Optimize Insertion in Deposit Trie (prysmaticlabs#4299)

* 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 · Feb 24, 2020 · 3db4601 · 3db4601
1 parent 2b380d9
commit 3db4601
Show file tree

Hide file tree

Showing 16 changed files with 164 additions and 123 deletions.
diff --git a/beacon-chain/core/state/state.go b/beacon-chain/core/state/state.go
@@ -152,7 +152,7 @@ func GenesisBeaconState(deposits []*ethpb.Deposit, genesisTime uint64, eth1Data
 		}
 		leaves = append(leaves, hash[:])
 	}
-	var trie *trieutil.MerkleTrie
+	var trie *trieutil.SparseMerkleTrie
 	if len(leaves) > 0 {
 		trie, err = trieutil.GenerateTrieFromItems(leaves, int(params.BeaconConfig().DepositContractTreeDepth))
 		if err != nil {

diff --git a/beacon-chain/powchain/log_processing.go b/beacon-chain/powchain/log_processing.go
@@ -110,7 +110,6 @@ func (s *Service) ProcessDepositLog(ctx context.Context, depositLog gethTypes.Lo
 
 	// We then decode the deposit input in order to create a deposit object
 	// we can store in our persistent DB.
-	validData := true
 	depositData := &ethpb.Deposit_Data{
 		Amount:                bytesutil.FromBytes8(amount),
 		PublicKey:             pubkey,
@@ -123,9 +122,7 @@ func (s *Service) ProcessDepositLog(ctx context.Context, depositLog gethTypes.Lo
 		return errors.Wrap(err, "Unable to determine hashed value of deposit")
 	}
 
-	if err := s.depositTrie.InsertIntoTrie(depositHash[:], int(index)); err != nil {
-		return errors.Wrap(err, "Unable to insert deposit into trie")
-	}
+	s.depositTrie.Insert(depositHash[:], int(index))
 
 	proof, err := s.depositTrie.MerkleProof(int(index))
 	if err != nil {
@@ -138,7 +135,7 @@ func (s *Service) ProcessDepositLog(ctx context.Context, depositLog gethTypes.Lo
 	}
 
 	// Make sure duplicates are rejected pre-chainstart.
-	if !s.chainStarted && validData {
+	if !s.chainStarted {
 		var pubkey = fmt.Sprintf("#%x", depositData.PublicKey)
 		if s.depositCache.PubkeyInChainstart(ctx, pubkey) {
 			log.Warnf("Pubkey %#x has already been submitted for chainstart", pubkey)
@@ -150,7 +147,7 @@ func (s *Service) ProcessDepositLog(ctx context.Context, depositLog gethTypes.Lo
 
 	// We always store all historical deposits in the DB.
 	s.depositCache.InsertDeposit(ctx, deposit, big.NewInt(int64(depositLog.BlockNumber)), int(index), s.depositTrie.Root())
-
+	validData := true
 	if !s.chainStarted {
 		s.chainStartDeposits = append(s.chainStartDeposits, deposit)
 		root := s.depositTrie.Root()

diff --git a/beacon-chain/powchain/service.go b/beacon-chain/powchain/service.go
@@ -125,7 +125,7 @@ type Service struct {
 	blockCache              *blockCache // cache to store block hash/block height.
 	depositContractCaller   *contracts.DepositContractCaller
 	depositRoot             []byte
-	depositTrie             *trieutil.MerkleTrie
+	depositTrie             *trieutil.SparseMerkleTrie
 	chainStartDeposits      []*ethpb.Deposit
 	chainStarted            bool
 	chainStartBlockNumber   *big.Int
@@ -255,7 +255,7 @@ func (s *Service) DepositRoot() [32]byte {
 
 // DepositTrie returns the sparse Merkle trie used for storing
 // deposits from the ETH1.0 deposit contract.
-func (s *Service) DepositTrie() *trieutil.MerkleTrie {
+func (s *Service) DepositTrie() *trieutil.SparseMerkleTrie {
 	return s.depositTrie
 }
 

diff --git a/beacon-chain/powchain/testing/faulty_mock.go b/beacon-chain/powchain/testing/faulty_mock.go
diff --git a/beacon-chain/powchain/testing/mock.go b/beacon-chain/powchain/testing/mock.go
@@ -39,8 +39,8 @@ func (m *POWChain) Eth2GenesisPowchainInfo() (uint64, *big.Int) {
 }
 
 // DepositTrie --
-func (m *POWChain) DepositTrie() *trieutil.MerkleTrie {
-	return &trieutil.MerkleTrie{}
+func (m *POWChain) DepositTrie() *trieutil.SparseMerkleTrie {
+	return &trieutil.SparseMerkleTrie{}
 }
 
 // BlockExists --

diff --git a/beacon-chain/rpc/proposer/server.go b/beacon-chain/rpc/proposer/server.go
@@ -356,7 +356,7 @@ func (ps *Server) defaultEth1DataResponse(ctx context.Context, currentHeight *bi
 	}, nil
 }
 
-func constructMerkleProof(trie *trieutil.MerkleTrie, index int, deposit *ethpb.Deposit) (*ethpb.Deposit, error) {
+func constructMerkleProof(trie *trieutil.SparseMerkleTrie, index int, deposit *ethpb.Deposit) (*ethpb.Deposit, error) {
 	proof, err := trie.MerkleProof(index)
 	if err != nil {
 		return nil, errors.Wrapf(err, "could not generate merkle proof for deposit at index %d", index)

diff --git a/beacon-chain/rpc/proposer/server_test.go b/beacon-chain/rpc/proposer/server_test.go
@@ -314,10 +314,7 @@ func TestPendingDeposits_OutsideEth1FollowWindow(t *testing.T) {
 			t.Fatalf("Unable to determine hashed value of deposit %v", err)
 		}
 
-		if err := depositTrie.InsertIntoTrie(depositHash[:], int(dp.Index)); err != nil {
-			t.Fatalf("Unable to insert deposit into trie %v", err)
-		}
-
+		depositTrie.Insert(depositHash[:], dp.Index)
 		depositCache.InsertDeposit(ctx, dp.Deposit, dp.Block, dp.Index, depositTrie.Root())
 	}
 	for _, dp := range recentDeposits {
@@ -467,10 +464,7 @@ func TestPendingDeposits_FollowsCorrectEth1Block(t *testing.T) {
 			t.Fatalf("Unable to determine hashed value of deposit %v", err)
 		}
 
-		if err := depositTrie.InsertIntoTrie(depositHash[:], int(dp.Index)); err != nil {
-			t.Fatalf("Unable to insert deposit into trie %v", err)
-		}
-
+		depositTrie.Insert(depositHash[:], dp.Index)
 		depositCache.InsertDeposit(ctx, dp.Deposit, dp.Block, dp.Index, depositTrie.Root())
 	}
 	for _, dp := range recentDeposits {
@@ -584,10 +578,7 @@ func TestPendingDeposits_CantReturnBelowStateEth1DepositIndex(t *testing.T) {
 			t.Fatalf("Unable to determine hashed value of deposit %v", err)
 		}
 
-		if err := depositTrie.InsertIntoTrie(depositHash[:], int(dp.Index)); err != nil {
-			t.Fatalf("Unable to insert deposit into trie %v", err)
-		}
-
+		depositTrie.Insert(depositHash[:], dp.Index)
 		depositCache.InsertDeposit(ctx, dp.Deposit, big.NewInt(int64(dp.Index)), dp.Index, depositTrie.Root())
 	}
 	for _, dp := range recentDeposits {
@@ -693,10 +684,7 @@ func TestPendingDeposits_CantReturnMoreThanMax(t *testing.T) {
 			t.Fatalf("Unable to determine hashed value of deposit %v", err)
 		}
 
-		if err := depositTrie.InsertIntoTrie(depositHash[:], int(dp.Index)); err != nil {
-			t.Fatalf("Unable to insert deposit into trie %v", err)
-		}
-
+		depositTrie.Insert(depositHash[:], dp.Index)
 		depositCache.InsertDeposit(ctx, dp.Deposit, big.NewInt(int64(dp.Index)), dp.Index, depositTrie.Root())
 	}
 	for _, dp := range recentDeposits {
@@ -800,10 +788,7 @@ func TestPendingDeposits_CantReturnMoreDepositCount(t *testing.T) {
 			t.Fatalf("Unable to determine hashed value of deposit %v", err)
 		}
 
-		if err := depositTrie.InsertIntoTrie(depositHash[:], int(dp.Index)); err != nil {
-			t.Fatalf("Unable to insert deposit into trie %v", err)
-		}
-
+		depositTrie.Insert(depositHash[:], dp.Index)
 		depositCache.InsertDeposit(ctx, dp.Deposit, big.NewInt(int64(dp.Index)), dp.Index, depositTrie.Root())
 	}
 	for _, dp := range recentDeposits {
@@ -1166,10 +1151,7 @@ func TestDeposits_ReturnsEmptyList_IfLatestEth1DataEqGenesisEth1Block(t *testing
 			t.Fatalf("Unable to determine hashed value of deposit %v", err)
 		}
 
-		if err := depositTrie.InsertIntoTrie(depositHash[:], int(dp.Index)); err != nil {
-			t.Fatalf("Unable to insert deposit into trie %v", err)
-		}
-
+		depositTrie.Insert(depositHash[:], dp.Index)
 		depositCache.InsertDeposit(ctx, dp.Deposit, big.NewInt(int64(dp.Index)), dp.Index, depositTrie.Root())
 	}
 	for _, dp := range recentDeposits {

diff --git a/beacon-chain/rpc/validator/status_test.go b/beacon-chain/rpc/validator/status_test.go
@@ -598,7 +598,7 @@ func TestMultipleValidatorStatus_OK(t *testing.T) {
 	dep = &ethpb.Deposit{
 		Data: depData,
 	}
-	depositTrie.InsertIntoTrie(dep.Data.Signature, 15)
+	depositTrie.Insert(dep.Data.Signature, 15)
 	depositCache.InsertDeposit(context.Background(), dep, big.NewInt(15), 0, depositTrie.Root())
 
 	if err := db.SaveValidatorIndex(ctx, bytesutil.ToBytes48(pubKeys[0]), 0); err != nil {

diff --git a/contracts/deposit-contract/deposit_tree_test.go b/contracts/deposit-contract/deposit_tree_test.go
@@ -58,16 +58,11 @@ func TestDepositTrieRoot_OK(t *testing.T) {
 			t.Fatal(err)
 		}
 
-		err = localTrie.InsertIntoTrie(item[:], i)
-		if err != nil {
-			t.Error(err)
-		}
-
+		localTrie.Insert(item[:], i)
 		depRoot, err = testAcc.Contract.GetDepositRoot(&bind.CallOpts{})
 		if err != nil {
 			t.Fatal(err)
 		}
-
 		if depRoot != localTrie.HashTreeRoot() {
 			t.Errorf("Local deposit trie root and contract deposit trie root are not equal for index %d. Expected %#x , Got %#x", i, depRoot, localTrie.Root())
 		}
@@ -122,10 +117,7 @@ func TestDepositTrieRoot_Fail(t *testing.T) {
 			t.Fatal(err)
 		}
 
-		err = localTrie.InsertIntoTrie(item[:], i)
-		if err != nil {
-			t.Error(err)
-		}
+		localTrie.Insert(item[:], i)
 
 		depRoot, err = testAcc.Contract.GetDepositRoot(&bind.CallOpts{})
 		if err != nil {

diff --git a/shared/interop/generate_genesis_state.go b/shared/interop/generate_genesis_state.go
@@ -60,7 +60,7 @@ func GenerateGenesisState(genesisTime, numValidators uint64) (*pb.BeaconState, [
 }
 
 // GenerateDepositsFromData a list of deposit items by creating proofs for each of them from a sparse Merkle trie.
-func GenerateDepositsFromData(depositDataItems []*ethpb.Deposit_Data, trie *trieutil.MerkleTrie) ([]*ethpb.Deposit, error) {
+func GenerateDepositsFromData(depositDataItems []*ethpb.Deposit_Data, trie *trieutil.SparseMerkleTrie) ([]*ethpb.Deposit, error) {
 	deposits := make([]*ethpb.Deposit, len(depositDataItems))
 	results, err := mputil.Scatter(len(depositDataItems), func(offset int, entries int, _ *sync.RWMutex) (interface{}, error) {
 		return generateDepositsFromData(depositDataItems[offset:offset+entries], offset, trie)
@@ -79,7 +79,7 @@ func GenerateDepositsFromData(depositDataItems []*ethpb.Deposit_Data, trie *trie
 }
 
 // generateDepositsFromData a list of deposit items by creating proofs for each of them from a sparse Merkle trie.
-func generateDepositsFromData(depositDataItems []*ethpb.Deposit_Data, offset int, trie *trieutil.MerkleTrie) ([]*ethpb.Deposit, error) {
+func generateDepositsFromData(depositDataItems []*ethpb.Deposit_Data, offset int, trie *trieutil.SparseMerkleTrie) ([]*ethpb.Deposit, error) {
 	deposits := make([]*ethpb.Deposit, len(depositDataItems))
 	for i, item := range depositDataItems {
 		proof, err := trie.MerkleProof(i + offset)

diff --git a/shared/testutil/deposits.go b/shared/testutil/deposits.go
@@ -22,7 +22,7 @@ var lock sync.Mutex
 // Caches
 var cachedDeposits []*ethpb.Deposit
 var privKeys []*bls.SecretKey
-var trie *trieutil.MerkleTrie
+var trie *trieutil.SparseMerkleTrie
 
 // DeterministicDepositsAndKeys returns the entered amount of deposits and secret keys.
 // The deposits are configured such that for deposit n the validator
@@ -81,9 +81,7 @@ func DeterministicDepositsAndKeys(numDeposits uint64) ([]*ethpb.Deposit, []*bls.
 				return nil, nil, errors.Wrap(err, "could not tree hash deposit data")
 			}
 
-			if err := trie.InsertIntoTrie(hashedDeposit[:], int(numExisting+i)); err != nil {
-				return nil, nil, errors.Wrap(err, "could not tree hash deposit data")
-			}
+			trie.Insert(hashedDeposit[:], int(numExisting+i))
 		}
 	}
 
@@ -105,7 +103,7 @@ func DeterministicDepositsAndKeys(numDeposits uint64) ([]*ethpb.Deposit, []*bls.
 
 // DeterministicDepositTrie returns a merkle trie of the requested size from the
 // deterministic deposits.
-func DeterministicDepositTrie(size int) (*trieutil.MerkleTrie, [][32]byte, error) {
+func DeterministicDepositTrie(size int) (*trieutil.SparseMerkleTrie, [][32]byte, error) {
 	items := trie.Items()
 	if size > len(items) {
 		return nil, [][32]byte{}, errors.New("requested a larger tree than amount of deposits")
@@ -162,7 +160,7 @@ func DeterministicGenesisState(t testing.TB, numValidators uint64) (*pb.BeaconSt
 }
 
 // DepositTrieFromDeposits takes an array of deposits and returns the deposit trie.
-func DepositTrieFromDeposits(deposits []*ethpb.Deposit) (*trieutil.MerkleTrie, [][32]byte, error) {
+func DepositTrieFromDeposits(deposits []*ethpb.Deposit) (*trieutil.SparseMerkleTrie, [][32]byte, error) {
 	encodedDeposits := make([][]byte, len(deposits))
 	for i := 0; i < len(encodedDeposits); i++ {
 		hashedDeposit, err := ssz.HashTreeRoot(deposits[i].Data)

diff --git a/shared/trieutil/BUILD.bazel b/shared/trieutil/BUILD.bazel
@@ -3,7 +3,7 @@ load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")
 go_library(
     name = "go_default_library",
     srcs = [
-        "merkle_trie.go",
+        "helpers.go",
         "sparse_merkle.go",
         "zerohashes.go",
     ],
@@ -20,12 +20,13 @@ go_test(
     name = "go_default_test",
     size = "small",
     srcs = [
-        "merkle_trie_test.go",
+        "helpers_test.go",
         "sparse_merkle_test.go",
     ],
     embed = [":go_default_library"],
     deps = [
         "//contracts/deposit-contract:go_default_library",
+        "//shared/bytesutil:go_default_library",
         "//shared/hashutil:go_default_library",
         "//shared/params:go_default_library",
         "@com_github_ethereum_go_ethereum//accounts/abi/bind:go_default_library",

diff --git a/shared/trieutil/merkle_trie.go → shared/trieutil/helpers.go b/shared/trieutil/merkle_trie.go → shared/trieutil/helpers.go
diff --git a/shared/trieutil/merkle_trie_test.go → shared/trieutil/helpers_test.go b/shared/trieutil/merkle_trie_test.go → shared/trieutil/helpers_test.go