tree: allow batching new leaf nodes #363
Conversation
gballet
force-pushed
the
jsign-cowleaves2
branch
3 times, most recently
from
7597228 to
9dbd43e
Compare
There was a problem hiding this comment.
@jsign ok so it was weird, but it wasn't really github action's fault: CI will merge your PR branch with main during the test, making sure that the merge can work. It's something that needs to happen.
The problem is that if you rebase your branch on top of master, there are a few problems appearing. This is why it worked locally for both of us: we were not rebasing. I have fixed the first problem, but there is now one with a nil commitment.
tree_test.go
Outdated
| valIdx := 42 | ||
| values[valIdx] = testValue | ||
| ln := NewLeafNode(ffx32KeyTest[:StemSize], values) |
There was a problem hiding this comment.
We can't create "direct leaf node" now since all the internal data is lazily computed. This caused a crash later when doing ln.Hash() called (*LeafNode).Commitment(), which panicked since there wasn't any calculated commitment.
The solution is always to use the tree from a properly created tree, since before inspecting internals of things you should always call tree.Commit() to do all the pending calculations.
Another solution is making (*LeafNode).Commitment() not panicked if commitment == nil, so compute it if that's the case. Technically speaking, that would be my preferred solution since we shouldn't have panicked but considering that case should never happen...
gballet
force-pushed
the
jsign-cowleaves2
branch
from
22e2491 to
1e7745d
Compare
|
Note to self when I revisit this: This PR is kept on hold for a little bit, as it needs to be rebased and it fill conflict with more upcoming PRs. It should not decrease performance, so it's safe to merge. |
Signed-off-by: Ignacio Hagopian <[email protected]> tree: simplify batching of new leaves creation Signed-off-by: Ignacio Hagopian <[email protected]> tree: fix insert new leaves test and benchmark Signed-off-by: Ignacio Hagopian <[email protected]> remove comment Signed-off-by: Ignacio Hagopian <[email protected]> remove conversion file Signed-off-by: Ignacio Hagopian <[email protected]> remove unused method Signed-off-by: Ignacio Hagopian <[email protected]> tree: add comment to explain different strategies on leaf node value updating Signed-off-by: Ignacio Hagopian <[email protected]>
Signed-off-by: Ignacio Hagopian <[email protected]>
Signed-off-by: Ignacio Hagopian <[email protected]>
Update: rebased to |
Try again to run #349 with a different branch name...