-
Notifications
You must be signed in to change notification settings - Fork 290
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
feat: finish msg inclusion api #681
Conversation
Codecov Report
@@ Coverage Diff @@
## main #681 +/- ##
==========================================
+ Coverage 39.28% 40.73% +1.44%
==========================================
Files 27 29 +2
Lines 2795 2897 +102
==========================================
+ Hits 1098 1180 +82
- Misses 1609 1628 +19
- Partials 88 89 +1
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
2bc32cf
to
09e022e
Compare
Co-authored-by: Rootul Patel <rootulp@gmail.com>
Co-authored-by: Rootul Patel <rootulp@gmail.com>
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.
LGTM. I'm not sure if my intuition on how to draw the merkle tree of row roots is correct so open to feedback there
expected []path | ||
} | ||
tests := []test{ | ||
{2, 0, 1, []path{{instructions: []WalkInstruction{WalkLeft}, row: 0}}}, |
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.
[no change needed] visual
row 0 -> |1| |
row 1 -> | | |
merkle tree of row 0
x
/ \
1 x
where I assume x
doesn't matter
{2, 0, 1, []path{{instructions: []WalkInstruction{WalkLeft}, row: 0}}}, | ||
{2, 2, 2, []path{{instructions: []WalkInstruction{}, row: 1}}}, | ||
{2, 1, 2, []path{{instructions: []WalkInstruction{}, row: 1}}}, | ||
{4, 2, 2, []path{{instructions: []WalkInstruction{WalkRight}, row: 0}}}, |
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.
[no change needed] visual
row 0 -> | | |2|2|
row 2 -> | | | | |
row 3 -> | | | | |
row 4 -> | | | | |
merkle tree of row 0
x
/ \
x y
/ \
2 2
where y
is what we want
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.
exactly!
going to merge this later tonight w/o further feedback, as its blocking creating a PR for the |
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.
🚀
|
||
return paths | ||
} | ||
|
||
// genSubTreeRootPath calculates the path to a given subtree root of a node, given the | ||
// depth and position of the node. note: the root of the tree is depth 0. | ||
// The following nolint can be removed after this function is used. | ||
//nolint:unused,deadcode |
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.
[Non Blocking][Suggestion]
Probably can be removed, as this function is now used.
//nolint:unused,deadcode |
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.
nice! good find, I'll if there is other feedback, I'll include this in this PR, if not, I'll be sure to include this in the next ProcessProposal
PR as to not dismiss reviews.
finishes the api for message inclusion
part of #382
closes #381