multiverse: add overlay points for universe trees #624
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
guggero
changed the title
Roasbeef
reviewed
Oct 25, 2023
Roasbeef
reviewed
Oct 26, 2023
guggero
force-pushed
the
multiverse-trees
branch
from
55bbfd9 to
7a9001b
Compare
|
Addressed all comments so far and added unit tests. |
guggero
force-pushed
the
multiverse-trees
branch
from
7a9001b to
e5ce9ff
Compare
Roasbeef
reviewed
Nov 2, 2023
universe/base.go
Outdated
| log.Debugf("Fetching multiverse root for proof type: %v", proofType) | ||
|
|
||
| leaveIDs, err := a.cfg.Multiverse.FetchMultiverseLeaves( | ||
| ctx, nil, nil, proofType, |
There was a problem hiding this comment.
Rather than fetch everything all the time, we can instead pass the identifier in (if they exist) and use db filtering for it.
There was a problem hiding this comment.
I don't think this is resolved in the latest commits. I think what Roas says still makes sense here. I'll unresolve this conversation.
There was a problem hiding this comment.
I think this is now addressed with @Roasbeef's additional commits.
|
Will re-open this against my main branch. I think the only thing that needs to change is that: the leaf value for the multiverse tree is the root of a universe tree itself. So when we go to try and make the in memory version of the tree, we can just use the value directly (hash+sum). |
|
I'm going to update the milestone for this PR to 0.3.2 because the issue it fixes has been moved to the 0.3.2 milestone. |
Roasbeef
force-pushed
the
multiverse-trees
branch
2 times, most recently
from
0ac6a37 to
acac981
Compare
Roasbeef
reviewed
Nov 14, 2023
Roasbeef
requested review from
ffranr and
jharveyb
and removed request for
jharveyb
Roasbeef
force-pushed
the
multiverse-trees
branch
from
44e27ee to
43e012e
Compare
guggero
force-pushed
the
multiverse-trees
branch
from
31653fd to
0693fb1
Compare
|
Need to add another unit test that @jharveyb highlighted |
guggero
force-pushed
the
multiverse-trees
branch
from
0693fb1 to
79a5d71
Compare
|
Okay, updated unit test is in, this would now catch the issue with the unique index. Ready for final review. |
guggero
force-pushed
the
multiverse-trees
branch
from
79a5d71 to
9d55839
Compare
This commit adds two new tables for the two multiverse trees we currently have: multiverse_roots (will currently only contain two entries, one for the issuance and one for the transfer multiverse roots) and multiverse_leaves (which will contain an entry for each issuance and proof universe we currently have). We already have all information needed to fill these tables for existing universes, so we can use conditional INSERT statements to create the entries for all existing universes. Any new universes will be added through the new upsert methods added.
Otherwise it conflicts with the new Option[T] type
By modifying this query we can simplify other logic as now we get the full context which includes the value+sum of the SMT leaf. In this case, the value is the universe root hash, and the sum the root sum.
This query will be used to fetch the multiverse leaf directly, without first needing to make a temp SMT tree.
In this commit, we rename RootNode to MultiverseRootNode as we can return multiple root types. We also modify the query to use the new `FetchMultiverseRoot` method as well.
These will be used in the database to modify the existing FetchMultiverseLeaves call to use a single DB query.
In this commit, we modify the FetchMultiverseLeaves method to allow it to return either all the leaves, or a subset of leaves filtered by the `MultiverseLeafDesc` type. In the future, we can further extend this to use postgres batch queries so we can get the resp in a single round trip.
In this commit, we modify the `MultiverseRoot` method to use a single DB query. If we don't have anything to filter, then we'll fetch the global root for that proof type. Otherwise, we'll query the DB for the leaves that match the set of IDs, then insert those into our in-memory tree. This saves us N DB queries where N is the size of the set of Identifier.
guggero
force-pushed
the
multiverse-trees
branch
from
9d55839 to
55f07b1
Compare
|
Post-merge approval 💯 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #574.
Fixes #621.
Depends on the caching PR (not up yet).