Skip to content

refactor(verification): finish moving all verification methods [part 5/5] #800

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

Merged
merged 1 commit into from
Oct 25, 2023

Conversation

glevco
Copy link
Contributor

@glevco glevco commented Sep 29, 2023

Depends on #799

Motivation

This is the last PR in a 5-part series of PRs that completely move verification-related code out of the vertex model classes.

This PR moves the verification method implementations of BaseTransaction, finishing the code migration. This also allows us to remove the verify_pow() simulator patch, as it can now be customized via injected verifiers.

Acceptance Criteria

  • Remove all BaseTransaction verification methods from the original model classes, moving the implementation to the VertexVerifier class.
  • Move each respective verify_outputs() implementation.
  • Create custom Simulator verifiers such that verify_pow() is overridden.
  • Update Simulator to use the custom verifiers, removing the verify_pow() patch.

Checklist

  • If you are requesting a merge into master, confirm this code is production-ready and can be included in future releases as soon as it gets merged

@glevco glevco self-assigned this Sep 29, 2023
@glevco glevco changed the title refactor(verification): finish moving all verification methods [part 5 refactor(verification): finish moving all verification methods [part 5] Sep 29, 2023
@glevco glevco changed the title refactor(verification): finish moving all verification methods [part 5] refactor(verification): finish moving all verification methods [part 5/5] Sep 29, 2023
@glevco glevco marked this pull request as ready for review September 29, 2023 20:46
jansegre
jansegre previously approved these changes Oct 3, 2023
@glevco glevco force-pushed the refactor/move-verification/transaction-methods branch from 9f2245e to 422ec12 Compare October 10, 2023 02:34
@glevco glevco force-pushed the refactor/move-verification/vertex-methods branch from bfc8907 to 9145c97 Compare October 10, 2023 02:35
@glevco glevco force-pushed the refactor/move-verification/transaction-methods branch from 422ec12 to 48feebd Compare October 10, 2023 02:53
@glevco glevco force-pushed the refactor/move-verification/vertex-methods branch from 9145c97 to 7093fa5 Compare October 10, 2023 02:54
@codecov
Copy link

codecov bot commented Oct 10, 2023

Codecov Report

Attention: 18 lines in your changes are missing coverage. Please review.

Comparison is base (deaaaf6) 84.60% compared to head (363be05) 84.57%.

❗ Current head 363be05 differs from pull request most recent head 0a78bb9. Consider uploading reports for the commit 0a78bb9 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #800      +/-   ##
==========================================
- Coverage   84.60%   84.57%   -0.03%     
==========================================
  Files         269      270       +1     
  Lines       22278    22289      +11     
  Branches     3402     3405       +3     
==========================================
+ Hits        18848    18852       +4     
- Misses       2761     2766       +5     
- Partials      669      671       +2     
Files Coverage Δ
hathor/simulator/simulator.py 94.51% <100.00%> (-0.10%) ⬇️
hathor/stratum/stratum.py 69.32% <100.00%> (ø)
hathor/transaction/base_transaction.py 94.15% <100.00%> (-0.01%) ⬇️
hathor/transaction/block.py 94.38% <100.00%> (-0.14%) ⬇️
hathor/transaction/merge_mined_block.py 64.86% <ø> (ø)
hathor/transaction/resources/create_tx.py 92.59% <100.00%> (ø)
hathor/transaction/token_creation_tx.py 99.15% <100.00%> (ø)
hathor/transaction/transaction.py 94.76% <100.00%> (-0.16%) ⬇️
hathor/verification/block_verifier.py 100.00% <100.00%> (ø)
hathor/verification/merge_mined_block_verifier.py 100.00% <100.00%> (ø)
... and 5 more

... and 6 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@glevco glevco force-pushed the refactor/move-verification/transaction-methods branch from 48feebd to 2bc5ebd Compare October 10, 2023 20:52
@glevco glevco force-pushed the refactor/move-verification/vertex-methods branch 2 times, most recently from ca40831 to db9591a Compare October 10, 2023 20:57
@glevco glevco force-pushed the refactor/move-verification/transaction-methods branch from 2bc5ebd to 4af2cfb Compare October 11, 2023 20:34
@glevco glevco force-pushed the refactor/move-verification/vertex-methods branch from db9591a to d6caa5c Compare October 11, 2023 20:35
@glevco glevco force-pushed the refactor/move-verification/transaction-methods branch from 4af2cfb to 7407fc6 Compare October 17, 2023 21:09
@glevco glevco force-pushed the refactor/move-verification/vertex-methods branch from d6caa5c to e62cc24 Compare October 17, 2023 21:14
@glevco glevco force-pushed the refactor/move-verification/transaction-methods branch from 7407fc6 to c5ae653 Compare October 19, 2023 20:30
@glevco glevco force-pushed the refactor/move-verification/vertex-methods branch from e62cc24 to 07ff57b Compare October 19, 2023 20:31
msbrogli
msbrogli previously approved these changes Oct 24, 2023
@glevco glevco force-pushed the refactor/move-verification/transaction-methods branch from c5ae653 to 78890ea Compare October 24, 2023 20:43
@glevco glevco force-pushed the refactor/move-verification/vertex-methods branch from 07ff57b to 363be05 Compare October 24, 2023 20:55
@glevco glevco force-pushed the refactor/move-verification/transaction-methods branch from 78890ea to 389903c Compare October 24, 2023 23:39
@glevco glevco force-pushed the refactor/move-verification/transaction-methods branch from 432abb0 to 2ca6cb6 Compare October 25, 2023 13:51
@glevco glevco force-pushed the refactor/move-verification/vertex-methods branch 2 times, most recently from bc5f263 to dc17bcb Compare October 25, 2023 14:03
Base automatically changed from refactor/move-verification/transaction-methods to master October 25, 2023 15:06
@glevco glevco dismissed stale reviews from msbrogli and jansegre October 25, 2023 15:06

The base branch was changed.

@glevco glevco force-pushed the refactor/move-verification/vertex-methods branch from dc17bcb to f5f7fe7 Compare October 25, 2023 15:07
@glevco glevco force-pushed the refactor/move-verification/vertex-methods branch from f5f7fe7 to bc6f21d Compare October 25, 2023 16:00
@glevco glevco force-pushed the refactor/move-verification/vertex-methods branch from bc6f21d to 0a78bb9 Compare October 25, 2023 17:04
@msbrogli msbrogli merged commit 0a78bb9 into master Oct 25, 2023
@msbrogli msbrogli deleted the refactor/move-verification/vertex-methods branch October 25, 2023 17:10
@jansegre jansegre mentioned this pull request Nov 13, 2023
2 tasks
This was referenced Dec 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants