Optimize Insertion in Deposit Trie #4299
Merged
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
…into optimizeDepositLogs
…into optimizeDepositLogs
rauljordan
reviewed
Dec 16, 2019
| enc := [32]byte{} | ||
| binary.LittleEndian.PutUint64(enc[:], uint64(len(m.originalItems))) | ||
| return hashutil.Hash(append(m.branches[len(m.branches)-1][0], enc[:]...)) | ||
| } | ||
|
|
||
| // InsertIntoTrie inserts an item(deposit hash) into the trie. | ||
| func (m *SparseMerkleTrie) InsertIntoTrie(item []byte, index int) { | ||
| for index >= len(m.branches[0]) { |
There was a problem hiding this comment.
Given this is a sparse merkle trie, sometimes we may be attempting to insert something at index 15 when only 5 original leaves exist in the trie, so we have to expand it on-demand
| } | ||
| parentIdx := currentIndex / 2 | ||
| // Update the cached layers at the parent index. | ||
| if len(m.branches[i+1]) == 0 || parentIdx >= len(m.branches[i+1]) { |
There was a problem hiding this comment.
We need some special handling here since the trie is sparse and there may not be an actual object at the index we're trying to modify
Codecov Report
@@ Coverage Diff @@
## master #4299 +/- ##
=========================================
Coverage ? 35.68%
=========================================
Files ? 9
Lines ? 524
Branches ? 0
=========================================
Hits ? 187
Misses ? 332
Partials ? 5 |
terencechain
requested changes
Dec 16, 2019
shared/trieutil/sparse_merkle.go
Outdated
| enc := [32]byte{} | ||
| binary.LittleEndian.PutUint64(enc[:], uint64(len(m.originalItems))) | ||
| return hashutil.Hash(append(m.branches[len(m.branches)-1][0], enc[:]...)) | ||
| } | ||
|
|
||
| // InsertIntoTrie inserts an item(deposit hash) into the trie. | ||
| func (m *SparseMerkleTrie) InsertIntoTrie(item []byte, index int) { |
There was a problem hiding this comment.
Would Insert be more clear? Given the receiver is already SparseMerkleTrie
| @@ -43,7 +44,7 @@ func TestDepositTrieRoot_OK(t *testing.T) { | |||
|
|
|||
| testAcc.TxOpts.Value = depositcontract.Amount32Eth() | |||
|
|
|||
| for i := 0; i < 100; i++ { | |||
| for i := 0; i < 5; i++ { | |||
| @@ -32,7 +33,7 @@ func TestDepositTrieRoot_OK(t *testing.T) { | |||
| t.Errorf("Local deposit trie root and contract deposit trie root are not equal. Expected %#x , Got %#x", depRoot, localTrie.Root()) | |||
| } | |||
|
|
|||
| privKeys, pubKeys, err := interop.DeterministicallyGenerateKeys(0 /*startIndex*/, 101) | |||
| privKeys, pubKeys, err := interop.DeterministicallyGenerateKeys(0 /*startIndex*/, 10) | |||
| @@ -6,6 +6,7 @@ import ( | |||
|
|
|||
| "github.com/ethereum/go-ethereum/accounts/abi/bind" | |||
| "github.com/prysmaticlabs/go-ssz" | |||
|
|
|||
…into optimizeDepositLogs
terencechain
previously approved these changes
Dec 17, 2019
rauljordan
approved these changes
Dec 17, 2019
cryptomental
pushed a commit
to cryptomental/prysm
that referenced
this pull request
Feb 24, 2020
* current changes * change algorithm for tree insert * almost done with getting this to pass * unit test passes * tests now pass * fix in repo * Merge branch 'master' into optimizeDepositLogs * fix build * Merge branch 'optimizeDepositLogs' of github.com:prysmaticlabs/prysm into optimizeDepositLogs * remove tautology * fix tautology * fix up sparsity * Merge branch 'master' into optimizeDepositLogs * further fixes * Merge branch 'optimizeDepositLogs' of github.com:prysmaticlabs/prysm into optimizeDepositLogs * Update shared/trieutil/sparse_merkle.go * comments * Merge branch 'optimizeDepositLogs' of github.com:prysmaticlabs/prysm into optimizeDepositLogs * add bench for optimized * gaz * Merge refs/heads/master into optimizeDepositLogs
cryptomental
pushed a commit
to cryptomental/prysm
that referenced
this pull request
Feb 28, 2020
* current changes * change algorithm for tree insert * almost done with getting this to pass * unit test passes * tests now pass * fix in repo * Merge branch 'master' into optimizeDepositLogs * fix build * Merge branch 'optimizeDepositLogs' of github.com:prysmaticlabs/prysm into optimizeDepositLogs * remove tautology * fix tautology * fix up sparsity * Merge branch 'master' into optimizeDepositLogs * further fixes * Merge branch 'optimizeDepositLogs' of github.com:prysmaticlabs/prysm into optimizeDepositLogs * Update shared/trieutil/sparse_merkle.go * comments * Merge branch 'optimizeDepositLogs' of github.com:prysmaticlabs/prysm into optimizeDepositLogs * add bench for optimized * gaz * Merge refs/heads/master into optimizeDepositLogs
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.
Resolves #[PR]
Background
Our current workflow for computing the Merkle trie of eth1 deposits is as follows:
This means every new deposit takes up O(n) in runtime, which makes it difficult to process a large number of deposit logs. This PR brings insertion down to O(log n), as instead we can just recompute the changed branch when a new deposit is seen. This will be a huge improvement to the initial deposit verification flow upon launching the beacon node.
Benchmark Results