-
Notifications
You must be signed in to change notification settings - Fork 959
Introduce the TransactionValidatorService
to allow plugins to add custom validation rules
#8793
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Introduce the TransactionValidatorService
to allow plugins to add custom validation rules
#8793
Conversation
TransactionValidatorService
to allow plugins to add custom validation rules
6665edd
to
f6192d6
Compare
f6192d6
to
4ffcaba
Compare
...core/src/main/java/org/hyperledger/besu/ethereum/mainnet/ExtendableTransactionValidator.java
Outdated
Show resolved
Hide resolved
...core/src/main/java/org/hyperledger/besu/ethereum/mainnet/ExtendableTransactionValidator.java
Outdated
Show resolved
Hide resolved
...core/src/main/java/org/hyperledger/besu/ethereum/mainnet/ExtendableTransactionValidator.java
Outdated
Show resolved
Hide resolved
* | ||
* @return the transaction validators | ||
*/ | ||
public List<TransactionValidationRule> getTransactionValidators() { |
There was a problem hiding this comment.
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 ?
There was a problem hiding this comment.
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
.../src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/response/RpcErrorType.java
Show resolved
Hide resolved
287c2b1
to
2c8b078
Compare
public void setAdditionalValidationRules( | ||
final List<TransactionValidationRule> additionalValidationRules) { | ||
final TransactionValidator baseTxValidator = transactionValidatorSupplier.get(); | ||
transactionValidatorSupplier = |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
There was a problem hiding this 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
…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>
2c8b078
to
a76bb93
Compare
Indeed, sounds good! |
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:
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 newTransactionValidatorService
but this could be the scope of a follow up.Fixed Issue(s)
Thanks for sending a pull request! Have you done the following?
doc-change-required
label to this PR if updates are required.Locally, you can run these tests to catch failures early:
./gradlew spotlessApply
./gradlew build
./gradlew acceptanceTest
./gradlew integrationTest
./gradlew ethereum:referenceTests:referenceTests