PIE-1717: Add TransactionValidationParam to TxProcessor (#1613)

* Updated TransactionValidationParams with checkLocalPermissioning flag
* Added TransactionValidationParam to TxProcessor
Signed-off-by: Adrian Sutton <adrian.sutton@consensys.net>

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

@@ -24,6 +24,7 @@ import tech.pegasys.pantheon.ethereum.eth.transactions.PendingTransactions;
 import tech.pegasys.pantheon.ethereum.eth.transactions.PendingTransactions.TransactionSelectionResult;
 import tech.pegasys.pantheon.ethereum.mainnet.MainnetBlockProcessor.TransactionReceiptFactory;
 import tech.pegasys.pantheon.ethereum.mainnet.TransactionProcessor;
+import tech.pegasys.pantheon.ethereum.mainnet.TransactionValidationParams;
 import tech.pegasys.pantheon.ethereum.mainnet.TransactionValidator.TransactionInvalidReason;
 import tech.pegasys.pantheon.ethereum.vm.BlockHashLookup;
 
@@ -169,7 +170,7 @@ public class BlockTransactionSelector {
             miningBeneficiary,
             blockHashLookup,
             false,
-            true);
+            TransactionValidationParams.mining());
 
     if (!result.isInvalid()) {
       worldStateUpdater.commit();

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

@@ -123,7 +123,7 @@ public class BlockTransactionSelectorTest {
     pendingTransactions.addRemoteTransaction(transaction);
 
     when(transactionProcessor.processTransaction(
-            any(), any(), any(), eq(transaction), any(), any(), anyBoolean(), anyBoolean()))
+            any(), any(), any(), eq(transaction), any(), any(), anyBoolean(), any()))
         .thenReturn(MainnetTransactionProcessor.Result.failed(5, ValidationResult.valid()));
 
     // The block should fit 3 transactions only
@@ -162,7 +162,7 @@ public class BlockTransactionSelectorTest {
     }
 
     when(transactionProcessor.processTransaction(
-            any(), any(), any(), any(), any(), any(), anyBoolean(), anyBoolean()))
+            any(), any(), any(), any(), any(), any(), anyBoolean(), any()))
         .thenReturn(
             MainnetTransactionProcessor.Result.successful(
                 new LogSeries(Lists.newArrayList()),
@@ -177,7 +177,7 @@ public class BlockTransactionSelectorTest {
             any(),
             any(),
             anyBoolean(),
-            anyBoolean()))
+            any()))
         .thenReturn(
             MainnetTransactionProcessor.Result.invalid(ValidationResult.invalid(NONCE_TOO_LOW)));
 
@@ -218,7 +218,7 @@ public class BlockTransactionSelectorTest {
     }
 
     when(transactionProcessor.processTransaction(
-            any(), any(), any(), any(), any(), any(), anyBoolean(), anyBoolean()))
+            any(), any(), any(), any(), any(), any(), anyBoolean(), any()))
         .thenReturn(
             MainnetTransactionProcessor.Result.successful(
                 new LogSeries(Lists.newArrayList()),
@@ -289,7 +289,7 @@ public class BlockTransactionSelectorTest {
     final ProcessableBlockHeader blockHeader = createBlockWithGasLimit(300);
 
     when(transactionProcessor.processTransaction(
-            any(), any(), any(), any(), any(), any(), anyBoolean(), anyBoolean()))
+            any(), any(), any(), any(), any(), any(), anyBoolean(), any()))
         .thenReturn(
             MainnetTransactionProcessor.Result.successful(
                 new LogSeries(Lists.newArrayList()),
@@ -346,7 +346,7 @@ public class BlockTransactionSelectorTest {
 
     // TransactionProcessor mock assumes all gas in the transaction was used (i.e. gasLimit).
     when(transactionProcessor.processTransaction(
-            any(), any(), any(), any(), any(), any(), anyBoolean(), anyBoolean()))
+            any(), any(), any(), any(), any(), any(), anyBoolean(), any()))
         .thenReturn(
             MainnetTransactionProcessor.Result.successful(
                 new LogSeries(Lists.newArrayList()),
@@ -442,7 +442,7 @@ public class BlockTransactionSelectorTest {
             any(),
             any(),
             anyBoolean(),
-            anyBoolean()))
+            any()))
         .thenReturn(
             Result.successful(
                 LogSeries.empty(), 10000, BytesValue.EMPTY, ValidationResult.valid()));
@@ -454,7 +454,7 @@ public class BlockTransactionSelectorTest {
             any(),
             any(),
             anyBoolean(),
-            anyBoolean()))
+            any()))
         .thenReturn(
             Result.invalid(
                 ValidationResult.invalid(TransactionInvalidReason.EXCEEDS_BLOCK_GAS_LIMIT)));
@@ -483,7 +483,7 @@ public class BlockTransactionSelectorTest {
             any(),
             any(),
             anyBoolean(),
-            anyBoolean()))
+            any()))
         .thenReturn(
             Result.invalid(ValidationResult.invalid(TransactionInvalidReason.INCORRECT_NONCE)));
 

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

@@ -30,6 +30,7 @@ import tech.pegasys.pantheon.ethereum.debug.TraceOptions;
 import tech.pegasys.pantheon.ethereum.mainnet.ProtocolSchedule;
 import tech.pegasys.pantheon.ethereum.mainnet.TransactionProcessor;
 import tech.pegasys.pantheon.ethereum.mainnet.TransactionProcessor.Result;
+import tech.pegasys.pantheon.ethereum.mainnet.TransactionValidationParams;
 import tech.pegasys.pantheon.ethereum.rlp.BytesValueRLPInput;
 import tech.pegasys.pantheon.ethereum.worldstate.WorldStateArchive;
 import tech.pegasys.pantheon.util.bytes.Bytes32;
@@ -93,7 +94,7 @@ public class TraceTransactionIntegrationTest {
             genesisBlock.getHeader().getCoinbase(),
             blockHashLookup,
             false,
-            false);
+            TransactionValidationParams.blockReplay());
     assertThat(result.isSuccessful()).isTrue();
     final Account createdContract =
         createTransactionUpdater.getTouchedAccounts().stream()

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

@@ -121,6 +121,7 @@ public class MainnetBlockProcessor implements BlockProcessor {
       final BlockHashLookup blockHashLookup = new BlockHashLookup(blockHeader, blockchain);
       final Address miningBeneficiary =
           miningBeneficiaryCalculator.calculateBeneficiary(blockHeader);
+
       final TransactionProcessor.Result result =
           transactionProcessor.processTransaction(
               blockchain,
@@ -130,7 +131,7 @@ public class MainnetBlockProcessor implements BlockProcessor {
               miningBeneficiary,
               blockHashLookup,
               true,
-              true);
+              TransactionValidationParams.processingBlock());
       if (result.isInvalid()) {
         return Result.failed();
       }

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

@@ -147,7 +147,7 @@ public class MainnetTransactionProcessor implements TransactionProcessor {
       final OperationTracer operationTracer,
       final BlockHashLookup blockHashLookup,
       final Boolean isPersistingState,
-      final Boolean checkOnchainPermissions) {
+      final TransactionValidationParams transactionValidationParams) {
     LOG.trace("Starting execution of {}", transaction);
 
     ValidationResult<TransactionInvalidReason> validationResult =
@@ -160,16 +160,10 @@ public class MainnetTransactionProcessor implements TransactionProcessor {
       return Result.invalid(validationResult);
     }
 
-    final TransactionValidationParams validationParams =
-        new TransactionValidationParams.Builder()
-            .allowFutureNonce(false)
-            .checkOnchainPermissions(checkOnchainPermissions)
-            .build();
-
     final Address senderAddress = transaction.getSender();
     final MutableAccount sender = worldState.getOrCreate(senderAddress);
     validationResult =
-        transactionValidator.validateForSender(transaction, sender, validationParams);
+        transactionValidator.validateForSender(transaction, sender, transactionValidationParams);
     if (!validationResult.isValid()) {
       LOG.warn("Invalid transaction: {}", validationResult.getErrorMessage());
       return Result.invalid(validationResult);

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.checkOnchainPermissions())) {
+    if (!isSenderAllowed(transaction, validationParams)) {
       return ValidationResult.invalid(
           TX_SENDER_NOT_AUTHORIZED,
           String.format("Sender %s is not on the Account Whitelist", transaction.getSender()));
@@ -163,10 +163,14 @@ public class MainnetTransactionValidator implements TransactionValidator {
   }
 
   private boolean isSenderAllowed(
-      final Transaction transaction, final boolean checkOnchainPermissions) {
+      final Transaction transaction, final TransactionValidationParams validationParams) {
-    return transactionFilter
+    if (validationParams.checkLocalPermissions() || validationParams.checkOnchainPermissions()) {
-        .map(c -> c.permitted(transaction, checkOnchainPermissions))
+      return transactionFilter
-        .orElse(true);
+          .map(c -> c.permitted(transaction, validationParams.checkOnchainPermissions()))
+          .orElse(true);
+    } else {
+      return true;
+    }
   }
 
   @Override

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

@@ -107,9 +107,11 @@ 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
+   * @param transactionValidationParams Validation parameters that will be used by the {@link
-   *     permissioning rules
+   *     TransactionValidator}
    * @return the transaction result
+   * @see TransactionValidator
+   * @see TransactionValidationParams
    */
   default Result processTransaction(
       final Blockchain blockchain,
@@ -119,7 +121,7 @@ public interface TransactionProcessor {
       final Address miningBeneficiary,
       final BlockHashLookup blockHashLookup,
       final Boolean isPersistingState,
-      final Boolean checkOnchainPermissions) {
+      final TransactionValidationParams transactionValidationParams) {
     return processTransaction(
         blockchain,
         worldState,
@@ -129,7 +131,7 @@ public interface TransactionProcessor {
         NO_TRACING,
         blockHashLookup,
         isPersistingState,
-        checkOnchainPermissions);
+        transactionValidationParams);
   }
 
   /**
@@ -163,7 +165,7 @@ public interface TransactionProcessor {
         operationTracer,
         blockHashLookup,
         isPersistingState,
-        false);
+        new TransactionValidationParams.Builder().build());
   }
 
   Result processTransaction(
@@ -175,5 +177,5 @@ public interface TransactionProcessor {
       OperationTracer operationTracer,
       BlockHashLookup blockHashLookup,
       Boolean isPersistingState,
-      Boolean checkOnchainPermissions);
+      TransactionValidationParams transactionValidationParams);
 }

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

@@ -16,11 +16,15 @@ public class TransactionValidationParams {
 
   private final boolean allowFutureNonce;
   private final boolean checkOnchainPermissions;
+  private final boolean checkLocalPermissions;
 
   private TransactionValidationParams(
-      final boolean allowFutureNonce, final boolean checkOnchainPermissions) {
+      final boolean allowFutureNonce,
+      final boolean checkOnchainPermissions,
+      final boolean checkLocalPermissions) {
     this.allowFutureNonce = allowFutureNonce;
     this.checkOnchainPermissions = checkOnchainPermissions;
+    this.checkLocalPermissions = checkLocalPermissions;
   }
 
   public boolean isAllowFutureNonce() {
@@ -31,10 +35,55 @@ public class TransactionValidationParams {
     return checkOnchainPermissions;
   }
 
-  public static class Builder {
+  public boolean checkLocalPermissions() {
+    return checkLocalPermissions;
+  }
+
+  public static TransactionValidationParams transactionSimulator() {
+    return new Builder()
+        .checkLocalPermissions(false)
+        .checkOnchainPermissions(false)
+        .allowFutureNonce(false)
+        .build();
+  }
+
+  public static TransactionValidationParams processingBlock() {
+    return new Builder()
+        .checkLocalPermissions(false)
+        .checkOnchainPermissions(true)
+        .allowFutureNonce(false)
+        .build();
+  }
+
+  public static TransactionValidationParams transactionPool() {
+    return new Builder()
+        .checkLocalPermissions(true)
+        .checkOnchainPermissions(false)
+        .allowFutureNonce(true)
+        .build();
+  }
+
+  public static TransactionValidationParams mining() {
+    return new Builder()
+        .checkLocalPermissions(true)
+        .checkOnchainPermissions(true)
+        .allowFutureNonce(false)
+        .build();
+  }
+
+  public static TransactionValidationParams blockReplay() {
+    return new Builder()
+        .checkLocalPermissions(false)
+        .checkOnchainPermissions(false)
+        .allowFutureNonce(false)
+        .build();
+  }
+
+  static class Builder {
 
     private boolean allowFutureNonce = false;
     private boolean checkOnchainPermissions = false;
+    private boolean checkLocalPermissions = true;
 
     public Builder allowFutureNonce(final boolean allowFutureNonce) {
       this.allowFutureNonce = allowFutureNonce;
@@ -46,8 +95,14 @@ public class TransactionValidationParams {
       return this;
     }
 
+    public Builder checkLocalPermissions(final boolean checkLocalPermissions) {
+      this.checkLocalPermissions = checkLocalPermissions;
+      return this;
+    }
+
     public TransactionValidationParams build() {
-      return new TransactionValidationParams(allowFutureNonce, checkOnchainPermissions);
+      return new TransactionValidationParams(
+          allowFutureNonce, checkOnchainPermissions, checkLocalPermissions);
     }
   }
 }

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

@@ -24,6 +24,7 @@ import tech.pegasys.pantheon.ethereum.core.Wei;
 import tech.pegasys.pantheon.ethereum.mainnet.ProtocolSchedule;
 import tech.pegasys.pantheon.ethereum.mainnet.ProtocolSpec;
 import tech.pegasys.pantheon.ethereum.mainnet.TransactionProcessor;
+import tech.pegasys.pantheon.ethereum.mainnet.TransactionValidationParams;
 import tech.pegasys.pantheon.ethereum.vm.BlockHashLookup;
 import tech.pegasys.pantheon.ethereum.worldstate.WorldStateArchive;
 import tech.pegasys.pantheon.util.bytes.BytesValue;
@@ -123,7 +124,7 @@ public class TransactionSimulator {
             protocolSpec.getMiningBeneficiaryCalculator().calculateBeneficiary(header),
             new BlockHashLookup(header, blockchain),
             false,
-            false);
+            TransactionValidationParams.transactionSimulator());
 
     return Optional.of(new TransactionSimulatorResult(transaction, result));
   }

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

@@ -65,13 +65,12 @@ public class MainnetTransactionProcessorTest {
   }
 
   @Test
-  public void
+  public void shouldCallTransactionValidatorWithExpectedTransactionValidationParams() {
-      shouldCallTransactionValidatorWithExpectedTransactionValidationParamsWhenNotPersistingState() {
     final ArgumentCaptor<TransactionValidationParams> txValidationParamCaptor =
         transactionValidationParamCaptor();
 
     final TransactionValidationParams expectedValidationParams =
-        new TransactionValidationParams.Builder().checkOnchainPermissions(false).build();
+        new TransactionValidationParams.Builder().build();
 
     transactionProcessor.processTransaction(
         blockchain,
@@ -81,30 +80,7 @@ public class MainnetTransactionProcessorTest {
         Address.fromHexString("1"),
         blockHashLookup,
         false,
-        false);
+        new TransactionValidationParams.Builder().build());
-
-    assertThat(txValidationParamCaptor.getValue())
-        .isEqualToComparingFieldByField(expectedValidationParams);
-  }
-
-  @Test
-  public void
-      shouldCallTransactionValidatorWithExpectedTransactionValidationParamsWhenPersistingState() {
-    final ArgumentCaptor<TransactionValidationParams> txValidationParamCaptor =
-        transactionValidationParamCaptor();
-
-    final TransactionValidationParams expectedValidationParams =
-        new TransactionValidationParams.Builder().checkOnchainPermissions(true).build();
-
-    transactionProcessor.processTransaction(
-        blockchain,
-        worldState,
-        blockHeader,
-        transaction,
-        Address.fromHexString("1"),
-        blockHashLookup,
-        true,
-        true);
 
     assertThat(txValidationParamCaptor.getValue())
         .isEqualToComparingFieldByField(expectedValidationParams);

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

@@ -16,6 +16,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.Mockito.mock;
+import static org.mockito.Mockito.verifyZeroInteractions;
 import static org.mockito.Mockito.when;
 import static tech.pegasys.pantheon.ethereum.mainnet.TransactionValidator.TransactionInvalidReason.INCORRECT_NONCE;
 import static tech.pegasys.pantheon.ethereum.mainnet.TransactionValidator.TransactionInvalidReason.INTRINSIC_GAS_EXCEEDS_GAS_LIMIT;
@@ -193,6 +194,28 @@ public class MainnetTransactionValidatorTest {
     assertThat(stateChangeParamCaptor.getValue()).isTrue();
   }
 
+  @Test
+  public void shouldNotCheckAccountPermissionIfBothValidationParamsCheckPermissionsAreFalse() {
+    final TransactionFilter transactionFilter = mock(TransactionFilter.class);
+
+    final MainnetTransactionValidator validator =
+        new MainnetTransactionValidator(gasCalculator, false, Optional.empty());
+    validator.setTransactionFilter(transactionFilter);
+
+    final TransactionValidationParams validationParams =
+        new TransactionValidationParams.Builder()
+            .checkOnchainPermissions(false)
+            .checkLocalPermissions(false)
+            .build();
+
+    validator.validateForSender(basicTransaction, accountWithNonce(0), validationParams);
+
+    assertThat(validator.validateForSender(basicTransaction, accountWithNonce(0), validationParams))
+        .isEqualTo(ValidationResult.valid());
+
+    verifyZeroInteractions(transactionFilter);
+  }
+
   private Account accountWithNonce(final long nonce) {
     return account(basicTransaction.getUpfrontCost(), nonce);
   }

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

@@ -345,14 +345,14 @@ public class TransactionSimulatorTest {
     }
 
     when(transactionProcessor.processTransaction(
-            any(), any(), any(), eq(transaction), any(), any(), anyBoolean(), anyBoolean()))
+            any(), any(), any(), eq(transaction), any(), any(), anyBoolean(), any()))
         .thenReturn(result);
   }
 
   private void verifyTransactionWasProcessed(final Transaction expectedTransaction) {
     verify(transactionProcessor)
         .processTransaction(
-            any(), any(), any(), eq(expectedTransaction), any(), any(), anyBoolean(), anyBoolean());
+            any(), any(), any(), eq(expectedTransaction), any(), any(), anyBoolean(), any());
   }
 
   private CallParameter callParameter() {

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

@@ -23,6 +23,7 @@ import tech.pegasys.pantheon.ethereum.core.Transaction;
 import tech.pegasys.pantheon.ethereum.core.WorldState;
 import tech.pegasys.pantheon.ethereum.core.WorldUpdater;
 import tech.pegasys.pantheon.ethereum.mainnet.TransactionProcessor;
+import tech.pegasys.pantheon.ethereum.mainnet.TransactionValidationParams;
 import tech.pegasys.pantheon.ethereum.rlp.RLP;
 import tech.pegasys.pantheon.ethereum.worldstate.DebuggableMutableWorldState;
 import tech.pegasys.pantheon.testutil.JsonTestParameters;
@@ -117,7 +118,7 @@ public class GeneralStateReferenceTestTools {
             blockHeader.getCoinbase(),
             new BlockHashLookup(blockHeader, blockchain),
             false,
-            false);
+            TransactionValidationParams.processingBlock());
     final Account coinbase = worldStateUpdater.getOrCreate(spec.blockHeader().getCoinbase());
     if (coinbase != null && coinbase.isEmpty() && shouldClearEmptyAccounts(spec.eip())) {
       worldStateUpdater.deleteAccount(coinbase.getAddress());

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

@@ -204,12 +204,6 @@ public class TransactionPool implements BlockAddedObserver {
               transaction.getGasLimit(), chainHeadBlockHeader.getGasLimit()));
     }
 
-    final TransactionValidationParams validationParams =
-        new TransactionValidationParams.Builder()
-            .allowFutureNonce(true)
-            .checkOnchainPermissions(false)
-            .build();
-
     return protocolContext
         .getWorldStateArchive()
         .get(chainHeadBlockHeader.getStateRoot())
@@ -217,7 +211,8 @@ public class TransactionPool implements BlockAddedObserver {
             worldState -> {
               final Account senderAccount = worldState.get(transaction.getSender());
               return getTransactionValidator()
-                  .validateForSender(transaction, senderAccount, validationParams);
+                  .validateForSender(
+                      transaction, senderAccount, TransactionValidationParams.transactionPool());
             })
         .orElseGet(() -> ValidationResult.invalid(CHAIN_HEAD_WORLD_STATE_NOT_AVAILABLE));
   }

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

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

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

@@ -22,6 +22,7 @@ import tech.pegasys.pantheon.ethereum.core.Transaction;
 import tech.pegasys.pantheon.ethereum.mainnet.ProtocolSchedule;
 import tech.pegasys.pantheon.ethereum.mainnet.ProtocolSpec;
 import tech.pegasys.pantheon.ethereum.mainnet.TransactionProcessor;
+import tech.pegasys.pantheon.ethereum.mainnet.TransactionValidationParams;
 import tech.pegasys.pantheon.ethereum.vm.BlockHashLookup;
 import tech.pegasys.pantheon.ethereum.worldstate.WorldStateArchive;
 
@@ -90,7 +91,7 @@ public class BlockReplay {
                   spec.getMiningBeneficiaryCalculator().calculateBeneficiary(header),
                   blockHashLookup,
                   false,
-                  false);
+                  TransactionValidationParams.blockReplay());
             }
           }
           return Optional.empty();
@@ -112,7 +113,7 @@ public class BlockReplay {
               spec.getMiningBeneficiaryCalculator().calculateBeneficiary(blockHeader),
               new BlockHashLookup(blockHeader, blockchain),
               false,
-              false);
+              TransactionValidationParams.blockReplay());
           return action.performAction(
               transaction, blockHeader, blockchain, worldState, transactionProcessor);
         });