PAN-2794: Including flag for onchain permissioning check on tx processor (#1571)

* Inclusing flag for onchain permissioning check on tx processor

Signed-off-by: Adrian Sutton <adrian.sutton@consensys.net>

ethereum/blockcreation/src/main/java/tech/pegasys/pantheon/ethereum/blockcreation/BlockTransactionSelector.java

@@ -168,7 +168,8 @@ public class BlockTransactionSelector {
             transaction,
             miningBeneficiary,
             blockHashLookup,
-            false);
+            false,
+            true);
 
     if (!result.isInvalid()) {
       worldStateUpdater.commit();

ethereum/blockcreation/src/test/java/tech/pegasys/pantheon/ethereum/blockcreation/BlockTransactionSelectorTest.java

@@ -14,6 +14,7 @@ package tech.pegasys.pantheon.ethereum.blockcreation;
 
 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 class BlockTransactionSelectorTest {
     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,7 +161,8 @@ public class BlockTransactionSelectorTest {
       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()),
@@ -168,7 +170,14 @@ public class BlockTransactionSelectorTest {
                 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 class BlockTransactionSelectorTest {
       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 class BlockTransactionSelectorTest {
   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 class BlockTransactionSelectorTest {
     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 class BlockTransactionSelectorTest {
             eq(validTransaction),
             any(),
             any(),
-            any()))
+            anyBoolean(),
+            anyBoolean()))
         .thenReturn(
             Result.successful(
                 LogSeries.empty(), 10000, BytesValue.EMPTY, ValidationResult.valid()));
@@ -440,7 +453,8 @@ public class BlockTransactionSelectorTest {
             eq(invalidTransaction),
             any(),
             any(),
-            any()))
+            anyBoolean(),
+            anyBoolean()))
         .thenReturn(
             Result.invalid(
                 ValidationResult.invalid(TransactionInvalidReason.EXCEEDS_BLOCK_GAS_LIMIT)));
@@ -468,7 +482,8 @@ public class BlockTransactionSelectorTest {
             eq(futureTransaction),
             any(),
             any(),
-            any()))
+            anyBoolean(),
+            anyBoolean()))
         .thenReturn(
             Result.invalid(ValidationResult.invalid(TransactionInvalidReason.INCORRECT_NONCE)));
 

ethereum/core/src/integration-test/java/tech/pegasys/pantheon/ethereum/vm/TraceTransactionIntegrationTest.java

@@ -92,6 +92,7 @@ public class TraceTransactionIntegrationTest {
             createTransaction,
             genesisBlock.getHeader().getCoinbase(),
             blockHashLookup,
+            false,
             false);
     assertThat(result.isSuccessful()).isTrue();
     final Account createdContract =

ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/core/TransactionFilter.java

@@ -14,5 +14,5 @@ package tech.pegasys.pantheon.ethereum.core;
 
 @FunctionalInterface
 public interface TransactionFilter {
-  boolean permitted(Transaction transaction, boolean isStateChange);
+  boolean permitted(Transaction transaction, boolean checkOnchainPermissions);
 }

ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/mainnet/MainnetBlockProcessor.java

@@ -129,6 +129,7 @@ public class MainnetBlockProcessor implements BlockProcessor {
               transaction,
               miningBeneficiary,
               blockHashLookup,
+              true,
               true);
       if (result.isInvalid()) {
         return Result.failed();

ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/mainnet/MainnetTransactionProcessor.java

@@ -146,7 +146,8 @@ public class MainnetTransactionProcessor implements TransactionProcessor {
       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 class MainnetTransactionProcessor implements TransactionProcessor {
     final TransactionValidationParams validationParams =
         new TransactionValidationParams.Builder()
             .allowFutureNonce(false)
-            .stateChange(isPersistingState)
+            .checkOnchainPermissions(checkOnchainPermissions)
             .build();
 
     final Address senderAddress = transaction.getSender();

ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/mainnet/MainnetTransactionValidator.java

@@ -114,7 +114,7 @@ public class MainnetTransactionValidator implements TransactionValidator {
               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 class MainnetTransactionValidator implements TransactionValidator {
     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

ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/mainnet/TransactionProcessor.java

@@ -107,6 +107,8 @@ public interface TransactionProcessor {
    * @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 @@ public interface TransactionProcessor {
       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 @@ public interface TransactionProcessor {
         miningBeneficiary,
         NO_TRACING,
         blockHashLookup,
-        isPersistingState);
+        isPersistingState,
+        checkOnchainPermissions);
   }
 
   /**
@@ -141,6 +145,27 @@ public interface TransactionProcessor {
    * @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 @@ public interface TransactionProcessor {
       Address miningBeneficiary,
       OperationTracer operationTracer,
       BlockHashLookup blockHashLookup,
-      Boolean isPersistingState);
+      Boolean isPersistingState,
+      Boolean checkOnchainPermissions);
 }

ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/mainnet/TransactionValidationParams.java

@@ -15,38 +15,39 @@ package tech.pegasys.pantheon.ethereum.mainnet;
 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);
     }
   }
 }

ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/transaction/TransactionSimulator.java

@@ -122,6 +122,7 @@ public class TransactionSimulator {
             transaction,
             protocolSpec.getMiningBeneficiaryCalculator().calculateBeneficiary(header),
             new BlockHashLookup(header, blockchain),
+            false,
             false);
 
     return Optional.of(new TransactionSimulatorResult(transaction, result));

ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/mainnet/MainnetTransactionProcessorTest.java

@@ -71,7 +71,7 @@ public class MainnetTransactionProcessorTest {
         transactionValidationParamCaptor();
 
     final TransactionValidationParams expectedValidationParams =
-        new TransactionValidationParams.Builder().stateChange(false).build();
+        new TransactionValidationParams.Builder().checkOnchainPermissions(false).build();
 
     transactionProcessor.processTransaction(
         blockchain,
@@ -80,6 +80,7 @@ public class MainnetTransactionProcessorTest {
         transaction,
         Address.fromHexString("1"),
         blockHashLookup,
+        false,
         false);
 
     assertThat(txValidationParamCaptor.getValue())
@@ -93,7 +94,7 @@ public class MainnetTransactionProcessorTest {
         transactionValidationParamCaptor();
 
     final TransactionValidationParams expectedValidationParams =
-        new TransactionValidationParams.Builder().stateChange(true).build();
+        new TransactionValidationParams.Builder().checkOnchainPermissions(true).build();
 
     transactionProcessor.processTransaction(
         blockchain,
@@ -102,6 +103,7 @@ public class MainnetTransactionProcessorTest {
         transaction,
         Address.fromHexString("1"),
         blockHashLookup,
+        true,
         true);
 
     assertThat(txValidationParamCaptor.getValue())

ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/mainnet/MainnetTransactionValidatorTest.java

@@ -186,7 +186,7 @@ public class MainnetTransactionValidatorTest {
     validator.setTransactionFilter(transactionFilter);
 
     final TransactionValidationParams validationParams =
-        new TransactionValidationParams.Builder().stateChange(true).build();
+        new TransactionValidationParams.Builder().checkOnchainPermissions(true).build();
 
     validator.validateForSender(basicTransaction, accountWithNonce(0), validationParams);
 

ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/transaction/TransactionSimulatorTest.java

@@ -14,6 +14,7 @@ package tech.pegasys.pantheon.ethereum.transaction;
 
 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 @@ public class TransactionSimulatorTest {
     }
 
     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() {

ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/vm/GeneralStateReferenceTestTools.java

@@ -116,6 +116,7 @@ public class GeneralStateReferenceTestTools {
             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())) {

ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/transactions/TransactionPool.java

@@ -193,7 +193,10 @@ public class TransactionPool implements BlockAddedObserver {
     }
 
     final TransactionValidationParams validationParams =
-        new TransactionValidationParams.Builder().allowFutureNonce(true).stateChange(false).build();
+        new TransactionValidationParams.Builder()
+            .allowFutureNonce(true)
+            .checkOnchainPermissions(false)
+            .build();
 
     return protocolContext
         .getWorldStateArchive()

ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/transactions/TransactionPoolTest.java

@@ -558,7 +558,10 @@ public class TransactionPoolTest {
         .thenReturn(valid());
 
     final TransactionValidationParams expectedValidationParams =
-        new TransactionValidationParams.Builder().stateChange(false).allowFutureNonce(true).build();
+        new TransactionValidationParams.Builder()
+            .checkOnchainPermissions(false)
+            .allowFutureNonce(true)
+            .build();
 
     transactionPool.addLocalTransaction(transaction1);
 

ethereum/jsonrpc/src/main/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/processor/BlockReplay.java

@@ -89,6 +89,7 @@ public class BlockReplay {
                   transaction,
                   spec.getMiningBeneficiaryCalculator().calculateBeneficiary(header),
                   blockHashLookup,
+                  false,
                   false);
             }
           }
@@ -110,6 +111,7 @@ public class BlockReplay {
               transaction,
               spec.getMiningBeneficiaryCalculator().calculateBeneficiary(blockHeader),
               new BlockHashLookup(blockHeader, blockchain),
+              false,
               false);
           return action.performAction(
               transaction, blockHeader, blockchain, worldState, transactionProcessor);