PAN-2794: Including flag for onchain permissioning check on tx proces…

…sor (#1571) * Inclusing flag for onchain permissioning check on tx processor
PegaSysEng · Jun 18, 2019 · b05179f · b05179f
1 parent 9ff9012
commit b05179f
Show file tree

Hide file tree

Showing 17 changed files with 98 additions and 35 deletions.
diff --git a/.../src/main/java/tech/pegasys/pantheon/ethereum/blockcreation/BlockTransactionSelector.java b/.../src/main/java/tech/pegasys/pantheon/ethereum/blockcreation/BlockTransactionSelector.java
@@ -168,7 +168,8 @@ private TransactionSelectionResult evaluateTransaction(final Transaction transac
             transaction,
             miningBeneficiary,
             blockHashLookup,
-            false);
+            false,
+            true);
 
     if (!result.isInvalid()) {
       worldStateUpdater.commit();

diff --git a/.../test/java/tech/pegasys/pantheon/ethereum/blockcreation/BlockTransactionSelectorTest.java b/.../test/java/tech/pegasys/pantheon/ethereum/blockcreation/BlockTransactionSelectorTest.java
@@ -14,6 +14,7 @@
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyBoolean;
 import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
@@ -122,7 +123,7 @@ public void failedTransactionsAreIncludedInTheBlock() {
     pendingTransactions.addRemoteTransaction(transaction);
 
     when(transactionProcessor.processTransaction(
-            any(), any(), any(), eq(transaction), any(), any(), any()))
+            any(), any(), any(), eq(transaction), any(), any(), anyBoolean(), anyBoolean()))
         .thenReturn(MainnetTransactionProcessor.Result.failed(5, ValidationResult.valid()));
 
     // The block should fit 3 transactions only
@@ -160,15 +161,23 @@ public void invalidTransactionsTransactionProcessingAreSkippedButBlockStillFills
       pendingTransactions.addRemoteTransaction(tx);
     }
 
-    when(transactionProcessor.processTransaction(any(), any(), any(), any(), any(), any(), any()))
+    when(transactionProcessor.processTransaction(
+            any(), any(), any(), any(), any(), any(), anyBoolean(), anyBoolean()))
         .thenReturn(
             MainnetTransactionProcessor.Result.successful(
                 new LogSeries(Lists.newArrayList()),
                 0,
                 BytesValue.EMPTY,
                 ValidationResult.valid()));
     when(transactionProcessor.processTransaction(
-            any(), any(), any(), eq(transactionsToInject.get(1)), any(), any(), any()))
+            any(),
+            any(),
+            any(),
+            eq(transactionsToInject.get(1)),
+            any(),
+            any(),
+            anyBoolean(),
+            anyBoolean()))
         .thenReturn(
             MainnetTransactionProcessor.Result.invalid(ValidationResult.invalid(NONCE_TOO_LOW)));
 
@@ -208,7 +217,8 @@ public void subsetOfPendingTransactionsIncludedWhenBlockGasLimitHit() {
       pendingTransactions.addRemoteTransaction(tx);
     }
 
-    when(transactionProcessor.processTransaction(any(), any(), any(), any(), any(), any(), any()))
+    when(transactionProcessor.processTransaction(
+            any(), any(), any(), any(), any(), any(), anyBoolean(), anyBoolean()))
         .thenReturn(
             MainnetTransactionProcessor.Result.successful(
                 new LogSeries(Lists.newArrayList()),
@@ -278,7 +288,8 @@ public void transactionOfferingGasPriceLessThanMinimumIsIdentifiedAndRemovedFrom
   public void transactionTooLargeForBlockDoesNotPreventMoreBeingAddedIfBlockOccupancyNotReached() {
     final ProcessableBlockHeader blockHeader = createBlockWithGasLimit(300);
 
-    when(transactionProcessor.processTransaction(any(), any(), any(), any(), any(), any(), any()))
+    when(transactionProcessor.processTransaction(
+            any(), any(), any(), any(), any(), any(), anyBoolean(), anyBoolean()))
         .thenReturn(
             MainnetTransactionProcessor.Result.successful(
                 new LogSeries(Lists.newArrayList()),
@@ -334,7 +345,8 @@ public void transactionSelectionStopsWhenSufficientBlockOccupancyIsReached() {
     final ProcessableBlockHeader blockHeader = createBlockWithGasLimit(300);
 
     // TransactionProcessor mock assumes all gas in the transaction was used (i.e. gasLimit).
-    when(transactionProcessor.processTransaction(any(), any(), any(), any(), any(), any(), any()))
+    when(transactionProcessor.processTransaction(
+            any(), any(), any(), any(), any(), any(), anyBoolean(), anyBoolean()))
         .thenReturn(
             MainnetTransactionProcessor.Result.successful(
                 new LogSeries(Lists.newArrayList()),
@@ -429,7 +441,8 @@ public void shouldDiscardTransactionsThatFailValidation() {
             eq(validTransaction),
             any(),
             any(),
-            any()))
+            anyBoolean(),
+            anyBoolean()))
         .thenReturn(
             Result.successful(
                 LogSeries.empty(), 10000, BytesValue.EMPTY, ValidationResult.valid()));
@@ -440,7 +453,8 @@ public void shouldDiscardTransactionsThatFailValidation() {
             eq(invalidTransaction),
             any(),
             any(),
-            any()))
+            anyBoolean(),
+            anyBoolean()))
         .thenReturn(
             Result.invalid(
                 ValidationResult.invalid(TransactionInvalidReason.EXCEEDS_BLOCK_GAS_LIMIT)));
@@ -468,7 +482,8 @@ public void transactionWithIncorrectNonceRemainsInPoolAndNotSelected() {
             eq(futureTransaction),
             any(),
             any(),
-            any()))
+            anyBoolean(),
+            anyBoolean()))
         .thenReturn(
             Result.invalid(ValidationResult.invalid(TransactionInvalidReason.INCORRECT_NONCE)));
 

diff --git a/...egration-test/java/tech/pegasys/pantheon/ethereum/vm/TraceTransactionIntegrationTest.java b/...egration-test/java/tech/pegasys/pantheon/ethereum/vm/TraceTransactionIntegrationTest.java
@@ -92,6 +92,7 @@ public void shouldTraceSStoreOperation() {
             createTransaction,
             genesisBlock.getHeader().getCoinbase(),
             blockHashLookup,
+            false,
             false);
     assertThat(result.isSuccessful()).isTrue();
     final Account createdContract =

diff --git a/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/core/TransactionFilter.java b/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/core/TransactionFilter.java
@@ -14,5 +14,5 @@
 
 @FunctionalInterface
 public interface TransactionFilter {
-  boolean permitted(Transaction transaction, boolean isStateChange);
+  boolean permitted(Transaction transaction, boolean checkOnchainPermissions);
 }
diff --git a/...reum/core/src/main/java/tech/pegasys/pantheon/ethereum/mainnet/MainnetBlockProcessor.java b/...reum/core/src/main/java/tech/pegasys/pantheon/ethereum/mainnet/MainnetBlockProcessor.java
@@ -129,6 +129,7 @@ public Result processBlock(
               transaction,
               miningBeneficiary,
               blockHashLookup,
+              true,
               true);
       if (result.isInvalid()) {
         return Result.failed();

diff --git a/...ore/src/main/java/tech/pegasys/pantheon/ethereum/mainnet/MainnetTransactionProcessor.java b/...ore/src/main/java/tech/pegasys/pantheon/ethereum/mainnet/MainnetTransactionProcessor.java
@@ -146,7 +146,8 @@ public Result processTransaction(
       final Address miningBeneficiary,
       final OperationTracer operationTracer,
       final BlockHashLookup blockHashLookup,
-      final Boolean isPersistingState) {
+      final Boolean isPersistingState,
+      final Boolean checkOnchainPermissions) {
     LOG.trace("Starting execution of {}", transaction);
 
     ValidationResult<TransactionInvalidReason> validationResult =
@@ -162,7 +163,7 @@ public Result processTransaction(
     final TransactionValidationParams validationParams =
         new TransactionValidationParams.Builder()
             .allowFutureNonce(false)
-            .stateChange(isPersistingState)
+            .checkOnchainPermissions(checkOnchainPermissions)
             .build();
 
     final Address senderAddress = transaction.getSender();

diff --git a/...ore/src/main/java/tech/pegasys/pantheon/ethereum/mainnet/MainnetTransactionValidator.java b/...ore/src/main/java/tech/pegasys/pantheon/ethereum/mainnet/MainnetTransactionValidator.java
@@ -114,7 +114,7 @@ public ValidationResult<TransactionInvalidReason> validateForSender(
               transaction.getNonce(), senderNonce));
     }
 
-    if (!isSenderAllowed(transaction, validationParams.isStateChange())) {
+    if (!isSenderAllowed(transaction, validationParams.checkOnchainPermissions())) {
       return ValidationResult.invalid(
           TX_SENDER_NOT_AUTHORIZED,
           String.format("Sender %s is not on the Account Whitelist", transaction.getSender()));
@@ -162,8 +162,11 @@ public ValidationResult<TransactionInvalidReason> validateTransactionSignature(
     return ValidationResult.valid();
   }
 
-  private boolean isSenderAllowed(final Transaction transaction, final boolean isStateChange) {
-    return transactionFilter.map(c -> c.permitted(transaction, isStateChange)).orElse(true);
+  private boolean isSenderAllowed(
+      final Transaction transaction, final boolean checkOnchainPermissions) {
+    return transactionFilter
+        .map(c -> c.permitted(transaction, checkOnchainPermissions))
+        .orElse(true);
   }
 
   @Override

diff --git a/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/mainnet/TransactionProcessor.java b/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/mainnet/TransactionProcessor.java
@@ -107,6 +107,8 @@ default boolean isSuccessful() {
    * @param miningBeneficiary The address which is to receive the transaction fee
    * @param blockHashLookup The {@link BlockHashLookup} to use for BLOCKHASH operations
    * @param isPersistingState Whether the state will be modified by this process
+   * @param checkOnchainPermissions Whether a transaction permissioning check should check onchain
+   *     permissioning rules
    * @return the transaction result
    */
   default Result processTransaction(
@@ -116,7 +118,8 @@ default Result processTransaction(
       final Transaction transaction,
       final Address miningBeneficiary,
       final BlockHashLookup blockHashLookup,
-      final Boolean isPersistingState) {
+      final Boolean isPersistingState,
+      final Boolean checkOnchainPermissions) {
     return processTransaction(
         blockchain,
         worldState,
@@ -125,7 +128,8 @@ default Result processTransaction(
         miningBeneficiary,
         NO_TRACING,
         blockHashLookup,
-        isPersistingState);
+        isPersistingState,
+        checkOnchainPermissions);
   }
 
   /**
@@ -141,6 +145,27 @@ default Result processTransaction(
    * @param isPersistingState Whether the state will be modified by this process
    * @return the transaction result
    */
+  default Result processTransaction(
+      final Blockchain blockchain,
+      final WorldUpdater worldState,
+      final ProcessableBlockHeader blockHeader,
+      final Transaction transaction,
+      final Address miningBeneficiary,
+      final OperationTracer operationTracer,
+      final BlockHashLookup blockHashLookup,
+      final Boolean isPersistingState) {
+    return processTransaction(
+        blockchain,
+        worldState,
+        blockHeader,
+        transaction,
+        miningBeneficiary,
+        operationTracer,
+        blockHashLookup,
+        isPersistingState,
+        false);
+  }
+
   Result processTransaction(
       Blockchain blockchain,
       WorldUpdater worldState,
@@ -149,5 +174,6 @@ Result processTransaction(
       Address miningBeneficiary,
       OperationTracer operationTracer,
       BlockHashLookup blockHashLookup,
-      Boolean isPersistingState);
+      Boolean isPersistingState,
+      Boolean checkOnchainPermissions);
 }
diff --git a/...ore/src/main/java/tech/pegasys/pantheon/ethereum/mainnet/TransactionValidationParams.java b/...ore/src/main/java/tech/pegasys/pantheon/ethereum/mainnet/TransactionValidationParams.java
@@ -15,38 +15,39 @@
 public class TransactionValidationParams {
 
   private final boolean allowFutureNonce;
-  private final boolean stateChange;
+  private final boolean checkOnchainPermissions;
 
-  private TransactionValidationParams(final boolean allowFutureNonce, final boolean stateChange) {
+  private TransactionValidationParams(
+      final boolean allowFutureNonce, final boolean checkOnchainPermissions) {
     this.allowFutureNonce = allowFutureNonce;
-    this.stateChange = stateChange;
+    this.checkOnchainPermissions = checkOnchainPermissions;
   }
 
   public boolean isAllowFutureNonce() {
     return allowFutureNonce;
   }
 
-  public boolean isStateChange() {
-    return stateChange;
+  public boolean checkOnchainPermissions() {
+    return checkOnchainPermissions;
   }
 
   public static class Builder {
 
     private boolean allowFutureNonce = false;
-    private boolean stateChange = false;
+    private boolean checkOnchainPermissions = false;
 
     public Builder allowFutureNonce(final boolean allowFutureNonce) {
       this.allowFutureNonce = allowFutureNonce;
       return this;
     }
 
-    public Builder stateChange(final boolean stateChange) {
-      this.stateChange = stateChange;
+    public Builder checkOnchainPermissions(final boolean checkOnchainPermissions) {
+      this.checkOnchainPermissions = checkOnchainPermissions;
       return this;
     }
 
     public TransactionValidationParams build() {
-      return new TransactionValidationParams(allowFutureNonce, stateChange);
+      return new TransactionValidationParams(allowFutureNonce, checkOnchainPermissions);
     }
   }
 }
diff --git a/...m/core/src/main/java/tech/pegasys/pantheon/ethereum/transaction/TransactionSimulator.java b/...m/core/src/main/java/tech/pegasys/pantheon/ethereum/transaction/TransactionSimulator.java
@@ -122,6 +122,7 @@ private Optional<TransactionSimulatorResult> process(
             transaction,
             protocolSpec.getMiningBeneficiaryCalculator().calculateBeneficiary(header),
             new BlockHashLookup(header, blockchain),
+            false,
             false);
 
     return Optional.of(new TransactionSimulatorResult(transaction, result));

diff --git a/...src/test/java/tech/pegasys/pantheon/ethereum/mainnet/MainnetTransactionProcessorTest.java b/...src/test/java/tech/pegasys/pantheon/ethereum/mainnet/MainnetTransactionProcessorTest.java
@@ -71,7 +71,7 @@ public void before() {
         transactionValidationParamCaptor();
 
     final TransactionValidationParams expectedValidationParams =
-        new TransactionValidationParams.Builder().stateChange(false).build();
+        new TransactionValidationParams.Builder().checkOnchainPermissions(false).build();
 
     transactionProcessor.processTransaction(
         blockchain,
@@ -80,6 +80,7 @@ public void before() {
         transaction,
         Address.fromHexString("1"),
         blockHashLookup,
+        false,
         false);
 
     assertThat(txValidationParamCaptor.getValue())
@@ -93,7 +94,7 @@ public void before() {
         transactionValidationParamCaptor();
 
     final TransactionValidationParams expectedValidationParams =
-        new TransactionValidationParams.Builder().stateChange(true).build();
+        new TransactionValidationParams.Builder().checkOnchainPermissions(true).build();
 
     transactionProcessor.processTransaction(
         blockchain,
@@ -102,6 +103,7 @@ public void before() {
         transaction,
         Address.fromHexString("1"),
         blockHashLookup,
+        true,
         true);
 
     assertThat(txValidationParamCaptor.getValue())

diff --git a/...src/test/java/tech/pegasys/pantheon/ethereum/mainnet/MainnetTransactionValidatorTest.java b/...src/test/java/tech/pegasys/pantheon/ethereum/mainnet/MainnetTransactionValidatorTest.java
@@ -186,7 +186,7 @@ public void shouldPropagateCorrectStateChangeParamToTransactionFilter() {
     validator.setTransactionFilter(transactionFilter);
 
     final TransactionValidationParams validationParams =
-        new TransactionValidationParams.Builder().stateChange(true).build();
+        new TransactionValidationParams.Builder().checkOnchainPermissions(true).build();
 
     validator.validateForSender(basicTransaction, accountWithNonce(0), validationParams);
 

diff --git a/...re/src/test/java/tech/pegasys/pantheon/ethereum/transaction/TransactionSimulatorTest.java b/...re/src/test/java/tech/pegasys/pantheon/ethereum/transaction/TransactionSimulatorTest.java
@@ -14,6 +14,7 @@
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyBoolean;
 import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.verify;
@@ -344,13 +345,14 @@ private void mockProcessorStatusForTransaction(
     }
 
     when(transactionProcessor.processTransaction(
-            any(), any(), any(), eq(transaction), any(), any(), any()))
+            any(), any(), any(), eq(transaction), any(), any(), anyBoolean(), anyBoolean()))
         .thenReturn(result);
   }
 
   private void verifyTransactionWasProcessed(final Transaction expectedTransaction) {
     verify(transactionProcessor)
-        .processTransaction(any(), any(), any(), eq(expectedTransaction), any(), any(), any());
+        .processTransaction(
+            any(), any(), any(), eq(expectedTransaction), any(), any(), anyBoolean(), anyBoolean());
   }
 
   private CallParameter callParameter() {

diff --git a/.../core/src/test/java/tech/pegasys/pantheon/ethereum/vm/GeneralStateReferenceTestTools.java b/.../core/src/test/java/tech/pegasys/pantheon/ethereum/vm/GeneralStateReferenceTestTools.java
@@ -116,6 +116,7 @@ public static void executeTest(final GeneralStateTestCaseEipSpec spec) {
             transaction,
             blockHeader.getCoinbase(),
             new BlockHashLookup(blockHeader, blockchain),
+            false,
             false);
     final Account coinbase = worldStateUpdater.getOrCreate(spec.blockHeader().getCoinbase());
     if (coinbase != null && coinbase.isEmpty() && shouldClearEmptyAccounts(spec.eip())) {

diff --git a/...um/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/transactions/TransactionPool.java b/...um/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/transactions/TransactionPool.java
@@ -193,7 +193,10 @@ private ValidationResult<TransactionInvalidReason> validateTransaction(
     }
 
     final TransactionValidationParams validationParams =
-        new TransactionValidationParams.Builder().allowFutureNonce(true).stateChange(false).build();
+        new TransactionValidationParams.Builder()
+            .allowFutureNonce(true)
+            .checkOnchainPermissions(false)
+            .build();
 
     return protocolContext
         .getWorldStateArchive()

diff --git a/...th/src/test/java/tech/pegasys/pantheon/ethereum/eth/transactions/TransactionPoolTest.java b/...th/src/test/java/tech/pegasys/pantheon/ethereum/eth/transactions/TransactionPoolTest.java
@@ -558,7 +558,10 @@ public void shouldCallValidatorWithExpectedValidationParameters() {
         .thenReturn(valid());
 
     final TransactionValidationParams expectedValidationParams =
-        new TransactionValidationParams.Builder().stateChange(false).allowFutureNonce(true).build();
+        new TransactionValidationParams.Builder()
+            .checkOnchainPermissions(false)
+            .allowFutureNonce(true)
+            .build();
 
     transactionPool.addLocalTransaction(transaction1);