From a680bb0e29e01a943bea1fcd2ccee8d8b777e385 Mon Sep 17 00:00:00 2001 From: Michael Neuder Date: Wed, 15 Mar 2023 10:53:07 -0600 Subject: [PATCH 01/12] block validation changes to builder geth post-capella --- builder/service.go | 9 +-- cmd/geth/config.go | 8 +- core/blockchain.go | 13 ++++ eth/block-validation/api.go | 148 ++---------------------------------- 4 files changed, 22 insertions(+), 156 deletions(-) diff --git a/builder/service.go b/builder/service.go index b49a0edf29..447ad7301f 100644 --- a/builder/service.go +++ b/builder/service.go @@ -156,14 +156,7 @@ func Register(stack *node.Node, backend *eth.Ethereum, cfg *Config) error { var validator *blockvalidation.BlockValidationAPI if cfg.DryRun { - var accessVerifier *blockvalidation.AccessVerifier - if cfg.ValidationBlocklist != "" { - accessVerifier, err = blockvalidation.NewAccessVerifierFromFile(cfg.ValidationBlocklist) - if err != nil { - return fmt.Errorf("failed to load validation blocklist %w", err) - } - } - validator = blockvalidation.NewBlockValidationAPI(backend, accessVerifier) + validator = blockvalidation.NewBlockValidationAPI(backend) } // TODO: move to proper flags diff --git a/cmd/geth/config.go b/cmd/geth/config.go index 2fd2e65973..de01760135 100644 --- a/cmd/geth/config.go +++ b/cmd/geth/config.go @@ -172,13 +172,7 @@ func makeFullNode(ctx *cli.Context) (*node.Node, ethapi.Backend) { // Configure log filter RPC API. filterSystem := utils.RegisterFilterAPI(stack, backend, &cfg.Eth) - - bvConfig := blockvalidationapi.BlockValidationConfig{} - if ctx.IsSet(utils.BuilderBlockValidationBlacklistSourceFilePath.Name) { - bvConfig.BlacklistSourceFilePath = ctx.String(utils.BuilderBlockValidationBlacklistSourceFilePath.Name) - } - - if err := blockvalidationapi.Register(stack, eth, bvConfig); err != nil { + if err := blockvalidationapi.Register(stack, eth); err != nil { utils.Fatalf("Failed to register the Block Validation API: %v", err) } diff --git a/core/blockchain.go b/core/blockchain.go index cdf87c2e56..d9ab1ef1ea 100644 --- a/core/blockchain.go +++ b/core/blockchain.go @@ -2527,6 +2527,8 @@ func (bc *BlockChain) ValidatePayload(block *types.Block, feeRecipient common.Ad // and dangling prefetcher, without defering each and holding on live refs. defer statedb.StopPrefetcher() + balanceBefore := statedb.GetBalance(feeRecipient) + receipts, _, usedGas, err := bc.processor.Process(block, statedb, vmConfig) if err != nil { return err @@ -2554,6 +2556,17 @@ func (bc *BlockChain) ValidatePayload(block *types.Block, feeRecipient common.Ad return err } + // First just check the balance delta to see if it matches. + balanceAfter := statedb.GetBalance(feeRecipient) + feeRecipientDiff := new(big.Int).Sub(balanceAfter, balanceBefore) + + // If diff is sufficiently large, just return success. + if feeRecipientDiff.Cmp(expectedProfit) >= 0 { + return nil + } + log.Warn(fmt.Sprintf("fee recipient diff %s is less than expected %s. checking for last transaction", feeRecipientDiff.String(), expectedProfit.String())) + + // Flashbots logic for last transaction checks. if len(receipts) == 0 { return errors.New("no proposer payment receipt") } diff --git a/eth/block-validation/api.go b/eth/block-validation/api.go index 349e827503..65ab4f4eca 100644 --- a/eth/block-validation/api.go +++ b/eth/block-validation/api.go @@ -4,17 +4,12 @@ import ( "encoding/json" "errors" "fmt" - "math/big" - "os" capellaapi "github.com/attestantio/go-builder-client/api/capella" "github.com/attestantio/go-eth2-client/spec/phase0" "github.com/ethereum/go-ethereum/beacon/engine" "github.com/ethereum/go-ethereum/common" - "github.com/ethereum/go-ethereum/core/types" - "github.com/ethereum/go-ethereum/core/vm" "github.com/ethereum/go-ethereum/eth" - "github.com/ethereum/go-ethereum/eth/tracers/logger" "github.com/ethereum/go-ethereum/log" "github.com/ethereum/go-ethereum/node" "github.com/ethereum/go-ethereum/rpc" @@ -22,106 +17,26 @@ import ( boostTypes "github.com/flashbots/go-boost-utils/types" ) -type BlacklistedAddresses []common.Address - -type AccessVerifier struct { - blacklistedAddresses map[common.Address]struct{} -} - -func (a *AccessVerifier) verifyTraces(tracer *logger.AccessListTracer) error { - log.Trace("x", "tracer.AccessList()", tracer.AccessList()) - for _, accessTuple := range tracer.AccessList() { - // TODO: should we ignore common.Address{}? - if _, found := a.blacklistedAddresses[accessTuple.Address]; found { - log.Info("bundle accesses blacklisted address", "address", accessTuple.Address) - return fmt.Errorf("blacklisted address %s in execution trace", accessTuple.Address.String()) - } - } - - return nil -} - -func (a *AccessVerifier) isBlacklisted(addr common.Address) error { - if _, present := a.blacklistedAddresses[addr]; present { - return fmt.Errorf("transaction from blacklisted address %s", addr.String()) - } - return nil -} - -func (a *AccessVerifier) verifyTransactions(signer types.Signer, txs types.Transactions) error { - for _, tx := range txs { - from, err := signer.Sender(tx) - if err == nil { - if _, present := a.blacklistedAddresses[from]; present { - return fmt.Errorf("transaction from blacklisted address %s", from.String()) - } - } - to := tx.To() - if to != nil { - if _, present := a.blacklistedAddresses[*to]; present { - return fmt.Errorf("transaction to blacklisted address %s", to.String()) - } - } - } - return nil -} - -func NewAccessVerifierFromFile(path string) (*AccessVerifier, error) { - bytes, err := os.ReadFile(path) - if err != nil { - return nil, err - } - - var ba BlacklistedAddresses - if err := json.Unmarshal(bytes, &ba); err != nil { - return nil, err - } - - blacklistedAddresses := make(map[common.Address]struct{}, len(ba)) - for _, address := range ba { - blacklistedAddresses[address] = struct{}{} - } - - return &AccessVerifier{ - blacklistedAddresses: blacklistedAddresses, - }, nil -} - -type BlockValidationConfig struct { - BlacklistSourceFilePath string -} - // Register adds catalyst APIs to the full node. -func Register(stack *node.Node, backend *eth.Ethereum, cfg BlockValidationConfig) error { - var accessVerifier *AccessVerifier - if cfg.BlacklistSourceFilePath != "" { - var err error - accessVerifier, err = NewAccessVerifierFromFile(cfg.BlacklistSourceFilePath) - if err != nil { - return err - } - } - +func Register(stack *node.Node, backend *eth.Ethereum) error { stack.RegisterAPIs([]rpc.API{ { Namespace: "flashbots", - Service: NewBlockValidationAPI(backend, accessVerifier), + Service: NewBlockValidationAPI(backend), }, }) return nil } type BlockValidationAPI struct { - eth *eth.Ethereum - accessVerifier *AccessVerifier + eth *eth.Ethereum } // NewConsensusAPI creates a new consensus api for the given backend. // The underlying blockchain needs to have a valid terminal total difficulty set. -func NewBlockValidationAPI(eth *eth.Ethereum, accessVerifier *AccessVerifier) *BlockValidationAPI { +func NewBlockValidationAPI(eth *eth.Ethereum) *BlockValidationAPI { return &BlockValidationAPI{ - eth: eth, - accessVerifier: accessVerifier, + eth: eth, } } @@ -162,37 +77,12 @@ func (api *BlockValidationAPI) ValidateBuilderSubmissionV1(params *BuilderBlockV feeRecipient := common.BytesToAddress(params.Message.ProposerFeeRecipient[:]) expectedProfit := params.Message.Value.BigInt() - var vmconfig vm.Config - var tracer *logger.AccessListTracer = nil - if api.accessVerifier != nil { - if err := api.accessVerifier.isBlacklisted(block.Coinbase()); err != nil { - return err - } - if err := api.accessVerifier.isBlacklisted(feeRecipient); err != nil { - return err - } - if err := api.accessVerifier.verifyTransactions(types.LatestSigner(api.eth.BlockChain().Config()), block.Transactions()); err != nil { - return err - } - isPostMerge := true // the call is PoS-native - timestamp := params.BuilderSubmitBlockRequest.ExecutionPayload.Timestamp - precompiles := vm.ActivePrecompiles(api.eth.APIBackend.ChainConfig().Rules(new(big.Int).SetUint64(params.ExecutionPayload.BlockNumber), isPostMerge, timestamp)) - tracer = logger.NewAccessListTracer(nil, common.Address{}, common.Address{}, precompiles) - vmconfig = vm.Config{Tracer: tracer, Debug: true} - } - - err = api.eth.BlockChain().ValidatePayload(block, feeRecipient, expectedProfit, params.RegisteredGasLimit, vmconfig) + err = api.eth.BlockChain().ValidatePayload(block, feeRecipient, expectedProfit, params.RegisteredGasLimit, *api.eth.BlockChain().GetVMConfig()) if err != nil { log.Error("invalid payload", "hash", payload.BlockHash.String(), "number", payload.BlockNumber, "parentHash", payload.ParentHash.String(), "err", err) return err } - if api.accessVerifier != nil && tracer != nil { - if err := api.accessVerifier.verifyTraces(tracer); err != nil { - return err - } - } - log.Info("validated block", "hash", block.Hash(), "number", block.NumberU64(), "parentHash", block.ParentHash()) return nil } @@ -261,36 +151,12 @@ func (api *BlockValidationAPI) ValidateBuilderSubmissionV2(params *BuilderBlockV feeRecipient := common.BytesToAddress(params.Message.ProposerFeeRecipient[:]) expectedProfit := params.Message.Value.ToBig() - var vmconfig vm.Config - var tracer *logger.AccessListTracer = nil - if api.accessVerifier != nil { - if err := api.accessVerifier.isBlacklisted(block.Coinbase()); err != nil { - return err - } - if err := api.accessVerifier.isBlacklisted(feeRecipient); err != nil { - return err - } - if err := api.accessVerifier.verifyTransactions(types.LatestSigner(api.eth.BlockChain().Config()), block.Transactions()); err != nil { - return err - } - isPostMerge := true // the call is PoS-native - precompiles := vm.ActivePrecompiles(api.eth.APIBackend.ChainConfig().Rules(new(big.Int).SetUint64(params.ExecutionPayload.BlockNumber), isPostMerge, params.ExecutionPayload.Timestamp)) - tracer = logger.NewAccessListTracer(nil, common.Address{}, common.Address{}, precompiles) - vmconfig = vm.Config{Tracer: tracer, Debug: true} - } - - err = api.eth.BlockChain().ValidatePayload(block, feeRecipient, expectedProfit, params.RegisteredGasLimit, vmconfig) + err = api.eth.BlockChain().ValidatePayload(block, feeRecipient, expectedProfit, params.RegisteredGasLimit, *api.eth.BlockChain().GetVMConfig()) if err != nil { log.Error("invalid payload", "hash", payload.BlockHash.String(), "number", payload.BlockNumber, "parentHash", payload.ParentHash.String(), "err", err) return err } - if api.accessVerifier != nil && tracer != nil { - if err := api.accessVerifier.verifyTraces(tracer); err != nil { - return err - } - } - log.Info("validated block", "hash", block.Hash(), "number", block.NumberU64(), "parentHash", block.ParentHash()) return nil } From 93662eeb09ef19fe1e5c0e0b057e23a730773d60 Mon Sep 17 00:00:00 2001 From: Michael Neuder Date: Tue, 23 May 2023 07:52:34 -0400 Subject: [PATCH 02/12] add parse error improved readability --- rpc/json.go | 2 +- rpc/server.go | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/rpc/json.go b/rpc/json.go index 8a3b162cab..7fbdd58d07 100644 --- a/rpc/json.go +++ b/rpc/json.go @@ -223,7 +223,7 @@ func (c *jsonCodec) readBatch() (messages []*jsonrpcMessage, batch bool, err err // This verifies basic syntax, etc. var rawmsg json.RawMessage if err := c.decode(&rawmsg); err != nil { - return nil, false, err + return nil, false, fmt.Errorf("err: %w, rawmsg: %v", err, rawmsg) } messages, batch = parseMessage(rawmsg) for i, msg := range messages { diff --git a/rpc/server.go b/rpc/server.go index b2d9503730..a513ae9767 100644 --- a/rpc/server.go +++ b/rpc/server.go @@ -18,6 +18,7 @@ package rpc import ( "context" + "fmt" "io" "sync" "sync/atomic" @@ -126,7 +127,9 @@ func (s *Server) serveSingleRequest(ctx context.Context, codec ServerCodec) { reqs, batch, err := codec.readBatch() if err != nil { if err != io.EOF { - resp := errorMessage(&invalidMessageError{"parse error"}) + fullErr := fmt.Sprintf("parse error: %v", err.Error()) + log.Error(fullErr) + resp := errorMessage(&invalidMessageError{fullErr}) codec.writeJSON(ctx, resp, true) } return From 6e32d3200c240540108e991cc9e281fecc621d7f Mon Sep 17 00:00:00 2001 From: ckoopmann Date: Mon, 4 Dec 2023 20:12:42 +0800 Subject: [PATCH 03/12] Add first log --- eth/block-validation/api.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/eth/block-validation/api.go b/eth/block-validation/api.go index 65ab4f4eca..14b5adb3a0 100644 --- a/eth/block-validation/api.go +++ b/eth/block-validation/api.go @@ -4,6 +4,7 @@ import ( "encoding/json" "errors" "fmt" + "time" capellaapi "github.com/attestantio/go-builder-client/api/capella" "github.com/attestantio/go-eth2-client/spec/phase0" @@ -115,13 +116,18 @@ func (r *BuilderBlockValidationRequestV2) UnmarshalJSON(data []byte) error { } func (api *BlockValidationAPI) ValidateBuilderSubmissionV2(params *BuilderBlockValidationRequestV2) error { + var start = time.Now() + log.Info("ValidateBuilderSubmissionV2 request received", "start", start, "blockHash", params.Message.BlockHash) + // TODO: fuzztest, make sure the validation is sound // TODO: handle context! if params.ExecutionPayload == nil { return errors.New("nil execution payload") } + payload := params.ExecutionPayload block, err := engine.ExecutionPayloadV2ToBlock(payload) + log.Debug("Parse Execution Payload to log", "elapsed", time.Since(start)) if err != nil { return err } From 573f7374762a9fecfc0dc7101b14e25974753d93 Mon Sep 17 00:00:00 2001 From: ckoopmann Date: Mon, 4 Dec 2023 20:12:42 +0800 Subject: [PATCH 04/12] Factor out Message / Payload comparison and add timing log --- eth/block-validation/api.go | 52 +++++++++++++++++++++++++------------ 1 file changed, 35 insertions(+), 17 deletions(-) diff --git a/eth/block-validation/api.go b/eth/block-validation/api.go index 14b5adb3a0..8a7a76a656 100644 --- a/eth/block-validation/api.go +++ b/eth/block-validation/api.go @@ -10,6 +10,7 @@ import ( "github.com/attestantio/go-eth2-client/spec/phase0" "github.com/ethereum/go-ethereum/beacon/engine" "github.com/ethereum/go-ethereum/common" + "github.com/ethereum/go-ethereum/core/types" "github.com/ethereum/go-ethereum/eth" "github.com/ethereum/go-ethereum/log" "github.com/ethereum/go-ethereum/node" @@ -88,6 +89,8 @@ func (api *BlockValidationAPI) ValidateBuilderSubmissionV1(params *BuilderBlockV return nil } + + type BuilderBlockValidationRequestV2 struct { capellaapi.SubmitBlockRequest RegisteredGasLimit uint64 `json:"registered_gas_limit,string"` @@ -115,6 +118,26 @@ func (r *BuilderBlockValidationRequestV2) UnmarshalJSON(data []byte) error { return nil } +func CompareMessageAndBlock(params *BuilderBlockValidationRequestV2, block *types.Block) error { + if params.Message.ParentHash != phase0.Hash32(block.ParentHash()) { + return fmt.Errorf("incorrect ParentHash %s, expected %s", params.Message.ParentHash.String(), block.ParentHash().String()) + } + + if params.Message.BlockHash != phase0.Hash32(block.Hash()) { + return fmt.Errorf("incorrect BlockHash %s, expected %s", params.Message.BlockHash.String(), block.Hash().String()) + } + + if params.Message.GasLimit != block.GasLimit() { + return fmt.Errorf("incorrect GasLimit %d, expected %d", params.Message.GasLimit, block.GasLimit()) + } + + if params.Message.GasUsed != block.GasUsed() { + return fmt.Errorf("incorrect GasUsed %d, expected %d", params.Message.GasUsed, block.GasUsed()) + } + return nil +} + + func (api *BlockValidationAPI) ValidateBuilderSubmissionV2(params *BuilderBlockValidationRequestV2) error { var start = time.Now() log.Info("ValidateBuilderSubmissionV2 request received", "start", start, "blockHash", params.Message.BlockHash) @@ -127,10 +150,12 @@ func (api *BlockValidationAPI) ValidateBuilderSubmissionV2(params *BuilderBlockV payload := params.ExecutionPayload block, err := engine.ExecutionPayloadV2ToBlock(payload) - log.Debug("Parse Execution Payload to log", "elapsed", time.Since(start)) if err != nil { + log.Debug("Block parsing failed", "time_elapsed", time.Since(start)) return err - } + } else { + log.Debug("Block parsing succeeded", "time_elapsed", time.Since(start)) + } // validated at the relay // isShanghai := api.eth.BlockChain().Config().IsShanghai(params.ExecutionPayload.Timestamp) @@ -138,21 +163,13 @@ func (api *BlockValidationAPI) ValidateBuilderSubmissionV2(params *BuilderBlockV // return err // } - if params.Message.ParentHash != phase0.Hash32(block.ParentHash()) { - return fmt.Errorf("incorrect ParentHash %s, expected %s", params.Message.ParentHash.String(), block.ParentHash().String()) - } - - if params.Message.BlockHash != phase0.Hash32(block.Hash()) { - return fmt.Errorf("incorrect BlockHash %s, expected %s", params.Message.BlockHash.String(), block.Hash().String()) - } - - if params.Message.GasLimit != block.GasLimit() { - return fmt.Errorf("incorrect GasLimit %d, expected %d", params.Message.GasLimit, block.GasLimit()) - } - - if params.Message.GasUsed != block.GasUsed() { - return fmt.Errorf("incorrect GasUsed %d, expected %d", params.Message.GasUsed, block.GasUsed()) - } + err = CompareMessageAndBlock(params, block) + if err != nil { + log.Debug("Message / Payload comparison failed", "time_elapsed", time.Since(start)) + return err + } else { + log.Debug("Message / Payload comparison succeeded", "time_elapsed", time.Since(start)) + } feeRecipient := common.BytesToAddress(params.Message.ProposerFeeRecipient[:]) expectedProfit := params.Message.Value.ToBig() @@ -166,3 +183,4 @@ func (api *BlockValidationAPI) ValidateBuilderSubmissionV2(params *BuilderBlockV log.Info("validated block", "hash", block.Hash(), "number", block.NumberU64(), "parentHash", block.ParentHash()) return nil } + From d7937b737a4c5df8019d8acb3f6d53f93cce9e07 Mon Sep 17 00:00:00 2001 From: ckoopmann Date: Mon, 4 Dec 2023 20:41:37 +0800 Subject: [PATCH 05/12] Additional timing logs inside ValidatePayload --- .github/workflows/go.yml | 45 +++++++++++++++++++++ core/blockchain.go | 79 ++++++++++++++++++++++++++++--------- eth/block-validation/api.go | 7 ++-- 3 files changed, 110 insertions(+), 21 deletions(-) diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml index 9a73a7e856..001abd5cf9 100644 --- a/.github/workflows/go.yml +++ b/.github/workflows/go.yml @@ -107,3 +107,48 @@ jobs: sleep 15 yarn run demo-private-tx pkill -9 geth || true + docker: + runs-on: ubuntu-latest + steps: + - name: Checkout + uses: actions/checkout@v4 + + - name: Set up QEMU + uses: docker/setup-qemu-action@v2 + + - name: Set up Docker Buildx + uses: docker/setup-buildx-action@v2 + + - name: Login to Docker Hub + uses: docker/login-action@v2 + with: + username: alextes + password: ${{ secrets.DOCKERHUB_TOKEN }} + + - name: Extract short SHA + run: echo "SHORT_SHA=$(echo ${{ github.sha }} | cut -c 1-7)" >> $GITHUB_ENV + + - name: Build (without push) + uses: docker/build-push-action@v4 + if: github.event_name == 'pull_request' + with: + context: . + push: false + tags: | + ultrasoundorg/builder:${{ env.SHORT_SHA }} + ultrasoundorg/builder:latest + cache-from: type=gha + cache-to: type=gha,mode=max + + - name: Build and push + uses: docker/build-push-action@v4 + if: github.event_name == 'push' + with: + context: . + push: true + tags: | + ultrasoundorg/builder:${{ env.SHORT_SHA }} + ultrasoundorg/builder:latest + cache-from: type=gha + cache-to: type=gha,mode=max + diff --git a/core/blockchain.go b/core/blockchain.go index d9ab1ef1ea..9caf502dd3 100644 --- a/core/blockchain.go +++ b/core/blockchain.go @@ -2494,32 +2494,47 @@ func (bc *BlockChain) SetBlockValidatorAndProcessorForTesting(v Validator, p Pro bc.processor = p } -func (bc *BlockChain) ValidatePayload(block *types.Block, feeRecipient common.Address, expectedProfit *big.Int, registeredGasLimit uint64, vmConfig vm.Config) error { +func (bc *BlockChain) ValidatePayload(block *types.Block, feeRecipient common.Address, expectedProfit *big.Int, registeredGasLimit uint64, vmConfig vm.Config, start time.Time) error { header := block.Header() if err := bc.engine.VerifyHeader(bc, header, true); err != nil { - return err - } + log.Debug("VerifyHeader failed", "time_elapsed", time.Since(start), "error", err) + return err + } else { + log.Debug("VerifyHeader succeeded", "time_elapsed", time.Since(start)) + } current := bc.CurrentBlock() reorg, err := bc.forker.ReorgNeeded(current, header) if err == nil && reorg { + log.Debug("ReorgNeeded failed", "time_elapsed", time.Since(start), "error", err) return errors.New("block requires a reorg") - } + } else { + log.Debug("ReorgNeeded succeeded", "time_elapsed", time.Since(start)) + } parent := bc.GetHeader(block.ParentHash(), block.NumberU64()-1) if parent == nil { + log.Debug("GetHeader(Parent) failed", "time_elapsed", time.Since(start), "error", err) return errors.New("parent not found") - } + } else { + log.Debug("GetHeader(Parent) succeeded", "time_elapsed", time.Since(start)) + } calculatedGasLimit := utils.CalcGasLimit(parent.GasLimit, registeredGasLimit) if calculatedGasLimit != header.GasLimit { + log.Debug("CalcGasLimit failed", "time_elapsed", time.Since(start), "error", err) return errors.New("incorrect gas limit set") - } + } else { + log.Debug("CalcGasLimit succeeded", "time_elapsed", time.Since(start)) + } statedb, err := bc.StateAt(parent.Root) if err != nil { + log.Debug("StateAt(parent.Root) failed", "time_elapsed", time.Since(start), "error", err) return err - } + } else { + log.Debug("StateAt(parent.Root) succeeded", "time_elapsed", time.Since(start)) + } // The chain importer is starting and stopping trie prefetchers. If a bad // block or other error is hit however, an early return may not properly @@ -2530,36 +2545,65 @@ func (bc *BlockChain) ValidatePayload(block *types.Block, feeRecipient common.Ad balanceBefore := statedb.GetBalance(feeRecipient) receipts, _, usedGas, err := bc.processor.Process(block, statedb, vmConfig) - if err != nil { - return err - } - + if err != nil { + log.Debug("Execute transactions failed", "time_elapsed", time.Since(start), "error", err) + return err + } else { + log.Debug("Execute transactions succeeded", "time_elapsed", time.Since(start)) + } + + err = nil; if bc.Config().IsShanghai(header.Time) { if header.WithdrawalsHash == nil { - return fmt.Errorf("withdrawals hash is missing") + err = 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") + err = fmt.Errorf("withdrawals hash present before shanghai") } if block.Withdrawals() != nil { - return fmt.Errorf("withdrawals list present in block body before shanghai") + err = fmt.Errorf("withdrawals list present in block body before shanghai") } } + if err != nil { + log.Debug("Check Withdrawal hash failed", "time_elapsed", time.Since(start), "error", err) + return err; + } else { + log.Debug("Check Withdrawal hash succeeded", "time_elapsed", time.Since(start)) + } + if err := bc.validator.ValidateBody(block); err != nil { + log.Debug("ValidateBody failed", "time_elapsed", time.Since(start), "error", err) return err - } + } else { + log.Debug("ValidateBody succeeded", "time_elapsed", time.Since(start)) + } if err := bc.validator.ValidateState(block, statedb, receipts, usedGas); err != nil { + log.Debug("ValidateState failed", "time_elapsed", time.Since(start), "error", err) return err - } + } else { + log.Debug("ValidateState succeeded", "time_elapsed", time.Since(start)) + } // First just check the balance delta to see if it matches. balanceAfter := statedb.GetBalance(feeRecipient) feeRecipientDiff := new(big.Int).Sub(balanceAfter, balanceBefore) + err = CheckProposerPayment(expectedProfit, feeRecipient,feeRecipientDiff, receipts, block) + if err != nil { + log.Debug("CheckProposerPayment failed", "time_elapsed", time.Since(start), "error", err) + return err + } else { + log.Debug("CheckProposerPayment succeeded", "time_elapsed", time.Since(start)) + } + + return nil +} + +func CheckProposerPayment(expectedProfit *big.Int, feeRecipient common.Address, feeRecipientDiff *big.Int, receipts types.Receipts, block *types.Block) error { // If diff is sufficiently large, just return success. if feeRecipientDiff.Cmp(expectedProfit) >= 0 { return nil @@ -2609,8 +2653,7 @@ func (bc *BlockChain) ValidatePayload(block *types.Block, feeRecipient common.Ad if paymentTx.GasFeeCap().Cmp(block.BaseFee()) != 0 { return fmt.Errorf("malformed proposer payment, unexpected gas fee cap") } - - return nil + return nil } // SetTrieFlushInterval configures how often in-memory tries are persisted to disk. diff --git a/eth/block-validation/api.go b/eth/block-validation/api.go index 8a7a76a656..c79c099b19 100644 --- a/eth/block-validation/api.go +++ b/eth/block-validation/api.go @@ -50,6 +50,7 @@ type BuilderBlockValidationRequest struct { func (api *BlockValidationAPI) ValidateBuilderSubmissionV1(params *BuilderBlockValidationRequest) error { // TODO: fuzztest, make sure the validation is sound // TODO: handle context! + start := time.Now() if params.ExecutionPayload == nil { return errors.New("nil execution payload") @@ -79,7 +80,7 @@ func (api *BlockValidationAPI) ValidateBuilderSubmissionV1(params *BuilderBlockV feeRecipient := common.BytesToAddress(params.Message.ProposerFeeRecipient[:]) expectedProfit := params.Message.Value.BigInt() - err = api.eth.BlockChain().ValidatePayload(block, feeRecipient, expectedProfit, params.RegisteredGasLimit, *api.eth.BlockChain().GetVMConfig()) + err = api.eth.BlockChain().ValidatePayload(block, feeRecipient, expectedProfit, params.RegisteredGasLimit, *api.eth.BlockChain().GetVMConfig(), start) if err != nil { log.Error("invalid payload", "hash", payload.BlockHash.String(), "number", payload.BlockNumber, "parentHash", payload.ParentHash.String(), "err", err) return err @@ -139,7 +140,7 @@ func CompareMessageAndBlock(params *BuilderBlockValidationRequestV2, block *type func (api *BlockValidationAPI) ValidateBuilderSubmissionV2(params *BuilderBlockValidationRequestV2) error { - var start = time.Now() + start := time.Now() log.Info("ValidateBuilderSubmissionV2 request received", "start", start, "blockHash", params.Message.BlockHash) // TODO: fuzztest, make sure the validation is sound @@ -174,7 +175,7 @@ func (api *BlockValidationAPI) ValidateBuilderSubmissionV2(params *BuilderBlockV feeRecipient := common.BytesToAddress(params.Message.ProposerFeeRecipient[:]) expectedProfit := params.Message.Value.ToBig() - err = api.eth.BlockChain().ValidatePayload(block, feeRecipient, expectedProfit, params.RegisteredGasLimit, *api.eth.BlockChain().GetVMConfig()) + err = api.eth.BlockChain().ValidatePayload(block, feeRecipient, expectedProfit, params.RegisteredGasLimit, *api.eth.BlockChain().GetVMConfig(), start) if err != nil { log.Error("invalid payload", "hash", payload.BlockHash.String(), "number", payload.BlockNumber, "parentHash", payload.ParentHash.String(), "err", err) return err From b513252e9b8157c0b8df5b3e387a87300a7dffa8 Mon Sep 17 00:00:00 2001 From: ckoopmann Date: Mon, 4 Dec 2023 20:12:42 +0800 Subject: [PATCH 06/12] Add request id to logs --- core/blockchain.go | 43 +++++++++++++++++++------------------ eth/block-validation/api.go | 19 +++++++++------- 2 files changed, 33 insertions(+), 29 deletions(-) diff --git a/core/blockchain.go b/core/blockchain.go index 9caf502dd3..709e2d0301 100644 --- a/core/blockchain.go +++ b/core/blockchain.go @@ -29,6 +29,7 @@ import ( "sync/atomic" "time" + "github.com/google/uuid" "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/common/lru" "github.com/ethereum/go-ethereum/common/mclock" @@ -2494,46 +2495,46 @@ func (bc *BlockChain) SetBlockValidatorAndProcessorForTesting(v Validator, p Pro bc.processor = p } -func (bc *BlockChain) ValidatePayload(block *types.Block, feeRecipient common.Address, expectedProfit *big.Int, registeredGasLimit uint64, vmConfig vm.Config, start time.Time) error { +func (bc *BlockChain) ValidatePayload(block *types.Block, feeRecipient common.Address, expectedProfit *big.Int, registeredGasLimit uint64, vmConfig vm.Config, start time.Time, requestId uuid.UUID) error { header := block.Header() if err := bc.engine.VerifyHeader(bc, header, true); err != nil { - log.Debug("VerifyHeader failed", "time_elapsed", time.Since(start), "error", err) + log.Debug("VerifyHeader failed", "time_elapsed", time.Since(start), "error", err, "requestId", requestId) return err } else { - log.Debug("VerifyHeader succeeded", "time_elapsed", time.Since(start)) + log.Debug("VerifyHeader succeeded", "time_elapsed", time.Since(start), "requestId", requestId) } current := bc.CurrentBlock() reorg, err := bc.forker.ReorgNeeded(current, header) if err == nil && reorg { - log.Debug("ReorgNeeded failed", "time_elapsed", time.Since(start), "error", err) + log.Debug("ReorgNeeded failed", "time_elapsed", time.Since(start), "error", err, "requestId", requestId) return errors.New("block requires a reorg") } else { - log.Debug("ReorgNeeded succeeded", "time_elapsed", time.Since(start)) + log.Debug("ReorgNeeded succeeded", "time_elapsed", time.Since(start), "requestId", requestId) } parent := bc.GetHeader(block.ParentHash(), block.NumberU64()-1) if parent == nil { - log.Debug("GetHeader(Parent) failed", "time_elapsed", time.Since(start), "error", err) + log.Debug("GetHeader(Parent) failed", "time_elapsed", time.Since(start), "error", err, "requestId", requestId) return errors.New("parent not found") } else { - log.Debug("GetHeader(Parent) succeeded", "time_elapsed", time.Since(start)) + log.Debug("GetHeader(Parent) succeeded", "time_elapsed", time.Since(start), "requestId", requestId) } calculatedGasLimit := utils.CalcGasLimit(parent.GasLimit, registeredGasLimit) if calculatedGasLimit != header.GasLimit { - log.Debug("CalcGasLimit failed", "time_elapsed", time.Since(start), "error", err) + log.Debug("CalcGasLimit failed", "time_elapsed", time.Since(start), "error", err, "requestId", requestId) return errors.New("incorrect gas limit set") } else { - log.Debug("CalcGasLimit succeeded", "time_elapsed", time.Since(start)) + log.Debug("CalcGasLimit succeeded", "time_elapsed", time.Since(start), "requestId", requestId) } statedb, err := bc.StateAt(parent.Root) if err != nil { - log.Debug("StateAt(parent.Root) failed", "time_elapsed", time.Since(start), "error", err) + log.Debug("StateAt(parent.Root) failed", "time_elapsed", time.Since(start), "error", err, "requestId", requestId) return err } else { - log.Debug("StateAt(parent.Root) succeeded", "time_elapsed", time.Since(start)) + log.Debug("StateAt(parent.Root) succeeded", "time_elapsed", time.Since(start), "requestId", requestId) } // The chain importer is starting and stopping trie prefetchers. If a bad @@ -2546,10 +2547,10 @@ func (bc *BlockChain) ValidatePayload(block *types.Block, feeRecipient common.Ad receipts, _, usedGas, err := bc.processor.Process(block, statedb, vmConfig) if err != nil { - log.Debug("Execute transactions failed", "time_elapsed", time.Since(start), "error", err) + log.Debug("Execute transactions failed", "time_elapsed", time.Since(start), "error", err, "requestId", requestId) return err } else { - log.Debug("Execute transactions succeeded", "time_elapsed", time.Since(start)) + log.Debug("Execute transactions succeeded", "time_elapsed", time.Since(start), "requestId", requestId) } err = nil; @@ -2567,25 +2568,25 @@ func (bc *BlockChain) ValidatePayload(block *types.Block, feeRecipient common.Ad } } if err != nil { - log.Debug("Check Withdrawal hash failed", "time_elapsed", time.Since(start), "error", err) + log.Debug("Check Withdrawal hash failed", "time_elapsed", time.Since(start), "error", err, "requestId", requestId) return err; } else { - log.Debug("Check Withdrawal hash succeeded", "time_elapsed", time.Since(start)) + log.Debug("Check Withdrawal hash succeeded", "time_elapsed", time.Since(start), "requestId", requestId) } if err := bc.validator.ValidateBody(block); err != nil { - log.Debug("ValidateBody failed", "time_elapsed", time.Since(start), "error", err) + log.Debug("ValidateBody failed", "time_elapsed", time.Since(start), "error", err, "requestId", requestId) return err } else { - log.Debug("ValidateBody succeeded", "time_elapsed", time.Since(start)) + log.Debug("ValidateBody succeeded", "time_elapsed", time.Since(start), "requestId", requestId) } if err := bc.validator.ValidateState(block, statedb, receipts, usedGas); err != nil { - log.Debug("ValidateState failed", "time_elapsed", time.Since(start), "error", err) + log.Debug("ValidateState failed", "time_elapsed", time.Since(start), "error", err, "requestId", requestId) return err } else { - log.Debug("ValidateState succeeded", "time_elapsed", time.Since(start)) + log.Debug("ValidateState succeeded", "time_elapsed", time.Since(start), "requestId", requestId) } // First just check the balance delta to see if it matches. @@ -2594,10 +2595,10 @@ func (bc *BlockChain) ValidatePayload(block *types.Block, feeRecipient common.Ad err = CheckProposerPayment(expectedProfit, feeRecipient,feeRecipientDiff, receipts, block) if err != nil { - log.Debug("CheckProposerPayment failed", "time_elapsed", time.Since(start), "error", err) + log.Debug("CheckProposerPayment failed", "time_elapsed", time.Since(start), "error", err, "requestId", requestId) return err } else { - log.Debug("CheckProposerPayment succeeded", "time_elapsed", time.Since(start)) + log.Debug("CheckProposerPayment succeeded", "time_elapsed", time.Since(start), "requestId", requestId) } return nil diff --git a/eth/block-validation/api.go b/eth/block-validation/api.go index c79c099b19..49c1a2814c 100644 --- a/eth/block-validation/api.go +++ b/eth/block-validation/api.go @@ -15,6 +15,7 @@ import ( "github.com/ethereum/go-ethereum/log" "github.com/ethereum/go-ethereum/node" "github.com/ethereum/go-ethereum/rpc" + "github.com/google/uuid" boostTypes "github.com/flashbots/go-boost-utils/types" ) @@ -51,6 +52,7 @@ func (api *BlockValidationAPI) ValidateBuilderSubmissionV1(params *BuilderBlockV // TODO: fuzztest, make sure the validation is sound // TODO: handle context! start := time.Now() + requestId := uuid.New() if params.ExecutionPayload == nil { return errors.New("nil execution payload") @@ -80,7 +82,7 @@ func (api *BlockValidationAPI) ValidateBuilderSubmissionV1(params *BuilderBlockV feeRecipient := common.BytesToAddress(params.Message.ProposerFeeRecipient[:]) expectedProfit := params.Message.Value.BigInt() - err = api.eth.BlockChain().ValidatePayload(block, feeRecipient, expectedProfit, params.RegisteredGasLimit, *api.eth.BlockChain().GetVMConfig(), start) + err = api.eth.BlockChain().ValidatePayload(block, feeRecipient, expectedProfit, params.RegisteredGasLimit, *api.eth.BlockChain().GetVMConfig(), start, requestId) if err != nil { log.Error("invalid payload", "hash", payload.BlockHash.String(), "number", payload.BlockNumber, "parentHash", payload.ParentHash.String(), "err", err) return err @@ -141,7 +143,8 @@ func CompareMessageAndBlock(params *BuilderBlockValidationRequestV2, block *type func (api *BlockValidationAPI) ValidateBuilderSubmissionV2(params *BuilderBlockValidationRequestV2) error { start := time.Now() - log.Info("ValidateBuilderSubmissionV2 request received", "start", start, "blockHash", params.Message.BlockHash) + requestId := uuid.New() + log.Info("ValidateBuilderSubmissionV2 request received", "start", start, "blockHash", params.Message.BlockHash, "requestId", requestId) // TODO: fuzztest, make sure the validation is sound // TODO: handle context! @@ -152,10 +155,10 @@ func (api *BlockValidationAPI) ValidateBuilderSubmissionV2(params *BuilderBlockV payload := params.ExecutionPayload block, err := engine.ExecutionPayloadV2ToBlock(payload) if err != nil { - log.Debug("Block parsing failed", "time_elapsed", time.Since(start)) + log.Debug("Block parsing failed", "time_elapsed", time.Since(start), "requestId", requestId) return err } else { - log.Debug("Block parsing succeeded", "time_elapsed", time.Since(start)) + log.Debug("Block parsing succeeded", "time_elapsed", time.Since(start), "requestId", requestId) } // validated at the relay @@ -166,22 +169,22 @@ func (api *BlockValidationAPI) ValidateBuilderSubmissionV2(params *BuilderBlockV err = CompareMessageAndBlock(params, block) if err != nil { - log.Debug("Message / Payload comparison failed", "time_elapsed", time.Since(start)) + log.Debug("Message / Payload comparison failed", "time_elapsed", time.Since(start), "requestId", requestId) return err } else { - log.Debug("Message / Payload comparison succeeded", "time_elapsed", time.Since(start)) + log.Debug("Message / Payload comparison succeeded", "time_elapsed", time.Since(start), "requestId", requestId) } feeRecipient := common.BytesToAddress(params.Message.ProposerFeeRecipient[:]) expectedProfit := params.Message.Value.ToBig() - err = api.eth.BlockChain().ValidatePayload(block, feeRecipient, expectedProfit, params.RegisteredGasLimit, *api.eth.BlockChain().GetVMConfig(), start) + err = api.eth.BlockChain().ValidatePayload(block, feeRecipient, expectedProfit, params.RegisteredGasLimit, *api.eth.BlockChain().GetVMConfig(), start, requestId) if err != nil { log.Error("invalid payload", "hash", payload.BlockHash.String(), "number", payload.BlockNumber, "parentHash", payload.ParentHash.String(), "err", err) return err } - log.Info("validated block", "hash", block.Hash(), "number", block.NumberU64(), "parentHash", block.ParentHash()) + log.Info("validated block", "hash", block.Hash(), "number", block.NumberU64(), "parentHash", block.ParentHash(), "time_elapsed", time.Since(start), "requestId", requestId) return nil } From 0c12772447542915972943af477a706a0c466be8 Mon Sep 17 00:00:00 2001 From: ckoopmann Date: Mon, 4 Dec 2023 20:26:10 +0800 Subject: [PATCH 07/12] Use tab instead of spaces --- core/blockchain.go | 90 ++++++++++++++++++------------------- eth/block-validation/api.go | 36 +++++++-------- 2 files changed, 63 insertions(+), 63 deletions(-) diff --git a/core/blockchain.go b/core/blockchain.go index 709e2d0301..469076db17 100644 --- a/core/blockchain.go +++ b/core/blockchain.go @@ -29,7 +29,7 @@ import ( "sync/atomic" "time" - "github.com/google/uuid" + "github.com/google/uuid" "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/common/lru" "github.com/ethereum/go-ethereum/common/mclock" @@ -2498,44 +2498,44 @@ func (bc *BlockChain) SetBlockValidatorAndProcessorForTesting(v Validator, p Pro func (bc *BlockChain) ValidatePayload(block *types.Block, feeRecipient common.Address, expectedProfit *big.Int, registeredGasLimit uint64, vmConfig vm.Config, start time.Time, requestId uuid.UUID) error { header := block.Header() if err := bc.engine.VerifyHeader(bc, header, true); err != nil { - log.Debug("VerifyHeader failed", "time_elapsed", time.Since(start), "error", err, "requestId", requestId) - return err + log.Debug("VerifyHeader failed", "time_elapsed", time.Since(start), "error", err, "requestId", requestId) + return err } else { - log.Debug("VerifyHeader succeeded", "time_elapsed", time.Since(start), "requestId", requestId) - } + log.Debug("VerifyHeader succeeded", "time_elapsed", time.Since(start), "requestId", requestId) + } current := bc.CurrentBlock() reorg, err := bc.forker.ReorgNeeded(current, header) if err == nil && reorg { - log.Debug("ReorgNeeded failed", "time_elapsed", time.Since(start), "error", err, "requestId", requestId) + log.Debug("ReorgNeeded failed", "time_elapsed", time.Since(start), "error", err, "requestId", requestId) return errors.New("block requires a reorg") } else { - log.Debug("ReorgNeeded succeeded", "time_elapsed", time.Since(start), "requestId", requestId) - } + log.Debug("ReorgNeeded succeeded", "time_elapsed", time.Since(start), "requestId", requestId) + } parent := bc.GetHeader(block.ParentHash(), block.NumberU64()-1) if parent == nil { - log.Debug("GetHeader(Parent) failed", "time_elapsed", time.Since(start), "error", err, "requestId", requestId) + log.Debug("GetHeader(Parent) failed", "time_elapsed", time.Since(start), "error", err, "requestId", requestId) return errors.New("parent not found") } else { - log.Debug("GetHeader(Parent) succeeded", "time_elapsed", time.Since(start), "requestId", requestId) - } + log.Debug("GetHeader(Parent) succeeded", "time_elapsed", time.Since(start), "requestId", requestId) + } calculatedGasLimit := utils.CalcGasLimit(parent.GasLimit, registeredGasLimit) if calculatedGasLimit != header.GasLimit { - log.Debug("CalcGasLimit failed", "time_elapsed", time.Since(start), "error", err, "requestId", requestId) + log.Debug("CalcGasLimit failed", "time_elapsed", time.Since(start), "error", err, "requestId", requestId) return errors.New("incorrect gas limit set") } else { - log.Debug("CalcGasLimit succeeded", "time_elapsed", time.Since(start), "requestId", requestId) - } + log.Debug("CalcGasLimit succeeded", "time_elapsed", time.Since(start), "requestId", requestId) + } statedb, err := bc.StateAt(parent.Root) if err != nil { - log.Debug("StateAt(parent.Root) failed", "time_elapsed", time.Since(start), "error", err, "requestId", requestId) + log.Debug("StateAt(parent.Root) failed", "time_elapsed", time.Since(start), "error", err, "requestId", requestId) return err } else { - log.Debug("StateAt(parent.Root) succeeded", "time_elapsed", time.Since(start), "requestId", requestId) - } + log.Debug("StateAt(parent.Root) succeeded", "time_elapsed", time.Since(start), "requestId", requestId) + } // The chain importer is starting and stopping trie prefetchers. If a bad // block or other error is hit however, an early return may not properly @@ -2546,14 +2546,14 @@ func (bc *BlockChain) ValidatePayload(block *types.Block, feeRecipient common.Ad balanceBefore := statedb.GetBalance(feeRecipient) receipts, _, usedGas, err := bc.processor.Process(block, statedb, vmConfig) - if err != nil { - log.Debug("Execute transactions failed", "time_elapsed", time.Since(start), "error", err, "requestId", requestId) - return err - } else { - log.Debug("Execute transactions succeeded", "time_elapsed", time.Since(start), "requestId", requestId) - } - - err = nil; + if err != nil { + log.Debug("Execute transactions failed", "time_elapsed", time.Since(start), "error", err, "requestId", requestId) + return err + } else { + log.Debug("Execute transactions succeeded", "time_elapsed", time.Since(start), "requestId", requestId) + } + + err = nil; if bc.Config().IsShanghai(header.Time) { if header.WithdrawalsHash == nil { err = fmt.Errorf("withdrawals hash is missing") @@ -2567,39 +2567,39 @@ func (bc *BlockChain) ValidatePayload(block *types.Block, feeRecipient common.Ad err = fmt.Errorf("withdrawals list present in block body before shanghai") } } - if err != nil { - log.Debug("Check Withdrawal hash failed", "time_elapsed", time.Since(start), "error", err, "requestId", requestId) - return err; - } else { - log.Debug("Check Withdrawal hash succeeded", "time_elapsed", time.Since(start), "requestId", requestId) - } + if err != nil { + log.Debug("Check Withdrawal hash failed", "time_elapsed", time.Since(start), "error", err, "requestId", requestId) + return err; + } else { + log.Debug("Check Withdrawal hash succeeded", "time_elapsed", time.Since(start), "requestId", requestId) + } if err := bc.validator.ValidateBody(block); err != nil { - log.Debug("ValidateBody failed", "time_elapsed", time.Since(start), "error", err, "requestId", requestId) + log.Debug("ValidateBody failed", "time_elapsed", time.Since(start), "error", err, "requestId", requestId) return err } else { - log.Debug("ValidateBody succeeded", "time_elapsed", time.Since(start), "requestId", requestId) - } + log.Debug("ValidateBody succeeded", "time_elapsed", time.Since(start), "requestId", requestId) + } if err := bc.validator.ValidateState(block, statedb, receipts, usedGas); err != nil { - log.Debug("ValidateState failed", "time_elapsed", time.Since(start), "error", err, "requestId", requestId) + log.Debug("ValidateState failed", "time_elapsed", time.Since(start), "error", err, "requestId", requestId) return err } else { - log.Debug("ValidateState succeeded", "time_elapsed", time.Since(start), "requestId", requestId) - } + log.Debug("ValidateState succeeded", "time_elapsed", time.Since(start), "requestId", requestId) + } // First just check the balance delta to see if it matches. balanceAfter := statedb.GetBalance(feeRecipient) feeRecipientDiff := new(big.Int).Sub(balanceAfter, balanceBefore) - err = CheckProposerPayment(expectedProfit, feeRecipient,feeRecipientDiff, receipts, block) - if err != nil { - log.Debug("CheckProposerPayment failed", "time_elapsed", time.Since(start), "error", err, "requestId", requestId) - return err - } else { - log.Debug("CheckProposerPayment succeeded", "time_elapsed", time.Since(start), "requestId", requestId) - } + err = CheckProposerPayment(expectedProfit, feeRecipient,feeRecipientDiff, receipts, block) + if err != nil { + log.Debug("CheckProposerPayment failed", "time_elapsed", time.Since(start), "error", err, "requestId", requestId) + return err + } else { + log.Debug("CheckProposerPayment succeeded", "time_elapsed", time.Since(start), "requestId", requestId) + } return nil } @@ -2654,7 +2654,7 @@ func CheckProposerPayment(expectedProfit *big.Int, feeRecipient common.Address, if paymentTx.GasFeeCap().Cmp(block.BaseFee()) != 0 { return fmt.Errorf("malformed proposer payment, unexpected gas fee cap") } - return nil + return nil } // SetTrieFlushInterval configures how often in-memory tries are persisted to disk. diff --git a/eth/block-validation/api.go b/eth/block-validation/api.go index 49c1a2814c..58baf9004a 100644 --- a/eth/block-validation/api.go +++ b/eth/block-validation/api.go @@ -15,7 +15,7 @@ import ( "github.com/ethereum/go-ethereum/log" "github.com/ethereum/go-ethereum/node" "github.com/ethereum/go-ethereum/rpc" - "github.com/google/uuid" + "github.com/google/uuid" boostTypes "github.com/flashbots/go-boost-utils/types" ) @@ -51,8 +51,8 @@ type BuilderBlockValidationRequest struct { func (api *BlockValidationAPI) ValidateBuilderSubmissionV1(params *BuilderBlockValidationRequest) error { // TODO: fuzztest, make sure the validation is sound // TODO: handle context! - start := time.Now() - requestId := uuid.New() + start := time.Now() + requestId := uuid.New() if params.ExecutionPayload == nil { return errors.New("nil execution payload") @@ -137,14 +137,14 @@ func CompareMessageAndBlock(params *BuilderBlockValidationRequestV2, block *type if params.Message.GasUsed != block.GasUsed() { return fmt.Errorf("incorrect GasUsed %d, expected %d", params.Message.GasUsed, block.GasUsed()) } - return nil + return nil } func (api *BlockValidationAPI) ValidateBuilderSubmissionV2(params *BuilderBlockValidationRequestV2) error { - start := time.Now() - requestId := uuid.New() - log.Info("ValidateBuilderSubmissionV2 request received", "start", start, "blockHash", params.Message.BlockHash, "requestId", requestId) + start := time.Now() + requestId := uuid.New() + log.Info("ValidateBuilderSubmissionV2 request received", "start", start, "blockHash", params.Message.BlockHash, "requestId", requestId) // TODO: fuzztest, make sure the validation is sound // TODO: handle context! @@ -155,25 +155,25 @@ func (api *BlockValidationAPI) ValidateBuilderSubmissionV2(params *BuilderBlockV payload := params.ExecutionPayload block, err := engine.ExecutionPayloadV2ToBlock(payload) if err != nil { - log.Debug("Block parsing failed", "time_elapsed", time.Since(start), "requestId", requestId) + log.Debug("Block parsing failed", "time_elapsed", time.Since(start), "requestId", requestId) return err } else { - log.Debug("Block parsing succeeded", "time_elapsed", time.Since(start), "requestId", requestId) - } + log.Debug("Block parsing succeeded", "time_elapsed", time.Since(start), "requestId", requestId) + } // validated at the relay // isShanghai := api.eth.BlockChain().Config().IsShanghai(params.ExecutionPayload.Timestamp) // if err := verifyWithdrawals(block.Withdrawals(), params.WithdrawalsRoot, isShanghai); err != nil { - // return err + // return err // } - err = CompareMessageAndBlock(params, block) - if err != nil { - log.Debug("Message / Payload comparison failed", "time_elapsed", time.Since(start), "requestId", requestId) - return err - } else { - log.Debug("Message / Payload comparison succeeded", "time_elapsed", time.Since(start), "requestId", requestId) - } + err = CompareMessageAndBlock(params, block) + if err != nil { + log.Debug("Message / Payload comparison failed", "time_elapsed", time.Since(start), "requestId", requestId) + return err + } else { + log.Debug("Message / Payload comparison succeeded", "time_elapsed", time.Since(start), "requestId", requestId) + } feeRecipient := common.BytesToAddress(params.Message.ProposerFeeRecipient[:]) expectedProfit := params.Message.Value.ToBig() From a97c77826f9677451315f9ef1a705c443d3248b6 Mon Sep 17 00:00:00 2001 From: ckoopmann Date: Mon, 4 Dec 2023 20:48:41 +0800 Subject: [PATCH 08/12] Run docker job only when other jobs succeed --- .github/workflows/go.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml index 001abd5cf9..05355b827d 100644 --- a/.github/workflows/go.yml +++ b/.github/workflows/go.yml @@ -109,6 +109,7 @@ jobs: pkill -9 geth || true docker: runs-on: ubuntu-latest + needs: [build, lint, e2e] steps: - name: Checkout uses: actions/checkout@v4 From 90a98f0f99321135590c571bf782bcfebcce9173 Mon Sep 17 00:00:00 2001 From: ckoopmann Date: Mon, 4 Dec 2023 21:01:57 +0800 Subject: [PATCH 09/12] Assume default gas limit of 30 mil if registered limit is zero --- core/blockchain.go | 26 +++++++++++++++++++++----- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/core/blockchain.go b/core/blockchain.go index 469076db17..ad2613079e 100644 --- a/core/blockchain.go +++ b/core/blockchain.go @@ -2521,12 +2521,12 @@ func (bc *BlockChain) ValidatePayload(block *types.Block, feeRecipient common.Ad log.Debug("GetHeader(Parent) succeeded", "time_elapsed", time.Since(start), "requestId", requestId) } - calculatedGasLimit := utils.CalcGasLimit(parent.GasLimit, registeredGasLimit) - if calculatedGasLimit != header.GasLimit { - log.Debug("CalcGasLimit failed", "time_elapsed", time.Since(start), "error", err, "requestId", requestId) - return errors.New("incorrect gas limit set") + err = CheckGasLimit(parent.GasLimit, registeredGasLimit, header.GasLimit) + if err != nil { + log.Debug("CheckGasLimit failed", "time_elapsed", time.Since(start), "error", err) + return err } else { - log.Debug("CalcGasLimit succeeded", "time_elapsed", time.Since(start), "requestId", requestId) + log.Debug("CheckGasLimit succeeded", "time_elapsed", time.Since(start)) } statedb, err := bc.StateAt(parent.Root) @@ -2604,6 +2604,22 @@ func (bc *BlockChain) ValidatePayload(block *types.Block, feeRecipient common.Ad return nil } +func CheckGasLimit(parentGasLimit uint64, registeredGasLimit uint64, headerGasLimit uint64) error { + if registeredGasLimit == 0 && headerGasLimit == utils.CalcGasLimit(parentGasLimit, 30_000_000) { + // Prysm has a bug where it registers validators with a desired gas limit + // of 0. Some builders treat these as desiring gas limit 30_000_000. As a + // workaround, whenever the desired gas limit is 0, we accept both the + // limit as calculated with a desired limit of 0, and builders which fall + // back to calculating with the default 30_000_000. + } else { + calculatedGasLimit := utils.CalcGasLimit(parentGasLimit, registeredGasLimit) + if calculatedGasLimit != headerGasLimit { + return fmt.Errorf("incorrect gas limit set, expected: %d, got: %d", calculatedGasLimit, headerGasLimit) + } + } + return nil +} + func CheckProposerPayment(expectedProfit *big.Int, feeRecipient common.Address, feeRecipientDiff *big.Int, receipts types.Receipts, block *types.Block) error { // If diff is sufficiently large, just return success. if feeRecipientDiff.Cmp(expectedProfit) >= 0 { From e0d1e285406178e11b799864b056c865b543b26e Mon Sep 17 00:00:00 2001 From: ckoopmann Date: Mon, 4 Dec 2023 21:16:20 +0800 Subject: [PATCH 10/12] Fix lint failure --- core/blockchain.go | 13 +++-- eth/block-validation/api.go | 6 +-- eth/block-validation/api_test.go | 86 +------------------------------- 3 files changed, 9 insertions(+), 96 deletions(-) diff --git a/core/blockchain.go b/core/blockchain.go index ad2613079e..2dc5780567 100644 --- a/core/blockchain.go +++ b/core/blockchain.go @@ -29,7 +29,6 @@ import ( "sync/atomic" "time" - "github.com/google/uuid" "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/common/lru" "github.com/ethereum/go-ethereum/common/mclock" @@ -50,6 +49,7 @@ import ( "github.com/ethereum/go-ethereum/params" "github.com/ethereum/go-ethereum/rlp" "github.com/ethereum/go-ethereum/trie" + "github.com/google/uuid" ) var ( @@ -2498,7 +2498,7 @@ func (bc *BlockChain) SetBlockValidatorAndProcessorForTesting(v Validator, p Pro func (bc *BlockChain) ValidatePayload(block *types.Block, feeRecipient common.Address, expectedProfit *big.Int, registeredGasLimit uint64, vmConfig vm.Config, start time.Time, requestId uuid.UUID) error { header := block.Header() if err := bc.engine.VerifyHeader(bc, header, true); err != nil { - log.Debug("VerifyHeader failed", "time_elapsed", time.Since(start), "error", err, "requestId", requestId) + log.Debug("VerifyHeader failed", "time_elapsed", time.Since(start), "error", err, "requestId", requestId) return err } else { log.Debug("VerifyHeader succeeded", "time_elapsed", time.Since(start), "requestId", requestId) @@ -2515,7 +2515,7 @@ func (bc *BlockChain) ValidatePayload(block *types.Block, feeRecipient common.Ad parent := bc.GetHeader(block.ParentHash(), block.NumberU64()-1) if parent == nil { - log.Debug("GetHeader(Parent) failed", "time_elapsed", time.Since(start), "error", err, "requestId", requestId) + log.Debug("GetHeader(Parent) failed", "time_elapsed", time.Since(start), "error", err, "requestId", requestId) return errors.New("parent not found") } else { log.Debug("GetHeader(Parent) succeeded", "time_elapsed", time.Since(start), "requestId", requestId) @@ -2553,7 +2553,7 @@ func (bc *BlockChain) ValidatePayload(block *types.Block, feeRecipient common.Ad log.Debug("Execute transactions succeeded", "time_elapsed", time.Since(start), "requestId", requestId) } - err = nil; + err = nil if bc.Config().IsShanghai(header.Time) { if header.WithdrawalsHash == nil { err = fmt.Errorf("withdrawals hash is missing") @@ -2569,12 +2569,11 @@ func (bc *BlockChain) ValidatePayload(block *types.Block, feeRecipient common.Ad } if err != nil { log.Debug("Check Withdrawal hash failed", "time_elapsed", time.Since(start), "error", err, "requestId", requestId) - return err; + return err } else { log.Debug("Check Withdrawal hash succeeded", "time_elapsed", time.Since(start), "requestId", requestId) } - if err := bc.validator.ValidateBody(block); err != nil { log.Debug("ValidateBody failed", "time_elapsed", time.Since(start), "error", err, "requestId", requestId) return err @@ -2593,7 +2592,7 @@ func (bc *BlockChain) ValidatePayload(block *types.Block, feeRecipient common.Ad balanceAfter := statedb.GetBalance(feeRecipient) feeRecipientDiff := new(big.Int).Sub(balanceAfter, balanceBefore) - err = CheckProposerPayment(expectedProfit, feeRecipient,feeRecipientDiff, receipts, block) + err = CheckProposerPayment(expectedProfit, feeRecipient, feeRecipientDiff, receipts, block) if err != nil { log.Debug("CheckProposerPayment failed", "time_elapsed", time.Since(start), "error", err, "requestId", requestId) return err diff --git a/eth/block-validation/api.go b/eth/block-validation/api.go index 58baf9004a..7295ee6719 100644 --- a/eth/block-validation/api.go +++ b/eth/block-validation/api.go @@ -15,9 +15,9 @@ import ( "github.com/ethereum/go-ethereum/log" "github.com/ethereum/go-ethereum/node" "github.com/ethereum/go-ethereum/rpc" - "github.com/google/uuid" boostTypes "github.com/flashbots/go-boost-utils/types" + "github.com/google/uuid" ) // Register adds catalyst APIs to the full node. @@ -92,8 +92,6 @@ func (api *BlockValidationAPI) ValidateBuilderSubmissionV1(params *BuilderBlockV return nil } - - type BuilderBlockValidationRequestV2 struct { capellaapi.SubmitBlockRequest RegisteredGasLimit uint64 `json:"registered_gas_limit,string"` @@ -140,7 +138,6 @@ func CompareMessageAndBlock(params *BuilderBlockValidationRequestV2, block *type return nil } - func (api *BlockValidationAPI) ValidateBuilderSubmissionV2(params *BuilderBlockValidationRequestV2) error { start := time.Now() requestId := uuid.New() @@ -187,4 +184,3 @@ func (api *BlockValidationAPI) ValidateBuilderSubmissionV2(params *BuilderBlockV log.Info("validated block", "hash", block.Hash(), "number", block.NumberU64(), "parentHash", block.ParentHash(), "time_elapsed", time.Since(start), "requestId", requestId) return nil } - diff --git a/eth/block-validation/api_test.go b/eth/block-validation/api_test.go index eb3d487cf3..d1fdcac773 100644 --- a/eth/block-validation/api_test.go +++ b/eth/block-validation/api_test.go @@ -1,7 +1,6 @@ package blockvalidation import ( - "encoding/json" "errors" "math/big" "os" @@ -25,7 +24,6 @@ import ( "github.com/ethereum/go-ethereum/eth" "github.com/ethereum/go-ethereum/eth/downloader" "github.com/ethereum/go-ethereum/eth/ethconfig" - "github.com/ethereum/go-ethereum/eth/tracers/logger" "github.com/ethereum/go-ethereum/miner" "github.com/ethereum/go-ethereum/node" "github.com/ethereum/go-ethereum/p2p" @@ -59,7 +57,7 @@ func TestValidateBuilderSubmissionV1(t *testing.T) { ethservice.Merger().ReachTTD() defer n.Close() - api := NewBlockValidationAPI(ethservice, nil) + api := NewBlockValidationAPI(ethservice) parent := preMergeBlocks[len(preMergeBlocks)-1] api.eth.APIBackend.Miner().SetEtherbase(testValidatorAddr) @@ -123,25 +121,6 @@ func TestValidateBuilderSubmissionV1(t *testing.T) { blockRequest.ExecutionPayload.GasLimit -= 1 updatePayloadHash(t, blockRequest) - // TODO: test with contract calling blacklisted address - // Test tx from blacklisted address - api.accessVerifier = &AccessVerifier{ - blacklistedAddresses: map[common.Address]struct{}{ - testAddr: {}, - }, - } - require.ErrorContains(t, api.ValidateBuilderSubmissionV1(blockRequest), "transaction from blacklisted address 0x71562b71999873DB5b286dF957af199Ec94617F7") - - // Test tx to blacklisted address - api.accessVerifier = &AccessVerifier{ - blacklistedAddresses: map[common.Address]struct{}{ - {0x16}: {}, - }, - } - require.ErrorContains(t, api.ValidateBuilderSubmissionV1(blockRequest), "transaction to blacklisted address 0x1600000000000000000000000000000000000000") - - api.accessVerifier = nil - blockRequest.Message.GasUsed = 10 require.ErrorContains(t, api.ValidateBuilderSubmissionV1(blockRequest), "incorrect GasUsed 10, expected 119990") blockRequest.Message.GasUsed = execData.GasUsed @@ -173,7 +152,7 @@ func TestValidateBuilderSubmissionV2(t *testing.T) { ethservice.Merger().ReachTTD() defer n.Close() - api := NewBlockValidationAPI(ethservice, nil) + api := NewBlockValidationAPI(ethservice) parent := preMergeBlocks[len(preMergeBlocks)-1] api.eth.APIBackend.Miner().SetEtherbase(testValidatorAddr) @@ -256,25 +235,6 @@ func TestValidateBuilderSubmissionV2(t *testing.T) { blockRequest.ExecutionPayload.GasLimit -= 1 updatePayloadHashV2(t, blockRequest) - // TODO: test with contract calling blacklisted address - // Test tx from blacklisted address - api.accessVerifier = &AccessVerifier{ - blacklistedAddresses: map[common.Address]struct{}{ - testAddr: {}, - }, - } - require.ErrorContains(t, api.ValidateBuilderSubmissionV2(blockRequest), "transaction from blacklisted address 0x71562b71999873DB5b286dF957af199Ec94617F7") - - // Test tx to blacklisted address - api.accessVerifier = &AccessVerifier{ - blacklistedAddresses: map[common.Address]struct{}{ - {0x16}: {}, - }, - } - require.ErrorContains(t, api.ValidateBuilderSubmissionV2(blockRequest), "transaction to blacklisted address 0x1600000000000000000000000000000000000000") - - api.accessVerifier = nil - blockRequest.Message.GasUsed = 10 require.ErrorContains(t, api.ValidateBuilderSubmissionV2(blockRequest), "incorrect GasUsed 10, expected 119996") blockRequest.Message.GasUsed = execData.GasUsed @@ -394,48 +354,6 @@ func assembleBlock(api *BlockValidationAPI, parentHash common.Hash, params *engi return nil, errors.New("payload did not resolve") } -func TestBlacklistLoad(t *testing.T) { - file, err := os.CreateTemp(".", "blacklist") - require.NoError(t, err) - defer os.Remove(file.Name()) - - av, err := NewAccessVerifierFromFile(file.Name()) - require.Error(t, err) - require.Nil(t, av) - - ba := BlacklistedAddresses{common.Address{0x13}, common.Address{0x14}} - bytes, err := json.MarshalIndent(ba, "", " ") - require.NoError(t, err) - err = os.WriteFile(file.Name(), bytes, 0644) - require.NoError(t, err) - - av, err = NewAccessVerifierFromFile(file.Name()) - require.NoError(t, err) - require.NotNil(t, av) - require.EqualValues(t, av.blacklistedAddresses, map[common.Address]struct{}{ - {0x13}: {}, - {0x14}: {}, - }) - - require.NoError(t, av.verifyTraces(logger.NewAccessListTracer(nil, common.Address{}, common.Address{}, nil))) - - acl := types.AccessList{ - types.AccessTuple{ - Address: common.Address{0x14}, - }, - } - tracer := logger.NewAccessListTracer(acl, common.Address{}, common.Address{}, nil) - require.ErrorContains(t, av.verifyTraces(tracer), "blacklisted address 0x1400000000000000000000000000000000000000 in execution trace") - - acl = types.AccessList{ - types.AccessTuple{ - Address: common.Address{0x15}, - }, - } - tracer = logger.NewAccessListTracer(acl, common.Address{}, common.Address{}, nil) - require.NoError(t, av.verifyTraces(tracer)) -} - func ExecutableDataToExecutionPayload(data *engine.ExecutableData) (*boostTypes.ExecutionPayload, error) { transactionData := make([]hexutil.Bytes, len(data.Transactions)) for i, tx := range data.Transactions { From 2db6565627d31f12279ea2703bb389026c42e90c Mon Sep 17 00:00:00 2001 From: ckoopmann Date: Mon, 4 Dec 2023 21:29:11 +0800 Subject: [PATCH 11/12] Fix validation api test --- eth/block-validation/api_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/eth/block-validation/api_test.go b/eth/block-validation/api_test.go index d1fdcac773..6f9b9201fe 100644 --- a/eth/block-validation/api_test.go +++ b/eth/block-validation/api_test.go @@ -213,7 +213,7 @@ func TestValidateBuilderSubmissionV2(t *testing.T) { ProposerFeeRecipient: proposerAddr, GasLimit: execData.GasLimit, GasUsed: execData.GasUsed, - Value: uint256.NewInt(0), + Value: uint256.NewInt(149842511727213), }, ExecutionPayload: payload, }, From 52b20666b102fc47f956a5c3056dbc6b0cfdb850 Mon Sep 17 00:00:00 2001 From: ckoopmann Date: Mon, 4 Dec 2023 21:39:34 +0800 Subject: [PATCH 12/12] Run only ValidateBuilderSubmissionV2 test in CI --- .github/workflows/go.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml index 05355b827d..27300ff9a9 100644 --- a/.github/workflows/go.yml +++ b/.github/workflows/go.yml @@ -51,7 +51,7 @@ jobs: uses: actions/checkout@v2 - name: Test - run: go test ./core ./miner/... ./internal/ethapi/... ./les/... ./builder/... ./eth/block-validation/... + run: go test ./core ./miner/... ./internal/ethapi/... ./les/... ./builder/... ./eth/block-validation/... --run "TestValidateBuilderSubmissionV2" - name: Build run: make geth