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(sync-v2): implement P2PStorage and P2PVertexHandler [part 9/12] #1024

Open
wants to merge 1 commit into
base: refactor/verification-dependencies-8
Choose a base branch
from

Conversation

glevco
Copy link
Contributor

@glevco glevco commented May 6, 2024

Depends on #1023


Motivation

This PR begins a refactor in Sync-v2 that will allow it to work asynchronously, meaning it can receive and download vertices from other peers even if previously received vertices have not been verified, through the consensus, and saved yet. This is necessary for the Multiprocess Verification project.

In order to do this, each agent saves received vertices in a memory storage local to only that agent. After a vertex completes the pipeline, it is saved in the persisted TransactionStorage and removed from the agent-specific memory storage.

This PR introduces the P2PStorage class, which is an abstraction layer over the TransactionStorage, and in the next PR an async version of this class will be implemented with in-memory capabilities. This is done by providing two method versions for each storage method that is used by Sync-v2: one "local" and one "non-local". Generally speaking, when downloading vertices, an agent uses "local" methods — meaning it can use its in-memory data to advance the sync. Conversely, when uploading vertices, the agent uses "non-local" methods, which are simply a forward to the persisted storage.

A similar abstraction is introduced for VertexHandler, by the P2PVertexHandler class. It is a single point of contact for Sync-v2 when handling new vertices.

Acceptance Criteria

  • Implement P2PStorage, an abstraction over TransactionStorage and indices to be used in Sync-v2.
  • Implement P2PVertexHandler, and abstraction over VertexHandler to be used in Sync-v2.
  • Update all Sync-v2 calls to the storage to use the respective equivalent P2PStorage method.
  • Move BaseTransaction.can_validate_full to the storage.
  • Update typings in the traversal module so it can receive the more generic VertexStorageProtocol instead of a TransactionStorage.

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

all_valid = False
if meta.validation.is_invalid():
# or any of them is invalid (which would make this one invalid too)
return True
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we return False here?

Copy link

codecov bot commented May 6, 2024

Codecov Report

Attention: Patch coverage is 92.12598% with 10 lines in your changes are missing coverage. Please review.

Project coverage is 84.67%. Comparing base (8416913) to head (bfc28b4).

Files Patch % Lines
hathor/transaction/storage/transaction_storage.py 75.00% 2 Missing and 2 partials ⚠️
hathor/p2p/sync_v2/agent.py 90.00% 0 Missing and 3 partials ⚠️
hathor/p2p/sync_v2/p2p_storage.py 95.65% 2 Missing ⚠️
hathor/manager.py 0.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@                           Coverage Diff                            @@
##           refactor/verification-dependencies-8    #1024      +/-   ##
========================================================================
+ Coverage                                 84.66%   84.67%   +0.01%     
========================================================================
  Files                                       302      304       +2     
  Lines                                     23146    23184      +38     
  Branches                                   3515     3515              
========================================================================
+ Hits                                      19596    19631      +35     
- Misses                                     2865     2868       +3     
  Partials                                    685      685              

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

@glevco glevco force-pushed the refactor/verification-dependencies-8 branch from 326d107 to aad0eab Compare May 7, 2024 00:45
@glevco glevco force-pushed the refactor/sync-v2/p2p-storage-9 branch from c35d431 to 98c0ab6 Compare May 7, 2024 02:36
@glevco glevco marked this pull request as ready for review May 7, 2024 03:29
@glevco glevco force-pushed the refactor/verification-dependencies-8 branch from aad0eab to f28c610 Compare May 8, 2024 03:07
@glevco glevco force-pushed the refactor/sync-v2/p2p-storage-9 branch 2 times, most recently from 98c0ab6 to 197c056 Compare May 8, 2024 03:12
@glevco glevco force-pushed the refactor/verification-dependencies-8 branch from f28c610 to 5811a89 Compare May 10, 2024 02:31
@glevco glevco force-pushed the refactor/sync-v2/p2p-storage-9 branch 3 times, most recently from 2c96d59 to 380b917 Compare May 10, 2024 02:34
@glevco glevco force-pushed the refactor/verification-dependencies-8 branch from 5811a89 to 3d42e7a Compare May 10, 2024 02:53
@glevco glevco force-pushed the refactor/sync-v2/p2p-storage-9 branch from 380b917 to df7d74e Compare May 10, 2024 02:53
@glevco glevco force-pushed the refactor/verification-dependencies-8 branch from 3d42e7a to 8416913 Compare May 13, 2024 14:32
@glevco glevco changed the title refactor(sync-v2): implement P2PStorage and P2PVertexHandler [part 9] refactor(sync-v2): implement P2PStorage and P2PVertexHandler [part 9/12] May 13, 2024
@glevco glevco force-pushed the refactor/sync-v2/p2p-storage-9 branch from df7d74e to bfc28b4 Compare May 13, 2024 14:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: In Progress (Done)
Development

Successfully merging this pull request may close these issues.

None yet

1 participant