Skip to content
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

refactor(verification): move transaction-only verification methods [part 4/5] #799

Merged
merged 1 commit into from
Oct 25, 2023

Conversation

glevco
Copy link
Contributor

@glevco glevco commented Sep 29, 2023

Depends on #798

Motivation

This is the fourth 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 the Transaction inheritance branch.

Acceptance Criteria

  • Remove all Transaction and TokenCreationTransaction verification methods from the original model classes, moving the implementation to each respective Verifier class. The only exception is the verify_outputs() method, which is part of the inheritance tree up to BaseTransaction, and will be moved in a separate PR.
  • check_authorities_and_deposit() is renamed to verify_authorities_and_deposit(), for consistency.
  • update_token_info_from_outputs() is also moved as it's only used by verification.

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): move transaction-only verification methods [part 4] refactor(verification): move transaction-only verification methods [part 4/5] Sep 29, 2023
@codecov
Copy link

codecov bot commented Sep 29, 2023

Codecov Report

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

Comparison is base (05f7228) 84.59% compared to head (2ca6cb6) 84.58%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #799      +/-   ##
==========================================
- Coverage   84.59%   84.58%   -0.02%     
==========================================
  Files         269      269              
  Lines       22296    22277      -19     
  Branches     3401     3401              
==========================================
- Hits        18861    18842      -19     
  Misses       2765     2765              
  Partials      670      670              
Files Coverage Δ
hathor/transaction/token_creation_tx.py 99.15% <100.00%> (-0.13%) ⬇️
hathor/transaction/transaction.py 94.92% <100.00%> (+1.25%) ⬆️
...erification/token_creation_transaction_verifier.py 100.00% <100.00%> (ø)
hathor/verification/transaction_verifier.py 91.89% <91.07%> (-1.99%) ⬇️

... and 6 files with indirect coverage changes

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

@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/block-methods branch from 3846066 to 222e32f Compare October 10, 2023 02:32
@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/block-methods branch from 222e32f to 6039c40 Compare October 10, 2023 02:53
@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/block-methods branch from 6039c40 to 727b499 Compare October 10, 2023 20:51
@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/block-methods branch from 31d5d85 to 95e2def Compare October 11, 2023 20:33
@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/block-methods branch from 95e2def to 5fd2094 Compare October 17, 2023 21:03
@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/block-methods branch from 5fd2094 to 2b0c232 Compare October 19, 2023 20:29
@glevco glevco force-pushed the refactor/move-verification/transaction-methods branch from 7407fc6 to c5ae653 Compare October 19, 2023 20:30
msbrogli
msbrogli previously approved these changes Oct 24, 2023
@glevco glevco force-pushed the refactor/move-verification/block-methods branch from 61cc04b to 1a5a08a Compare October 24, 2023 19:50
@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/block-methods branch 2 times, most recently from 40baa10 to 94b9075 Compare October 24, 2023 21:45
Base automatically changed from refactor/move-verification/block-methods to master October 24, 2023 23:37
@glevco glevco dismissed stale reviews from msbrogli and jansegre October 24, 2023 23:37

The base branch was changed.

@glevco glevco force-pushed the refactor/move-verification/transaction-methods branch from 78890ea to 389903c Compare October 24, 2023 23:39
msbrogli
msbrogli previously approved these changes Oct 25, 2023
@glevco glevco force-pushed the refactor/move-verification/transaction-methods branch from 432abb0 to 2ca6cb6 Compare October 25, 2023 13:51
@glevco glevco merged commit bd11bc4 into master Oct 25, 2023
@glevco glevco deleted the refactor/move-verification/transaction-methods branch October 25, 2023 15:06
@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