Introduce transaction validator interface (phase 2) (hyperledger#5682)

Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>

besu/src/main/java/org/hyperledger/besu/RunnerBuilder.java

@@ -1136,7 +1136,7 @@ private Optional<AccountPermissioningController> buildAccountPermissioningContro
           permissioningController ->
               besuController
                   .getProtocolSchedule()
-                  .setTransactionFilter(permissioningController::isPermitted));
+                  .setPermissionTransactionFilter(permissioningController::isPermitted));
 
       return accountPermissioningController;
     } else {

besu/src/test/java/org/hyperledger/besu/services/BesuEventsImplTest.java

@@ -51,7 +51,7 @@
 import org.hyperledger.besu.ethereum.mainnet.MainnetBlockHeaderFunctions;
 import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule;
 import org.hyperledger.besu.ethereum.mainnet.ProtocolSpec;
-import org.hyperledger.besu.ethereum.mainnet.TransactionValidator;
+import org.hyperledger.besu.ethereum.mainnet.TransactionValidatorFactory;
 import org.hyperledger.besu.ethereum.mainnet.ValidationResult;
 import org.hyperledger.besu.ethereum.mainnet.feemarket.FeeMarket;
 import org.hyperledger.besu.ethereum.storage.keyvalue.KeyValueStoragePrefixedKeyBlockchainStorage;
@@ -79,6 +79,7 @@
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
+import org.mockito.Answers;
 import org.mockito.Mock;
 import org.mockito.junit.MockitoJUnitRunner;
 
@@ -99,7 +100,10 @@ public class BesuEventsImplTest {
   @Mock private EthContext mockEthContext;
   @Mock private EthMessages mockEthMessages;
   @Mock private EthScheduler mockEthScheduler;
-  @Mock private TransactionValidator mockTransactionValidator;
+
+  @Mock(answer = Answers.RETURNS_DEEP_STUBS)
+  private TransactionValidatorFactory mockTransactionValidatorFactory;
+
   @Mock private ProtocolSpec mockProtocolSpec;
   @Mock private WorldStateArchive mockWorldStateArchive;
   @Mock private MutableWorldState mockWorldState;
@@ -128,11 +132,12 @@ public void setUp() {
     when(mockProtocolContext.getBlockchain()).thenReturn(blockchain);
     when(mockProtocolContext.getWorldStateArchive()).thenReturn(mockWorldStateArchive);
     when(mockProtocolSchedule.getByBlockHeader(any())).thenReturn(mockProtocolSpec);
-    when(mockProtocolSpec.getTransactionValidator()).thenReturn(mockTransactionValidator);
+    when(mockProtocolSpec.getTransactionValidatorFactory())
+        .thenReturn(mockTransactionValidatorFactory);
     when(mockProtocolSpec.getFeeMarket()).thenReturn(FeeMarket.london(0L));
-    when(mockTransactionValidator.validate(any(), any(Optional.class), any()))
+    when(mockTransactionValidatorFactory.get().validate(any(), any(Optional.class), any()))
         .thenReturn(ValidationResult.valid());
-    when(mockTransactionValidator.validateForSender(any(), any(), any()))
+    when(mockTransactionValidatorFactory.get().validateForSender(any(), any(), any()))
         .thenReturn(ValidationResult.valid());
     when(mockWorldStateArchive.getMutable(any(), anyBoolean()))
         .thenReturn(Optional.of(mockWorldState));

consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/TransitionProtocolSchedule.java

@@ -219,9 +219,11 @@ public String listMilestones() {
    * @param permissionTransactionFilter the transaction filter
    */
   @Override
-  public void setTransactionFilter(final PermissionTransactionFilter permissionTransactionFilter) {
+  public void setPermissionTransactionFilter(
+      final PermissionTransactionFilter permissionTransactionFilter) {
     transitionUtils.dispatchConsumerAccordingToMergeState(
-        protocolSchedule -> protocolSchedule.setTransactionFilter(permissionTransactionFilter));
+        protocolSchedule ->
+            protocolSchedule.setPermissionTransactionFilter(permissionTransactionFilter));
   }
 
   /**

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/ClassicProtocolSpecs.java

@@ -76,7 +76,7 @@ public static ProtocolSpecBuilder tangerineWhistleDefinition(
         .gasCalculator(TangerineWhistleGasCalculator::new)
         .transactionValidatorBuilder(
             (gasCalculator, gasLimitCalculator) ->
-                new MainnetTransactionValidator(gasCalculator, gasLimitCalculator, true, chainId))
+                new TransactionValidatorFactory(gasCalculator, gasLimitCalculator, true, chainId))
         .name("ClassicTangerineWhistle");
   }
 
@@ -130,7 +130,7 @@ public static ProtocolSpecBuilder defuseDifficultyBombDefinition(
         .difficultyCalculator(ClassicDifficultyCalculators.DIFFICULTY_BOMB_REMOVED)
         .transactionValidatorBuilder(
             (gasCalculator, gasLimitCalculator) ->
-                new MainnetTransactionValidator(gasCalculator, gasLimitCalculator, true, chainId))
+                new TransactionValidatorFactory(gasCalculator, gasLimitCalculator, true, chainId))
         .name("DefuseDifficultyBomb");
   }
 
@@ -293,7 +293,7 @@ public static ProtocolSpecBuilder magnetoDefinition(
         .gasCalculator(BerlinGasCalculator::new)
         .transactionValidatorBuilder(
             (gasCalculator, gasLimitCalculator) ->
-                new MainnetTransactionValidator(
+                new TransactionValidatorFactory(
                     gasCalculator,
                     gasLimitCalculator,
                     true,

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/DefaultProtocolSchedule.java

@@ -111,11 +111,12 @@ public boolean anyMatch(final Predicate<ScheduledProtocolSpec> predicate) {
   }
 
   @Override
-  public void setTransactionFilter(final PermissionTransactionFilter permissionTransactionFilter) {
+  public void setPermissionTransactionFilter(
+      final PermissionTransactionFilter permissionTransactionFilter) {
     protocolSpecs.forEach(
         spec ->
             spec.spec()
-                .getTransactionValidator()
+                .getTransactionValidatorFactory()
                 .setPermissionTransactionFilter(permissionTransactionFilter));
   }
 

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/MainnetProtocolSpecs.java

@@ -124,16 +124,16 @@ public static ProtocolSpecBuilder frontierDefinition(
                     0))
         .transactionValidatorBuilder(
             (gasCalculator, gasLimitCalculator) ->
-                new MainnetTransactionValidator(
+                new TransactionValidatorFactory(
                     gasCalculator, gasLimitCalculator, false, Optional.empty()))
         .transactionProcessorBuilder(
             (gasCalculator,
-                transactionValidator,
+                transactionValidatorFactory,
                 contractCreationProcessor,
                 messageCallProcessor) ->
                 new MainnetTransactionProcessor(
                     gasCalculator,
-                    transactionValidator,
+                    transactionValidatorFactory,
                     contractCreationProcessor,
                     messageCallProcessor,
                     false,
@@ -142,12 +142,12 @@ public static ProtocolSpecBuilder frontierDefinition(
                     FeeMarket.legacy(),
                     CoinbaseFeePriceCalculator.frontier()))
         .privateTransactionProcessorBuilder(
-            (transactionValidator,
+            (transactionValidatorFactory,
                 contractCreationProcessor,
                 messageCallProcessor,
                 privateTransactionValidator) ->
                 new PrivateTransactionProcessor(
-                    transactionValidator,
+                    transactionValidatorFactory,
                     contractCreationProcessor,
                     messageCallProcessor,
                     false,
@@ -199,7 +199,7 @@ public static ProtocolSpecBuilder homesteadDefinition(
                     0))
         .transactionValidatorBuilder(
             (gasCalculator, gasLimitCalculator) ->
-                new MainnetTransactionValidator(
+                new TransactionValidatorFactory(
                     gasCalculator, gasLimitCalculator, true, Optional.empty()))
         .difficultyCalculator(MainnetDifficultyCalculators.HOMESTEAD)
         .name("Homestead");
@@ -277,7 +277,7 @@ public static ProtocolSpecBuilder spuriousDragonDefinition(
                     SPURIOUS_DRAGON_FORCE_DELETE_WHEN_EMPTY_ADDRESSES))
         .transactionValidatorBuilder(
             (gasCalculator, gasLimitCalculator) ->
-                new MainnetTransactionValidator(gasCalculator, gasLimitCalculator, true, chainId))
+                new TransactionValidatorFactory(gasCalculator, gasLimitCalculator, true, chainId))
         .transactionProcessorBuilder(
             (gasCalculator,
                 transactionValidator,
@@ -412,7 +412,7 @@ static ProtocolSpecBuilder berlinDefinition(
         .gasCalculator(BerlinGasCalculator::new)
         .transactionValidatorBuilder(
             (gasCalculator, gasLimitCalculator) ->
-                new MainnetTransactionValidator(
+                new TransactionValidatorFactory(
                     gasCalculator,
                     gasLimitCalculator,
                     true,
@@ -452,7 +452,7 @@ static ProtocolSpecBuilder londonDefinition(
             new LondonTargetingGasLimitCalculator(londonForkBlockNumber, londonFeeMarket))
         .transactionValidatorBuilder(
             (gasCalculator, gasLimitCalculator) ->
-                new MainnetTransactionValidator(
+                new TransactionValidatorFactory(
                     gasCalculator,
                     gasLimitCalculator,
                     londonFeeMarket,
@@ -613,7 +613,7 @@ static ProtocolSpecBuilder shanghaiDefinition(
         // Contract creation rules for EIP-3860 Limit and meter intitcode
         .transactionValidatorBuilder(
             (gasCalculator, gasLimitCalculator) ->
-                new MainnetTransactionValidator(
+                new TransactionValidatorFactory(
                     gasCalculator,
                     gasLimitCalculator,
                     londonFeeMarket,
@@ -696,7 +696,7 @@ static ProtocolSpecBuilder cancunDefinition(
         // change to check for max data gas per block for EIP-4844
         .transactionValidatorBuilder(
             (gasCalculator, gasLimitCalculator) ->
-                new MainnetTransactionValidator(
+                new TransactionValidatorFactory(
                     gasCalculator,
                     gasLimitCalculator,
                     cancunFeeMarket,

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/MainnetTransactionProcessor.java

@@ -64,7 +64,7 @@ public class MainnetTransactionProcessor {
 
   protected final GasCalculator gasCalculator;
 
-  protected final TransactionValidator transactionValidator;
+  protected final TransactionValidatorFactory transactionValidatorFactory;
 
   private final AbstractMessageProcessor contractCreationProcessor;
 
@@ -81,7 +81,7 @@ public class MainnetTransactionProcessor {
 
   public MainnetTransactionProcessor(
       final GasCalculator gasCalculator,
-      final TransactionValidator transactionValidator,
+      final TransactionValidatorFactory transactionValidatorFactory,
       final AbstractMessageProcessor contractCreationProcessor,
       final AbstractMessageProcessor messageCallProcessor,
       final boolean clearEmptyAccounts,
@@ -90,7 +90,7 @@ public MainnetTransactionProcessor(
       final FeeMarket feeMarket,
       final CoinbaseFeePriceCalculator coinbaseFeePriceCalculator) {
     this.gasCalculator = gasCalculator;
-    this.transactionValidator = transactionValidator;
+    this.transactionValidatorFactory = transactionValidatorFactory;
     this.contractCreationProcessor = contractCreationProcessor;
     this.messageCallProcessor = messageCallProcessor;
     this.clearEmptyAccounts = clearEmptyAccounts;
@@ -271,6 +271,7 @@ public TransactionProcessingResult processTransaction(
       final PrivateMetadataUpdater privateMetadataUpdater,
       final Wei dataGasPrice) {
     try {
+      final var transactionValidator = transactionValidatorFactory.get();
       LOG.trace("Starting execution of {}", transaction);
       ValidationResult<TransactionInvalidReason> validationResult =
           transactionValidator.validate(
@@ -498,10 +499,6 @@ public TransactionProcessingResult processTransaction(
     }
   }
 
-  public TransactionValidator getTransactionValidator() {
-    return transactionValidator;
-  }
-
   protected void process(final MessageFrame frame, final OperationTracer operationTracer) {
     final AbstractMessageProcessor executor = getMessageProcessor(frame.getType());
 

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/MainnetTransactionValidator.java

@@ -22,7 +22,6 @@
 import org.hyperledger.besu.datatypes.TransactionType;
 import org.hyperledger.besu.datatypes.Wei;
 import org.hyperledger.besu.ethereum.GasLimitCalculator;
-import org.hyperledger.besu.ethereum.core.PermissionTransactionFilter;
 import org.hyperledger.besu.ethereum.core.Transaction;
 import org.hyperledger.besu.ethereum.mainnet.feemarket.FeeMarket;
 import org.hyperledger.besu.ethereum.transaction.TransactionInvalidReason;
@@ -49,40 +48,10 @@ public class MainnetTransactionValidator implements TransactionValidator {
 
   private final Optional<BigInteger> chainId;
 
-  private Optional<PermissionTransactionFilter> permissionTransactionFilter = Optional.empty();
   private final Set<TransactionType> acceptedTransactionTypes;
 
   private final int maxInitcodeSize;
 
-  public MainnetTransactionValidator(
-      final GasCalculator gasCalculator,
-      final GasLimitCalculator gasLimitCalculator,
-      final boolean checkSignatureMalleability,
-      final Optional<BigInteger> chainId) {
-    this(
-        gasCalculator,
-        gasLimitCalculator,
-        checkSignatureMalleability,
-        chainId,
-        Set.of(TransactionType.FRONTIER));
-  }
-
-  public MainnetTransactionValidator(
-      final GasCalculator gasCalculator,
-      final GasLimitCalculator gasLimitCalculator,
-      final boolean checkSignatureMalleability,
-      final Optional<BigInteger> chainId,
-      final Set<TransactionType> acceptedTransactionTypes) {
-    this(
-        gasCalculator,
-        gasLimitCalculator,
-        FeeMarket.legacy(),
-        checkSignatureMalleability,
-        chainId,
-        acceptedTransactionTypes,
-        Integer.MAX_VALUE);
-  }
-
   public MainnetTransactionValidator(
       final GasCalculator gasCalculator,
       final GasLimitCalculator gasLimitCalculator,
@@ -239,12 +208,6 @@ public ValidationResult<TransactionInvalidReason> validateForSender(
               transaction.getSender()));
     }
 
-    if (!isSenderAllowed(transaction, validationParams)) {
-      return ValidationResult.invalid(
-          TransactionInvalidReason.TX_SENDER_NOT_AUTHORIZED,
-          String.format("Sender %s is not on the Account Allowlist", transaction.getSender()));
-    }
-
     return ValidationResult.valid();
   }
 
@@ -286,26 +249,4 @@ private ValidationResult<TransactionInvalidReason> validateTransactionSignature(
     }
     return ValidationResult.valid();
   }
-
-  private boolean isSenderAllowed(
-      final Transaction transaction, final TransactionValidationParams validationParams) {
-    if (validationParams.checkLocalPermissions() || validationParams.checkOnchainPermissions()) {
-      return permissionTransactionFilter
-          .map(
-              c ->
-                  c.permitted(
-                      transaction,
-                      validationParams.checkLocalPermissions(),
-                      validationParams.checkOnchainPermissions()))
-          .orElse(true);
-    } else {
-      return true;
-    }
-  }
-
-  @Override
-  public void setPermissionTransactionFilter(
-      final PermissionTransactionFilter permissionTransactionFilter) {
-    this.permissionTransactionFilter = Optional.of(permissionTransactionFilter);
-  }
 }