Merge pull request #166 from chainbound/fix/devnet/multiproof-orderin…

…g-logic fix(mevboost, relay): ordering logic for multiproofs
chainbound · Jul 26, 2024 · 0d1f0be · 0d1f0be
2 parents 56de49b + 32f9543
commit 0d1f0be
Show file tree

Hide file tree

Showing 5 changed files with 53 additions and 178 deletions.
diff --git a/mev-boost-relay/services/api/proofs.go b/mev-boost-relay/services/api/proofs.go
@@ -6,16 +6,19 @@ import (
 	"time"
 
 	"github.com/attestantio/go-eth2-client/spec/phase0"
+	gethCommon "github.com/ethereum/go-ethereum/common"
 	fastSsz "github.com/ferranbt/fastssz"
 	"github.com/flashbots/mev-boost-relay/common"
 	"github.com/sirupsen/logrus"
 )
 
 var (
-	ErrNilConstraint = errors.New("nil constraint")
-	ErrNilProof      = errors.New("nil proof")
-	ErrInvalidProofs = errors.New("proof verification failed")
-	ErrInvalidRoot   = errors.New("failed getting tx root from bid")
+	ErrNilConstraint             = errors.New("nil constraint")
+	ErrNilProof                  = errors.New("nil proof")
+	ErrInvalidProofs             = errors.New("proof verification failed")
+	ErrInvalidRoot               = errors.New("failed getting tx root from bid")
+	ErrHashesIndexesMismatch     = errors.New("proof transaction hashes and indexes length mismatch")
+	ErrHashesConstraintsMismatch = errors.New("proof transaction hashes and constraints length mismatch")
 )
 
 // verifyInclusionProof verifies the proofs against the constraints, and returns an error if the proofs are invalid.
@@ -26,12 +29,20 @@ func verifyInclusionProof(log *logrus.Entry, transactionsRoot phase0.Root, proof
 		return ErrNilProof
 	}
 
-	constraints := ParseConstraintsDecoded(hashToConstraints)
+	if len(proof.TransactionHashes) != len(proof.GeneralizedIndexes) {
+		return ErrHashesIndexesMismatch
+	}
+
+	if len(proof.TransactionHashes) != len(hashToConstraints) {
+		return ErrHashesIndexesMismatch
+	}
 
-	leaves := make([][]byte, len(constraints))
+	leaves := make([][]byte, len(hashToConstraints))
+	indexes := make([]int, len(proof.GeneralizedIndexes))
 
-	for i, constraint := range constraints {
-		if constraint == nil {
+	for i, hash := range proof.TransactionHashes {
+		constraint, ok := hashToConstraints[gethCommon.Hash(hash)]
+		if constraint == nil || !ok {
 			return ErrNilConstraint
 		}
 
@@ -50,17 +61,14 @@ func verifyInclusionProof(log *logrus.Entry, transactionsRoot phase0.Root, proof
 		}
 
 		leaves[i] = txHashTreeRoot[:]
+		indexes[i] = int(proof.GeneralizedIndexes[i])
 		i++
 	}
 
 	hashes := make([][]byte, len(proof.MerkleHashes))
 	for i, hash := range proof.MerkleHashes {
 		hashes[i] = []byte(*hash)
 	}
-	indexes := make([]int, len(proof.GeneralizedIndexes))
-	for i, index := range proof.GeneralizedIndexes {
-		indexes[i] = int(index)
-	}
 
 	currentTime := time.Now()
 	ok, err := fastSsz.VerifyMultiproof(transactionsRoot[:], hashes, leaves, indexes)

diff --git a/mev-boost-relay/services/api/types.go b/mev-boost-relay/services/api/types.go
@@ -4,8 +4,6 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
-	"slices"
-	"sort"
 
 	"github.com/attestantio/go-eth2-client/spec/phase0"
 	gethCommon "github.com/ethereum/go-ethereum/common"
@@ -51,43 +49,3 @@ type (
 		Tx    *types.Transaction
 	}
 )
-
-// ParseConstraintsDecoded receives a map of constraints and
-// - creates a slice of constraints sorted by index
-// - creates a slice of constraints without index sorted by nonce and hash
-// Returns the concatenation of the slices
-func ParseConstraintsDecoded(constraints HashToConstraintDecoded) []*ConstraintDecoded {
-	// Here we initialize and track the constraints left to be executed along
-	// with their gas requirements
-	constraintsOrderedByIndex := make([]*ConstraintDecoded, 0, len(constraints))
-	constraintsWithoutIndex := make([]*ConstraintDecoded, 0, len(constraints))
-
-	for _, constraint := range constraints {
-		if constraint.Index == nil {
-			constraintsWithoutIndex = append(constraintsWithoutIndex, constraint)
-		} else {
-			constraintsOrderedByIndex = append(constraintsOrderedByIndex, constraint)
-		}
-	}
-
-	// Sorts the constraints by index ascending
-	sort.Slice(constraintsOrderedByIndex, func(i, j int) bool {
-		// By assumption, all constraints here have a non-nil index
-		return *constraintsOrderedByIndex[i].Index < *constraintsOrderedByIndex[j].Index
-	})
-
-	// Sorts the unindexed constraints by nonce ascending and by hash
-	sort.Slice(constraintsWithoutIndex, func(i, j int) bool {
-		iNonce := constraintsWithoutIndex[i].Tx.Nonce()
-		jNonce := constraintsWithoutIndex[j].Tx.Nonce()
-		// Sort by hash
-		if iNonce == jNonce {
-			return constraintsWithoutIndex[i].Tx.Hash().Cmp(constraintsWithoutIndex[j].Tx.Hash()) < 0
-		}
-		return iNonce < jNonce
-	})
-
-	constraintsConcat := slices.Concat(constraintsOrderedByIndex, constraintsWithoutIndex)
-
-	return constraintsConcat
-}
diff --git a/mev-boost/server/constraints.go b/mev-boost/server/constraints.go
@@ -1,9 +1,6 @@
 package server
 
 import (
-	"slices"
-	"sort"
-
 	"github.com/attestantio/go-eth2-client/spec/phase0"
 	gethCommon "github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/core/types"
@@ -119,43 +116,3 @@ type (
 		Tx    *types.Transaction
 	}
 )
-
-// ParseConstraintsDecoded receives a map of constraints and
-// - creates a slice of constraints sorted by index
-// - creates a slice of constraints without index sorted by nonce and hash
-// Returns the concatenation of the slices
-func ParseConstraintsDecoded(constraints HashToConstraintDecoded) []*ConstraintDecoded {
-	// Here we initialize and track the constraints left to be executed along
-	// with their gas requirements
-	constraintsOrderedByIndex := make([]*ConstraintDecoded, 0, len(constraints))
-	constraintsWithoutIndex := make([]*ConstraintDecoded, 0, len(constraints))
-
-	for _, constraint := range constraints {
-		if constraint.Index == nil {
-			constraintsWithoutIndex = append(constraintsWithoutIndex, constraint)
-		} else {
-			constraintsOrderedByIndex = append(constraintsOrderedByIndex, constraint)
-		}
-	}
-
-	// Sorts the constraints by index ascending
-	sort.Slice(constraintsOrderedByIndex, func(i, j int) bool {
-		// By assumption, all constraints here have a non-nil index
-		return *constraintsOrderedByIndex[i].Index < *constraintsOrderedByIndex[j].Index
-	})
-
-	// Sorts the unindexed constraints by nonce ascending and by hash
-	sort.Slice(constraintsWithoutIndex, func(i, j int) bool {
-		iNonce := constraintsWithoutIndex[i].Tx.Nonce()
-		jNonce := constraintsWithoutIndex[j].Tx.Nonce()
-		// Sort by hash
-		if iNonce == jNonce {
-			return constraintsWithoutIndex[i].Tx.Hash().Cmp(constraintsWithoutIndex[j].Tx.Hash()) < 0
-		}
-		return iNonce < jNonce
-	})
-
-	constraintsConcat := slices.Concat(constraintsOrderedByIndex, constraintsWithoutIndex)
-
-	return constraintsConcat
-}
diff --git a/mev-boost/server/constraints_test.go b/mev-boost/server/constraints_test.go
diff --git a/mev-boost/server/service.go b/mev-boost/server/service.go
@@ -22,6 +22,7 @@ import (
 	eth2ApiV1Capella "github.com/attestantio/go-eth2-client/api/v1/capella"
 	eth2ApiV1Deneb "github.com/attestantio/go-eth2-client/api/v1/deneb"
 	"github.com/attestantio/go-eth2-client/spec/phase0"
+	gethCommon "github.com/ethereum/go-ethereum/common"
 	gethTypes "github.com/ethereum/go-ethereum/core/types"
 	fastSsz "github.com/ferranbt/fastssz"
 	"github.com/flashbots/go-boost-utils/ssz"
@@ -46,11 +47,14 @@ var (
 
 // Bolt errors
 var (
-	errNilProof          = errors.New("nil proof")
-	errMissingConstraint = errors.New("missing constraint")
-	errMismatchProofSize = errors.New("proof size mismatch")
-	errInvalidProofs     = errors.New("proof verification failed")
-	errInvalidRoot       = errors.New("failed getting tx root from bid")
+	errNilProof                  = errors.New("nil proof")
+	errMissingConstraint         = errors.New("missing constraint")
+	errMismatchProofSize         = errors.New("proof size mismatch")
+	errInvalidProofs             = errors.New("proof verification failed")
+	errInvalidRoot               = errors.New("failed getting tx root from bid")
+	errNilConstraint             = errors.New("nil constraint")
+	errHashesIndexesMismatch     = errors.New("proof transaction hashes and indexes length mismatch")
+	errHashesConstraintsMismatch = errors.New("proof transaction hashes and constraints length mismatch")
 )
 
 var (
@@ -338,7 +342,7 @@ func (m *BoostService) handleRegisterValidator(w http.ResponseWriter, req *http.
 }
 
 // verifyInclusionProof verifies the proofs against the constraints, and returns an error if the proofs are invalid.
-func (m *BoostService) verifyInclusionProof(responsePayload *BidWithInclusionProofs, slot uint64) error {
+func (m *BoostService) verifyInclusionProof(transactionsRoot phase0.Root, proof *InclusionProof, slot uint64) error {
 	log := m.log.WithFields(logrus.Fields{})
 
 	// BOLT: get constraints for the slot
@@ -349,21 +353,19 @@ func (m *BoostService) verifyInclusionProof(responsePayload *BidWithInclusionPro
 		return errMissingConstraint
 	}
 
-	if responsePayload.Proofs == nil {
+	if proof == nil {
 		return errNilProof
 	}
 
-	if len(responsePayload.Proofs.TransactionHashes) != len(inclusionConstraints) {
+	if len(proof.TransactionHashes) != len(inclusionConstraints) {
 		return errMismatchProofSize
 	}
-
-	log.Infof("[BOLT]: Verifying merkle multiproofs for %d transactions", len(responsePayload.Proofs.TransactionHashes))
-
-	transactionsRoot, err := responsePayload.Bid.TransactionsRoot()
-	if err != nil {
-		return errInvalidRoot
+	if len(proof.TransactionHashes) != len(proof.GeneralizedIndexes) {
+		return errHashesIndexesMismatch
 	}
 
+	log.Infof("[BOLT]: Verifying merkle multiproofs for %d transactions", len(proof.TransactionHashes))
+
 	// Decode the constraints, and sort them according to the utility function used
 	// TODO: this should be done before verification ideally
 	hashToConstraint := make(HashToConstraintDecoded)
@@ -379,18 +381,18 @@ func (m *BoostService) verifyInclusionProof(responsePayload *BidWithInclusionPro
 			Index: constraint.Index,
 		}
 	}
-	constraints := ParseConstraintsDecoded(hashToConstraint)
+	leaves := make([][]byte, len(inclusionConstraints))
+	indexes := make([]int, len(proof.GeneralizedIndexes))
 
-	leaves := make([][]byte, len(constraints))
+	for i, hash := range proof.TransactionHashes {
+		constraint, ok := hashToConstraint[gethCommon.Hash(hash)]
+		if constraint == nil || !ok {
+			return errNilConstraint
+		}
 
-	for i, constraint := range constraints {
 		// Compute the hash tree root for the raw preconfirmed transaction
 		// and use it as "Leaf" in the proof to be verified against
-
-		// TODO: this is pretty inefficient, we should work with the transaction already
-		// parsed without the blob here to avoid unmarshalling and marshalling again
-		transaction := constraint.Tx
-		encoded, err := transaction.MarshalBinary()
+		encoded, err := constraint.Tx.MarshalBinary()
 		if err != nil {
 			log.WithError(err).Error("error marshalling transaction without blob tx sidecar")
 			return err
@@ -403,17 +405,14 @@ func (m *BoostService) verifyInclusionProof(responsePayload *BidWithInclusionPro
 		}
 
 		leaves[i] = txHashTreeRoot[:]
+		indexes[i] = int(proof.GeneralizedIndexes[i])
 		i++
 	}
 
-	hashes := make([][]byte, len(responsePayload.Proofs.MerkleHashes))
-	for i, hash := range responsePayload.Proofs.MerkleHashes {
+	hashes := make([][]byte, len(proof.MerkleHashes))
+	for i, hash := range proof.MerkleHashes {
 		hashes[i] = []byte(*hash)
 	}
-	indexes := make([]int, len(responsePayload.Proofs.GeneralizedIndexes))
-	for i, index := range responsePayload.Proofs.GeneralizedIndexes {
-		indexes[i] = int(index)
-	}
 
 	currentTime := time.Now()
 	ok, err := fastSsz.VerifyMultiproof(transactionsRoot[:], hashes, leaves, indexes)
@@ -881,7 +880,12 @@ func (m *BoostService) handleGetHeaderWithProofs(w http.ResponseWriter, req *htt
 			// BOLT: verify preconfirmation inclusion proofs. If they don't match, we don't consider the bid to be valid.
 			if responsePayload.Proofs != nil {
 				// BOLT: verify the proofs against the constraints. If they don't match, we don't consider the bid to be valid.
-				if err := m.verifyInclusionProof(responsePayload, slotUint); err != nil {
+				transactionsRoot, err := responsePayload.Bid.TransactionsRoot()
+				if err != nil {
+					log.WithError(err).Error("[BOLT]: error getting transaction root")
+					return
+				}
+				if err := m.verifyInclusionProof(transactionsRoot, responsePayload.Proofs, slotUint); err != nil {
 					log.Warnf("[BOLT]: Proof verification failed for relay %s: %s", relay.URL, err)
 					return
 				}