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): implement VerificationDependencies and VerificationModel [part 7/12] #1022

Open
wants to merge 1 commit into
base: feat/multiv/async-on-new-vertex-6
Choose a base branch
from

Conversation

glevco
Copy link
Contributor

@glevco glevco commented May 6, 2024

Depends on #1019


Motivation

This PR introduces the concept of Verification Dependencies and Model dataclasses. Before, to perform vertex verification, it was necessary to retrieve data from the vertex itself and also from multiple other sources, for example from the TransactionStorage.

Now, instead of doing this dependency retrieval one by one interspersed in the whole verification process, a single point retrieves all necessary dependencies and everything is verified from the vertex data plus those pre-calculated dependencies. This prevents unnecessary duplicate calculations, and will be necessary for the Multiprocess Verification project.

In previous cancelled PRs, we were going to use a class abstraction over the TransactionStorage, a SimpleMemoryStorage, but this was becoming convoluted in later PRs, considering Sync-V2 agents will also have their own local memory storages. The abstraction had to include several methods to comply with all callers, however each caller only used a few of them. Therefore, we cancelled that approach and went with a functional abstraction instead: methods that calculate verification dependencies and that rely on a storage (for example, the DAA methods), are now higher-order functions, accepting functional parameters to perform their calculations.

Note: in this PR, not all ad-hoc dependencies have been moved yet. This will be completed in the next PR, by using this functional approach for all necessary calculations.

Acceptance Criteria

  • Implement verification dependencies and model dataclasses.
  • Remove SimpleMemoryStorage.
  • Refactor DAA methods to use a functional abstraction for dependencies.
  • Removed unused DAA from VertexVerifier.

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

Copy link

codecov bot commented May 6, 2024

Codecov Report

Attention: Patch coverage is 56.75676% with 48 lines in your changes missing coverage. Please review.

Please upload report for BASE (feat/multiv/async-on-new-vertex-6@054a739). Learn more about missing BASE report.

Files with missing lines Patch % Lines
hathor/verification/verification_model.py 52.63% 27 Missing ⚠️
hathor/verification/verification_dependencies.py 61.11% 21 Missing ⚠️
Additional details and impacted files
@@                         Coverage Diff                          @@
##             feat/multiv/async-on-new-vertex-6    #1022   +/-   ##
====================================================================
  Coverage                                     ?   84.25%           
====================================================================
  Files                                        ?      320           
  Lines                                        ?    24356           
  Branches                                     ?     3713           
====================================================================
  Hits                                         ?    20521           
  Misses                                       ?     3113           
  Partials                                     ?      722           

☔ 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 May 7, 2024 03:05
@glevco glevco requested review from jansegre and msbrogli as code owners May 7, 2024 03:05
@glevco glevco force-pushed the feat/multiv/async-on-new-vertex-6 branch from 9535b77 to 4b6a318 Compare May 8, 2024 03:05
@glevco glevco force-pushed the refactor/verification-dependencies-7 branch from 42f7cbf to 7a6d76d Compare May 8, 2024 03:06
@glevco glevco force-pushed the feat/multiv/async-on-new-vertex-6 branch from 4b6a318 to 552dc05 Compare May 10, 2024 02:16
@glevco glevco force-pushed the refactor/verification-dependencies-7 branch from 7a6d76d to e799dce Compare May 10, 2024 02:28
@glevco glevco force-pushed the feat/multiv/async-on-new-vertex-6 branch from 552dc05 to 2cdd9b3 Compare May 10, 2024 02:50
@glevco glevco force-pushed the refactor/verification-dependencies-7 branch 2 times, most recently from 1782d6b to 3d4fb3e Compare May 10, 2024 02:51
@glevco glevco force-pushed the feat/multiv/async-on-new-vertex-6 branch from 2cdd9b3 to 264da11 Compare May 13, 2024 14:26
@glevco glevco force-pushed the refactor/verification-dependencies-7 branch from 3d4fb3e to 10e8be8 Compare May 13, 2024 14:31
@glevco glevco changed the title refactor(verification): implement VerificationDependencies and VerificationModel [part 7] refactor(verification): implement VerificationDependencies and VerificationModel [part 7/12] May 13, 2024
@glevco glevco force-pushed the feat/multiv/async-on-new-vertex-6 branch from 264da11 to 3529d4a Compare August 23, 2024 21:47
@glevco glevco force-pushed the refactor/verification-dependencies-7 branch 3 times, most recently from 5f3632f to 5222dfe Compare August 26, 2024 17:20
@glevco glevco force-pushed the feat/multiv/async-on-new-vertex-6 branch from 3529d4a to 054a739 Compare September 18, 2024 18:06
@glevco glevco force-pushed the refactor/verification-dependencies-7 branch from 5222dfe to e65288f Compare September 18, 2024 18:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Todo
Development

Successfully merging this pull request may close these issues.

1 participant