Skip to content

refactor(verification): remove token info duplication [part 9/9] #868

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

Conversation

glevco
Copy link
Contributor

@glevco glevco commented Nov 10, 2023

Depends on #837

Motivation

Remove unnecessary duplicate calculation of token_info, as requested in this comment: #831 (comment).

Acceptance Criteria

  • Remove TransactionVerifier.verify_sum() as it only called verify_authorities_and_deposit().
  • Change TokenCreationTransactionVerifier.verify_minted_tokens() to receive a token_dict.
  • Change VerificationService._verify_tx() and _verify_token_creation_tx() to prevent duplicate calculation of the token_dict.

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 Nov 10, 2023
@glevco glevco changed the title refactor(verification): remove token info duplication refactor(verification): remove token info duplication [part 9/9] Nov 10, 2023
@glevco glevco marked this pull request as ready for review November 10, 2023 20:06
Copy link

codecov bot commented Nov 10, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (db91e96) 85.25% compared to head (4680b0d) 85.31%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #868      +/-   ##
==========================================
+ Coverage   85.25%   85.31%   +0.06%     
==========================================
  Files         283      283              
  Lines       22415    22416       +1     
  Branches     3397     3397              
==========================================
+ Hits        19110    19125      +15     
+ Misses       2616     2605      -11     
+ Partials      689      686       -3     

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

@glevco glevco force-pushed the refactor/verification-inheritance/8-remove-vertex-tx branch from 53a7f36 to cdc3256 Compare November 16, 2023 17:05
@glevco glevco force-pushed the refactor/verification-inheritance/9-remove-token-info-duplication branch from f88dc48 to ce8988e Compare November 16, 2023 17:05
@glevco glevco force-pushed the refactor/verification-inheritance/8-remove-vertex-tx branch from cdc3256 to aaef3db Compare November 16, 2023 20:06
@glevco glevco force-pushed the refactor/verification-inheritance/9-remove-token-info-duplication branch from ce8988e to 7265352 Compare November 16, 2023 20:06
@glevco glevco force-pushed the refactor/verification-inheritance/8-remove-vertex-tx branch from aaef3db to 230e953 Compare November 16, 2023 21:26
@glevco glevco force-pushed the refactor/verification-inheritance/9-remove-token-info-duplication branch from 7265352 to 97dbeef Compare November 16, 2023 21:26
@glevco glevco force-pushed the refactor/verification-inheritance/9-remove-token-info-duplication branch from 97dbeef to 527a824 Compare November 27, 2023 20:22
@glevco glevco force-pushed the refactor/verification-inheritance/8-remove-vertex-tx branch from b3974c0 to f5048ea Compare November 29, 2023 02:15
@glevco glevco force-pushed the refactor/verification-inheritance/9-remove-token-info-duplication branch from 527a824 to 157e50e Compare November 29, 2023 02:15
@glevco glevco force-pushed the refactor/verification-inheritance/8-remove-vertex-tx branch from f5048ea to 31630fd Compare November 29, 2023 03:09
@glevco glevco force-pushed the refactor/verification-inheritance/9-remove-token-info-duplication branch from 157e50e to 376eaa9 Compare November 29, 2023 03:09
@glevco glevco force-pushed the refactor/verification-inheritance/8-remove-vertex-tx branch from 31630fd to 9cd52aa Compare December 5, 2023 14:34
@glevco glevco force-pushed the refactor/verification-inheritance/9-remove-token-info-duplication branch from 376eaa9 to c24538e Compare December 5, 2023 14:34
jansegre
jansegre previously approved these changes Dec 5, 2023
@glevco glevco force-pushed the refactor/verification-inheritance/8-remove-vertex-tx branch from 9cd52aa to e21db39 Compare December 6, 2023 17:11
@glevco glevco force-pushed the refactor/verification-inheritance/9-remove-token-info-duplication branch from c24538e to c8af584 Compare December 6, 2023 17:11
msbrogli
msbrogli previously approved these changes Dec 6, 2023
Base automatically changed from refactor/verification-inheritance/8-remove-vertex-tx to master December 7, 2023 18:24
@glevco glevco dismissed stale reviews from msbrogli and jansegre December 7, 2023 18:24

The base branch was changed.

@glevco glevco force-pushed the refactor/verification-inheritance/9-remove-token-info-duplication branch from f218205 to 6d9eac9 Compare December 7, 2023 18:25
@glevco glevco force-pushed the refactor/verification-inheritance/9-remove-token-info-duplication branch from 6d9eac9 to 4680b0d Compare December 7, 2023 19:04
@glevco glevco merged commit 4a97f92 into master Dec 7, 2023
@glevco glevco deleted the refactor/verification-inheritance/9-remove-token-info-duplication branch December 7, 2023 19:48
@jansegre jansegre mentioned this pull request Dec 11, 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