-
Notifications
You must be signed in to change notification settings - Fork 26
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): externalize verification dependencies [part 1/2] #859
refactor(verification): externalize verification dependencies [part 1/2] #859
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## refactor/feature-activation/enable-usage #859 +/- ##
===========================================================================
Coverage ? 85.43%
===========================================================================
Files ? 290
Lines ? 22500
Branches ? 3384
===========================================================================
Hits ? 19222
Misses ? 2609
Partials ? 669 ☔ View full report in Codecov by Sentry. |
8cff879
to
a7635d4
Compare
8c08e8c
to
581399b
Compare
a7635d4
to
1bef803
Compare
581399b
to
a89dde8
Compare
2da64c0
to
492f364
Compare
|
||
|
||
@dataclass(frozen=True, slots=True, kw_only=True) | ||
class VertexDependencies: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can simplify it a lot by just implementing a simple memory database.
class SimpleMemoryDB:
def __init__(self):
self._db: Dict[VertexId, BaseTransaction] = {}
def add_vertex(self, vertex: BaseTransaction) -> None:
self._db[vertex.hash] = vertex
def get_vertex(self, _id: VertexId) -> BaseTransaction:
return self._db[_id]
It might have support for metadata too if needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR was completely changed, so I recommend re-reviewing it. Indeed the SimpleMemoryDB
was a good solution for tx parents and spent txs, and I implemented it. But I needed to keep the verification model abstractions for some other dependencies.
50171eb
to
5693c13
Compare
2473e1b
to
b5266b2
Compare
b5266b2
to
774ccf4
Compare
c8261f4
to
021a595
Compare
d0f339c
to
a73d566
Compare
1618015
to
2a6d4d0
Compare
4540afe
to
d82a5db
Compare
2a6d4d0
to
02efe61
Compare
d82a5db
to
a12297b
Compare
02efe61
to
6397eb3
Compare
a12297b
to
eb0f05d
Compare
91f19c2
to
9e7caf3
Compare
eb0f05d
to
43d2174
Compare
f156d96
to
6b0c206
Compare
43d2174
to
3911dbf
Compare
3911dbf
to
96c6ab0
Compare
6b0c206
to
6578961
Compare
Depends on #896
Motivation
This PR introduces the concept of verification dependencies. Before, to perform verifications, 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.
There are also some other side-benefits:
TransactionStorage
from inside vertices.TransactionStorage
(as discussed here).VerificationService
methods.Note: in this PR, not all ad-hoc dependencies have been moved yet. This will be completed in the next PR.
Acceptance Criteria
SimpleMemoryStorage
.Checklist
master
, confirm this code is production-ready and can be included in future releases as soon as it gets merged