Fix(multiproofs verification): don't skip visiting intermediate hashes

proof.go

@@ -36,6 +36,10 @@ func VerifyProof(root []byte, proof *Proof) (bool, error) {
 
 // VerifyMultiproof verifies a proof for multiple leaves against the given root.
 func VerifyMultiproof(root []byte, proof [][]byte, leaves [][]byte, indices []int) (bool, error) {
+	if len(indices) == 0 {
+		return false, errors.New("indices length is zero")
+	}
+
 	if len(leaves) != len(indices) {
 		return false, errors.New("number of leaves and indices mismatch")
 	}
@@ -45,50 +49,85 @@ func VerifyMultiproof(root []byte, proof [][]byte, leaves [][]byte, indices []in
 		return false, fmt.Errorf("number of proof hashes %d and required indices %d mismatch", len(proof), len(reqIndices))
 	}
 
-	keys := make([]int, len(indices)+len(reqIndices))
-	nk := 0
-	// Create database of index -> value (hash)
-	// from inputs
+	// userGenIndices contains all generalised indices between leaves and proof hashes
+	// i.e., the indices retrieved from the user of this function
+	userGenIndices := make([]int, len(indices)+len(reqIndices))
+	pos := 0
+	// Create database of index -> value (hash) from inputs
 	db := make(map[int][]byte)
 	for i, leaf := range leaves {
 		db[indices[i]] = leaf
-		keys[nk] = indices[i]
-		nk++
+		userGenIndices[pos] = indices[i]
+		pos++
 	}
 	for i, h := range proof {
 		db[reqIndices[i]] = h
-		keys[nk] = reqIndices[i]
-		nk++
+		userGenIndices[pos] = reqIndices[i]
+		pos++
 	}
-	sort.Sort(sort.Reverse(sort.IntSlice(keys)))
 
-	pos := 0
+	// Make sure keys are sorted in reverse order since we start from the leaves
+	sort.Sort(sort.Reverse(sort.IntSlice(userGenIndices)))
+
+	// The depth of the tree up to the greatest index
+	cap := int(math.Log2(float64(userGenIndices[0])))
+
+	// Allocate space for auxiliary keys created when computing intermediate hashes
+	// Auxiliary indices are useful to avoid using store all indices to traverse
+	// in a single array and sort upon an insertion, which would be inefficient.
+	auxGenIndices := make([]int, 0, cap)
+
+	// To keep track the current position to inspect in both arrays
+	pos = 0
+	posAux := 0
+
 	tmp := make([]byte, 64)
-	for pos < len(keys) {
-		k := keys[pos]
+	var index int
+
+	// Iter over the tree, computing hashes and storing them
+	// in the in-memory database, until the root is reached.
+	//
+	// EXIT CONDITION: no more indices to use in both arrays
+	for posAux < len(auxGenIndices) || pos < len(userGenIndices) {
+		// We need to establish from which array we're going to take the next index
+		//
+		// 1. If we've no auxiliary indices yet, we're going to use the generalised ones
+		// 2. If we have no more client indices, we're going to use the auxiliary ones
+		// 3. If we both, then we're going to compare them and take the biggest one
+		if len(auxGenIndices) == 0 || (pos < len(userGenIndices) && auxGenIndices[posAux] < userGenIndices[pos]) {
+			index = userGenIndices[pos]
+			pos++
+		} else {
+			index = auxGenIndices[posAux]
+			posAux++
+		}
+
 		// Root has been reached
-		if k == 1 {
+		if index == 1 {
 			break
 		}
 
-		_, hasParent := db[getParent(k)]
+		// If the parent is already computed, we don't need to calculate the intermediate hash
+		_, hasParent := db[getParent(index)]
 		if hasParent {
-			pos++
 			continue
 		}
 
-		left, hasLeft := db[(k|1)^1]
-		right, hasRight := db[k|1]
+		left, hasLeft := db[(index|1)^1]
+		right, hasRight := db[index|1]
 		if !hasRight || !hasLeft {
-			return false, fmt.Errorf("proof is missing required nodes, either %d or %d", (k|1)^1, k|1)
+			return false, fmt.Errorf("proof is missing required nodes, either %d or %d", (index|1)^1, index|1)
 		}
 
 		copy(tmp[:32], left[:])
 		copy(tmp[32:], right[:])
-		db[getParent(k)] = hashFn(tmp)
-		keys = append(keys, getParent(k))
+		parentIndex := getParent(index)
+		db[parentIndex] = hashFn(tmp)
+
+		// An intermediate hash has been computed, as such we need to store its index
+		// to remember to examine it later
+		auxGenIndices = append(auxGenIndices, parentIndex)
 
-		pos++
 	}
 
 	res, ok := db[1]

spectests/beacon_state_bellatrix_test.go

@@ -54,17 +54,9 @@ func TestBeaconHeader_MultiProof(t *testing.T) {
 
 	multiProof, err := objectTree.ProveMulti(proofAtIndices)
 	require.NoError(t, err)
-
-	require.Equal(t, 3, len(multiProof.Leaves), "multi proof leaf hashes length incorrect")
-	require.Equal(t, hex.EncodeToString(multiProof.Leaves[0]), "a064480000000000000000000000000000000000000000000000000000000000")
-	require.Equal(t, hex.EncodeToString(multiProof.Leaves[1]), "7859010000000000000000000000000000000000000000000000000000000000")
-	require.Equal(t, hex.EncodeToString(multiProof.Leaves[2]), "0000000000000000000000000000000000000000000000000000000000000000")
-
-	require.Equal(t, proofAtIndices, multiProof.Indices)
-	require.Equal(t, 3, len(multiProof.Hashes), "proof hashes length incorrect")
-	require.Equal(t, hex.EncodeToString(multiProof.Hashes[0]), "445fab586d7d52993d7713c29da316d7e0fe04fd053983198af93fb131ce02ed")
-	require.Equal(t, hex.EncodeToString(multiProof.Hashes[1]), "f5a5fd42d16a20302798ef6ed309979b43003d2320d9f0e8ea9831a92759fb4b")
-	require.Equal(t, hex.EncodeToString(multiProof.Hashes[2]), "007c0d1e0260fb9a6fa86a39569aaebc9a95aaab0180f2865da2fc25180e2242")
+	ok, err := ssz.VerifyMultiproof(objectTree.Hash(), multiProof.Hashes, multiProof.Leaves, proofAtIndices)
+	require.NoError(t, err)
+	require.True(t, ok)
 }
 
 func TestBeaconState_BlockRootAtIndexProof(t *testing.T) {

spectests/structs.go

@@ -267,11 +267,9 @@ type ErrorResponse struct {
 	Message []byte `ssz-max:"256"`
 }
 
-type Dummy struct {
-}
+type Dummy struct{}
 
-type Interface interface {
-}
+type Interface interface{}
 
 type SyncCommittee struct {
 	PubKeys         [][]byte `json:"pubkeys" ssz-size:"512,48"`
@@ -319,6 +317,11 @@ type ExecutionPayloadHeader struct {
 	TransactionsRoot []byte `json:"transactions_root" ssz-size:"32"`
 }
 
+// ExecutionPayloadTransactions provides information about transactions.
+type ExecutionPayloadTransactions struct {
+	Transactions [][]byte `ssz-max:"1048576,1073741824" ssz-size:"?,?"`
+}
+
 // Capella types
 
 type Uint256 [32]byte

tests/codetrie_test.go

@@ -3,11 +3,13 @@ package tests
 import (
 	"bytes"
 	"encoding/hex"
+	"math"
 	"math/rand"
 	"testing"
 	"time"
 
 	ssz "github.com/ferranbt/fastssz"
+	"github.com/ferranbt/fastssz/spectests"
 	"github.com/minio/sha256-simd"
 )
 
@@ -146,6 +148,50 @@ func TestVerifyCodeTrieProof(t *testing.T) {
 	}
 }
 
+func TestVerifyCodeTrieMultiProof2(t *testing.T) {
+	// https://etherscan.io/tx/0x138a5f8ba7950521d9dec66ee760b101e0c875039e695c9fcfb34f5ef02a881b
+	// 0x02f873011a8405f5e10085037fcc60e182520894f7eaaf75cb6ec4d0e2b53964ce6733f54f7d3ffc880b6139a7cbd2000080c080a095a7a3cbb7383fc3e7d217054f861b890a935adc1adf4f05e3a2f23688cf2416a00875cdc45f4395257e44d709d04990349b105c22c11034a60d7af749ffea2765
+	// https://etherscan.io/tx/0xfb0ee9de8941c8ad50e6a3d2999cd6ef7a541ec9cb1ba5711b76fcfd1662dfa9
+	// 0xf8708305dc6885029332e35883019a2894500b0107e172e420561565c8177c28ac0f62017f8810ffb80e6cc327008025a0e9c0b380c68f040ae7affefd11979f5ed18ae82c00e46aa3238857c372a358eca06b26e179dd2f7a7f1601755249f4cff56690c4033553658f0d73e26c36fe7815
+	// https://etherscan.io/tx/0x45e7ee9ba1a1d0145de29a764a33bb7fc5620486b686d68ec8cb3182d137bc90
+	// 0xf86c0785028fa6ae0082520894098d880c4753d0332ca737aa592332ed2522cd22880d2f09f6558750008026a0963e58027576b3a8930d7d9b4a49253b6e1a2060e259b2102e34a451d375ce87a063f802538d3efed17962c96fcea431388483bbe3860ea9bb3ef01d4781450fbf
+	// https://etherscan.io/tx/0x9d48b4a021898a605b7ae49bf93ad88fa6bd7050e9448f12dde064c10f22fe9c
+	// 0x02f87601836384348477359400850517683ba883019a28943678fce4028b6745eb04fa010d9c8e4b36d6288c872b0f1366ad800080c080a0b6b7aba1954160d081b2c8612e039518b9c46cd7df838b405a03f927ad196158a071d2fb6813e5b5184def6bd90fb5f29e0c52671dea433a7decb289560a58416e
+
+	raw := []string{"0x02f873011a8405f5e10085037fcc60e182520894f7eaaf75cb6ec4d0e2b53964ce6733f54f7d3ffc880b6139a7cbd2000080c080a095a7a3cbb7383fc3e7d217054f861b890a935adc1adf4f05e3a2f23688cf2416a00875cdc45f4395257e44d709d04990349b105c22c11034a60d7af749ffea2765", "0xf8708305dc6885029332e35883019a2894500b0107e172e420561565c8177c28ac0f62017f8810ffb80e6cc327008025a0e9c0b380c68f040ae7affefd11979f5ed18ae82c00e46aa3238857c372a358eca06b26e179dd2f7a7f1601755249f4cff56690c4033553658f0d73e26c36fe7815", "0xf86c0785028fa6ae0082520894098d880c4753d0332ca737aa592332ed2522cd22880d2f09f6558750008026a0963e58027576b3a8930d7d9b4a49253b6e1a2060e259b2102e34a451d375ce87a063f802538d3efed17962c96fcea431388483bbe3860ea9bb3ef01d4781450fbf", "0x02f87601836384348477359400850517683ba883019a28943678fce4028b6745eb04fa010d9c8e4b36d6288c872b0f1366ad800080c080a0b6b7aba1954160d081b2c8612e039518b9c46cd7df838b405a03f927ad196158a071d2fb6813e5b5184def6bd90fb5f29e0c52671dea433a7decb289560a58416e"}
+
+	byteTxs := make([][]byte, len(raw))
+	for i := range byteTxs {
+		byteTxs[i], _ = hex.DecodeString(raw[i][2:])
+	}
+
+	bellatrixPayloadTxs := spectests.ExecutionPayloadTransactions{Transactions: byteTxs}
+
+	rootNode, err := bellatrixPayloadTxs.GetTree()
+	if err != nil {
+		t.Errorf("Failed to construct tree for transactions: %v\n", err)
+	}
+
+	rootNode.Hash()
+
+	// using gen index formula: 2 ** 21
+	baseGeneralizedIndex := int(math.Pow(float64(2), float64(21)))
+	generalizedIndexes := make([]int, 2)
+	// prove inclusion of some transactions
+	generalizedIndexes[0] = baseGeneralizedIndex
+	generalizedIndexes[1] = baseGeneralizedIndex + 3
+
+	multiProof, err := rootNode.ProveMulti(generalizedIndexes)
+	if err != nil {
+		t.Errorf("Failed to generate multiproof: %v\n", err)
+	}
+	if ok, err := ssz.VerifyMultiproof(rootNode.Hash(), multiProof.Hashes, multiProof.Leaves, multiProof.Indices); !ok || err != nil {
+		t.Errorf("NOT OK while verifying multiproof: %v\n", err)
+	} else {
+		t.Logf("OK while verifying multiproof\n")
+	}
+}
+
 func TestVerifyCodeTrieMultiProof(t *testing.T) {
 	testCases := []struct {
 		root    string