From 4ff4c10949681d4d0b312e133ac5335560f89106 Mon Sep 17 00:00:00 2001 From: Vitaly Drogan Date: Thu, 7 Mar 2024 15:54:17 +0100 Subject: [PATCH] exclude CL withdrawals from profit calculation (#144) --- builder/config.go | 2 + builder/service.go | 2 +- cmd/geth/config.go | 3 + cmd/geth/main.go | 1 + cmd/utils/flags.go | 7 ++ core/blockchain.go | 11 +++- eth/block-validation/api.go | 13 ++-- eth/block-validation/api_test.go | 108 +++++++++++++++++++++++++++++-- 8 files changed, 136 insertions(+), 11 deletions(-) diff --git a/builder/config.go b/builder/config.go index de9868caaf..e3099fd5c5 100644 --- a/builder/config.go +++ b/builder/config.go @@ -22,6 +22,7 @@ type Config struct { SecondaryRemoteRelayEndpoints []string `toml:",omitempty"` ValidationBlocklist string `toml:",omitempty"` ValidationUseCoinbaseDiff bool `toml:",omitempty"` + ValidationExcludeWithdrawals bool `toml:",omitempty"` BuilderRateLimitDuration string `toml:",omitempty"` BuilderRateLimitMaxBurst int `toml:",omitempty"` BuilderRateLimitResubmitInterval string `toml:",omitempty"` @@ -52,6 +53,7 @@ var DefaultConfig = Config{ SecondaryRemoteRelayEndpoints: nil, ValidationBlocklist: "", ValidationUseCoinbaseDiff: false, + ValidationExcludeWithdrawals: false, BuilderRateLimitDuration: RateLimitIntervalDefault.String(), BuilderRateLimitMaxBurst: RateLimitBurstDefault, DiscardRevertibleTxOnErr: false, diff --git a/builder/service.go b/builder/service.go index 67e052b5bf..09be2c632f 100644 --- a/builder/service.go +++ b/builder/service.go @@ -214,7 +214,7 @@ func Register(stack *node.Node, backend *eth.Ethereum, cfg *Config) error { return fmt.Errorf("failed to load validation blocklist %w", err) } } - validator = blockvalidation.NewBlockValidationAPI(backend, accessVerifier, cfg.ValidationUseCoinbaseDiff) + validator = blockvalidation.NewBlockValidationAPI(backend, accessVerifier, cfg.ValidationUseCoinbaseDiff, cfg.ValidationExcludeWithdrawals) } // Set up builder rate limiter based on environment variables or CLI flags. diff --git a/cmd/geth/config.go b/cmd/geth/config.go index d2c698b631..9a43f0f72f 100644 --- a/cmd/geth/config.go +++ b/cmd/geth/config.go @@ -180,6 +180,9 @@ func makeFullNode(ctx *cli.Context) (*node.Node, ethapi.Backend) { if ctx.IsSet(utils.BuilderBlockValidationUseBalanceDiff.Name) { bvConfig.UseBalanceDiffProfit = ctx.Bool(utils.BuilderBlockValidationUseBalanceDiff.Name) } + if ctx.IsSet(utils.BuilderBlockValidationExcludeWithdrawals.Name) { + bvConfig.ExcludeWithdrawals = ctx.Bool(utils.BuilderBlockValidationExcludeWithdrawals.Name) + } if err := blockvalidationapi.Register(stack, eth, bvConfig); err != nil { utils.Fatalf("Failed to register the Block Validation API: %v", err) diff --git a/cmd/geth/main.go b/cmd/geth/main.go index 329a348451..1eaa57a495 100644 --- a/cmd/geth/main.go +++ b/cmd/geth/main.go @@ -163,6 +163,7 @@ var ( utils.BuilderEnableValidatorChecks, utils.BuilderBlockValidationBlacklistSourceFilePath, utils.BuilderBlockValidationUseBalanceDiff, + utils.BuilderBlockValidationExcludeWithdrawals, utils.BuilderEnableLocalRelay, utils.BuilderSecondsInSlot, utils.BuilderSlotsInEpoch, diff --git a/cmd/utils/flags.go b/cmd/utils/flags.go index 2944ccdc91..dd5832fa26 100644 --- a/cmd/utils/flags.go +++ b/cmd/utils/flags.go @@ -741,6 +741,12 @@ var ( Value: false, Category: flags.BuilderCategory, } + BuilderBlockValidationExcludeWithdrawals = &cli.BoolFlag{ + Name: "builder.validation_exclude_withdrawals", + Usage: "Block validation API will exclude CL withdrawals to the fee recipient from the balance delta.", + Value: false, + Category: flags.BuilderCategory, + } BuilderEnableLocalRelay = &cli.BoolFlag{ Name: "builder.local_relay", Usage: "Enable the local relay", @@ -1724,6 +1730,7 @@ func SetBuilderConfig(ctx *cli.Context, cfg *builder.Config) { cfg.ValidationBlocklist = ctx.String(BuilderBlockValidationBlacklistSourceFilePath.Name) } cfg.ValidationUseCoinbaseDiff = ctx.Bool(BuilderBlockValidationUseBalanceDiff.Name) + cfg.ValidationExcludeWithdrawals = ctx.Bool(BuilderBlockValidationExcludeWithdrawals.Name) cfg.BuilderRateLimitDuration = ctx.String(BuilderRateLimitDuration.Name) cfg.BuilderRateLimitMaxBurst = ctx.Int(BuilderRateLimitMaxBurst.Name) cfg.BuilderSubmissionOffset = ctx.Duration(BuilderSubmissionOffset.Name) diff --git a/core/blockchain.go b/core/blockchain.go index 8e5372c905..c4caf08e02 100644 --- a/core/blockchain.go +++ b/core/blockchain.go @@ -2498,7 +2498,8 @@ func (bc *BlockChain) SetBlockValidatorAndProcessorForTesting(v Validator, p Pro // It returns nil if the payload is valid, otherwise it returns an error. // - `useBalanceDiffProfit` if set to false, proposer payment is assumed to be in the last transaction of the block // otherwise we use proposer balance changes after the block to calculate proposer payment (see details in the code) -func (bc *BlockChain) ValidatePayload(block *types.Block, feeRecipient common.Address, expectedProfit *big.Int, registeredGasLimit uint64, vmConfig vm.Config, useBalanceDiffProfit bool) error { +// - `excludeWithdrawals` if set to true, withdrawals to the fee recipient are excluded from the balance change +func (bc *BlockChain) ValidatePayload(block *types.Block, feeRecipient common.Address, expectedProfit *big.Int, registeredGasLimit uint64, vmConfig vm.Config, useBalanceDiffProfit, excludeWithdrawals bool) error { header := block.Header() if err := bc.engine.VerifyHeader(bc, header, true); err != nil { return err @@ -2540,6 +2541,14 @@ func (bc *BlockChain) ValidatePayload(block *types.Block, feeRecipient common.Ad feeRecipientBalanceDelta := new(big.Int).Set(statedb.GetBalance(feeRecipient)) feeRecipientBalanceDelta.Sub(feeRecipientBalanceDelta, feeRecipientBalanceBefore) + if excludeWithdrawals { + for _, w := range block.Withdrawals() { + if w.Address == feeRecipient { + amount := new(big.Int).Mul(new(big.Int).SetUint64(w.Amount), big.NewInt(params.GWei)) + feeRecipientBalanceDelta.Sub(feeRecipientBalanceDelta, amount) + } + } + } if bc.Config().IsShanghai(header.Time) { if header.WithdrawalsHash == nil { diff --git a/eth/block-validation/api.go b/eth/block-validation/api.go index 58ad8dc93d..4bb7234aec 100644 --- a/eth/block-validation/api.go +++ b/eth/block-validation/api.go @@ -90,6 +90,8 @@ type BlockValidationConfig struct { BlacklistSourceFilePath string // If set to true, proposer payment is calculated as a balance difference of the fee recipient. UseBalanceDiffProfit bool + // If set to true, withdrawals to the fee recipient are excluded from the balance difference. + ExcludeWithdrawals bool } // Register adds catalyst APIs to the full node. @@ -106,7 +108,7 @@ func Register(stack *node.Node, backend *eth.Ethereum, cfg BlockValidationConfig stack.RegisterAPIs([]rpc.API{ { Namespace: "flashbots", - Service: NewBlockValidationAPI(backend, accessVerifier, cfg.UseBalanceDiffProfit), + Service: NewBlockValidationAPI(backend, accessVerifier, cfg.UseBalanceDiffProfit, cfg.ExcludeWithdrawals), }, }) return nil @@ -117,15 +119,18 @@ type BlockValidationAPI struct { accessVerifier *AccessVerifier // If set to true, proposer payment is calculated as a balance difference of the fee recipient. useBalanceDiffProfit bool + // If set to true, withdrawals to the fee recipient are excluded from the balance delta. + excludeWithdrawals bool } // 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, useBalanceDiffProfit bool) *BlockValidationAPI { +func NewBlockValidationAPI(eth *eth.Ethereum, accessVerifier *AccessVerifier, useBalanceDiffProfit, excludeWithdrawals bool) *BlockValidationAPI { return &BlockValidationAPI{ eth: eth, accessVerifier: accessVerifier, useBalanceDiffProfit: useBalanceDiffProfit, + excludeWithdrawals: excludeWithdrawals, } } @@ -185,7 +190,7 @@ func (api *BlockValidationAPI) ValidateBuilderSubmissionV1(params *BuilderBlockV vmconfig = vm.Config{Tracer: tracer, Debug: true} } - err = api.eth.BlockChain().ValidatePayload(block, feeRecipient, expectedProfit, params.RegisteredGasLimit, vmconfig, api.useBalanceDiffProfit) + err = api.eth.BlockChain().ValidatePayload(block, feeRecipient, expectedProfit, params.RegisteredGasLimit, vmconfig, api.useBalanceDiffProfit, api.excludeWithdrawals) if err != nil { log.Error("invalid payload", "hash", payload.BlockHash.String(), "number", payload.BlockNumber, "parentHash", payload.ParentHash.String(), "err", err) return err @@ -277,7 +282,7 @@ func (api *BlockValidationAPI) ValidateBuilderSubmissionV2(params *BuilderBlockV vmconfig = vm.Config{Tracer: tracer, Debug: true} } - err = api.eth.BlockChain().ValidatePayload(block, feeRecipient, expectedProfit, params.RegisteredGasLimit, vmconfig, api.useBalanceDiffProfit) + err = api.eth.BlockChain().ValidatePayload(block, feeRecipient, expectedProfit, params.RegisteredGasLimit, vmconfig, api.useBalanceDiffProfit, api.excludeWithdrawals) if err != nil { log.Error("invalid payload", "hash", payload.BlockHash.String(), "number", payload.BlockNumber, "parentHash", payload.ParentHash.String(), "err", err) return err diff --git a/eth/block-validation/api_test.go b/eth/block-validation/api_test.go index 5a925f4316..5072330260 100644 --- a/eth/block-validation/api_test.go +++ b/eth/block-validation/api_test.go @@ -69,7 +69,7 @@ func TestValidateBuilderSubmissionV1(t *testing.T) { ethservice.Merger().ReachTTD() defer n.Close() - api := NewBlockValidationAPI(ethservice, nil, true) + api := NewBlockValidationAPI(ethservice, nil, true, true) parent := preMergeBlocks[len(preMergeBlocks)-1] api.eth.APIBackend.Miner().SetEtherbase(testValidatorAddr) @@ -179,7 +179,7 @@ func TestValidateBuilderSubmissionV2(t *testing.T) { ethservice.Merger().ReachTTD() defer n.Close() - api := NewBlockValidationAPI(ethservice, nil, true) + api := NewBlockValidationAPI(ethservice, nil, true, true) parent := preMergeBlocks[len(preMergeBlocks)-1] api.eth.APIBackend.Miner().SetEtherbase(testBuilderAddr) @@ -623,7 +623,7 @@ func TestValidateBuilderSubmissionV2_CoinbasePaymentDefault(t *testing.T) { ethservice.Merger().ReachTTD() defer n.Close() - api := NewBlockValidationAPI(ethservice, nil, true) + api := NewBlockValidationAPI(ethservice, nil, true, true) baseFee := misc.CalcBaseFee(ethservice.BlockChain().Config(), lastBlock.Header()) txs := make(types.Transactions, 0) @@ -735,8 +735,8 @@ func TestValidateBuilderSubmissionV2_Blocklist(t *testing.T) { }, } - apiWithBlock := NewBlockValidationAPI(ethservice, accessVerifier, true) - apiNoBlock := NewBlockValidationAPI(ethservice, nil, true) + apiWithBlock := NewBlockValidationAPI(ethservice, accessVerifier, true, true) + apiNoBlock := NewBlockValidationAPI(ethservice, nil, true, true) baseFee := misc.CalcBaseFee(ethservice.BlockChain().Config(), lastBlock.Header()) blockedTxs := make(types.Transactions, 0) @@ -782,3 +782,101 @@ func TestValidateBuilderSubmissionV2_Blocklist(t *testing.T) { }) } } + +// This tests payment when the proposer fee recipient receives CL withdrawal. +func TestValidateBuilderSubmissionV2_ExcludeWithdrawals(t *testing.T) { + genesis, preMergeBlocks := generatePreMergeChain(20) + lastBlock := preMergeBlocks[len(preMergeBlocks)-1] + time := lastBlock.Time() + 5 + genesis.Config.ShanghaiTime = &time + n, ethservice := startEthService(t, genesis, preMergeBlocks) + ethservice.Merger().ReachTTD() + defer n.Close() + + api := NewBlockValidationAPI(ethservice, nil, true, true) + + baseFee := misc.CalcBaseFee(ethservice.BlockChain().Config(), lastBlock.Header()) + txs := make(types.Transactions, 0) + + statedb, _ := ethservice.BlockChain().StateAt(lastBlock.Root()) + nonce := statedb.GetNonce(testAddr) + signer := types.LatestSigner(ethservice.BlockChain().Config()) + + expectedProfit := uint64(0) + + tx1, _ := types.SignTx(types.NewTransaction(nonce, common.Address{0x16}, big.NewInt(10), 21000, big.NewInt(2*baseFee.Int64()), nil), signer, testKey) + txs = append(txs, tx1) + expectedProfit += 21000 * baseFee.Uint64() + + // this tx will use 56996 gas + tx2, _ := types.SignTx(types.NewContractCreation(nonce+1, new(big.Int), 1000000, big.NewInt(2*baseFee.Int64()), logCode), signer, testKey) + txs = append(txs, tx2) + expectedProfit += 56996 * baseFee.Uint64() + + tx3, _ := types.SignTx(types.NewTransaction(nonce+2, testAddr, big.NewInt(10), 21000, baseFee, nil), signer, testKey) + txs = append(txs, tx3) + + // this transaction sends 7 wei to the proposer fee recipient, this should count as a profit + tx4, _ := types.SignTx(types.NewTransaction(nonce+3, testValidatorAddr, big.NewInt(7), 21000, baseFee, nil), signer, testKey) + txs = append(txs, tx4) + expectedProfit += 7 + + withdrawals := []*types.Withdrawal{ + { + Index: 0, + Validator: 1, + Amount: 100, + Address: testAddr, + }, + { + Index: 1, + Validator: 1, + Amount: 17, + Address: testValidatorAddr, + }, + { + Index: 1, + Validator: 1, + Amount: 21, + Address: testValidatorAddr, + }, + } + withdrawalsRoot := types.DeriveSha(types.Withdrawals(withdrawals), trie.NewStackTrie(nil)) + + buildBlockArgs := buildBlockArgs{ + parentHash: lastBlock.Hash(), + parentRoot: lastBlock.Root(), + feeRecipient: testValidatorAddr, + txs: txs, + random: common.Hash{}, + number: lastBlock.NumberU64() + 1, + gasLimit: lastBlock.GasLimit(), + timestamp: lastBlock.Time() + 5, + extraData: nil, + baseFeePerGas: baseFee, + withdrawals: withdrawals, + } + + execData, err := buildBlock(buildBlockArgs, ethservice.BlockChain()) + require.NoError(t, err) + + value := big.NewInt(int64(expectedProfit)) + + req, err := executableDataToBlockValidationRequest(execData, testValidatorAddr, value, withdrawalsRoot) + require.NoError(t, err) + require.NoError(t, api.ValidateBuilderSubmissionV2(req)) + + // try to claim less profit than expected, should work + value.SetUint64(expectedProfit - 1) + + req, err = executableDataToBlockValidationRequest(execData, testValidatorAddr, value, withdrawalsRoot) + require.NoError(t, err) + require.NoError(t, api.ValidateBuilderSubmissionV2(req)) + + // try to claim more profit than expected, should fail + value.SetUint64(expectedProfit + 1) + + req, err = executableDataToBlockValidationRequest(execData, testValidatorAddr, value, withdrawalsRoot) + require.NoError(t, err) + require.ErrorContains(t, api.ValidateBuilderSubmissionV2(req), "payment") +}