From 41f77d894ca08a316642a8032e718fab88c7672a Mon Sep 17 00:00:00 2001 From: Mateusz Morusiewicz <11313015+Ruteri@users.noreply.github.com> Date: Tue, 22 Nov 2022 10:41:33 +0100 Subject: [PATCH] Merge changes from validation (#4) * Implement block validation API * Validate proposer payment assuming its the last transaction in the block (#4) * Validate that the coinbase and feeRecipient are not blacklisted (#5) * Validate that the proposer payment has no calldata and its gas usage (#6) * Validate gas limit is set correctly wrt registered (#8) * Pass validation-specific config (#9) --- builder/builder.go | 10 +++---- cmd/geth/config.go | 7 ++++- cmd/utils/flags.go | 4 +-- core/block_validator.go | 25 ++-------------- core/blockchain.go | 1 + core/utils/gas_limit.go | 29 +++++++++++++++++++ eth/block-validation/api.go | 35 +++++++++++++++++++---- eth/block-validation/api_test.go | 49 ++++++++++++++++++++------------ go.sum | 2 ++ 9 files changed, 107 insertions(+), 55 deletions(-) create mode 100644 core/utils/gas_limit.go diff --git a/builder/builder.go b/builder/builder.go index adada18749cf..7dde6fe325b3 100644 --- a/builder/builder.go +++ b/builder/builder.go @@ -97,7 +97,7 @@ func (b *Builder) Stop() error { return nil } -func (b *Builder) onSealedBlock(block *types.Block, ordersClosedAt time.Time, sealedAt time.Time, commitedBundles []types.SimulatedBundle, allBundles []types.SimulatedBundle, proposerPubkey boostTypes.PublicKey, proposerFeeRecipient boostTypes.Address, attrs *BuilderPayloadAttributes) error { +func (b *Builder) onSealedBlock(block *types.Block, ordersClosedAt time.Time, sealedAt time.Time, commitedBundles []types.SimulatedBundle, allBundles []types.SimulatedBundle, proposerPubkey boostTypes.PublicKey, proposerFeeRecipient boostTypes.Address, proposerRegisteredGasLimit uint64, attrs *BuilderPayloadAttributes) error { executableData := beacon.BlockToExecutableData(block) payload, err := executableDataToExecutionPayload(executableData) if err != nil { @@ -137,7 +137,7 @@ func (b *Builder) onSealedBlock(block *types.Block, ordersClosedAt time.Time, se } if b.dryRun { - err = b.validator.ValidateBuilderSubmissionV1(&blockSubmitReq) + err = b.validator.ValidateBuilderSubmissionV1(&blockvalidation.BuilderBlockValidationRequest{blockSubmitReq, proposerRegisteredGasLimit}) if err != nil { log.Error("could not validate block", "err", err) } @@ -208,7 +208,7 @@ func (b *Builder) OnPayloadAttribute(attrs *BuilderPayloadAttributes) error { } b.slotAttrs = append(b.slotAttrs, *attrs) - go b.runBuildingJob(b.slotCtx, proposerPubkey, vd.FeeRecipient, attrs) + go b.runBuildingJob(b.slotCtx, proposerPubkey, vd.FeeRecipient, vd.GasLimit, attrs) return nil } @@ -220,7 +220,7 @@ type blockQueueEntry struct { allBundles []types.SimulatedBundle } -func (b *Builder) runBuildingJob(slotCtx context.Context, proposerPubkey boostTypes.PublicKey, feeRecipient boostTypes.Address, attrs *BuilderPayloadAttributes) { +func (b *Builder) runBuildingJob(slotCtx context.Context, proposerPubkey boostTypes.PublicKey, feeRecipient boostTypes.Address, proposerRegisteredGasLimit uint64, attrs *BuilderPayloadAttributes) { ctx, cancel := context.WithTimeout(slotCtx, 12*time.Second) defer cancel() @@ -245,7 +245,7 @@ func (b *Builder) runBuildingJob(slotCtx context.Context, proposerPubkey boostTy submitBestBlock := func() { queueMu.Lock() if queueLastSubmittedProfit.Cmp(queueBestProfit) < 0 { - err := b.onSealedBlock(queueBestEntry.block, queueBestEntry.ordersCloseTime, queueBestEntry.sealedAt, queueBestEntry.commitedBundles, queueBestEntry.allBundles, proposerPubkey, feeRecipient, attrs) + err := b.onSealedBlock(queueBestEntry.block, queueBestEntry.ordersCloseTime, queueBestEntry.sealedAt, queueBestEntry.commitedBundles, queueBestEntry.allBundles, proposerPubkey, feeRecipient, proposerRegisteredGasLimit, attrs) if err != nil { log.Error("could not run sealed block hook", "err", err) diff --git a/cmd/geth/config.go b/cmd/geth/config.go index d5b01815292e..7e16eecf1f95 100644 --- a/cmd/geth/config.go +++ b/cmd/geth/config.go @@ -168,7 +168,12 @@ func makeFullNode(ctx *cli.Context) (*node.Node, ethapi.Backend) { // Configure log filter RPC API. filterSystem := utils.RegisterFilterAPI(stack, backend, &cfg.Eth) - if err := blockvalidationapi.Register(stack, eth, ctx.String(utils.BuilderBlockValidationBlacklistSourceFilePath.Name)); err != nil { + 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 { utils.Fatalf("Failed to register the Block Validation API: %v", err) } diff --git a/cmd/utils/flags.go b/cmd/utils/flags.go index ec4b6853e030..710defff8dfc 100644 --- a/cmd/utils/flags.go +++ b/cmd/utils/flags.go @@ -1085,8 +1085,8 @@ Please note that --` + MetricsHTTPFlag.Name + ` must be set to start the server. // Builder API flags BuilderBlockValidationBlacklistSourceFilePath = &cli.StringFlag{ Name: "builder.validation_blacklist", - Usage: "Path to file containing blacklisted addresses, json-encoded list of strings. Default assumes no blacklist", - Value: "", + Usage: "Path to file containing blacklisted addresses, json-encoded list of strings. Default assumes CWD is repo's root", + Value: "ofac_blacklist.json", Category: flags.EthCategory, } ) diff --git a/core/block_validator.go b/core/block_validator.go index db7ea3568723..1309d50a0fe6 100644 --- a/core/block_validator.go +++ b/core/block_validator.go @@ -22,6 +22,7 @@ import ( "github.com/ethereum/go-ethereum/consensus" "github.com/ethereum/go-ethereum/core/state" "github.com/ethereum/go-ethereum/core/types" + "github.com/ethereum/go-ethereum/core/utils" "github.com/ethereum/go-ethereum/params" "github.com/ethereum/go-ethereum/trie" ) @@ -118,28 +119,6 @@ func (v *BlockValidator) ValidateState(block *types.Block, statedb *state.StateD return nil } -// CalcGasLimit computes the gas limit of the next block after parent. It aims -// to keep the baseline gas close to the provided target, and increase it towards -// the target if the baseline gas is lower. func CalcGasLimit(parentGasLimit, desiredLimit uint64) uint64 { - delta := parentGasLimit/params.GasLimitBoundDivisor - 1 - limit := parentGasLimit - if desiredLimit < params.MinGasLimit { - desiredLimit = params.MinGasLimit - } - // If we're outside our allowed gas range, we try to hone towards them - if limit < desiredLimit { - limit = parentGasLimit + delta - if limit > desiredLimit { - limit = desiredLimit - } - return limit - } - if limit > desiredLimit { - limit = parentGasLimit - delta - if limit < desiredLimit { - limit = desiredLimit - } - } - return limit + return utils.CalcGasLimit(parentGasLimit, desiredLimit) } diff --git a/core/blockchain.go b/core/blockchain.go index f22562ccfa94..327b9b614d44 100644 --- a/core/blockchain.go +++ b/core/blockchain.go @@ -38,6 +38,7 @@ import ( "github.com/ethereum/go-ethereum/core/state" "github.com/ethereum/go-ethereum/core/state/snapshot" "github.com/ethereum/go-ethereum/core/types" + "github.com/ethereum/go-ethereum/core/utils" "github.com/ethereum/go-ethereum/core/vm" "github.com/ethereum/go-ethereum/ethdb" "github.com/ethereum/go-ethereum/event" diff --git a/core/utils/gas_limit.go b/core/utils/gas_limit.go new file mode 100644 index 000000000000..8be0a6a43e15 --- /dev/null +++ b/core/utils/gas_limit.go @@ -0,0 +1,29 @@ +package utils + +import "github.com/ethereum/go-ethereum/params" + +// CalcGasLimit computes the gas limit of the next block after parent. It aims +// to keep the baseline gas close to the provided target, and increase it towards +// the target if the baseline gas is lower. +func CalcGasLimit(parentGasLimit, desiredLimit uint64) uint64 { + delta := parentGasLimit/params.GasLimitBoundDivisor - 1 + limit := parentGasLimit + if desiredLimit < params.MinGasLimit { + desiredLimit = params.MinGasLimit + } + // If we're outside our allowed gas range, we try to hone towards them + if limit < desiredLimit { + limit = parentGasLimit + delta + if limit > desiredLimit { + limit = desiredLimit + } + return limit + } + if limit > desiredLimit { + limit = parentGasLimit - delta + if limit < desiredLimit { + limit = desiredLimit + } + } + return limit +} diff --git a/eth/block-validation/api.go b/eth/block-validation/api.go index 38984e214894..4cc36dbd57ab 100644 --- a/eth/block-validation/api.go +++ b/eth/block-validation/api.go @@ -16,6 +16,7 @@ import ( "github.com/ethereum/go-ethereum/log" "github.com/ethereum/go-ethereum/node" "github.com/ethereum/go-ethereum/rpc" + boostTypes "github.com/flashbots/go-boost-utils/types" ) @@ -26,7 +27,7 @@ type AccessVerifier struct { } func (a *AccessVerifier) verifyTraces(tracer *logger.AccessListTracer) error { - log.Info("x", "tracer.AccessList()", tracer.AccessList()) + 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 { @@ -38,6 +39,13 @@ func (a *AccessVerifier) verifyTraces(tracer *logger.AccessListTracer) error { 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) @@ -77,12 +85,16 @@ func NewAccessVerifierFromFile(path string) (*AccessVerifier, error) { }, nil } +type BlockValidationConfig struct { + BlacklistSourceFilePath string +} + // Register adds catalyst APIs to the full node. -func Register(stack *node.Node, backend *eth.Ethereum, blockValidationBlocklistFile string) error { +func Register(stack *node.Node, backend *eth.Ethereum, cfg BlockValidationConfig) error { var accessVerifier *AccessVerifier - if blockValidationBlocklistFile != "" { + if cfg.BlacklistSourceFilePath != "" { var err error - accessVerifier, err = NewAccessVerifierFromFile(blockValidationBlocklistFile) + accessVerifier, err = NewAccessVerifierFromFile(cfg.BlacklistSourceFilePath) if err != nil { return err } @@ -111,7 +123,12 @@ func NewBlockValidationAPI(eth *eth.Ethereum, accessVerifier *AccessVerifier) *B } } -func (api *BlockValidationAPI) ValidateBuilderSubmissionV1(params *boostTypes.BuilderSubmitBlockRequest) error { +type BuilderBlockValidationRequest struct { + boostTypes.BuilderSubmitBlockRequest + RegisteredGasLimit uint64 `json:"registered_gas_limit,string"` +} + +func (api *BlockValidationAPI) ValidateBuilderSubmissionV1(params *BuilderBlockValidationRequest) error { // TODO: fuzztest, make sure the validation is sound // TODO: handle context! @@ -146,6 +163,12 @@ func (api *BlockValidationAPI) ValidateBuilderSubmissionV1(params *boostTypes.Bu 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 } @@ -155,7 +178,7 @@ func (api *BlockValidationAPI) ValidateBuilderSubmissionV1(params *boostTypes.Bu vmconfig = vm.Config{Tracer: tracer, Debug: true} } - err = api.eth.BlockChain().ValidatePayload(block, feeRecipient, expectedProfit, vmconfig) + err = api.eth.BlockChain().ValidatePayload(block, feeRecipient, expectedProfit, params.RegisteredGasLimit, vmconfig) 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 9169abc77554..fca96454e855 100644 --- a/eth/block-validation/api_test.go +++ b/eth/block-validation/api_test.go @@ -11,6 +11,7 @@ import ( "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/common/hexutil" "github.com/ethereum/go-ethereum/consensus/ethash" + "github.com/ethereum/go-ethereum/consensus/misc" "github.com/ethereum/go-ethereum/core" "github.com/ethereum/go-ethereum/core/beacon" "github.com/ethereum/go-ethereum/core/rawdb" @@ -20,7 +21,6 @@ import ( "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/log" "github.com/ethereum/go-ethereum/node" "github.com/ethereum/go-ethereum/p2p" "github.com/ethereum/go-ethereum/params" @@ -68,14 +68,14 @@ func TestValidateBuilderSubmissionV1(t *testing.T) { cc, _ := types.SignTx(types.NewContractCreation(nonce+1, new(big.Int), 1000000, big.NewInt(2*params.InitialBaseFee), logCode), types.LatestSigner(ethservice.BlockChain().Config()), testKey) ethservice.TxPool().AddLocal(cc) - tx2, _ := types.SignTx(types.NewTransaction(nonce+2, testAddr, big.NewInt(10), 21000, big.NewInt(2*params.InitialBaseFee), nil), types.LatestSigner(ethservice.BlockChain().Config()), testKey) + baseFee := misc.CalcBaseFee(params.AllEthashProtocolChanges, preMergeBlocks[len(preMergeBlocks)-1].Header()) + tx2, _ := types.SignTx(types.NewTransaction(nonce+2, testAddr, big.NewInt(10), 21000, baseFee, nil), types.LatestSigner(ethservice.BlockChain().Config()), testKey) ethservice.TxPool().AddLocal(tx2) execData, err := assembleBlock(api, parent.Hash(), &beacon.PayloadAttributesV1{ Timestamp: parent.Time() + 5, SuggestedFeeRecipient: testValidatorAddr, }) - require.NoError(t, err) require.EqualValues(t, len(execData.Transactions), 4) require.NoError(t, err) @@ -85,22 +85,37 @@ func TestValidateBuilderSubmissionV1(t *testing.T) { proposerAddr := boostTypes.Address{} proposerAddr.FromSlice(testValidatorAddr[:]) - blockRequest := &boostTypes.BuilderSubmitBlockRequest{ - Signature: boostTypes.Signature{}, - Message: &boostTypes.BidTrace{ - ParentHash: boostTypes.Hash(execData.ParentHash), - BlockHash: boostTypes.Hash(execData.BlockHash), - ProposerFeeRecipient: proposerAddr, - GasLimit: execData.GasLimit, - GasUsed: execData.GasUsed, + blockRequest := &BuilderBlockValidationRequest{ + BuilderSubmitBlockRequest: boostTypes.BuilderSubmitBlockRequest{ + Signature: boostTypes.Signature{}, + Message: &boostTypes.BidTrace{ + ParentHash: boostTypes.Hash(execData.ParentHash), + BlockHash: boostTypes.Hash(execData.BlockHash), + ProposerFeeRecipient: proposerAddr, + GasLimit: execData.GasLimit, + GasUsed: execData.GasUsed, + }, + ExecutionPayload: payload, }, - ExecutionPayload: payload, + RegisteredGasLimit: execData.GasLimit, } + blockRequest.Message.Value = boostTypes.IntToU256(190526394825529) require.ErrorContains(t, api.ValidateBuilderSubmissionV1(blockRequest), "inaccurate payment") - blockRequest.Message.Value = boostTypes.IntToU256(190526394825530) + blockRequest.Message.Value = boostTypes.IntToU256(149830884438530) require.NoError(t, api.ValidateBuilderSubmissionV1(blockRequest)) + blockRequest.Message.GasLimit += 1 + blockRequest.ExecutionPayload.GasLimit += 1 + + oldHash := blockRequest.Message.BlockHash + copy(blockRequest.Message.BlockHash[:], hexutil.MustDecode("0x56cbdd508966f89cfb6ba16535e3676b59ae3ac3774478b631466bc99c1033c9")[:32]) + require.ErrorContains(t, api.ValidateBuilderSubmissionV1(blockRequest), "incorrect gas limit set") + + blockRequest.Message.GasLimit -= 1 + blockRequest.ExecutionPayload.GasLimit -= 1 + blockRequest.Message.BlockHash = oldHash + // TODO: test with contract calling blacklisted address // Test tx from blacklisted address api.accessVerifier = &AccessVerifier{ @@ -130,8 +145,7 @@ func TestValidateBuilderSubmissionV1(t *testing.T) { txData, err := invalidTx.MarshalBinary() require.NoError(t, err) - execData.Transactions = append(execData.Transactions, execData.Transactions[3]) - execData.Transactions[3] = txData + execData.Transactions = append(execData.Transactions, txData) invalidPayload, err := ExecutableDataToExecutionPayload(execData) require.NoError(t, err) @@ -139,8 +153,8 @@ func TestValidateBuilderSubmissionV1(t *testing.T) { invalidPayload.LogsBloom = boostTypes.Bloom{} copy(invalidPayload.ReceiptsRoot[:], hexutil.MustDecode("0x56e81f171bcc55a6ff8345e692c0f86e5b48e01b996cadc001622fb5e363b421")[:32]) blockRequest.ExecutionPayload = invalidPayload - copy(blockRequest.Message.BlockHash[:], hexutil.MustDecode("0x2ff468dee2e05f1f58744d5496f3ab22fdc23c8141f86f907b4b0f2c8e22afc4")[:32]) - require.ErrorContains(t, api.ValidateBuilderSubmissionV1(blockRequest), "could not apply tx 3", "insufficient funds for gas * price + value") + copy(blockRequest.Message.BlockHash[:], hexutil.MustDecode("0x595cba7ab70a18b7e11ae7541661cb6692909a0acd3eba3f1cf6ae694f85a8bd")[:32]) + require.ErrorContains(t, api.ValidateBuilderSubmissionV1(blockRequest), "could not apply tx 4", "insufficient funds for gas * price + value") } func generatePreMergeChain(n int) (*core.Genesis, []*types.Block) { @@ -211,7 +225,6 @@ func assembleBlock(api *BlockValidationAPI, parentHash common.Hash, params *beac if err != nil { return nil, err } - log.Info("b", "block", block) return beacon.BlockToExecutableData(block), nil } diff --git a/go.sum b/go.sum index daaab1319d19..1fef7b4ebd9f 100644 --- a/go.sum +++ b/go.sum @@ -877,6 +877,8 @@ gopkg.in/natefinch/npipe.v2 v2.0.0-20160621034901-c1b8fa8bdcce h1:+JknDZhAj8YMt7 gopkg.in/natefinch/npipe.v2 v2.0.0-20160621034901-c1b8fa8bdcce/go.mod h1:5AcXVHNjg+BDxry382+8OKon8SEWiKktQR07RKPsv1c= gopkg.in/tomb.v1 v1.0.0-20141024135613-dd632973f1e7 h1:uRGJdciOHaEIrze2W8Q3AKkepLTh2hOroT7a+7czfdQ= gopkg.in/tomb.v1 v1.0.0-20141024135613-dd632973f1e7/go.mod h1:dt/ZhP58zS4L8KSrWDmTeBkI65Dw0HsyUHuEVlX15mw= +gopkg.in/urfave/cli.v1 v1.20.0 h1:NdAVW6RYxDif9DhDHaAortIu956m2c0v+09AZBPTbE0= +gopkg.in/urfave/cli.v1 v1.20.0/go.mod h1:vuBzUtMdQeixQj8LVd+/98pzhxNGQoyuPBlsXHOQNO0= gopkg.in/yaml.v2 v2.2.1/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= gopkg.in/yaml.v2 v2.2.3/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=