From 66fc7a03dfa6af87411047128714fc91154345f3 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 +++- core/txpool/blobpool/blobpool.go | 2 +- core/txpool/blobpool/config.go | 12 ++-- core/txpool/subpool.go | 2 +- eth/block-validation/api.go | 11 +++- eth/block-validation/api_test.go | 110 +++++++++++++++++++++++++++++-- eth/handler.go | 2 +- internal/ethapi/sbundle_api.go | 3 +- 13 files changed, 147 insertions(+), 21 deletions(-) diff --git a/builder/config.go b/builder/config.go index de9868caa..e3099fd5c 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 67e052b5b..09be2c632 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 649a16387..9dd0f7cdf 100644 --- a/cmd/geth/config.go +++ b/cmd/geth/config.go @@ -210,6 +210,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 b31482155..052020578 100644 --- a/cmd/geth/main.go +++ b/cmd/geth/main.go @@ -160,6 +160,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 4ead07ad6..d7d63db78 100644 --- a/cmd/utils/flags.go +++ b/cmd/utils/flags.go @@ -646,6 +646,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", @@ -1624,6 +1630,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 317881fc8..88308a795 100644 --- a/core/blockchain.go +++ b/core/blockchain.go @@ -2453,7 +2453,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); err != nil { return err @@ -2495,6 +2496,14 @@ func (bc *BlockChain) ValidatePayload(block *types.Block, feeRecipient common.Ad feeRecipientBalanceDelta := new(uint256.Int).Set(statedb.GetBalance(feeRecipient)) feeRecipientBalanceDelta.Sub(feeRecipientBalanceDelta, feeRecipientBalanceBefore) + if excludeWithdrawals { + for _, w := range block.Withdrawals() { + if w.Address == feeRecipient { + amount := new(uint256.Int).Mul(new(uint256.Int).SetUint64(w.Amount), uint256.NewInt(params.GWei)) + feeRecipientBalanceDelta.Sub(feeRecipientBalanceDelta, amount) + } + } + } if bc.Config().IsShanghai(header.Number, header.Time) { if header.WithdrawalsHash == nil { diff --git a/core/txpool/blobpool/blobpool.go b/core/txpool/blobpool/blobpool.go index 3f2b29143..1fe22c6c4 100644 --- a/core/txpool/blobpool/blobpool.go +++ b/core/txpool/blobpool/blobpool.go @@ -1664,4 +1664,4 @@ func (p *BlobPool) Status(hash common.Hash) txpool.TxStatus { func (p *BlobPool) IsPrivateTxHash(hash common.Hash) bool { return p.privateTxs.Contains(hash) -} \ No newline at end of file +} diff --git a/core/txpool/blobpool/config.go b/core/txpool/blobpool/config.go index 97154e11c..bb27bcf6b 100644 --- a/core/txpool/blobpool/config.go +++ b/core/txpool/blobpool/config.go @@ -24,17 +24,17 @@ import ( // Config are the configuration parameters of the blob transaction pool. type Config struct { - Datadir string // Data directory containing the currently executable blobs - Datacap uint64 // Soft-cap of database storage (hard cap is larger due to overhead) - PriceBump uint64 // Minimum price bump percentage to replace an already existing nonce + Datadir string // Data directory containing the currently executable blobs + Datacap uint64 // Soft-cap of database storage (hard cap is larger due to overhead) + PriceBump uint64 // Minimum price bump percentage to replace an already existing nonce PrivateTxLifetime time.Duration // Maximum amount of time to keep private transactions private } // DefaultConfig contains the default configurations for the transaction pool. var DefaultConfig = Config{ - Datadir: "blobpool", - Datacap: 10 * 1024 * 1024 * 1024 / 4, // TODO(karalabe): /4 handicap for rollout, gradually bump back up to 10GB - PriceBump: 100, // either have patience or be aggressive, no mushy ground + Datadir: "blobpool", + Datacap: 10 * 1024 * 1024 * 1024 / 4, // TODO(karalabe): /4 handicap for rollout, gradually bump back up to 10GB + PriceBump: 100, // either have patience or be aggressive, no mushy ground PrivateTxLifetime: 3 * 24 * time.Hour, } diff --git a/core/txpool/subpool.go b/core/txpool/subpool.go index c75580255..96c131698 100644 --- a/core/txpool/subpool.go +++ b/core/txpool/subpool.go @@ -130,7 +130,7 @@ type SubPool interface { Add(txs []*types.Transaction, local bool, sync bool, private bool) []error IsPrivateTxHash(hash common.Hash) bool - + // Pending retrieves all currently processable transactions, grouped by origin // account and sorted by nonce. // diff --git a/eth/block-validation/api.go b/eth/block-validation/api.go index a17579109..30f02ac46 100644 --- a/eth/block-validation/api.go +++ b/eth/block-validation/api.go @@ -93,6 +93,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. @@ -109,7 +111,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 @@ -120,15 +122,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, } } @@ -278,7 +283,7 @@ func (api *BlockValidationAPI) validateBlock(block *types.Block, msg *builderApi vmconfig = vm.Config{Tracer: tracer} } - err := api.eth.BlockChain().ValidatePayload(block, feeRecipient, expectedProfit, registeredGasLimit, vmconfig, api.useBalanceDiffProfit) + err := api.eth.BlockChain().ValidatePayload(block, feeRecipient, expectedProfit, registeredGasLimit, vmconfig, api.useBalanceDiffProfit, api.excludeWithdrawals) if err != nil { return err } diff --git a/eth/block-validation/api_test.go b/eth/block-validation/api_test.go index 13052102d..4340e99b3 100644 --- a/eth/block-validation/api_test.go +++ b/eth/block-validation/api_test.go @@ -76,7 +76,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) @@ -186,7 +186,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) @@ -318,7 +318,7 @@ func TestValidateBuilderSubmissionV3(t *testing.T) { ethservice.Merger().ReachTTD() defer n.Close() - api := NewBlockValidationAPI(ethservice, nil, true) + api := NewBlockValidationAPI(ethservice, nil, true, false) parent := ethservice.BlockChain().CurrentHeader() api.eth.APIBackend.Miner().SetEtherbase(testBuilderAddr) @@ -855,7 +855,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 := eip1559.CalcBaseFee(ethservice.BlockChain().Config(), lastBlock.Header()) txs := make(types.Transactions, 0) @@ -967,8 +967,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 := eip1559.CalcBaseFee(ethservice.BlockChain().Config(), lastBlock.Header()) blockedTxs := make(types.Transactions, 0) @@ -1014,3 +1014,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 := eip1559.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") +} diff --git a/eth/handler.go b/eth/handler.go index 48a54aada..65a17a9ad 100644 --- a/eth/handler.go +++ b/eth/handler.go @@ -79,7 +79,7 @@ type txPool interface { // can decide whether to receive notifications only for newly seen transactions // or also for reorged out ones. SubscribeTransactions(ch chan<- core.NewTxsEvent, reorgs bool) event.Subscription - + // IsPrivateTxHash indicates if the transaction hash should not // be broadcast on public channels IsPrivateTxHash(hash common.Hash) bool diff --git a/internal/ethapi/sbundle_api.go b/internal/ethapi/sbundle_api.go index 9e3a27427..a7dab3c34 100644 --- a/internal/ethapi/sbundle_api.go +++ b/internal/ethapi/sbundle_api.go @@ -4,10 +4,11 @@ import ( "context" "errors" "fmt" - "github.com/ethereum/go-ethereum/consensus/misc/eip4844" "math/big" "time" + "github.com/ethereum/go-ethereum/consensus/misc/eip4844" + "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/common/hexutil" "github.com/ethereum/go-ethereum/consensus/misc/eip1559"