From 6afc14b90b034ebd025fe056364742788a7876f5 Mon Sep 17 00:00:00 2001 From: avalonche Date: Fri, 10 Mar 2023 18:09:53 +1100 Subject: [PATCH 1/4] Prevent uint256 overflow during conversion --- builder/builder.go | 52 +++++++++++++++++++++++++++-------------- core/block_validator.go | 4 ++++ 2 files changed, 39 insertions(+), 17 deletions(-) diff --git a/builder/builder.go b/builder/builder.go index 082cd34d9b..261919a0d3 100644 --- a/builder/builder.go +++ b/builder/builder.go @@ -166,18 +166,18 @@ func (b *Builder) submitBellatrixBlock(block *types.Block, blockValue *big.Int, if b.dryRun { err = b.validator.ValidateBuilderSubmissionV1(&blockvalidation.BuilderBlockValidationRequest{BuilderSubmitBlockRequest: blockSubmitReq, RegisteredGasLimit: vd.GasLimit}) if err != nil { - log.Error("could not validate block", "err", err) + log.Error("could not validate block for bellatrix", "err", err) } } else { go b.ds.ConsumeBuiltBlock(block, blockValue, ordersClosedAt, sealedAt, commitedBundles, allBundles, &blockBidMsg) err = b.relay.SubmitBlock(&blockSubmitReq, vd) if err != nil { - log.Error("could not submit block", "err", err, "#commitedBundles", len(commitedBundles)) + log.Error("could not submit bellatrix block", "err", err, "#commitedBundles", len(commitedBundles)) return err } } - log.Info("submitted block", "slot", blockBidMsg.Slot, "value", blockBidMsg.Value.String(), "parent", blockBidMsg.ParentHash, "hash", block.Hash(), "#commitedBundles", len(commitedBundles)) + log.Info("submitted bellatrix block", "slot", blockBidMsg.Slot, "value", blockBidMsg.Value.String(), "parent", blockBidMsg.ParentHash, "hash", block.Hash(), "#commitedBundles", len(commitedBundles)) return nil } @@ -190,10 +190,9 @@ func (b *Builder) submitCapellaBlock(block *types.Block, blockValue *big.Int, or return err } - value := new(boostTypes.U256Str) - err = value.FromBig(blockValue) - if err != nil { - log.Error("could not set block value", "err", err) + value, overflow := uint256.FromBig(blockValue) + if overflow { + log.Error("could not set block value due to value overflow") return err } @@ -206,10 +205,14 @@ func (b *Builder) submitCapellaBlock(block *types.Block, blockValue *big.Int, or ProposerFeeRecipient: bellatrix.ExecutionAddress(vd.FeeRecipient), GasLimit: executableData.ExecutionPayload.GasLimit, GasUsed: executableData.ExecutionPayload.GasUsed, - Value: uint256.NewInt(value.BigInt().Uint64()), + Value: value, } - boostBidTrace := convertBidTrace(blockBidMsg) + boostBidTrace, err := convertBidTrace(blockBidMsg) + if err != nil { + log.Error("could not convert bid trace", "err", err) + return err + } signature, err := boostTypes.SignMessage(&blockBidMsg, b.builderSigningDomain, b.builderSecretKey) if err != nil { @@ -223,12 +226,21 @@ func (b *Builder) submitCapellaBlock(block *types.Block, blockValue *big.Int, or ExecutionPayload: payload, } - go b.ds.ConsumeBuiltBlock(block, blockValue, ordersClosedAt, sealedAt, commitedBundles, allBundles, &boostBidTrace) - err = b.relay.SubmitBlockCapella(&blockSubmitReq, vd) - if err != nil { - log.Error("could not submit block", "err", err, "#commitedBundles", len(commitedBundles)) - return err + if b.dryRun { + err = b.validator.ValidateBuilderSubmissionV2(&blockvalidation.BuilderBlockValidationRequestV2{SubmitBlockRequest: blockSubmitReq, RegisteredGasLimit: vd.GasLimit}) + if err != nil { + log.Error("could not validate block for capella", "err", err) + } + } else { + go b.ds.ConsumeBuiltBlock(block, blockValue, ordersClosedAt, sealedAt, commitedBundles, allBundles, &boostBidTrace) + err = b.relay.SubmitBlockCapella(&blockSubmitReq, vd) + if err != nil { + log.Error("could not submit capella block", "err", err, "#commitedBundles", len(commitedBundles)) + return err + } } + + log.Info("submitted capella block", "slot", blockBidMsg.Slot, "value", blockBidMsg.Value.String(), "parent", blockBidMsg.ParentHash, "hash", block.Hash(), "#commitedBundles", len(commitedBundles)) return nil } @@ -444,7 +456,13 @@ func executableDataToCapellaExecutionPayload(data *engine.ExecutableData) (*cape }, nil } -func convertBidTrace(bidTrace apiv1.BidTrace) boostTypes.BidTrace { +func convertBidTrace(bidTrace apiv1.BidTrace) (boostTypes.BidTrace, error) { + value := new(boostTypes.U256Str) + err := value.FromBig(bidTrace.Value.ToBig()) + if err != nil { + return boostTypes.BidTrace{}, err + } + return boostTypes.BidTrace{ Slot: bidTrace.Slot, ParentHash: boostTypes.Hash(bidTrace.ParentHash), @@ -454,6 +472,6 @@ func convertBidTrace(bidTrace apiv1.BidTrace) boostTypes.BidTrace { ProposerFeeRecipient: boostTypes.Address(bidTrace.ProposerFeeRecipient), GasLimit: bidTrace.GasLimit, GasUsed: bidTrace.GasUsed, - Value: boostTypes.IntToU256(bidTrace.Value.Uint64()), - } + Value: *value, + }, nil } diff --git a/core/block_validator.go b/core/block_validator.go index 0faa35c8b4..d363781ab9 100644 --- a/core/block_validator.go +++ b/core/block_validator.go @@ -77,6 +77,10 @@ func (v *BlockValidator) ValidateBody(block *types.Block) error { if hash := types.DeriveSha(block.Withdrawals(), trie.NewStackTrie(nil)); hash != *header.WithdrawalsHash { return fmt.Errorf("withdrawals root hash mismatch (header value %x, calculated %x)", *header.WithdrawalsHash, hash) } + } else { + if block.Withdrawals() != nil { + return fmt.Errorf("withdrawals list present in block body for nil withdrawals hash") + } } if !v.bc.HasBlockAndState(block.ParentHash(), block.NumberU64()-1) { From dd63b6f45593f9ccb05776168ecd62f43739b188 Mon Sep 17 00:00:00 2001 From: avalonche Date: Sat, 11 Mar 2023 00:40:10 +1100 Subject: [PATCH 2/4] add shanghai checks --- builder/builder.go | 2 +- core/block_validator.go | 12 +++++++++--- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/builder/builder.go b/builder/builder.go index 261919a0d3..3bcd005865 100644 --- a/builder/builder.go +++ b/builder/builder.go @@ -166,7 +166,7 @@ func (b *Builder) submitBellatrixBlock(block *types.Block, blockValue *big.Int, if b.dryRun { err = b.validator.ValidateBuilderSubmissionV1(&blockvalidation.BuilderBlockValidationRequest{BuilderSubmitBlockRequest: blockSubmitReq, RegisteredGasLimit: vd.GasLimit}) if err != nil { - log.Error("could not validate block for bellatrix", "err", err) + log.Error("could not validate bellatrix block", "err", err) } } else { go b.ds.ConsumeBuiltBlock(block, blockValue, ordersClosedAt, sealedAt, commitedBundles, allBundles, &blockBidMsg) diff --git a/core/block_validator.go b/core/block_validator.go index d363781ab9..b7197d1cd8 100644 --- a/core/block_validator.go +++ b/core/block_validator.go @@ -68,8 +68,11 @@ func (v *BlockValidator) ValidateBody(block *types.Block) error { if hash := types.DeriveSha(block.Transactions(), trie.NewStackTrie(nil)); hash != header.TxHash { return fmt.Errorf("transaction root hash mismatch (header value %x, calculated %x)", header.TxHash, hash) } - // Withdrawals are present after the Shanghai fork. - if header.WithdrawalsHash != nil { + if v.config.IsShanghai(header.Time) { + if header.WithdrawalsHash == nil { + return fmt.Errorf("withdrawals hash is missing") + } + // Withdrawals are present after the Shanghai fork. // Withdrawals list must be present in body after Shanghai. if block.Withdrawals() == nil { return fmt.Errorf("missing withdrawals in block body") @@ -78,8 +81,11 @@ func (v *BlockValidator) ValidateBody(block *types.Block) error { return fmt.Errorf("withdrawals root hash mismatch (header value %x, calculated %x)", *header.WithdrawalsHash, hash) } } else { + if header.WithdrawalsHash != nil { + return fmt.Errorf("withdrawals hash present before shanghai") + } if block.Withdrawals() != nil { - return fmt.Errorf("withdrawals list present in block body for nil withdrawals hash") + return fmt.Errorf("withdrawals list present in block body before shanghai") } } From 239404d226607eba910fe17206688711afcd8173 Mon Sep 17 00:00:00 2001 From: avalonche Date: Mon, 13 Mar 2023 13:05:02 +1100 Subject: [PATCH 3/4] remove unused functions --- eth/block-validation/api.go | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/eth/block-validation/api.go b/eth/block-validation/api.go index 0fecfeb0b3..349e827503 100644 --- a/eth/block-validation/api.go +++ b/eth/block-validation/api.go @@ -18,7 +18,6 @@ import ( "github.com/ethereum/go-ethereum/log" "github.com/ethereum/go-ethereum/node" "github.com/ethereum/go-ethereum/rpc" - "github.com/ethereum/go-ethereum/trie" boostTypes "github.com/flashbots/go-boost-utils/types" ) @@ -67,22 +66,6 @@ func (a *AccessVerifier) verifyTransactions(signer types.Signer, txs types.Trans return nil } -func verifyWithdrawals(withdrawals types.Withdrawals, expectedWithdrawalsRoot common.Hash, isShanghai bool) error { - if !isShanghai { - // Reject payload attributes with withdrawals before shanghai - if withdrawals != nil { - return errors.New("withdrawals before shanghai") - } - return nil - } - - withdrawalsRoot := types.DeriveSha(types.Withdrawals(withdrawals), trie.NewStackTrie(nil)) - if withdrawalsRoot != expectedWithdrawalsRoot { - return fmt.Errorf("withdrawals mismatch") - } - return nil -} - func NewAccessVerifierFromFile(path string) (*AccessVerifier, error) { bytes, err := os.ReadFile(path) if err != nil { From 0b92131445866adf0d72131ff78b7e10ad95261f Mon Sep 17 00:00:00 2001 From: avalonche Date: Tue, 14 Mar 2023 05:19:41 +1100 Subject: [PATCH 4/4] move validation to ValidatePayload --- core/block_validator.go | 16 +++------------- core/blockchain.go | 14 ++++++++++++++ 2 files changed, 17 insertions(+), 13 deletions(-) diff --git a/core/block_validator.go b/core/block_validator.go index b7197d1cd8..7e0e9cc67f 100644 --- a/core/block_validator.go +++ b/core/block_validator.go @@ -68,11 +68,8 @@ func (v *BlockValidator) ValidateBody(block *types.Block) error { if hash := types.DeriveSha(block.Transactions(), trie.NewStackTrie(nil)); hash != header.TxHash { return fmt.Errorf("transaction root hash mismatch (header value %x, calculated %x)", header.TxHash, hash) } - if v.config.IsShanghai(header.Time) { - if header.WithdrawalsHash == nil { - return fmt.Errorf("withdrawals hash is missing") - } - // Withdrawals are present after the Shanghai fork. + // Withdrawals are present after the Shanghai fork. + if header.WithdrawalsHash != nil { // Withdrawals list must be present in body after Shanghai. if block.Withdrawals() == nil { return fmt.Errorf("missing withdrawals in block body") @@ -80,15 +77,8 @@ func (v *BlockValidator) ValidateBody(block *types.Block) error { if hash := types.DeriveSha(block.Withdrawals(), trie.NewStackTrie(nil)); hash != *header.WithdrawalsHash { return fmt.Errorf("withdrawals root hash mismatch (header value %x, calculated %x)", *header.WithdrawalsHash, hash) } - } else { - if header.WithdrawalsHash != nil { - return fmt.Errorf("withdrawals hash present before shanghai") - } - if block.Withdrawals() != nil { - return fmt.Errorf("withdrawals list present in block body before shanghai") - } } - + if !v.bc.HasBlockAndState(block.ParentHash(), block.NumberU64()-1) { if !v.bc.HasBlock(block.ParentHash(), block.NumberU64()-1) { return consensus.ErrUnknownAncestor diff --git a/core/blockchain.go b/core/blockchain.go index e3461bd964..ce8ef2090f 100644 --- a/core/blockchain.go +++ b/core/blockchain.go @@ -2507,6 +2507,20 @@ func (bc *BlockChain) ValidatePayload(block *types.Block, feeRecipient common.Ad return err } + if bc.Config().IsShanghai(header.Time) { + if header.WithdrawalsHash == nil { + return fmt.Errorf("withdrawals hash is missing") + } + // withdrawals hash and withdrawals validated later in ValidateBody + } else { + if header.WithdrawalsHash != nil { + return fmt.Errorf("withdrawals hash present before shanghai") + } + if block.Withdrawals() != nil { + return fmt.Errorf("withdrawals list present in block body before shanghai") + } + } + if err := bc.validator.ValidateBody(block); err != nil { return err }