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): refactor verify_sum [part 2/9] #831

Merged
merged 1 commit into from
Nov 16, 2023

Conversation

glevco
Copy link
Contributor

@glevco glevco commented Oct 27, 2023

Depends on #830

Motivation

Currently, TokenCreationTransactionVerifier.verify_sum() has duplicate code from TransactionVerifier.verify_sum(). This PR changes it so it calls the super method instead.

Acceptance Criteria

  • Implement TransactionVerifier.get_complete_token_info() and update its verify_sum() to use it.
  • Implement TokenCreationTransaction.get_token_info_from_inputs(), overriding the Transaction implementation.
  • Update TokenCreationTransaction.verify_sum() to use the new get_complete_token_info().
  • No behavior should be changed by this PR.

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 Oct 27, 2023
@glevco glevco changed the title refactor(verification): refactor verify_sum refactor(verification): refactor verify_sum [part 2/9] Oct 27, 2023
@codecov
Copy link

codecov bot commented Oct 27, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (13e49bf) 85.28% compared to head (c084103) 85.19%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #831      +/-   ##
==========================================
- Coverage   85.28%   85.19%   -0.09%     
==========================================
  Files         281      281              
  Lines       22354    22363       +9     
  Branches     3387     3388       +1     
==========================================
- Hits        19065    19053      -12     
- Misses       2608     2624      +16     
- Partials      681      686       +5     

☔ 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 November 3, 2023 16:10
@glevco glevco requested a review from jansegre as a code owner November 3, 2023 16:10
@glevco glevco force-pushed the refactor/verification-inheritance/1-improvements branch 3 times, most recently from 99af497 to 50281e7 Compare November 7, 2023 16:22
@glevco glevco force-pushed the refactor/verification-inheritance/2-verify-sum branch from 6811073 to 005adad Compare November 7, 2023 21:20
@glevco glevco force-pushed the refactor/verification-inheritance/1-improvements branch from 50281e7 to c632ffe Compare November 7, 2023 21:31
@glevco glevco force-pushed the refactor/verification-inheritance/2-verify-sum branch from 005adad to 84f38ae Compare November 7, 2023 21:31
Base automatically changed from refactor/verification-inheritance/1-improvements to master November 7, 2023 22:26
@glevco glevco force-pushed the refactor/verification-inheritance/2-verify-sum branch from 84f38ae to ba246bc Compare November 9, 2023 12:23
@glevco glevco force-pushed the refactor/verification-inheritance/2-verify-sum branch 2 times, most recently from c69073d to 2e709e9 Compare November 9, 2023 19:27
@glevco glevco force-pushed the refactor/verification-inheritance/2-verify-sum branch 2 times, most recently from dc55893 to 073d6f6 Compare November 10, 2023 16:31
@glevco glevco force-pushed the refactor/verification-inheritance/2-verify-sum branch from 073d6f6 to c084103 Compare November 16, 2023 17:02
@glevco glevco merged commit 351e4d3 into master Nov 16, 2023
9 checks passed
@glevco glevco deleted the refactor/verification-inheritance/2-verify-sum branch November 16, 2023 20:01
@jansegre jansegre mentioned this pull request Nov 24, 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