refactor(verification): move vertex verification to its own service #779
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation
This PR is the first one on a series of refactors related to vertex verification, aiming to move all verification related code out of model classes and to their own files, improving code modularity and enabling other future improvements related to injection of dependencies, patching, etc.
The 4 main entrypoint methods used for validation/verification (detailed below) were moved to a new class and modules. Now, instead of using inheritance, we implement concrete methods and functions that are explicit about from where each implementation is coming from. This also allows to remove unused arguments from method signatures, as they're not overrides of the same method anymore. Verification of each vertex type accepts a specific set of arguments. Other than this change of structure, no behavior should be changed by this PR, it's only a refactor moving code.
In the next PRs, we'll move the remaining verification methods.
Acceptance Criteria
VerificationService
class and(block|transaction|token_creation_transaction)_verification
modules.BaseTransaction
methods toVerificationService
. The only change in them is adding*
to make the signatures more explicit.validate_basic()
andvalidate_full()
concrete methods as-is.verify_basic()
andverify()
abstract methods, making them concrete.validate_tx_error()
renaming it tovalidate_vertex_error()
.Block
,Transaction
andTokenCreationTransaction
methodsverify_basic()
andverify()
to each respective*_verification
module. The only change is also adding*
to their signatures.HathorManager
to use aVerificationService
, and updateBuilder
andCliBuilder
accordingly.VerificationService
instead of calling verification methods directly on vertices, including in tests.Checklist
master
, confirm this code is production-ready and can be included in future releases as soon as it gets merged