From 9a7811c906d72d3b1fbcb2a17b43c8a09cad0a8a Mon Sep 17 00:00:00 2001 From: thedevbirb Date: Fri, 26 Jul 2024 10:39:26 +0200 Subject: [PATCH 1/4] fix(mevboost): ordering logic for multiproofs --- mev-boost/server/service.go | 62 ++++++++++++++++++++----------------- 1 file changed, 33 insertions(+), 29 deletions(-) diff --git a/mev-boost/server/service.go b/mev-boost/server/service.go index 17f77766..36a50179 100644 --- 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 } From 7884c735e97237db7741c882096c20556c37836e Mon Sep 17 00:00:00 2001 From: thedevbirb Date: Fri, 26 Jul 2024 10:39:43 +0200 Subject: [PATCH 2/4] fix(relay): ordering logic for multiproofs --- mev-boost-relay/services/api/proofs.go | 32 ++++++++++++++++---------- 1 file changed, 20 insertions(+), 12 deletions(-) diff --git a/mev-boost-relay/services/api/proofs.go b/mev-boost-relay/services/api/proofs.go index 6505b0cb..fce7d86e 100644 --- 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,6 +61,7 @@ func verifyInclusionProof(log *logrus.Entry, transactionsRoot phase0.Root, proof } leaves[i] = txHashTreeRoot[:] + indexes[i] = int(proof.GeneralizedIndexes[i]) i++ } @@ -57,10 +69,6 @@ func verifyInclusionProof(log *logrus.Entry, transactionsRoot phase0.Root, proof 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) From 4e0442e88318168ae7a5e4d65546005c0443e121 Mon Sep 17 00:00:00 2001 From: thedevbirb Date: Fri, 26 Jul 2024 14:39:34 +0200 Subject: [PATCH 3/4] chore(mevboost): remove unused utility --- mev-boost/server/constraints.go | 43 ----------------------- mev-boost/server/constraints_test.go | 52 ---------------------------- 2 files changed, 95 deletions(-) delete mode 100644 mev-boost/server/constraints_test.go diff --git a/mev-boost/server/constraints.go b/mev-boost/server/constraints.go index e6edec36..4f5914aa 100644 --- 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 deleted file mode 100644 index 56e3f811..00000000 --- a/mev-boost/server/constraints_test.go +++ /dev/null @@ -1,52 +0,0 @@ -package server - -import ( - "encoding/hex" - "testing" - - gethTypes "github.com/ethereum/go-ethereum/core/types" - "github.com/stretchr/testify/require" -) - -func Test_ParseContraintsDecoded(t *testing.T) { - rawTxs := []string{ - // These two will have index set, and nonce 367, 368 - "f86882016f84042343e082520894deaddeaddeaddeaddeaddeaddeaddeaddeaddead07808360306ba0a5b07edf4e7074a679b08cfc474364f3378e87006d82843c8bf306fc1c6e9e57a07927c7f92ac2f9a5166433e2b9bbc5f48ebf9d366d437c568c465cdf9ac148d8", - "f86882017084042343e082520894deaddeaddeaddeaddeaddeaddeaddeaddeaddead3b808360306ba082d4f1a817f12d59d21bbf1b156715bc1ab307f160b4a3e1527ec915a7757273a073a51224caa582e0cb34388ff188a68d022a6a283fcb9b4e6dfecece8ccf21e6", - // These three will not - // The first two will have same nonce 369, but one is to aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa just to have different hash: 0x678a4d09b8dd43ebd675b9e3f1983185f5a31f7b44e3f5815436a8fae647d1f9 - "f86882017184042343e082520894aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa27808360306ca00499b985ef217b1f9c03ef039190ae877b4578114e0b03a1ebbae977d5ca7d5fa0086a6d280a7dd5fdb64c4ddc727329f0ba1fa8c49deab9cafe71207f21dbf81b", - // This has hash 0x1c8e21622617cc02111389c67f542b5059cf5b024b265c5fdbcac529ae7ab7e0, so it will appear first - "f86882017184042343e082520894deaddeaddeaddeaddeaddeaddeaddeaddeaddead27808360306ca00499b985ef217b1f9c03ef039190ae877b4578114e0b03a1ebbae977d5ca7d5fa0086a6d280a7dd5fdb64c4ddc727329f0ba1fa8c49deab9cafe71207f21dbf81b", - // This will have nonce 370 - "f86882017284042343e082520894deaddeaddeaddeaddeaddeaddeaddeaddeaddead11808360306ba0891ff5261562c21a3f89f12d95391aef865c21f5cf72f97c4602aa9f072c0489a04c4482a46802d160c9a812cffc90be0cd6ffc1206c9dd2f5b53111d9098ff207", - // "f86882017384042343e082520894deaddeaddeaddeaddeaddeaddeaddeaddeaddead4b808360306ca087d083fadadeba27f213ebb2b428003aa730035686202547e2260cadfb824ee5a00c93b0f0403fbb78b5810e90b9f5bb63368dd6bcd5c31c56ed1b132167e7d69f", - } - - hashToConstraint := make(HashToConstraintDecoded) - - for i, rawTx := range rawTxs { - rawTxBytes, err := hex.DecodeString(rawTx) - require.NoError(t, err) - tx := new(gethTypes.Transaction) - err = tx.UnmarshalBinary(rawTxBytes) - - require.NoError(t, err) - var index *uint64 - if i < 2 { - index = new(uint64) - *index = uint64(i) - } - hashToConstraint[tx.Hash()] = &ConstraintDecoded{ - Tx: tx, - Index: index, - } - } - - constraintsParsed := ParseConstraintsDecoded(hashToConstraint) - require.Equal(t, uint64(367), constraintsParsed[0].Tx.Nonce()) - require.Equal(t, uint64(368), constraintsParsed[1].Tx.Nonce()) - require.Equal(t, "0x1c8e21622617cc02111389c67f542b5059cf5b024b265c5fdbcac529ae7ab7e0", constraintsParsed[2].Tx.Hash().String()) - require.Equal(t, uint64(369), constraintsParsed[3].Tx.Nonce()) - require.Equal(t, uint64(370), constraintsParsed[4].Tx.Nonce()) -} From 32f9543bd78ffcc061ec75748dc74c36102d91b8 Mon Sep 17 00:00:00 2001 From: thedevbirb Date: Fri, 26 Jul 2024 14:39:54 +0200 Subject: [PATCH 4/4] chore(relay): remove unused utility --- mev-boost-relay/services/api/types.go | 42 --------------------------- 1 file changed, 42 deletions(-) diff --git a/mev-boost-relay/services/api/types.go b/mev-boost-relay/services/api/types.go index 2454a0b5..298b52a7 100644 --- 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 -}