Skip to content

Conversation

fab-10
Copy link
Contributor

@fab-10 fab-10 commented Jun 10, 2025

PR description

This PR fills a gap and allows for plugin to introduce additional transaction validation rules, on top of the standard protocol validation that is already performed, and such the rules introduced by the plugin can only restrict what is valid, and cannot revert any standard validation.
For example a network that want to ban a specific kind of txs, can do that implementing a plugin that make use of this new service.

The additional validation rules are enforced every time a transaction validation is needed, basically they extend the protocol validation rules for transactions:

  • before accepting the tx in the pool
  • when importing a block
  • when building a block

This new feature complements the other 2 feature that extend the transaction validator, but are meant for different use cases:

  • TransactionPoolValidatorService is only meant to validate tx before they are accepted in the pool, and does not have any effect on block time validation, and does not alter the protocol rules.
  • PermissioningService is only meant for validating if the sender is permitted to send the tx, but does not alter in any way the protocol rules. In theory this specific case could be implemented on top of the more generic new TransactionValidatorService but this could be the scope of a follow up.

Fixed Issue(s)

Thanks for sending a pull request! Have you done the following?

  • Checked out our contribution guidelines?
  • Considered documentation and added the doc-change-required label to this PR if updates are required.
  • Considered the changelog and included an update if required.
  • For database changes (e.g. KeyValueSegmentIdentifier) considered compatibility and performed forwards and backwards compatibility tests

Locally, you can run these tests to catch failures early:

  • spotless: ./gradlew spotlessApply
  • unit tests: ./gradlew build
  • acceptance tests: ./gradlew acceptanceTest
  • integration tests: ./gradlew integrationTest
  • reference tests: ./gradlew ethereum:referenceTests:referenceTests

@fab-10 fab-10 changed the title Extend transaction validation by plugin Introduce the TransactionValidatorService to allow plugins to add custom validation rules Jun 10, 2025
@fab-10 fab-10 force-pushed the extend-transaction-validation-by-plugin branch 5 times, most recently from 6665edd to f6192d6 Compare June 11, 2025 07:54
@fab-10 fab-10 marked this pull request as ready for review June 11, 2025 08:25
@fab-10 fab-10 force-pushed the extend-transaction-validation-by-plugin branch from f6192d6 to 4ffcaba Compare June 11, 2025 08:48
*
* @return the transaction validators
*/
public List<TransactionValidationRule> getTransactionValidators() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the naming is a bit confusing - should this be getTransactionValidationRules ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good catch, I missed to rename this one

@fab-10 fab-10 force-pushed the extend-transaction-validation-by-plugin branch 2 times, most recently from 287c2b1 to 2c8b078 Compare June 11, 2025 13:10
public void setAdditionalValidationRules(
final List<TransactionValidationRule> additionalValidationRules) {
final TransactionValidator baseTxValidator = transactionValidatorSupplier.get();
transactionValidatorSupplier =
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: can we return the baseTxValidator if no additiionalValidationRules are present?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this only called when there are additional rules, because the check to see if the list is empty is done in the RunnerBuilder but probably is clearer to move it here

Copy link
Contributor

@Gabriel-Trintinalia Gabriel-Trintinalia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Very nice feature! It gave me an idea to refactor the MainnetTransactionValidator to have a list of rules as well. Similar to what we do for the transaction selectors. Then we can extend it by forks similar to what we do the header validation rules. I think I will work on a refactor next week

fab-10 and others added 4 commits June 11, 2025 16:40
…ustom validation rules

Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
Co-authored-by: Sally MacFarlane <macfarla.github@gmail.com>
Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
@fab-10 fab-10 force-pushed the extend-transaction-validation-by-plugin branch from 2c8b078 to a76bb93 Compare June 11, 2025 14:40
@fab-10
Copy link
Contributor Author

fab-10 commented Jun 11, 2025

LGTM. Very nice feature! It gave me an idea to refactor the MainnetTransactionValidator to have a list of rules as well. Similar to what we do for the transaction selectors. Then we can extend it by forks similar to what we do the header validation rules. I think I will work on a refactor next week

Indeed, sounds good!

@fab-10 fab-10 merged commit 2783ebb into hyperledger:main Jun 11, 2025
48 checks passed
@fab-10 fab-10 deleted the extend-transaction-validation-by-plugin branch June 11, 2025 15:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants