From b57faeb2498e96292a0a1c3faf2b12ad4ebbe82a Mon Sep 17 00:00:00 2001 From: Hamdi Allam Date: Wed, 26 Jun 2024 18:00:06 -0400 Subject: [PATCH 01/16] eth_sendRawTransactionConditional --- cmd/geth/main.go | 2 + cmd/utils/flags.go | 17 ++ core/state/statedb.go | 32 +++ core/state/statedb_test.go | 218 +++++++++++++++ core/txpool/legacypool/legacypool.go | 18 +- core/txpool/legacypool/legacypool_test.go | 37 +++ core/types/block.go | 17 ++ core/types/block_test.go | 106 ++++++++ .../types/gen_transaction_conditional_json.go | 66 +++++ core/types/transaction.go | 17 ++ core/types/transaction_conditional.go | 137 ++++++++++ core/types/transaction_conditional_test.go | 257 ++++++++++++++++++ eth/backend.go | 6 + eth/ethconfig/config.go | 1 + eth/protocols/eth/broadcast.go | 5 +- fork.yaml | 19 ++ internal/sequencerapi/api.go | 97 +++++++ internal/sequencerapi/errors.go | 20 ++ miner/miner.go | 25 +- miner/miner_test.go | 59 +++- miner/worker.go | 42 +++ 21 files changed, 1184 insertions(+), 14 deletions(-) create mode 100644 core/types/gen_transaction_conditional_json.go create mode 100644 core/types/transaction_conditional.go create mode 100644 core/types/transaction_conditional_test.go create mode 100644 internal/sequencerapi/api.go create mode 100644 internal/sequencerapi/errors.go diff --git a/cmd/geth/main.go b/cmd/geth/main.go index 085b892059..3ac62eb355 100644 --- a/cmd/geth/main.go +++ b/cmd/geth/main.go @@ -155,6 +155,8 @@ var ( utils.GpoIgnoreGasPriceFlag, utils.GpoMinSuggestedPriorityFeeFlag, utils.RollupSequencerHTTPFlag, + utils.RollupSequencerEnableTxConditionalFlag, + utils.RollupSequencerTxConditionalRateLimitFlag, utils.RollupHistoricalRPCFlag, utils.RollupHistoricalRPCTimeoutFlag, utils.RollupDisableTxPoolGossipFlag, diff --git a/cmd/utils/flags.go b/cmd/utils/flags.go index f894a11834..77f20c7e5c 100644 --- a/cmd/utils/flags.go +++ b/cmd/utils/flags.go @@ -976,6 +976,18 @@ var ( Category: flags.RollupCategory, Value: true, } + RollupSequencerEnableTxConditionalFlag = &cli.BoolFlag{ + Name: "rollup.sequencerenabletxconditional", + Usage: "Serve the eth_sendRawTransactionConditional endpoint and apply the conditional constraints on mempool inclusion & block building", + Category: flags.RollupCategory, + Value: false, + } + RollupSequencerTxConditionalRateLimitFlag = &cli.Uint64Flag{ + Name: "rollup.sequencertxconditionalratelimit", + Usage: "Maximum cost -- storage lookups -- allowed for conditional transactions in a given second", + Category: flags.RollupCategory, + Value: 5000, + } // Metrics flags MetricsEnabledFlag = &cli.BoolFlag{ @@ -1717,6 +1729,9 @@ func setMiner(ctx *cli.Context, cfg *miner.Config) { if ctx.IsSet(RollupComputePendingBlock.Name) { cfg.RollupComputePendingBlock = ctx.Bool(RollupComputePendingBlock.Name) } + if ctx.IsSet(RollupSequencerTxConditionalRateLimitFlag.Name) { + cfg.RollupTransactionConditionalBurstRate = int(ctx.Uint64(RollupSequencerTxConditionalRateLimitFlag.Name)) + } } func setRequiredBlocks(ctx *cli.Context, cfg *ethconfig.Config) { @@ -1955,6 +1970,8 @@ func SetEthConfig(ctx *cli.Context, stack *node.Node, cfg *ethconfig.Config) { cfg.RollupDisableTxPoolAdmission = cfg.RollupSequencerHTTP != "" && !ctx.Bool(RollupEnableTxPoolAdmissionFlag.Name) cfg.RollupHaltOnIncompatibleProtocolVersion = ctx.String(RollupHaltOnIncompatibleProtocolVersionFlag.Name) cfg.ApplySuperchainUpgrades = ctx.Bool(RollupSuperchainUpgradesFlag.Name) + cfg.RollupSequencerEnableTxConditional = ctx.Bool(RollupSequencerEnableTxConditionalFlag.Name) + // Override any default configs for hard coded networks. switch { case ctx.Bool(MainnetFlag.Name): diff --git a/core/state/statedb.go b/core/state/statedb.go index 20994d6cfe..e525eee551 100644 --- a/core/state/statedb.go +++ b/core/state/statedb.go @@ -425,6 +425,38 @@ func (s *StateDB) HasSelfDestructed(addr common.Address) bool { return false } +// CheckTransactionConditional validates the account preconditions against the statedb. +// +// NOTE: A lock is not held on the db while the conditional is checked. The caller must +// ensure no state changes occur while this check is executed. +func (s *StateDB) CheckTransactionConditional(cond *types.TransactionConditional) error { + cost := cond.Cost() + if cost > types.TransactionConditionalMaxCost { + return fmt.Errorf("conditional cost, %d, exceeded max: %d", cost, types.TransactionConditionalMaxCost) + } + + for addr, acct := range cond.KnownAccounts { + if root, isRoot := acct.Root(); isRoot { + storageRoot := s.GetStorageRoot(addr) + if storageRoot == (common.Hash{}) { // if the root is not found, replace with the empty root hash + storageRoot = types.EmptyRootHash + } + if root != storageRoot { + return fmt.Errorf("failed account storage root constraint. Got %s, Expected %s", storageRoot, root) + } + } + if slots, isSlots := acct.Slots(); isSlots { + for key, state := range slots { + accState := s.GetState(addr, key) + if state != accState { + return fmt.Errorf("failed account storage slot key %s constraint. Got %s, Expected %s", key, accState, state) + } + } + } + } + return nil +} + /* * SETTERS */ diff --git a/core/state/statedb_test.go b/core/state/statedb_test.go index 2ce2b868fa..7e16e0b58e 100644 --- a/core/state/statedb_test.go +++ b/core/state/statedb_test.go @@ -1373,3 +1373,221 @@ func TestStorageDirtiness(t *testing.T) { state.RevertToSnapshot(snap) checkDirty(common.Hash{0x1}, common.Hash{0x1}, true) } + +func TestCheckTransactionConditional(t *testing.T) { + type preAction struct { + Account common.Address + Slots map[common.Hash]common.Hash + } + + tests := []struct { + name string + preActions []preAction + cond types.TransactionConditional + valid bool + }{ + { + // Clean Prestate, no defined cond + name: "clean prestate", + preActions: []preAction{}, + cond: types.TransactionConditional{}, + valid: true, + }, + { + // Prestate: + // - address(1) + // - bytes32(0): bytes32(1) + // cond: + // - address(1) + // - bytes32(0): bytes32(1) + name: "matching storage slots", + preActions: []preAction{ + { + Account: common.Address{19: 1}, + Slots: map[common.Hash]common.Hash{ + common.Hash{}: common.Hash{31: 1}, + }, + }, + }, + cond: types.TransactionConditional{ + KnownAccounts: map[common.Address]types.KnownAccount{ + common.Address{19: 1}: types.KnownAccount{ + StorageSlots: map[common.Hash]common.Hash{ + common.Hash{}: common.Hash{31: 1}, + }, + }, + }, + }, + valid: true, + }, + { + // Prestate: + // - address(1) + // - bytes32(0): bytes32(1) + // cond: + // - address(1) + // - bytes32(0): bytes32(2) + name: "mismatched storage slots", + preActions: []preAction{ + { + Account: common.Address{19: 1}, + Slots: map[common.Hash]common.Hash{ + common.Hash{}: common.Hash{31: 1}, + }, + }, + }, + cond: types.TransactionConditional{ + KnownAccounts: map[common.Address]types.KnownAccount{ + common.Address{19: 1}: types.KnownAccount{ + StorageSlots: map[common.Hash]common.Hash{ + common.Hash{}: common.Hash{31: 2}, + }, + }, + }, + }, + valid: false, + }, + { + // Clean Prestate + // cond: + // - address(1) + // - emptyRoot + name: "matching storage root", + preActions: []preAction{}, + cond: types.TransactionConditional{ + KnownAccounts: map[common.Address]types.KnownAccount{ + common.Address{19: 1}: types.KnownAccount{ + StorageRoot: &types.EmptyRootHash, + }, + }, + }, + valid: true, + }, + { + // Prestate: + // - address(1) + // - bytes32(0): bytes32(1) + // cond: + // - address(1) + // - emptyRoot + name: "mismatched storage root", + preActions: []preAction{ + { + Account: common.Address{19: 1}, + Slots: map[common.Hash]common.Hash{ + common.Hash{}: common.Hash{31: 1}, + }, + }, + }, + cond: types.TransactionConditional{ + KnownAccounts: map[common.Address]types.KnownAccount{ + common.Address{19: 1}: types.KnownAccount{ + StorageRoot: &types.EmptyRootHash, + }, + }, + }, + valid: false, + }, + { + // Prestate: + // - address(1) + // - bytes32(0): bytes32(1) + // - address(2) + // - bytes32(0): bytes32(2) + // cond: + // - address(1) + // - bytes32(0): bytes32(1) + // - address(2) + // - bytes32(0): bytes32(2) + name: "multiple matching", + preActions: []preAction{ + { + Account: common.Address{19: 1}, + Slots: map[common.Hash]common.Hash{ + common.Hash{}: common.Hash{31: 1}, + }, + }, + { + Account: common.Address{19: 2}, + Slots: map[common.Hash]common.Hash{ + common.Hash{}: common.Hash{31: 2}, + }, + }, + }, + cond: types.TransactionConditional{ + KnownAccounts: map[common.Address]types.KnownAccount{ + common.Address{19: 1}: types.KnownAccount{ + StorageSlots: map[common.Hash]common.Hash{ + common.Hash{}: common.Hash{31: 1}, + }, + }, + common.Address{19: 2}: types.KnownAccount{ + StorageSlots: map[common.Hash]common.Hash{ + common.Hash{}: common.Hash{31: 2}, + }, + }, + }, + }, + valid: true, + }, + { + // Prestate: + // - address(1) + // - bytes32(0): bytes32(1) + // - address(2) + // - bytes32(0): bytes32(3) + // cond: + // - address(1) + // - bytes32(0): bytes32(1) + // - address(2) + // - bytes32(0): bytes32(2) + name: "multiple mismatch single", + preActions: []preAction{ + { + Account: common.Address{19: 1}, + Slots: map[common.Hash]common.Hash{ + common.Hash{}: common.Hash{31: 1}, + }, + }, + { + Account: common.Address{19: 2}, + Slots: map[common.Hash]common.Hash{ + common.Hash{}: common.Hash{31: 3}, + }, + }, + }, + cond: types.TransactionConditional{ + KnownAccounts: map[common.Address]types.KnownAccount{ + common.Address{19: 1}: types.KnownAccount{ + StorageSlots: map[common.Hash]common.Hash{ + common.Hash{}: common.Hash{31: 1}, + }, + }, + common.Address{19: 2}: types.KnownAccount{ + StorageSlots: map[common.Hash]common.Hash{ + common.Hash{}: common.Hash{31: 2}, + }, + }, + }, + }, + valid: false, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + state, _ := New(types.EmptyRootHash, NewDatabase(rawdb.NewMemoryDatabase()), nil) + for _, action := range test.preActions { + for key, value := range action.Slots { + state.SetState(action.Account, key, value) + } + } + + // write modifications to the trie + state.IntermediateRoot(false) + if err := state.CheckTransactionConditional(&test.cond); err == nil && !test.valid { + t.Errorf("Test %s got unvalid value: want %v, got err %v", test.name, test.valid, err) + } + }) + } +} diff --git a/core/txpool/legacypool/legacypool.go b/core/txpool/legacypool/legacypool.go index 6e004ed356..7036c66429 100644 --- a/core/txpool/legacypool/legacypool.go +++ b/core/txpool/legacypool/legacypool.go @@ -1724,6 +1724,7 @@ func (pool *LegacyPool) demoteUnexecutables() { } balance := pool.currentState.GetBalance(addr) balance = pool.reduceBalanceByL1Cost(list, balance) + // Drop all transactions that are too costly (low balance or out of gas), and queue any invalids back for later drops, invalids := list.Filter(balance, gasLimit) for _, tx := range drops { @@ -1733,6 +1734,19 @@ func (pool *LegacyPool) demoteUnexecutables() { } pendingNofundsMeter.Mark(int64(len(drops))) + // Drop all conditional transactions that was rejected by the miner + conditionalDrops := list.txs.Filter(func(tx *types.Transaction) bool { + if conditional := tx.Conditional(); conditional != nil { + return conditional.Rejected.Load() + } + return false + }) + for _, tx := range conditionalDrops { + hash := tx.Hash() + log.Trace("Removed conditional transaction rejected by the miner", "hash", hash) + pool.all.Remove(hash) + } + for _, tx := range invalids { hash := tx.Hash() log.Trace("Demoting pending transaction", "hash", hash) @@ -1740,9 +1754,9 @@ func (pool *LegacyPool) demoteUnexecutables() { // Internal shuffle shouldn't touch the lookup set. pool.enqueueTx(hash, tx, false, false) } - pendingGauge.Dec(int64(len(olds) + len(drops) + len(invalids))) + pendingGauge.Dec(int64(len(olds) + len(drops) + len(invalids) + len(conditionalDrops))) if pool.locals.contains(addr) { - localGauge.Dec(int64(len(olds) + len(drops) + len(invalids))) + localGauge.Dec(int64(len(olds) + len(drops) + len(invalids) + len(conditionalDrops))) } // If there's a gap in front, alert (should never happen) and postpone all transactions if list.Len() > 0 && list.txs.Get(nonce) == nil { diff --git a/core/txpool/legacypool/legacypool_test.go b/core/txpool/legacypool/legacypool_test.go index c71544b48c..ac6af8c420 100644 --- a/core/txpool/legacypool/legacypool_test.go +++ b/core/txpool/legacypool/legacypool_test.go @@ -690,6 +690,43 @@ func TestDropping(t *testing.T) { } } +// Tests that transactions marked as reject (by the miner in practice) +// are removed from the pool +func TestRejectedConditionalDropping(t *testing.T) { + t.Parallel() + + pool, key := setupPool() + defer pool.Close() + + account := crypto.PubkeyToAddress(key.PublicKey) + testAddBalance(pool, account, big.NewInt(1000)) + + // create some txs. tx0 has a conditional + tx0, tx1 := transaction(0, 100, key), transaction(1, 200, key) + tx0.SetConditional(&types.TransactionConditional{ + BlockNumberMin: big.NewInt(1), + Rejected: &atomic.Bool{}, + }) + + pool.all.Add(tx0, false) + pool.all.Add(tx1, false) + pool.promoteTx(account, tx0.Hash(), tx0) + pool.promoteTx(account, tx1.Hash(), tx1) + + // pool state is unchanged + <-pool.requestReset(nil, nil) + if pool.all.Count() != 2 { + t.Errorf("total transaction mismatch: have %d, want %d", pool.all.Count(), 2) + } + + // tx0 conditional is marked as rejected and should be removed + tx0.Conditional().Rejected.Store(true) + <-pool.requestReset(nil, nil) + if pool.all.Count() != 1 { + t.Errorf("total transaction mismatch: have %d, want %d", pool.all.Count(), 1) + } +} + // Tests that if a transaction is dropped from the current pending pool (e.g. out // of fund), all consecutive (still valid, but not executable) transactions are // postponed back into the future queue to prevent broadcasting them. diff --git a/core/types/block.go b/core/types/block.go index 4857cd6e50..3773a12853 100644 --- a/core/types/block.go +++ b/core/types/block.go @@ -166,6 +166,23 @@ func (h *Header) EmptyReceipts() bool { return h.ReceiptHash == EmptyReceiptsHash } +// CheckTransactionConditional validates the block preconditions against the header +func (h *Header) CheckTransactionConditional(cond *TransactionConditional) error { + if cond.BlockNumberMin != nil && cond.BlockNumberMin.Cmp(h.Number) > 0 { + return fmt.Errorf("failed block number minimum constraint") + } + if cond.BlockNumberMax != nil && cond.BlockNumberMax.Cmp(h.Number) < 0 { + return fmt.Errorf("failed block number maximmum constraint") + } + if cond.TimestampMin != nil && *cond.TimestampMin > h.Time { + return fmt.Errorf("failed timestamp minimum constraint") + } + if cond.TimestampMax != nil && *cond.TimestampMax < h.Time { + return fmt.Errorf("failed timestamp maximum constraint") + } + return nil +} + // Body is a simple (mutable, non-safe) data container for storing and moving // a block's data contents (transactions and uncles) together. type Body struct { diff --git a/core/types/block_test.go b/core/types/block_test.go index 1af5b9d7bf..16cb64fa78 100644 --- a/core/types/block_test.go +++ b/core/types/block_test.go @@ -317,3 +317,109 @@ func TestRlpDecodeParentHash(t *testing.T) { } } } + +func TestCheckTransactionConditional(t *testing.T) { + u64Ptr := func(n uint64) *uint64 { + return &n + } + + tests := []struct { + name string + header Header + cond TransactionConditional + valid bool + }{ + { + "BlockNumberMaxFails", + Header{Number: big.NewInt(2)}, + TransactionConditional{BlockNumberMax: big.NewInt(1)}, + false, + }, + { + "BlockNumberMaxEqualSucceeds", + Header{Number: big.NewInt(2)}, + TransactionConditional{BlockNumberMax: big.NewInt(2)}, + true, + }, + { + "BlockNumberMaxSucceeds", + Header{Number: big.NewInt(1)}, + TransactionConditional{BlockNumberMax: big.NewInt(2)}, + true, + }, + { + "BlockNumberMinFails", + Header{Number: big.NewInt(1)}, + TransactionConditional{BlockNumberMin: big.NewInt(2)}, + false, + }, + { + "BlockNumberMinEqualSuccess", + Header{Number: big.NewInt(2)}, + TransactionConditional{BlockNumberMin: big.NewInt(2)}, + true, + }, + { + "BlockNumberMinSuccess", + Header{Number: big.NewInt(4)}, + TransactionConditional{BlockNumberMin: big.NewInt(3)}, + true, + }, + { + "BlockNumberRangeSucceeds", + Header{Number: big.NewInt(5)}, + TransactionConditional{BlockNumberMin: big.NewInt(1), BlockNumberMax: big.NewInt(10)}, + true, + }, + { + "BlockNumberRangeFails", + Header{Number: big.NewInt(15)}, + TransactionConditional{BlockNumberMin: big.NewInt(1), BlockNumberMax: big.NewInt(10)}, + false, + }, + { + "BlockTimestampMinFails", + Header{Time: 1}, + TransactionConditional{TimestampMin: u64Ptr(2)}, + false, + }, + { + "BlockTimestampMinSucceeds", + Header{Time: 2}, + TransactionConditional{TimestampMin: u64Ptr(1)}, + true, + }, + { + "BlockTimestampMinEqualSucceeds", + Header{Time: 1}, + TransactionConditional{TimestampMin: u64Ptr(1)}, + true, + }, + { + "BlockTimestampMaxFails", + Header{Time: 2}, + TransactionConditional{TimestampMax: u64Ptr(1)}, + false, + }, + { + "BlockTimestampMaxSucceeds", + Header{Time: 1}, + TransactionConditional{TimestampMax: u64Ptr(2)}, + true, + }, + { + "BlockTimestampMaxEqualSucceeds", + Header{Time: 2}, + TransactionConditional{TimestampMax: u64Ptr(2)}, + true, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + if err := test.header.CheckTransactionConditional(&test.cond); err == nil && !test.valid { + t.Errorf("Test %s got unvalid value, want %v, got err %v", test.name, test.valid, err) + } + }) + } +} diff --git a/core/types/gen_transaction_conditional_json.go b/core/types/gen_transaction_conditional_json.go new file mode 100644 index 0000000000..b168b60206 --- /dev/null +++ b/core/types/gen_transaction_conditional_json.go @@ -0,0 +1,66 @@ +// Code generated by github.com/fjl/gencodec. DO NOT EDIT. + +package types + +import ( + "encoding/json" + "math/big" + "bytes" + + "github.com/ethereum/go-ethereum/common/hexutil" +) + +var _ = (*transactionConditionalMarshalling)(nil) + +// MarshalJSON marshals as JSON. +func (t TransactionConditional) MarshalJSON() ([]byte, error) { + type TransactionConditional struct { + KnownAccounts KnownAccounts `json:"knownAccounts"` + BlockNumberMin *hexutil.Big `json:"blockNumberMin,omitempty"` + BlockNumberMax *hexutil.Big `json:"blockNumberMax,omitempty"` + TimestampMin *hexutil.Uint64 `json:"timestampMin,omitempty"` + TimestampMax *hexutil.Uint64 `json:"timestampMax,omitempty"` + } + var enc TransactionConditional + enc.KnownAccounts = t.KnownAccounts + enc.BlockNumberMin = (*hexutil.Big)(t.BlockNumberMin) + enc.BlockNumberMax = (*hexutil.Big)(t.BlockNumberMax) + enc.TimestampMin = (*hexutil.Uint64)(t.TimestampMin) + enc.TimestampMax = (*hexutil.Uint64)(t.TimestampMax) + return json.Marshal(&enc) +} + +// UnmarshalJSON unmarshals from JSON. +func (t *TransactionConditional) UnmarshalJSON(input []byte) error { + type TransactionConditional struct { + KnownAccounts *KnownAccounts `json:"knownAccounts"` + BlockNumberMin *hexutil.Big `json:"blockNumberMin,omitempty"` + BlockNumberMax *hexutil.Big `json:"blockNumberMax,omitempty"` + TimestampMin *hexutil.Uint64 `json:"timestampMin,omitempty"` + TimestampMax *hexutil.Uint64 `json:"timestampMax,omitempty"` + } + var dec TransactionConditional + // --- Not Generated. Disallow unknown fields + decoder := json.NewDecoder(bytes.NewReader(input)) + decoder.DisallowUnknownFields() // Force errors + // --- + if err := decoder.Decode(&dec); err != nil { + return err + } + if dec.KnownAccounts != nil { + t.KnownAccounts = *dec.KnownAccounts + } + if dec.BlockNumberMin != nil { + t.BlockNumberMin = (*big.Int)(dec.BlockNumberMin) + } + if dec.BlockNumberMax != nil { + t.BlockNumberMax = (*big.Int)(dec.BlockNumberMax) + } + if dec.TimestampMin != nil { + t.TimestampMin = (*uint64)(dec.TimestampMin) + } + if dec.TimestampMax != nil { + t.TimestampMax = (*uint64)(dec.TimestampMax) + } + return nil +} diff --git a/core/types/transaction.go b/core/types/transaction.go index d0fa5308f0..25e5e55054 100644 --- a/core/types/transaction.go +++ b/core/types/transaction.go @@ -64,6 +64,9 @@ type Transaction struct { // cache of details to compute the data availability fee rollupCostData atomic.Value + + // optional preconditions for inclusion + conditional atomic.Value } // NewTx creates a new transaction. @@ -387,6 +390,20 @@ func (tx *Transaction) RollupCostData() RollupCostData { return out } +// Conditional returns the conditional attached to the transaction +func (tx *Transaction) Conditional() *TransactionConditional { + v := tx.conditional.Load() + if v == nil { + return nil + } + return v.(*TransactionConditional) +} + +// SetConditional attaches a conditional to the transaction +func (tx *Transaction) SetConditional(cond *TransactionConditional) { + tx.conditional.Store(cond) +} + // RawSignatureValues returns the V, R, S signature values of the transaction. // The return values should not be modified by the caller. // The return values may be nil or zero, if the transaction is unsigned. diff --git a/core/types/transaction_conditional.go b/core/types/transaction_conditional.go new file mode 100644 index 0000000000..dd28c2a11d --- /dev/null +++ b/core/types/transaction_conditional.go @@ -0,0 +1,137 @@ +package types + +import ( + "encoding/json" + "fmt" + "math/big" + "sync/atomic" + "time" + + "github.com/ethereum/go-ethereum/common" + "github.com/ethereum/go-ethereum/common/hexutil" +) + +const ( + TransactionConditionalMaxCost = 1000 + + TransactionConditionalRejectedErrCode = -32003 + TransactionConditionalCostExceededMaxErrCode = -32005 +) + +// KnownAccounts represents a set of KnownAccounts +type KnownAccounts map[common.Address]KnownAccount + +// KnownAccount allows for a user to express their preference of a known +// prestate at a particular account. Only one of the storage root or +// storage slots is allowed to be set. If the storage root is set, then +// the user prefers their transaction to only be included in a block if +// the account's storage root matches. If the storage slots are set, +// then the user prefers their transaction to only be included if the +// particular storage slot values from state match. +type KnownAccount struct { + StorageRoot *common.Hash `rlp:"nil"` + StorageSlots map[common.Hash]common.Hash +} + +// UnmarshalJSON will parse the JSON bytes into a KnownAccount struct. +func (ka *KnownAccount) UnmarshalJSON(data []byte) error { + var hash common.Hash + if err := json.Unmarshal(data, &hash); err == nil { + ka.StorageRoot = &hash + ka.StorageSlots = make(map[common.Hash]common.Hash) + return nil + } + + var mapping map[common.Hash]common.Hash + if err := json.Unmarshal(data, &mapping); err != nil { + return err + } + ka.StorageSlots = mapping + return nil +} + +// MarshalJSON will serialize the KnownAccount into JSON bytes. +func (ka *KnownAccount) MarshalJSON() ([]byte, error) { + if ka.StorageRoot != nil { + return json.Marshal(ka.StorageRoot) + } + return json.Marshal(ka.StorageSlots) +} + +// Root will return the storage root and true when the user prefers +// execution against an account's storage root, otherwise it will +// return false. +func (ka *KnownAccount) Root() (common.Hash, bool) { + if ka.StorageRoot == nil { + return common.Hash{}, false + } + return *ka.StorageRoot, true +} + +// Slots will return the storage slots and true when the user prefers +// execution against an account's particular storage slots, StorageRoot == nil, +// otherwise it will return false. +func (ka *KnownAccount) Slots() (map[common.Hash]common.Hash, bool) { + if ka.StorageRoot != nil { + return ka.StorageSlots, false + } + return ka.StorageSlots, true +} + +//go:generate go run github.com/fjl/gencodec -type TransactionConditional -field-override transactionConditionalMarshalling -out gen_transaction_conditional_json.go + +// TransactionConditional represents the preconditions that determine the +// inclusion of the transaction, enforced out-of-protocol by the sequencer. +type TransactionConditional struct { + // KnownAccounts represents account prestate conditions + KnownAccounts KnownAccounts `json:"knownAccounts"` + + // Header state conditionals + BlockNumberMin *big.Int `json:"blockNumberMin,omitempty"` + BlockNumberMax *big.Int `json:"blockNumberMax,omitempty"` + TimestampMin *uint64 `json:"timestampMin,omitempty"` + TimestampMax *uint64 `json:"timestampMax,omitempty"` + + // Internal fields used for metrics and inclusion tracking + SubmissionTime time.Time `json:"-"` + Rejected *atomic.Bool `json:"-"` +} + +// field type overrides for gencodec +type transactionConditionalMarshalling struct { + BlockNumberMax *hexutil.Big + BlockNumberMin *hexutil.Big + TimestampMin *hexutil.Uint64 + TimestampMax *hexutil.Uint64 +} + +// Validate will perform sanity checks on the preconditions. This does not check the aggregate cost of the preconditions. +func (cond *TransactionConditional) Validate() error { + if cond.BlockNumberMin != nil && cond.BlockNumberMax != nil && cond.BlockNumberMin.Cmp(cond.BlockNumberMax) > 0 { + return fmt.Errorf("block number minimum constraint must be less than the maximum") + } + if cond.TimestampMin != nil && cond.TimestampMax != nil && *cond.TimestampMin > *cond.TimestampMax { + return fmt.Errorf("timestamp minimum constraint must be less than the maximum") + } + return nil +} + +// Cost computes the aggregate cost of the preconditions; total number of storage lookups required +func (opts *TransactionConditional) Cost() int { + cost := 0 + for _, account := range opts.KnownAccounts { + if _, isRoot := account.Root(); isRoot { + cost += 1 + } + if slots, isSlots := account.Slots(); isSlots { + cost += len(slots) + } + } + if opts.BlockNumberMin != nil || opts.BlockNumberMax != nil { + cost += 1 + } + if opts.TimestampMin != nil || opts.TimestampMax != nil { + cost += 1 + } + return cost +} diff --git a/core/types/transaction_conditional_test.go b/core/types/transaction_conditional_test.go new file mode 100644 index 0000000000..abb8d8ab30 --- /dev/null +++ b/core/types/transaction_conditional_test.go @@ -0,0 +1,257 @@ +package types + +import ( + "encoding/json" + "math/big" + "reflect" + "testing" + + "github.com/ethereum/go-ethereum/common" +) + +func TestTransactionConditionalCost(t *testing.T) { + uint64Ptr := func(num uint64) *uint64 { + return &num + } + + tests := []struct { + name string + cond TransactionConditional + cost int + }{ + { + name: "empty conditional", + cond: TransactionConditional{}, + cost: 0, + }, + { + name: "block number lookup counts once", + cond: TransactionConditional{BlockNumberMin: big.NewInt(1), BlockNumberMax: big.NewInt(2)}, + cost: 1, + }, + { + name: "timestamp lookup counts once", + cond: TransactionConditional{TimestampMin: uint64Ptr(0), TimestampMax: uint64Ptr(5)}, + cost: 1, + }, + { + name: "storage root lookup", + cond: TransactionConditional{KnownAccounts: map[common.Address]KnownAccount{ + common.Address{19: 1}: KnownAccount{ + StorageRoot: &EmptyRootHash, + }}}, + cost: 1, + }, + { + name: "cost per storage slot lookup", + cond: TransactionConditional{KnownAccounts: map[common.Address]KnownAccount{ + common.Address{19: 1}: KnownAccount{ + StorageSlots: map[common.Hash]common.Hash{ + common.Hash{}: common.Hash{31: 1}, + common.Hash{31: 1}: common.Hash{31: 1}, + }, + }}}, + cost: 2, + }, + { + name: "cost summed together", + cond: TransactionConditional{ + BlockNumberMin: big.NewInt(1), + TimestampMin: uint64Ptr(1), + KnownAccounts: map[common.Address]KnownAccount{ + common.Address{19: 1}: KnownAccount{StorageRoot: &EmptyRootHash}, + common.Address{19: 2}: KnownAccount{ + StorageSlots: map[common.Hash]common.Hash{ + common.Hash{}: common.Hash{31: 1}, + common.Hash{31: 1}: common.Hash{31: 1}, + }, + }}}, + cost: 5, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + cost := test.cond.Cost() + if cost != test.cost { + t.Errorf("Test %s mismatch in TransactionConditional cost. Got %d, Expected: %d", test.name, cost, test.cost) + } + }) + } +} + +func TestTransactionConditionalValidation(t *testing.T) { + uint64Ptr := func(num uint64) *uint64 { + return &num + } + + tests := []struct { + name string + cond TransactionConditional + mustFail bool + }{ + { + name: "empty conditional", + cond: TransactionConditional{}, + mustFail: false, + }, + { + name: "equal block constraint", + cond: TransactionConditional{BlockNumberMin: big.NewInt(1), BlockNumberMax: big.NewInt(1)}, + mustFail: false, + }, + { + name: "block min greater than max", + cond: TransactionConditional{BlockNumberMin: big.NewInt(2), BlockNumberMax: big.NewInt(1)}, + mustFail: true, + }, + { + name: "equal timestamp constraint", + cond: TransactionConditional{TimestampMin: uint64Ptr(1), TimestampMax: uint64Ptr(1)}, + mustFail: false, + }, + { + name: "timestamp min greater than max", + cond: TransactionConditional{TimestampMin: uint64Ptr(2), TimestampMax: uint64Ptr(1)}, + mustFail: true, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + err := test.cond.Validate() + if test.mustFail && err == nil { + t.Errorf("Test %s should fail", test.name) + } + if !test.mustFail && err != nil { + t.Errorf("Test %s should pass but got err: %v", test.name, err) + } + }) + } +} + +func TestTransactionConditionalSerDeser(t *testing.T) { + uint64Ptr := func(num uint64) *uint64 { + return &num + } + hashPtr := func(hash common.Hash) *common.Hash { + return &hash + } + + tests := []struct { + name string + input string + mustFail bool + expected TransactionConditional + }{ + { + name: "StateRoot", + input: `{"knownAccounts":{"0x6b3A8798E5Fb9fC5603F3aB5eA2e8136694e55d0":"0x290decd9548b62a8d60345a988386fc84ba6bc95484008f6362f93160ef3e563"}}`, + mustFail: false, + expected: TransactionConditional{ + KnownAccounts: map[common.Address]KnownAccount{ + common.HexToAddress("0x6b3A8798E5Fb9fC5603F3aB5eA2e8136694e55d0"): KnownAccount{ + StorageRoot: hashPtr(common.HexToHash("0x290decd9548b62a8d60345a988386fc84ba6bc95484008f6362f93160ef3e563")), + StorageSlots: make(map[common.Hash]common.Hash), + }, + }, + }, + }, + { + name: "StorageSlots", + input: `{"knownAccounts":{"0x6b3A8798E5Fb9fC5603F3aB5eA2e8136694e55d0":{"0xc65a7bb8d6351c1cf70c95a316cc6a92839c986682d98bc35f958f4883f9d2a8":"0x0000000000000000000000000000000000000000000000000000000000000000"}}}`, + mustFail: false, + expected: TransactionConditional{ + KnownAccounts: map[common.Address]KnownAccount{ + common.HexToAddress("0x6b3A8798E5Fb9fC5603F3aB5eA2e8136694e55d0"): KnownAccount{ + StorageRoot: nil, + StorageSlots: map[common.Hash]common.Hash{ + common.HexToHash("0xc65a7bb8d6351c1cf70c95a316cc6a92839c986682d98bc35f958f4883f9d2a8"): common.HexToHash("0x"), + }, + }, + }, + }, + }, + { + name: "EmptyObject", + input: `{"knownAccounts":{}}`, + mustFail: false, + expected: TransactionConditional{ + KnownAccounts: make(map[common.Address]KnownAccount), + }, + }, + { + name: "EmptyStrings", + input: `{"knownAccounts":{"":""}}`, + mustFail: true, + expected: TransactionConditional{KnownAccounts: nil}, + }, + { + name: "BlockNumberMin", + input: `{"blockNumberMin":"0x1"}`, + mustFail: false, + expected: TransactionConditional{ + BlockNumberMin: big.NewInt(1), + }, + }, + { + name: "BlockNumberMax", + input: `{"blockNumberMin":"0x1", "blockNumberMax":"0x2"}`, + mustFail: false, + expected: TransactionConditional{ + BlockNumberMin: big.NewInt(1), + BlockNumberMax: big.NewInt(2), + }, + }, + { + name: "TimestampMin", + input: `{"timestampMin":"0xffff"}`, + mustFail: false, + expected: TransactionConditional{ + TimestampMin: uint64Ptr(uint64(0xffff)), + }, + }, + { + name: "TimestampMax", + input: `{"timestampMax":"0xffffff"}`, + mustFail: false, + expected: TransactionConditional{ + TimestampMax: uint64Ptr(uint64(0xffffff)), + }, + }, + { + name: "SubmissionTime", + input: `{"submissionTime": 1234}`, + mustFail: true, + expected: TransactionConditional{KnownAccounts: nil}, + }, + { + name: "Rejected", + input: `{"rejected": false}`, + mustFail: true, + expected: TransactionConditional{KnownAccounts: nil}, + }, + { + name: "UnknownField", + input: `{"foobarbaz": 1234}`, + mustFail: true, + expected: TransactionConditional{KnownAccounts: nil}, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + var cond TransactionConditional + err := json.Unmarshal([]byte(test.input), &cond) + if test.mustFail && err == nil { + t.Errorf("Test %s should fail", test.name) + } + if !test.mustFail && err != nil { + t.Errorf("Test %s should pass but got err: %v", test.name, err) + } + if !reflect.DeepEqual(cond, test.expected) { + t.Errorf("Test %s got unexpected value, want %#v, got %#v", test.name, test.expected, cond) + } + }) + } +} diff --git a/eth/backend.go b/eth/backend.go index 7081c6cfb5..98577869ca 100644 --- a/eth/backend.go +++ b/eth/backend.go @@ -48,6 +48,7 @@ import ( "github.com/ethereum/go-ethereum/ethdb" "github.com/ethereum/go-ethereum/event" "github.com/ethereum/go-ethereum/internal/ethapi" + "github.com/ethereum/go-ethereum/internal/sequencerapi" "github.com/ethereum/go-ethereum/internal/shutdowncheck" "github.com/ethereum/go-ethereum/log" "github.com/ethereum/go-ethereum/miner" @@ -374,6 +375,11 @@ func (s *Ethereum) APIs() []rpc.API { // Append any APIs exposed explicitly by the consensus engine apis = append(apis, s.engine.APIs(s.BlockChain())...) + // Append any Sequencer APIs as enabled + if s.config.RollupSequencerEnableTxConditional { + apis = append(apis, sequencerapi.GetSendRawTxConditionalAPI(s.APIBackend)) + } + // Append all the local APIs and return return append(apis, []rpc.API{ { diff --git a/eth/ethconfig/config.go b/eth/ethconfig/config.go index 2dc03013d5..b2ac0dcb4a 100644 --- a/eth/ethconfig/config.go +++ b/eth/ethconfig/config.go @@ -189,6 +189,7 @@ type Config struct { RollupDisableTxPoolGossip bool RollupDisableTxPoolAdmission bool RollupHaltOnIncompatibleProtocolVersion string + RollupSequencerEnableTxConditional bool } // CreateConsensusEngine creates a consensus engine for the given chain config. diff --git a/eth/protocols/eth/broadcast.go b/eth/protocols/eth/broadcast.go index f0ed1d6bc9..e41e0aaf74 100644 --- a/eth/protocols/eth/broadcast.go +++ b/eth/protocols/eth/broadcast.go @@ -47,7 +47,10 @@ func (p *Peer) broadcastTransactions() { size common.StorageSize ) for i := 0; i < len(queue) && size < maxTxPacketSize; i++ { - if tx := p.txpool.Get(queue[i]); tx != nil { + // Transaction conditionals are tied to the block builder it was + // submitted to. Thus we do not broadcast transactions to peers that + // have a conditional attached to them. + if tx := p.txpool.Get(queue[i]); tx != nil && tx.Conditional() == nil { txs = append(txs, tx) size += common.StorageSize(tx.Size()) } diff --git a/fork.yaml b/fork.yaml index 134b463ca3..5b52d37352 100644 --- a/fork.yaml +++ b/fork.yaml @@ -275,6 +275,25 @@ def: and the chain ID formula on signature data must not be used, or an underflow happens. globs: - "internal/ethapi/testdata/eth_getBlockByNumber-tag-pending-fullTx.json" + - title: "4337 Improvements" + description: "" + sub: + - title: eth_sendRawTransactionConditional + description: sequencer api for conditional transaction inclusion enforced out of protocol + globs: + - "cmd/geth/main.go" + - "cmd/utils/flags.go" + - "core/state/statedb.go" + - "core/types/block.go" + - "core/types/transaction.go" + - "core/types/transaction_conditional.go" + - "eth/backend.go" + - "eth/ethconfig/config.go" + - "eth/protocols/eth/broadcast.go" + - "internal/sequencerapi/api.go" + - "internal/sequencerapi/errors.go" + - "miner/worker.go" + - "miner/miner.go" - title: "Geth extras" description: Extend the tools available in geth to improve external testing and tooling. sub: diff --git a/internal/sequencerapi/api.go b/internal/sequencerapi/api.go new file mode 100644 index 0000000000..33dcdfd6f6 --- /dev/null +++ b/internal/sequencerapi/api.go @@ -0,0 +1,97 @@ +package sequencerapi + +import ( + "context" + "fmt" + "sync/atomic" + "time" + + "github.com/ethereum/go-ethereum/common" + "github.com/ethereum/go-ethereum/common/hexutil" + "github.com/ethereum/go-ethereum/core/types" + "github.com/ethereum/go-ethereum/internal/ethapi" + "github.com/ethereum/go-ethereum/metrics" + "github.com/ethereum/go-ethereum/rpc" +) + +var ( + sendRawTxConditionalCostMeter = metrics.NewRegisteredMeter("sequencer/sendRawTransactionConditional/cost", nil) + sendRawTxConditionalRequestsCounter = metrics.NewRegisteredCounter("sequencer/sendRawTransactionConditional/requests", nil) + sendRawTxConditionalAcceptedCounter = metrics.NewRegisteredCounter("sequencer/sendRawTransactionConditional/accepted", nil) +) + +type sendRawTxCond struct { + b ethapi.Backend +} + +func GetSendRawTxConditionalAPI(b ethapi.Backend) rpc.API { + return rpc.API{ + Namespace: "eth", + Service: &sendRawTxCond{b}, + } +} + +func (s *sendRawTxCond) SendRawTransactionConditional(ctx context.Context, txBytes hexutil.Bytes, cond types.TransactionConditional) (common.Hash, error) { + sendRawTxConditionalRequestsCounter.Inc(1) + + cost := cond.Cost() + sendRawTxConditionalCostMeter.Mark(int64(cost)) + if cost > types.TransactionConditionalMaxCost { + return common.Hash{}, &jsonRpcError{ + message: fmt.Sprintf("conditional cost, %d, exceeded max: %d", cost, types.TransactionConditionalMaxCost), + code: types.TransactionConditionalCostExceededMaxErrCode, + } + } + + // Perform sanity validation prior to state lookups + if err := cond.Validate(); err != nil { + return common.Hash{}, &jsonRpcError{ + message: fmt.Sprintf("failed conditional validation: %s", err), + code: types.TransactionConditionalRejectedErrCode, + } + } + + state, header, err := s.b.StateAndHeaderByNumber(context.Background(), rpc.LatestBlockNumber) + if err != nil { + return common.Hash{}, err + } + if err := header.CheckTransactionConditional(&cond); err != nil { + return common.Hash{}, &jsonRpcError{ + message: fmt.Sprintf("failed header check: %s", err), + code: types.TransactionConditionalRejectedErrCode, + } + } + if err := state.CheckTransactionConditional(&cond); err != nil { + return common.Hash{}, &jsonRpcError{ + message: fmt.Sprintf("failed state check: %s", err), + code: types.TransactionConditionalRejectedErrCode, + } + } + + // We also check against the parent block to eliminate the MEV incentive in comparison with sendRawTransaction + parentBlock := rpc.BlockNumberOrHash{BlockHash: &header.ParentHash} + parentState, _, err := s.b.StateAndHeaderByNumberOrHash(context.Background(), parentBlock) + if err != nil { + return common.Hash{}, err + } + if err := parentState.CheckTransactionConditional(&cond); err != nil { + return common.Hash{}, &jsonRpcError{ + message: fmt.Sprintf("failed parent block %s state check: %s", header.ParentHash, err), + code: types.TransactionConditionalRejectedErrCode, + } + } + + tx := new(types.Transaction) + if err := tx.UnmarshalBinary(txBytes); err != nil { + return common.Hash{}, err + } + + // Set internal fields + cond.SubmissionTime = time.Now() + cond.Rejected = &atomic.Bool{} + + tx.SetConditional(&cond) + sendRawTxConditionalAcceptedCounter.Inc(1) + + return ethapi.SubmitTransaction(ctx, s.b, tx) +} diff --git a/internal/sequencerapi/errors.go b/internal/sequencerapi/errors.go new file mode 100644 index 0000000000..9265a241bd --- /dev/null +++ b/internal/sequencerapi/errors.go @@ -0,0 +1,20 @@ +package sequencerapi + +import ( + "github.com/ethereum/go-ethereum/rpc" +) + +var _ rpc.Error = new(jsonRpcError) + +type jsonRpcError struct { + message string + code int +} + +func (j *jsonRpcError) Error() string { + return j.message +} + +func (j *jsonRpcError) ErrorCode() int { + return j.code +} diff --git a/miner/miner.go b/miner/miner.go index 8368b96e15..069c9e5a89 100644 --- a/miner/miner.go +++ b/miner/miner.go @@ -33,6 +33,7 @@ import ( "github.com/ethereum/go-ethereum/core/types" "github.com/ethereum/go-ethereum/eth/tracers" "github.com/ethereum/go-ethereum/params" + "golang.org/x/time/rate" ) // Backend wraps all methods required for mining. Only full node is capable @@ -55,8 +56,10 @@ type Config struct { GasPrice *big.Int // Minimum gas price for mining a transaction Recommit time.Duration // The time interval for miner to re-create mining work. - RollupComputePendingBlock bool // Compute the pending block from tx-pool, instead of copying the latest-block - EffectiveGasCeil uint64 // if non-zero, a gas ceiling to apply independent of the header's gaslimit value + RollupComputePendingBlock bool // Compute the pending block from tx-pool, instead of copying the latest-block + RollupTransactionConditionalBurstRate int // Total number of conditional cost units allowed in a second + + EffectiveGasCeil uint64 // if non-zero, a gas ceiling to apply independent of the header's gaslimit value } // DefaultConfig contains default settings for miner. @@ -84,18 +87,22 @@ type Miner struct { pendingMu sync.Mutex // Lock protects the pending block backend Backend + + // TransactionConditional safegaurds + conditionalLimiter *rate.Limiter } // New creates a new miner with provided config. func New(eth Backend, config Config, engine consensus.Engine) *Miner { return &Miner{ - backend: eth, - config: &config, - chainConfig: eth.BlockChain().Config(), - engine: engine, - txpool: eth.TxPool(), - chain: eth.BlockChain(), - pending: &pending{}, + backend: eth, + config: &config, + chainConfig: eth.BlockChain().Config(), + engine: engine, + txpool: eth.TxPool(), + chain: eth.BlockChain(), + pending: &pending{}, + conditionalLimiter: rate.NewLimiter(types.TransactionConditionalMaxCost, config.RollupTransactionConditionalBurstRate), } } diff --git a/miner/miner_test.go b/miner/miner_test.go index da133ad8d0..9042558160 100644 --- a/miner/miner_test.go +++ b/miner/miner_test.go @@ -20,7 +20,9 @@ package miner import ( "math/big" "sync" + "sync/atomic" "testing" + "time" "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/consensus/clique" @@ -74,6 +76,7 @@ func (bc *testBlockChain) CurrentBlock() *types.Header { return &types.Header{ Number: new(big.Int), GasLimit: bc.gasLimit, + BaseFee: big.NewInt(params.InitialBaseFee), } } @@ -139,16 +142,18 @@ func minerTestGenesisBlock(period uint64, gasLimit uint64, faucet common.Address func createMiner(t *testing.T) *Miner { // Create Ethash config config := Config{ - PendingFeeRecipient: common.HexToAddress("123456789"), + PendingFeeRecipient: common.HexToAddress("123456789"), + RollupTransactionConditionalBurstRate: types.TransactionConditionalMaxCost, } // Create chainConfig chainDB := rawdb.NewMemoryDatabase() triedb := triedb.NewDatabase(chainDB, nil) - genesis := minerTestGenesisBlock(15, 11_500_000, common.HexToAddress("12345")) + genesis := minerTestGenesisBlock(15, 11_500_000, testBankAddress) chainConfig, _, err := core.SetupGenesisBlock(chainDB, triedb, genesis) if err != nil { t.Fatalf("can't create new chain config: %v", err) } + // Create consensus engine engine := clique.New(chainConfig.Clique, chainDB) // Create Ethereum backend @@ -167,3 +172,53 @@ func createMiner(t *testing.T) *Miner { miner := New(backend, config, engine) return miner } + +func TestRejectedConditionalTx(t *testing.T) { + miner := createMiner(t) + timestamp := uint64(time.Now().Unix()) + uint64Ptr := func(num uint64) *uint64 { return &num } + + // add a conditional transaction to be rejected + signer := types.LatestSigner(miner.chainConfig) + tx := types.MustSignNewTx(testBankKey, signer, &types.LegacyTx{ + Nonce: 0, + To: &testUserAddress, + Value: big.NewInt(1000), + Gas: params.TxGas, + GasPrice: big.NewInt(params.InitialBaseFee), + }) + tx.SetConditional(&types.TransactionConditional{ + TimestampMax: uint64Ptr(timestamp - 1), + Rejected: &atomic.Bool{}, + }) + + // 1 pending tx + miner.txpool.Add(types.Transactions{tx}, true, false) + if !miner.txpool.Has(tx.Hash()) { + t.Fatalf("conditional tx is not in the mempool") + } + + // request block + r := miner.generateWork(&generateParams{ + parentHash: miner.chain.CurrentBlock().Hash(), + timestamp: timestamp, + random: common.HexToHash("0xcafebabe"), + noTxs: false, + forceTime: true, + }) + + if len(r.block.Transactions()) != 0 { + t.Fatalf("block should be empty") + } + + // conditional is rejected + if !tx.Conditional().Rejected.Load() { + t.Fatalf("conditional tx is not marked as rejected") + } + + // rejected conditional is evicted from the txpool + miner.txpool.Sync() + if miner.txpool.Has(tx.Hash()) { + t.Fatalf("conditional tx is still in the mempool") + } +} diff --git a/miner/worker.go b/miner/worker.go index a408852273..cba42a8e18 100644 --- a/miner/worker.go +++ b/miner/worker.go @@ -35,6 +35,7 @@ import ( "github.com/ethereum/go-ethereum/core/vm" "github.com/ethereum/go-ethereum/eth/tracers" "github.com/ethereum/go-ethereum/log" + "github.com/ethereum/go-ethereum/metrics" "github.com/ethereum/go-ethereum/params" "github.com/holiman/uint256" ) @@ -46,10 +47,16 @@ const ( ) var ( + errTxConditionalInvalid = errors.New("transaction conditional failed") + errTxConditionalRateLimited = errors.New("transaction conditional rate limited") + errBlockInterruptedByNewHead = errors.New("new head arrived while building block") errBlockInterruptedByRecommit = errors.New("recommit interrupt while building block") errBlockInterruptedByTimeout = errors.New("timeout while building block") errBlockInterruptedByResolve = errors.New("payload resolution while building block") + + txConditionalRejectedCounter = metrics.NewRegisteredCounter("miner/transactionConditional/rejected", nil) + txConditionalMinedTimer = metrics.NewRegisteredTimer("miner/transactionConditional/elapsedtime", nil) ) // environment is the worker's current environment and holds all @@ -287,6 +294,29 @@ func (miner *Miner) commitTransaction(env *environment, tx *types.Transaction) e if tx.Type() == types.BlobTxType { return miner.commitBlobTransaction(env, tx) } + + // If a conditional is set, check prior to applying + if conditional := tx.Conditional(); conditional != nil { + now, cost := time.Now(), conditional.Cost() + if !miner.conditionalLimiter.AllowN(now, cost) { + return fmt.Errorf("exceeded rate limit: cost %d, tokens %f: %w", cost, miner.conditionalLimiter.Tokens(), errTxConditionalRateLimited) + } + + // ditch the reservation as we've checked that we're allowed `cost` units by the limiter + miner.conditionalLimiter.ReserveN(now, cost) + txConditionalMinedTimer.UpdateSince(conditional.SubmissionTime) + + // check the conditional + if err := env.header.CheckTransactionConditional(conditional); err != nil { + conditional.Rejected.Store(true) + return fmt.Errorf("failed header check: %s: %w", err, errTxConditionalInvalid) + } + if err := env.state.CheckTransactionConditional(conditional); err != nil { + conditional.Rejected.Store(true) + return fmt.Errorf("failed state check: %s: %w", err, errTxConditionalInvalid) + } + } + receipt, err := miner.applyTransaction(env, tx) if err != nil { return err @@ -422,6 +452,18 @@ func (miner *Miner) commitTransactions(env *environment, plainTxs, blobTxs *tran log.Trace("Skipping transaction with low nonce", "hash", ltx.Hash, "sender", from, "nonce", tx.Nonce()) txs.Shift() + case errors.Is(err, errTxConditionalInvalid): + // err contains contextual info on the failed conditional + txConditionalRejectedCounter.Inc(1) + log.Debug("Skipping account, transaction with failed conditional", "sender", from, "hash", ltx.Hash, "err", err) + txs.Pop() + + case errors.Is(err, errTxConditionalRateLimited): + // err contains contextual info of the cost and limiter tokens available + txConditionalRejectedCounter.Inc(1) + log.Debug("Skipping account, transaction with conditional rate limited", "sender", from, "hash", ltx.Hash, "err", err) + txs.Pop() + case errors.Is(err, nil): // Everything ok, collect the logs and shift in the next transaction from the same account txs.Shift() From 61ebf14fc37c445b9159e8a763109629ee309c7e Mon Sep 17 00:00:00 2001 From: Hamdi Allam Date: Wed, 14 Aug 2024 12:47:39 -0400 Subject: [PATCH 02/16] lint --- core/types/transaction_conditional.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/core/types/transaction_conditional.go b/core/types/transaction_conditional.go index dd28c2a11d..ddccb38974 100644 --- a/core/types/transaction_conditional.go +++ b/core/types/transaction_conditional.go @@ -117,9 +117,9 @@ func (cond *TransactionConditional) Validate() error { } // Cost computes the aggregate cost of the preconditions; total number of storage lookups required -func (opts *TransactionConditional) Cost() int { +func (cond *TransactionConditional) Cost() int { cost := 0 - for _, account := range opts.KnownAccounts { + for _, account := range cond.KnownAccounts { if _, isRoot := account.Root(); isRoot { cost += 1 } @@ -127,10 +127,10 @@ func (opts *TransactionConditional) Cost() int { cost += len(slots) } } - if opts.BlockNumberMin != nil || opts.BlockNumberMax != nil { + if cond.BlockNumberMin != nil || cond.BlockNumberMax != nil { cost += 1 } - if opts.TimestampMin != nil || opts.TimestampMax != nil { + if cond.TimestampMin != nil || cond.TimestampMax != nil { cost += 1 } return cost From 895cedba9d7cfd2cc21c14c93f87b01a19733950 Mon Sep 17 00:00:00 2001 From: Hamdi Allam Date: Mon, 26 Aug 2024 15:22:26 -0400 Subject: [PATCH 03/16] feedback --- core/state/statedb.go | 4 +-- core/txpool/legacypool/legacypool.go | 1 - core/types/transaction_conditional.go | 7 ----- internal/sequencerapi/api.go | 37 ++++++++++++--------------- internal/sequencerapi/errors.go | 20 --------------- miner/miner.go | 2 +- miner/miner_test.go | 2 +- miner/worker.go | 7 ++--- params/conditional_tx_params.go | 8 ++++++ rpc/client_test.go | 2 +- rpc/json.go | 12 ++++----- 11 files changed, 39 insertions(+), 63 deletions(-) delete mode 100644 internal/sequencerapi/errors.go create mode 100644 params/conditional_tx_params.go diff --git a/core/state/statedb.go b/core/state/statedb.go index e525eee551..3dbad5e48f 100644 --- a/core/state/statedb.go +++ b/core/state/statedb.go @@ -431,8 +431,8 @@ func (s *StateDB) HasSelfDestructed(addr common.Address) bool { // ensure no state changes occur while this check is executed. func (s *StateDB) CheckTransactionConditional(cond *types.TransactionConditional) error { cost := cond.Cost() - if cost > types.TransactionConditionalMaxCost { - return fmt.Errorf("conditional cost, %d, exceeded max: %d", cost, types.TransactionConditionalMaxCost) + if cost > params.TransactionConditionalMaxCost { + return fmt.Errorf("conditional cost, %d, exceeded max: %d", cost, params.TransactionConditionalMaxCost) } for addr, acct := range cond.KnownAccounts { diff --git a/core/txpool/legacypool/legacypool.go b/core/txpool/legacypool/legacypool.go index 7036c66429..84b4924590 100644 --- a/core/txpool/legacypool/legacypool.go +++ b/core/txpool/legacypool/legacypool.go @@ -1724,7 +1724,6 @@ func (pool *LegacyPool) demoteUnexecutables() { } balance := pool.currentState.GetBalance(addr) balance = pool.reduceBalanceByL1Cost(list, balance) - // Drop all transactions that are too costly (low balance or out of gas), and queue any invalids back for later drops, invalids := list.Filter(balance, gasLimit) for _, tx := range drops { diff --git a/core/types/transaction_conditional.go b/core/types/transaction_conditional.go index ddccb38974..a9020ab9de 100644 --- a/core/types/transaction_conditional.go +++ b/core/types/transaction_conditional.go @@ -11,13 +11,6 @@ import ( "github.com/ethereum/go-ethereum/common/hexutil" ) -const ( - TransactionConditionalMaxCost = 1000 - - TransactionConditionalRejectedErrCode = -32003 - TransactionConditionalCostExceededMaxErrCode = -32005 -) - // KnownAccounts represents a set of KnownAccounts type KnownAccounts map[common.Address]KnownAccount diff --git a/internal/sequencerapi/api.go b/internal/sequencerapi/api.go index 33dcdfd6f6..6e4f1ea6cb 100644 --- a/internal/sequencerapi/api.go +++ b/internal/sequencerapi/api.go @@ -11,6 +11,7 @@ import ( "github.com/ethereum/go-ethereum/core/types" "github.com/ethereum/go-ethereum/internal/ethapi" "github.com/ethereum/go-ethereum/metrics" + "github.com/ethereum/go-ethereum/params" "github.com/ethereum/go-ethereum/rpc" ) @@ -36,48 +37,42 @@ func (s *sendRawTxCond) SendRawTransactionConditional(ctx context.Context, txByt cost := cond.Cost() sendRawTxConditionalCostMeter.Mark(int64(cost)) - if cost > types.TransactionConditionalMaxCost { - return common.Hash{}, &jsonRpcError{ - message: fmt.Sprintf("conditional cost, %d, exceeded max: %d", cost, types.TransactionConditionalMaxCost), - code: types.TransactionConditionalCostExceededMaxErrCode, + if cost > params.TransactionConditionalMaxCost { + return common.Hash{}, &rpc.JsonError{ + Message: fmt.Sprintf("conditional cost, %d, exceeded max: %d", cost, params.TransactionConditionalMaxCost), + Code: params.TransactionConditionalCostExceededMaxErrCode, } } // Perform sanity validation prior to state lookups if err := cond.Validate(); err != nil { - return common.Hash{}, &jsonRpcError{ - message: fmt.Sprintf("failed conditional validation: %s", err), - code: types.TransactionConditionalRejectedErrCode, + return common.Hash{}, &rpc.JsonError{ + Message: fmt.Sprintf("failed conditional validation: %s", err), + Code: params.TransactionConditionalRejectedErrCode, } } - state, header, err := s.b.StateAndHeaderByNumber(context.Background(), rpc.LatestBlockNumber) + header, err := s.b.HeaderByNumber(context.Background(), rpc.LatestBlockNumber) if err != nil { return common.Hash{}, err } if err := header.CheckTransactionConditional(&cond); err != nil { - return common.Hash{}, &jsonRpcError{ - message: fmt.Sprintf("failed header check: %s", err), - code: types.TransactionConditionalRejectedErrCode, - } - } - if err := state.CheckTransactionConditional(&cond); err != nil { - return common.Hash{}, &jsonRpcError{ - message: fmt.Sprintf("failed state check: %s", err), - code: types.TransactionConditionalRejectedErrCode, + return common.Hash{}, &rpc.JsonError{ + Message: fmt.Sprintf("failed header check: %s", err), + Code: params.TransactionConditionalRejectedErrCode, } } - // We also check against the parent block to eliminate the MEV incentive in comparison with sendRawTransaction + // State is checked against an older block to remove the MEV incentive for this endpoint compared with sendRawTransaction parentBlock := rpc.BlockNumberOrHash{BlockHash: &header.ParentHash} parentState, _, err := s.b.StateAndHeaderByNumberOrHash(context.Background(), parentBlock) if err != nil { return common.Hash{}, err } if err := parentState.CheckTransactionConditional(&cond); err != nil { - return common.Hash{}, &jsonRpcError{ - message: fmt.Sprintf("failed parent block %s state check: %s", header.ParentHash, err), - code: types.TransactionConditionalRejectedErrCode, + return common.Hash{}, &rpc.JsonError{ + Message: fmt.Sprintf("failed parent block %s state check: %s", header.ParentHash, err), + Code: params.TransactionConditionalRejectedErrCode, } } diff --git a/internal/sequencerapi/errors.go b/internal/sequencerapi/errors.go deleted file mode 100644 index 9265a241bd..0000000000 --- a/internal/sequencerapi/errors.go +++ /dev/null @@ -1,20 +0,0 @@ -package sequencerapi - -import ( - "github.com/ethereum/go-ethereum/rpc" -) - -var _ rpc.Error = new(jsonRpcError) - -type jsonRpcError struct { - message string - code int -} - -func (j *jsonRpcError) Error() string { - return j.message -} - -func (j *jsonRpcError) ErrorCode() int { - return j.code -} diff --git a/miner/miner.go b/miner/miner.go index 069c9e5a89..ffb360065d 100644 --- a/miner/miner.go +++ b/miner/miner.go @@ -102,7 +102,7 @@ func New(eth Backend, config Config, engine consensus.Engine) *Miner { txpool: eth.TxPool(), chain: eth.BlockChain(), pending: &pending{}, - conditionalLimiter: rate.NewLimiter(types.TransactionConditionalMaxCost, config.RollupTransactionConditionalBurstRate), + conditionalLimiter: rate.NewLimiter(params.TransactionConditionalMaxCost, config.RollupTransactionConditionalBurstRate), } } diff --git a/miner/miner_test.go b/miner/miner_test.go index 9042558160..4a7ada4f47 100644 --- a/miner/miner_test.go +++ b/miner/miner_test.go @@ -143,7 +143,7 @@ func createMiner(t *testing.T) *Miner { // Create Ethash config config := Config{ PendingFeeRecipient: common.HexToAddress("123456789"), - RollupTransactionConditionalBurstRate: types.TransactionConditionalMaxCost, + RollupTransactionConditionalBurstRate: params.TransactionConditionalMaxCost, } // Create chainConfig chainDB := rawdb.NewMemoryDatabase() diff --git a/miner/worker.go b/miner/worker.go index cba42a8e18..04fff138b7 100644 --- a/miner/worker.go +++ b/miner/worker.go @@ -308,11 +308,9 @@ func (miner *Miner) commitTransaction(env *environment, tx *types.Transaction) e // check the conditional if err := env.header.CheckTransactionConditional(conditional); err != nil { - conditional.Rejected.Store(true) return fmt.Errorf("failed header check: %s: %w", err, errTxConditionalInvalid) } if err := env.state.CheckTransactionConditional(conditional); err != nil { - conditional.Rejected.Store(true) return fmt.Errorf("failed state check: %s: %w", err, errTxConditionalInvalid) } } @@ -453,8 +451,11 @@ func (miner *Miner) commitTransactions(env *environment, plainTxs, blobTxs *tran txs.Shift() case errors.Is(err, errTxConditionalInvalid): - // err contains contextual info on the failed conditional + // err contains contextual info on the failed conditional. txConditionalRejectedCounter.Inc(1) + + // we know this tx has a conditional and mark it as rejected so that it can be ejected from the mempool + tx.Conditional().Rejected.Store(true) log.Debug("Skipping account, transaction with failed conditional", "sender", from, "hash", ltx.Hash, "err", err) txs.Pop() diff --git a/params/conditional_tx_params.go b/params/conditional_tx_params.go new file mode 100644 index 0000000000..149339307e --- /dev/null +++ b/params/conditional_tx_params.go @@ -0,0 +1,8 @@ +package params + +const ( + TransactionConditionalMaxCost = 1000 + + TransactionConditionalRejectedErrCode = -32003 + TransactionConditionalCostExceededMaxErrCode = -32005 +) diff --git a/rpc/client_test.go b/rpc/client_test.go index e692f7752d..635281bbd6 100644 --- a/rpc/client_test.go +++ b/rpc/client_test.go @@ -161,7 +161,7 @@ func TestClientBatchRequest(t *testing.T) { Method: "no_such_method", Args: []interface{}{1, 2, 3}, Result: new(int), - Error: &jsonError{Code: -32601, Message: "the method no_such_method does not exist/is not available"}, + Error: &JsonError{Code: -32601, Message: "the method no_such_method does not exist/is not available"}, }, } if !reflect.DeepEqual(batch, wantResult) { diff --git a/rpc/json.go b/rpc/json.go index e932389d17..e1d1fabf6f 100644 --- a/rpc/json.go +++ b/rpc/json.go @@ -64,7 +64,7 @@ type jsonrpcMessage struct { ID json.RawMessage `json:"id,omitempty"` Method string `json:"method,omitempty"` Params json.RawMessage `json:"params,omitempty"` - Error *jsonError `json:"error,omitempty"` + Error *JsonError `json:"error,omitempty"` Result json.RawMessage `json:"result,omitempty"` } @@ -121,7 +121,7 @@ func (msg *jsonrpcMessage) response(result interface{}) *jsonrpcMessage { } func errorMessage(err error) *jsonrpcMessage { - msg := &jsonrpcMessage{Version: vsn, ID: null, Error: &jsonError{ + msg := &jsonrpcMessage{Version: vsn, ID: null, Error: &JsonError{ Code: errcodeDefault, Message: err.Error(), }} @@ -136,24 +136,24 @@ func errorMessage(err error) *jsonrpcMessage { return msg } -type jsonError struct { +type JsonError struct { Code int `json:"code"` Message string `json:"message"` Data interface{} `json:"data,omitempty"` } -func (err *jsonError) Error() string { +func (err *JsonError) Error() string { if err.Message == "" { return fmt.Sprintf("json-rpc error %d", err.Code) } return err.Message } -func (err *jsonError) ErrorCode() int { +func (err *JsonError) ErrorCode() int { return err.Code } -func (err *jsonError) ErrorData() interface{} { +func (err *JsonError) ErrorData() interface{} { return err.Data } From 0134764d94ba408e57875848affe939a1fe20b8f Mon Sep 17 00:00:00 2001 From: Hamdi Allam Date: Fri, 30 Aug 2024 19:31:26 -0400 Subject: [PATCH 04/16] nits --- core/types/transaction_conditional.go | 2 +- eth/backend.go | 7 ++++++- eth/ethconfig/config.go | 2 +- 3 files changed, 8 insertions(+), 3 deletions(-) diff --git a/core/types/transaction_conditional.go b/core/types/transaction_conditional.go index a9020ab9de..6593d5e51c 100644 --- a/core/types/transaction_conditional.go +++ b/core/types/transaction_conditional.go @@ -79,7 +79,7 @@ type TransactionConditional struct { // KnownAccounts represents account prestate conditions KnownAccounts KnownAccounts `json:"knownAccounts"` - // Header state conditionals + // Header state conditionals (inclusive ranges) BlockNumberMin *big.Int `json:"blockNumberMin,omitempty"` BlockNumberMax *big.Int `json:"blockNumberMax,omitempty"` TimestampMin *uint64 `json:"timestampMin,omitempty"` diff --git a/eth/backend.go b/eth/backend.go index 98577869ca..3f685ebadb 100644 --- a/eth/backend.go +++ b/eth/backend.go @@ -377,7 +377,12 @@ func (s *Ethereum) APIs() []rpc.API { // Append any Sequencer APIs as enabled if s.config.RollupSequencerEnableTxConditional { - apis = append(apis, sequencerapi.GetSendRawTxConditionalAPI(s.APIBackend)) + // an effort to detect if running in a non-sequencer mode + if len(s.config.RollupSequencerHTTP) > 0 { + log.Warn("SendRawTransactionConditional enabled when RollupSequencerHTTP is defined. Skipping...") + } else { + apis = append(apis, sequencerapi.GetSendRawTxConditionalAPI(s.APIBackend)) + } } // Append all the local APIs and return diff --git a/eth/ethconfig/config.go b/eth/ethconfig/config.go index b2ac0dcb4a..28849a8519 100644 --- a/eth/ethconfig/config.go +++ b/eth/ethconfig/config.go @@ -184,12 +184,12 @@ type Config struct { ApplySuperchainUpgrades bool `toml:",omitempty"` RollupSequencerHTTP string + RollupSequencerEnableTxConditional bool RollupHistoricalRPC string RollupHistoricalRPCTimeout time.Duration RollupDisableTxPoolGossip bool RollupDisableTxPoolAdmission bool RollupHaltOnIncompatibleProtocolVersion string - RollupSequencerEnableTxConditional bool } // CreateConsensusEngine creates a consensus engine for the given chain config. From 36dbc2d50a346b99bb9b1e10c309134a016e1532 Mon Sep 17 00:00:00 2001 From: Hamdi Allam Date: Thu, 5 Sep 2024 15:14:40 -0400 Subject: [PATCH 05/16] JsonError type alias --- rpc/client_test.go | 2 +- rpc/json.go | 14 ++++++++------ 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/rpc/client_test.go b/rpc/client_test.go index 635281bbd6..e692f7752d 100644 --- a/rpc/client_test.go +++ b/rpc/client_test.go @@ -161,7 +161,7 @@ func TestClientBatchRequest(t *testing.T) { Method: "no_such_method", Args: []interface{}{1, 2, 3}, Result: new(int), - Error: &JsonError{Code: -32601, Message: "the method no_such_method does not exist/is not available"}, + Error: &jsonError{Code: -32601, Message: "the method no_such_method does not exist/is not available"}, }, } if !reflect.DeepEqual(batch, wantResult) { diff --git a/rpc/json.go b/rpc/json.go index e1d1fabf6f..bc8d69f143 100644 --- a/rpc/json.go +++ b/rpc/json.go @@ -41,6 +41,8 @@ const ( var null = json.RawMessage("null") +type JsonError = jsonError + type subscriptionResult struct { ID string `json:"subscription"` Result json.RawMessage `json:"result,omitempty"` @@ -64,7 +66,7 @@ type jsonrpcMessage struct { ID json.RawMessage `json:"id,omitempty"` Method string `json:"method,omitempty"` Params json.RawMessage `json:"params,omitempty"` - Error *JsonError `json:"error,omitempty"` + Error *jsonError `json:"error,omitempty"` Result json.RawMessage `json:"result,omitempty"` } @@ -121,7 +123,7 @@ func (msg *jsonrpcMessage) response(result interface{}) *jsonrpcMessage { } func errorMessage(err error) *jsonrpcMessage { - msg := &jsonrpcMessage{Version: vsn, ID: null, Error: &JsonError{ + msg := &jsonrpcMessage{Version: vsn, ID: null, Error: &jsonError{ Code: errcodeDefault, Message: err.Error(), }} @@ -136,24 +138,24 @@ func errorMessage(err error) *jsonrpcMessage { return msg } -type JsonError struct { +type jsonError struct { Code int `json:"code"` Message string `json:"message"` Data interface{} `json:"data,omitempty"` } -func (err *JsonError) Error() string { +func (err *jsonError) Error() string { if err.Message == "" { return fmt.Sprintf("json-rpc error %d", err.Code) } return err.Message } -func (err *JsonError) ErrorCode() int { +func (err *jsonError) ErrorCode() int { return err.Code } -func (err *JsonError) ErrorData() interface{} { +func (err *jsonError) ErrorData() interface{} { return err.Data } From 658122bdd1484e48eec9819d5a64c475a339a3fe Mon Sep 17 00:00:00 2001 From: Hamdi Allam Date: Thu, 5 Sep 2024 16:26:22 -0400 Subject: [PATCH 06/16] use tx.Time() instead of manual tracking --- core/types/transaction.go | 6 +++--- core/types/transaction_conditional.go | 4 +--- core/types/transaction_conditional_test.go | 6 ------ internal/sequencerapi/api.go | 2 +- miner/worker.go | 2 +- 5 files changed, 6 insertions(+), 14 deletions(-) diff --git a/core/types/transaction.go b/core/types/transaction.go index 25e5e55054..b2e79f2f81 100644 --- a/core/types/transaction.go +++ b/core/types/transaction.go @@ -566,9 +566,9 @@ func (tx *Transaction) WithBlobTxSidecar(sideCar *BlobTxSidecar) *Transaction { return cpy } -// SetTime sets the decoding time of a transaction. This is used by tests to set -// arbitrary times and by persistent transaction pools when loading old txs from -// disk. +// SetTime sets the decoding time of a transaction. Used by the sequencer API to +// determine mempool time spent by conditional txs and by tests to set arbitrary +// times and by persistent transaction pools when loading old txs from disk. func (tx *Transaction) SetTime(t time.Time) { tx.time = t } diff --git a/core/types/transaction_conditional.go b/core/types/transaction_conditional.go index 6593d5e51c..b0a2bac809 100644 --- a/core/types/transaction_conditional.go +++ b/core/types/transaction_conditional.go @@ -5,7 +5,6 @@ import ( "fmt" "math/big" "sync/atomic" - "time" "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/common/hexutil" @@ -86,8 +85,7 @@ type TransactionConditional struct { TimestampMax *uint64 `json:"timestampMax,omitempty"` // Internal fields used for metrics and inclusion tracking - SubmissionTime time.Time `json:"-"` - Rejected *atomic.Bool `json:"-"` + Rejected *atomic.Bool `json:"-"` } // field type overrides for gencodec diff --git a/core/types/transaction_conditional_test.go b/core/types/transaction_conditional_test.go index abb8d8ab30..edcea48244 100644 --- a/core/types/transaction_conditional_test.go +++ b/core/types/transaction_conditional_test.go @@ -219,12 +219,6 @@ func TestTransactionConditionalSerDeser(t *testing.T) { TimestampMax: uint64Ptr(uint64(0xffffff)), }, }, - { - name: "SubmissionTime", - input: `{"submissionTime": 1234}`, - mustFail: true, - expected: TransactionConditional{KnownAccounts: nil}, - }, { name: "Rejected", input: `{"rejected": false}`, diff --git a/internal/sequencerapi/api.go b/internal/sequencerapi/api.go index 6e4f1ea6cb..f089e26d5c 100644 --- a/internal/sequencerapi/api.go +++ b/internal/sequencerapi/api.go @@ -82,7 +82,7 @@ func (s *sendRawTxCond) SendRawTransactionConditional(ctx context.Context, txByt } // Set internal fields - cond.SubmissionTime = time.Now() + tx.SetTime(time.Now()) cond.Rejected = &atomic.Bool{} tx.SetConditional(&cond) diff --git a/miner/worker.go b/miner/worker.go index 04fff138b7..35ae5d0eaa 100644 --- a/miner/worker.go +++ b/miner/worker.go @@ -304,7 +304,7 @@ func (miner *Miner) commitTransaction(env *environment, tx *types.Transaction) e // ditch the reservation as we've checked that we're allowed `cost` units by the limiter miner.conditionalLimiter.ReserveN(now, cost) - txConditionalMinedTimer.UpdateSince(conditional.SubmissionTime) + txConditionalMinedTimer.UpdateSince(tx.Time()) // check the conditional if err := env.header.CheckTransactionConditional(conditional); err != nil { From ddc869f5c4b32195522a6b5ae514e5297d89c915 Mon Sep 17 00:00:00 2001 From: Hamdi Allam Date: Fri, 6 Sep 2024 12:10:32 -0400 Subject: [PATCH 07/16] some feedback --- cmd/utils/flags.go | 4 ++-- core/state/statedb.go | 2 ++ core/types/transaction_conditional.go | 2 ++ miner/miner.go | 15 ++++++++------- params/conditional_tx_params.go | 1 + 5 files changed, 15 insertions(+), 9 deletions(-) diff --git a/cmd/utils/flags.go b/cmd/utils/flags.go index 77f20c7e5c..04acaeb096 100644 --- a/cmd/utils/flags.go +++ b/cmd/utils/flags.go @@ -982,7 +982,7 @@ var ( Category: flags.RollupCategory, Value: false, } - RollupSequencerTxConditionalRateLimitFlag = &cli.Uint64Flag{ + RollupSequencerTxConditionalRateLimitFlag = &cli.IntFlag{ Name: "rollup.sequencertxconditionalratelimit", Usage: "Maximum cost -- storage lookups -- allowed for conditional transactions in a given second", Category: flags.RollupCategory, @@ -1730,7 +1730,7 @@ func setMiner(ctx *cli.Context, cfg *miner.Config) { cfg.RollupComputePendingBlock = ctx.Bool(RollupComputePendingBlock.Name) } if ctx.IsSet(RollupSequencerTxConditionalRateLimitFlag.Name) { - cfg.RollupTransactionConditionalBurstRate = int(ctx.Uint64(RollupSequencerTxConditionalRateLimitFlag.Name)) + cfg.RollupTransactionConditionalBurstRate = ctx.Int(RollupSequencerTxConditionalRateLimitFlag.Name) } } diff --git a/core/state/statedb.go b/core/state/statedb.go index 3dbad5e48f..d06b9ff250 100644 --- a/core/state/statedb.go +++ b/core/state/statedb.go @@ -431,6 +431,8 @@ func (s *StateDB) HasSelfDestructed(addr common.Address) bool { // ensure no state changes occur while this check is executed. func (s *StateDB) CheckTransactionConditional(cond *types.TransactionConditional) error { cost := cond.Cost() + + // The max cost as an inclusive limit. if cost > params.TransactionConditionalMaxCost { return fmt.Errorf("conditional cost, %d, exceeded max: %d", cost, params.TransactionConditionalMaxCost) } diff --git a/core/types/transaction_conditional.go b/core/types/transaction_conditional.go index b0a2bac809..e4702f48e5 100644 --- a/core/types/transaction_conditional.go +++ b/core/types/transaction_conditional.go @@ -111,6 +111,8 @@ func (cond *TransactionConditional) Validate() error { func (cond *TransactionConditional) Cost() int { cost := 0 for _, account := range cond.KnownAccounts { + // default cost to handle empty accounts + cost += 1 if _, isRoot := account.Root(); isRoot { cost += 1 } diff --git a/miner/miner.go b/miner/miner.go index ffb360065d..8fade06caf 100644 --- a/miner/miner.go +++ b/miner/miner.go @@ -95,13 +95,14 @@ type Miner struct { // New creates a new miner with provided config. func New(eth Backend, config Config, engine consensus.Engine) *Miner { return &Miner{ - backend: eth, - config: &config, - chainConfig: eth.BlockChain().Config(), - engine: engine, - txpool: eth.TxPool(), - chain: eth.BlockChain(), - pending: &pending{}, + backend: eth, + config: &config, + chainConfig: eth.BlockChain().Config(), + engine: engine, + txpool: eth.TxPool(), + chain: eth.BlockChain(), + pending: &pending{}, + // setup the rate limit imposed on conditional transactions when block building conditionalLimiter: rate.NewLimiter(params.TransactionConditionalMaxCost, config.RollupTransactionConditionalBurstRate), } } diff --git a/params/conditional_tx_params.go b/params/conditional_tx_params.go index 149339307e..a3d27a9837 100644 --- a/params/conditional_tx_params.go +++ b/params/conditional_tx_params.go @@ -1,6 +1,7 @@ package params const ( + // An inclusive limit on the max cost for the conditional attached to a tx TransactionConditionalMaxCost = 1000 TransactionConditionalRejectedErrCode = -32003 From cc96057e46f99d0b0c17b7ed2d7bb3a05d027767 Mon Sep 17 00:00:00 2001 From: Hamdi Allam Date: Fri, 6 Sep 2024 12:34:51 -0400 Subject: [PATCH 08/16] lift rejected into transaction --- core/txpool/legacypool/legacypool.go | 17 +++++++---------- core/txpool/legacypool/legacypool_test.go | 8 ++------ core/types/transaction.go | 21 +++++++++++++++------ core/types/transaction_conditional.go | 4 ---- core/types/transaction_conditional_test.go | 20 +++++++++++--------- internal/sequencerapi/api.go | 3 --- miner/miner_test.go | 10 +++------- miner/worker.go | 6 ++++-- 8 files changed, 42 insertions(+), 47 deletions(-) diff --git a/core/txpool/legacypool/legacypool.go b/core/txpool/legacypool/legacypool.go index 84b4924590..92466dc579 100644 --- a/core/txpool/legacypool/legacypool.go +++ b/core/txpool/legacypool/legacypool.go @@ -1733,16 +1733,13 @@ func (pool *LegacyPool) demoteUnexecutables() { } pendingNofundsMeter.Mark(int64(len(drops))) - // Drop all conditional transactions that was rejected by the miner - conditionalDrops := list.txs.Filter(func(tx *types.Transaction) bool { - if conditional := tx.Conditional(); conditional != nil { - return conditional.Rejected.Load() - } - return false + // Drop all transactions that were rejected by the miner + rejectedDrops := list.txs.Filter(func(tx *types.Transaction) bool { + return tx.Rejected() }) - for _, tx := range conditionalDrops { + for _, tx := range rejectedDrops { hash := tx.Hash() - log.Trace("Removed conditional transaction rejected by the miner", "hash", hash) + log.Trace("Removed rejected transaction by the miner", "hash", hash) pool.all.Remove(hash) } @@ -1753,9 +1750,9 @@ func (pool *LegacyPool) demoteUnexecutables() { // Internal shuffle shouldn't touch the lookup set. pool.enqueueTx(hash, tx, false, false) } - pendingGauge.Dec(int64(len(olds) + len(drops) + len(invalids) + len(conditionalDrops))) + pendingGauge.Dec(int64(len(olds) + len(drops) + len(invalids) + len(rejectedDrops))) if pool.locals.contains(addr) { - localGauge.Dec(int64(len(olds) + len(drops) + len(invalids) + len(conditionalDrops))) + localGauge.Dec(int64(len(olds) + len(drops) + len(invalids) + len(rejectedDrops))) } // If there's a gap in front, alert (should never happen) and postpone all transactions if list.Len() > 0 && list.txs.Get(nonce) == nil { diff --git a/core/txpool/legacypool/legacypool_test.go b/core/txpool/legacypool/legacypool_test.go index ac6af8c420..0240d93e14 100644 --- a/core/txpool/legacypool/legacypool_test.go +++ b/core/txpool/legacypool/legacypool_test.go @@ -692,7 +692,7 @@ func TestDropping(t *testing.T) { // Tests that transactions marked as reject (by the miner in practice) // are removed from the pool -func TestRejectedConditionalDropping(t *testing.T) { +func TestRejectedDropping(t *testing.T) { t.Parallel() pool, key := setupPool() @@ -703,10 +703,6 @@ func TestRejectedConditionalDropping(t *testing.T) { // create some txs. tx0 has a conditional tx0, tx1 := transaction(0, 100, key), transaction(1, 200, key) - tx0.SetConditional(&types.TransactionConditional{ - BlockNumberMin: big.NewInt(1), - Rejected: &atomic.Bool{}, - }) pool.all.Add(tx0, false) pool.all.Add(tx1, false) @@ -720,7 +716,7 @@ func TestRejectedConditionalDropping(t *testing.T) { } // tx0 conditional is marked as rejected and should be removed - tx0.Conditional().Rejected.Store(true) + tx0.SetRejected() <-pool.requestReset(nil, nil) if pool.all.Count() != 1 { t.Errorf("total transaction mismatch: have %d, want %d", pool.all.Count(), 1) diff --git a/core/types/transaction.go b/core/types/transaction.go index b2e79f2f81..f685bb46cc 100644 --- a/core/types/transaction.go +++ b/core/types/transaction.go @@ -66,7 +66,10 @@ type Transaction struct { rollupCostData atomic.Value // optional preconditions for inclusion - conditional atomic.Value + conditional atomic.Pointer[TransactionConditional] + + // an indicator if this transaction is rejected during block building + rejected atomic.Bool } // NewTx creates a new transaction. @@ -392,11 +395,7 @@ func (tx *Transaction) RollupCostData() RollupCostData { // Conditional returns the conditional attached to the transaction func (tx *Transaction) Conditional() *TransactionConditional { - v := tx.conditional.Load() - if v == nil { - return nil - } - return v.(*TransactionConditional) + return tx.conditional.Load() } // SetConditional attaches a conditional to the transaction @@ -404,6 +403,16 @@ func (tx *Transaction) SetConditional(cond *TransactionConditional) { tx.conditional.Store(cond) } +// Rejected will mark this transaction as rejected. +func (tx *Transaction) SetRejected() { + tx.rejected.Store(true) +} + +// Rejected returns the rejected status of this tx +func (tx *Transaction) Rejected() bool { + return tx.rejected.Load() +} + // RawSignatureValues returns the V, R, S signature values of the transaction. // The return values should not be modified by the caller. // The return values may be nil or zero, if the transaction is unsigned. diff --git a/core/types/transaction_conditional.go b/core/types/transaction_conditional.go index e4702f48e5..4b58d78df8 100644 --- a/core/types/transaction_conditional.go +++ b/core/types/transaction_conditional.go @@ -4,7 +4,6 @@ import ( "encoding/json" "fmt" "math/big" - "sync/atomic" "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/common/hexutil" @@ -83,9 +82,6 @@ type TransactionConditional struct { BlockNumberMax *big.Int `json:"blockNumberMax,omitempty"` TimestampMin *uint64 `json:"timestampMin,omitempty"` TimestampMax *uint64 `json:"timestampMax,omitempty"` - - // Internal fields used for metrics and inclusion tracking - Rejected *atomic.Bool `json:"-"` } // field type overrides for gencodec diff --git a/core/types/transaction_conditional_test.go b/core/types/transaction_conditional_test.go index edcea48244..f7e3bb7ef1 100644 --- a/core/types/transaction_conditional_test.go +++ b/core/types/transaction_conditional_test.go @@ -34,13 +34,21 @@ func TestTransactionConditionalCost(t *testing.T) { cond: TransactionConditional{TimestampMin: uint64Ptr(0), TimestampMax: uint64Ptr(5)}, cost: 1, }, + { + name: "default cost per account", + cond: TransactionConditional{KnownAccounts: map[common.Address]KnownAccount{ + common.Address{19: 1}: KnownAccount{}, + common.Address{19: 2}: KnownAccount{}, + }}, + cost: 2, + }, { name: "storage root lookup", cond: TransactionConditional{KnownAccounts: map[common.Address]KnownAccount{ common.Address{19: 1}: KnownAccount{ StorageRoot: &EmptyRootHash, }}}, - cost: 1, + cost: 2, }, { name: "cost per storage slot lookup", @@ -51,7 +59,7 @@ func TestTransactionConditionalCost(t *testing.T) { common.Hash{31: 1}: common.Hash{31: 1}, }, }}}, - cost: 2, + cost: 3, }, { name: "cost summed together", @@ -66,7 +74,7 @@ func TestTransactionConditionalCost(t *testing.T) { common.Hash{31: 1}: common.Hash{31: 1}, }, }}}, - cost: 5, + cost: 7, }, } @@ -219,12 +227,6 @@ func TestTransactionConditionalSerDeser(t *testing.T) { TimestampMax: uint64Ptr(uint64(0xffffff)), }, }, - { - name: "Rejected", - input: `{"rejected": false}`, - mustFail: true, - expected: TransactionConditional{KnownAccounts: nil}, - }, { name: "UnknownField", input: `{"foobarbaz": 1234}`, diff --git a/internal/sequencerapi/api.go b/internal/sequencerapi/api.go index f089e26d5c..6c1dc4dcd5 100644 --- a/internal/sequencerapi/api.go +++ b/internal/sequencerapi/api.go @@ -3,7 +3,6 @@ package sequencerapi import ( "context" "fmt" - "sync/atomic" "time" "github.com/ethereum/go-ethereum/common" @@ -83,8 +82,6 @@ func (s *sendRawTxCond) SendRawTransactionConditional(ctx context.Context, txByt // Set internal fields tx.SetTime(time.Now()) - cond.Rejected = &atomic.Bool{} - tx.SetConditional(&cond) sendRawTxConditionalAcceptedCounter.Inc(1) diff --git a/miner/miner_test.go b/miner/miner_test.go index 4a7ada4f47..0a0bdaf792 100644 --- a/miner/miner_test.go +++ b/miner/miner_test.go @@ -20,7 +20,6 @@ package miner import ( "math/big" "sync" - "sync/atomic" "testing" "time" @@ -187,10 +186,7 @@ func TestRejectedConditionalTx(t *testing.T) { Gas: params.TxGas, GasPrice: big.NewInt(params.InitialBaseFee), }) - tx.SetConditional(&types.TransactionConditional{ - TimestampMax: uint64Ptr(timestamp - 1), - Rejected: &atomic.Bool{}, - }) + tx.SetConditional(&types.TransactionConditional{TimestampMax: uint64Ptr(timestamp - 1)}) // 1 pending tx miner.txpool.Add(types.Transactions{tx}, true, false) @@ -211,8 +207,8 @@ func TestRejectedConditionalTx(t *testing.T) { t.Fatalf("block should be empty") } - // conditional is rejected - if !tx.Conditional().Rejected.Load() { + // tx is rejected + if !tx.Rejected() { t.Fatalf("conditional tx is not marked as rejected") } diff --git a/miner/worker.go b/miner/worker.go index 35ae5d0eaa..a7482de9ee 100644 --- a/miner/worker.go +++ b/miner/worker.go @@ -454,14 +454,16 @@ func (miner *Miner) commitTransactions(env *environment, plainTxs, blobTxs *tran // err contains contextual info on the failed conditional. txConditionalRejectedCounter.Inc(1) - // we know this tx has a conditional and mark it as rejected so that it can be ejected from the mempool - tx.Conditional().Rejected.Store(true) + // mark as rejected so that it can be ejected from the mempool + tx.SetRejected() log.Debug("Skipping account, transaction with failed conditional", "sender", from, "hash", ltx.Hash, "err", err) txs.Pop() case errors.Is(err, errTxConditionalRateLimited): // err contains contextual info of the cost and limiter tokens available txConditionalRejectedCounter.Inc(1) + + // note: we do not mark the tx as rejected as it is still eligible for inclusion at a later time log.Debug("Skipping account, transaction with conditional rate limited", "sender", from, "hash", ltx.Hash, "err", err) txs.Pop() From eec006a340acf7872032466263956c0cf5b33792 Mon Sep 17 00:00:00 2001 From: Hamdi Allam Date: Fri, 6 Sep 2024 12:52:39 -0400 Subject: [PATCH 09/16] support for forwarding sendRawTransactionConditional to seqRPC --- eth/backend.go | 7 +------ internal/sequencerapi/api.go | 33 ++++++++++++++++++++------------- 2 files changed, 21 insertions(+), 19 deletions(-) diff --git a/eth/backend.go b/eth/backend.go index 3f685ebadb..ebe2dfb9ba 100644 --- a/eth/backend.go +++ b/eth/backend.go @@ -377,12 +377,7 @@ func (s *Ethereum) APIs() []rpc.API { // Append any Sequencer APIs as enabled if s.config.RollupSequencerEnableTxConditional { - // an effort to detect if running in a non-sequencer mode - if len(s.config.RollupSequencerHTTP) > 0 { - log.Warn("SendRawTransactionConditional enabled when RollupSequencerHTTP is defined. Skipping...") - } else { - apis = append(apis, sequencerapi.GetSendRawTxConditionalAPI(s.APIBackend)) - } + apis = append(apis, sequencerapi.GetSendRawTxConditionalAPI(s.APIBackend, s.seqRPCService)) } // Append all the local APIs and return diff --git a/internal/sequencerapi/api.go b/internal/sequencerapi/api.go index 6c1dc4dcd5..c4bf7a4843 100644 --- a/internal/sequencerapi/api.go +++ b/internal/sequencerapi/api.go @@ -21,13 +21,14 @@ var ( ) type sendRawTxCond struct { - b ethapi.Backend + b ethapi.Backend + seqRPC *rpc.Client } -func GetSendRawTxConditionalAPI(b ethapi.Backend) rpc.API { +func GetSendRawTxConditionalAPI(b ethapi.Backend, seqRPC *rpc.Client) rpc.API { return rpc.API{ Namespace: "eth", - Service: &sendRawTxCond{b}, + Service: &sendRawTxCond{b, seqRPC}, } } @@ -75,15 +76,21 @@ func (s *sendRawTxCond) SendRawTransactionConditional(ctx context.Context, txByt } } - tx := new(types.Transaction) - if err := tx.UnmarshalBinary(txBytes); err != nil { - return common.Hash{}, err - } - - // Set internal fields - tx.SetTime(time.Now()) - tx.SetConditional(&cond) - sendRawTxConditionalAcceptedCounter.Inc(1) + // forward if seqRPC is set, otherwise submit the tx + if s.seqRPC != nil { + var hash common.Hash + err := s.seqRPC.CallContext(ctx, &hash, "eth_sendRawTransactionConditional", txBytes, cond) + return hash, err + } else { + tx := new(types.Transaction) + if err := tx.UnmarshalBinary(txBytes); err != nil { + return common.Hash{}, err + } - return ethapi.SubmitTransaction(ctx, s.b, tx) + // Set out-of-consensus internal tx fields + tx.SetTime(time.Now()) + tx.SetConditional(&cond) + sendRawTxConditionalAcceptedCounter.Inc(1) + return ethapi.SubmitTransaction(ctx, s.b, tx) + } } From de987be012da0b2433eeb6c999f52e2901277134 Mon Sep 17 00:00:00 2001 From: Hamdi Allam Date: Mon, 9 Sep 2024 12:36:22 +0200 Subject: [PATCH 10/16] feedback on limiter reservation. log skipped conditional txs at warn to start --- cmd/utils/flags.go | 6 +++--- internal/sequencerapi/api.go | 1 + miner/worker.go | 10 +++++----- 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/cmd/utils/flags.go b/cmd/utils/flags.go index 04acaeb096..026d024fbe 100644 --- a/cmd/utils/flags.go +++ b/cmd/utils/flags.go @@ -1729,9 +1729,9 @@ func setMiner(ctx *cli.Context, cfg *miner.Config) { if ctx.IsSet(RollupComputePendingBlock.Name) { cfg.RollupComputePendingBlock = ctx.Bool(RollupComputePendingBlock.Name) } - if ctx.IsSet(RollupSequencerTxConditionalRateLimitFlag.Name) { - cfg.RollupTransactionConditionalBurstRate = ctx.Int(RollupSequencerTxConditionalRateLimitFlag.Name) - } + + // This flag has a default rate limit so always set + cfg.RollupTransactionConditionalBurstRate = ctx.Int(RollupSequencerTxConditionalRateLimitFlag.Name) } func setRequiredBlocks(ctx *cli.Context, cfg *ethconfig.Config) { diff --git a/internal/sequencerapi/api.go b/internal/sequencerapi/api.go index c4bf7a4843..ca2fdeb8f1 100644 --- a/internal/sequencerapi/api.go +++ b/internal/sequencerapi/api.go @@ -90,6 +90,7 @@ func (s *sendRawTxCond) SendRawTransactionConditional(ctx context.Context, txByt // Set out-of-consensus internal tx fields tx.SetTime(time.Now()) tx.SetConditional(&cond) + sendRawTxConditionalAcceptedCounter.Inc(1) return ethapi.SubmitTransaction(ctx, s.b, tx) } diff --git a/miner/worker.go b/miner/worker.go index a7482de9ee..23f9f8310d 100644 --- a/miner/worker.go +++ b/miner/worker.go @@ -298,12 +298,12 @@ func (miner *Miner) commitTransaction(env *environment, tx *types.Transaction) e // If a conditional is set, check prior to applying if conditional := tx.Conditional(); conditional != nil { now, cost := time.Now(), conditional.Cost() - if !miner.conditionalLimiter.AllowN(now, cost) { + reservation := miner.conditionalLimiter.ReserveN(now, cost) + if !reservation.OK() { + reservation.Cancel() return fmt.Errorf("exceeded rate limit: cost %d, tokens %f: %w", cost, miner.conditionalLimiter.Tokens(), errTxConditionalRateLimited) } - // ditch the reservation as we've checked that we're allowed `cost` units by the limiter - miner.conditionalLimiter.ReserveN(now, cost) txConditionalMinedTimer.UpdateSince(tx.Time()) // check the conditional @@ -456,7 +456,7 @@ func (miner *Miner) commitTransactions(env *environment, plainTxs, blobTxs *tran // mark as rejected so that it can be ejected from the mempool tx.SetRejected() - log.Debug("Skipping account, transaction with failed conditional", "sender", from, "hash", ltx.Hash, "err", err) + log.Warn("Skipping account, transaction with failed conditional", "sender", from, "hash", ltx.Hash, "err", err) txs.Pop() case errors.Is(err, errTxConditionalRateLimited): @@ -464,7 +464,7 @@ func (miner *Miner) commitTransactions(env *environment, plainTxs, blobTxs *tran txConditionalRejectedCounter.Inc(1) // note: we do not mark the tx as rejected as it is still eligible for inclusion at a later time - log.Debug("Skipping account, transaction with conditional rate limited", "sender", from, "hash", ltx.Hash, "err", err) + log.Warn("Skipping account, transaction with conditional rate limited", "sender", from, "hash", ltx.Hash, "err", err) txs.Pop() case errors.Is(err, nil): From 79fc6fe5d134621644dedbe62f7b35468ee76000 Mon Sep 17 00:00:00 2001 From: Hamdi Allam Date: Mon, 9 Sep 2024 20:32:13 +0200 Subject: [PATCH 11/16] update forkdiff --- core/state/statedb.go | 2 +- fork.yaml | 11 +++++++++-- internal/sequencerapi/api.go | 3 +++ 3 files changed, 13 insertions(+), 3 deletions(-) diff --git a/core/state/statedb.go b/core/state/statedb.go index d06b9ff250..160774e441 100644 --- a/core/state/statedb.go +++ b/core/state/statedb.go @@ -432,7 +432,7 @@ func (s *StateDB) HasSelfDestructed(addr common.Address) bool { func (s *StateDB) CheckTransactionConditional(cond *types.TransactionConditional) error { cost := cond.Cost() - // The max cost as an inclusive limit. + // The max cost is an inclusive limit. if cost > params.TransactionConditionalMaxCost { return fmt.Errorf("conditional cost, %d, exceeded max: %d", cost, params.TransactionConditionalMaxCost) } diff --git a/fork.yaml b/fork.yaml index 5b52d37352..1f8f790bde 100644 --- a/fork.yaml +++ b/fork.yaml @@ -284,16 +284,23 @@ def: - "cmd/geth/main.go" - "cmd/utils/flags.go" - "core/state/statedb.go" + - "core/state/statedb_test.go" - "core/types/block.go" + - "core/types/block_test.go" - "core/types/transaction.go" - "core/types/transaction_conditional.go" + - "core/types/transaction_conditional.go" + - "core/types/transaction_conditional_test.go" + - "core/types/gen_transaction_conditional_json.go" - "eth/backend.go" - "eth/ethconfig/config.go" - "eth/protocols/eth/broadcast.go" - "internal/sequencerapi/api.go" - - "internal/sequencerapi/errors.go" - - "miner/worker.go" - "miner/miner.go" + - "miner/miner_test.go" + - "miner/worker.go" + - "params/conditional_tx_params.go" + - "rpc/json.go" - title: "Geth extras" description: Extend the tools available in geth to improve external testing and tooling. sub: diff --git a/internal/sequencerapi/api.go b/internal/sequencerapi/api.go index ca2fdeb8f1..d575009956 100644 --- a/internal/sequencerapi/api.go +++ b/internal/sequencerapi/api.go @@ -91,6 +91,9 @@ func (s *sendRawTxCond) SendRawTransactionConditional(ctx context.Context, txByt tx.SetTime(time.Now()) tx.SetConditional(&cond) + // `SubmitTransaction` which forwards to `b.SendTx` also checks if its internal `seqRPC` client is + // set. Since both of these client are constructed if `RollupSequencerHTTP` is set, the above block + // ensures that we're only adding to the txpool in this block. sendRawTxConditionalAcceptedCounter.Inc(1) return ethapi.SubmitTransaction(ctx, s.b, tx) } From 3f2b4c91e8a15025b04bb77e56d2702ddc63ef57 Mon Sep 17 00:00:00 2001 From: Hamdi Allam Date: Mon, 9 Sep 2024 20:58:51 +0200 Subject: [PATCH 12/16] limiter reservation delay needs to be checked --- miner/worker.go | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/miner/worker.go b/miner/worker.go index 23f9f8310d..b35d03558d 100644 --- a/miner/worker.go +++ b/miner/worker.go @@ -298,10 +298,14 @@ func (miner *Miner) commitTransaction(env *environment, tx *types.Transaction) e // If a conditional is set, check prior to applying if conditional := tx.Conditional(); conditional != nil { now, cost := time.Now(), conditional.Cost() - reservation := miner.conditionalLimiter.ReserveN(now, cost) - if !reservation.OK() { - reservation.Cancel() - return fmt.Errorf("exceeded rate limit: cost %d, tokens %f: %w", cost, miner.conditionalLimiter.Tokens(), errTxConditionalRateLimited) + res := miner.conditionalLimiter.ReserveN(now, cost) + if !res.OK() { + res.Cancel() + return fmt.Errorf("exceeded rate limiter burst: cost %d, burst %d: %w", cost, miner.conditionalLimiter.Burst(), errTxConditionalInvalid) + } + if res.Delay() > 0 { + res.Cancel() + return fmt.Errorf("exceeded rate limit: cost %d, available tokens %f: %w", cost, miner.conditionalLimiter.Tokens(), errTxConditionalRateLimited) } txConditionalMinedTimer.UpdateSince(tx.Time()) From 9987de30874a19d3de334b7253c9a4fa1d213026 Mon Sep 17 00:00:00 2001 From: Hamdi Allam Date: Tue, 10 Sep 2024 17:28:38 +0200 Subject: [PATCH 13/16] feedback --- core/types/transaction_conditional.go | 2 +- internal/sequencerapi/api.go | 12 +++++++++--- miner/worker.go | 1 - 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/core/types/transaction_conditional.go b/core/types/transaction_conditional.go index 4b58d78df8..90a77c7d81 100644 --- a/core/types/transaction_conditional.go +++ b/core/types/transaction_conditional.go @@ -20,7 +20,7 @@ type KnownAccounts map[common.Address]KnownAccount // then the user prefers their transaction to only be included if the // particular storage slot values from state match. type KnownAccount struct { - StorageRoot *common.Hash `rlp:"nil"` + StorageRoot *common.Hash StorageSlots map[common.Hash]common.Hash } diff --git a/internal/sequencerapi/api.go b/internal/sequencerapi/api.go index d575009956..232bd47f1d 100644 --- a/internal/sequencerapi/api.go +++ b/internal/sequencerapi/api.go @@ -52,7 +52,7 @@ func (s *sendRawTxCond) SendRawTransactionConditional(ctx context.Context, txByt } } - header, err := s.b.HeaderByNumber(context.Background(), rpc.LatestBlockNumber) + state, header, err := s.b.StateAndHeaderByNumber(context.Background(), rpc.LatestBlockNumber) if err != nil { return common.Hash{}, err } @@ -62,6 +62,12 @@ func (s *sendRawTxCond) SendRawTransactionConditional(ctx context.Context, txByt Code: params.TransactionConditionalRejectedErrCode, } } + if err := state.CheckTransactionConditional(&cond); err != nil { + return common.Hash{}, &rpc.JsonError{ + Message: fmt.Sprintf("failed state check: %s", err), + Code: params.TransactionConditionalRejectedErrCode, + } + } // State is checked against an older block to remove the MEV incentive for this endpoint compared with sendRawTransaction parentBlock := rpc.BlockNumberOrHash{BlockHash: &header.ParentHash} @@ -92,8 +98,8 @@ func (s *sendRawTxCond) SendRawTransactionConditional(ctx context.Context, txByt tx.SetConditional(&cond) // `SubmitTransaction` which forwards to `b.SendTx` also checks if its internal `seqRPC` client is - // set. Since both of these client are constructed if `RollupSequencerHTTP` is set, the above block - // ensures that we're only adding to the txpool in this block. + // set. Since both of these client are constructed when `RollupSequencerHTTP` is supplied, the above + // block ensures that we're only adding to the txpool for this node. sendRawTxConditionalAcceptedCounter.Inc(1) return ethapi.SubmitTransaction(ctx, s.b, tx) } diff --git a/miner/worker.go b/miner/worker.go index b35d03558d..d17199b89f 100644 --- a/miner/worker.go +++ b/miner/worker.go @@ -300,7 +300,6 @@ func (miner *Miner) commitTransaction(env *environment, tx *types.Transaction) e now, cost := time.Now(), conditional.Cost() res := miner.conditionalLimiter.ReserveN(now, cost) if !res.OK() { - res.Cancel() return fmt.Errorf("exceeded rate limiter burst: cost %d, burst %d: %w", cost, miner.conditionalLimiter.Burst(), errTxConditionalInvalid) } if res.Delay() > 0 { From ba020050d8a63f4a375372f60fb6df0103d7ab8f Mon Sep 17 00:00:00 2001 From: Hamdi Allam Date: Tue, 10 Sep 2024 17:48:28 +0200 Subject: [PATCH 14/16] feedback on limit. lol sorry for the originally taking the commit! --- cmd/utils/flags.go | 2 +- miner/miner.go | 4 ++-- miner/miner_test.go | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/cmd/utils/flags.go b/cmd/utils/flags.go index 026d024fbe..74de8611e1 100644 --- a/cmd/utils/flags.go +++ b/cmd/utils/flags.go @@ -1731,7 +1731,7 @@ func setMiner(ctx *cli.Context, cfg *miner.Config) { } // This flag has a default rate limit so always set - cfg.RollupTransactionConditionalBurstRate = ctx.Int(RollupSequencerTxConditionalRateLimitFlag.Name) + cfg.RollupTransactionConditionalRateLimit = ctx.Int(RollupSequencerTxConditionalRateLimitFlag.Name) } func setRequiredBlocks(ctx *cli.Context, cfg *ethconfig.Config) { diff --git a/miner/miner.go b/miner/miner.go index 8fade06caf..2e14bb10fe 100644 --- a/miner/miner.go +++ b/miner/miner.go @@ -57,7 +57,7 @@ type Config struct { Recommit time.Duration // The time interval for miner to re-create mining work. RollupComputePendingBlock bool // Compute the pending block from tx-pool, instead of copying the latest-block - RollupTransactionConditionalBurstRate int // Total number of conditional cost units allowed in a second + RollupTransactionConditionalRateLimit int // Total number of conditional cost units allowed in a second EffectiveGasCeil uint64 // if non-zero, a gas ceiling to apply independent of the header's gaslimit value } @@ -103,7 +103,7 @@ func New(eth Backend, config Config, engine consensus.Engine) *Miner { chain: eth.BlockChain(), pending: &pending{}, // setup the rate limit imposed on conditional transactions when block building - conditionalLimiter: rate.NewLimiter(params.TransactionConditionalMaxCost, config.RollupTransactionConditionalBurstRate), + conditionalLimiter: rate.NewLimiter(rate.Limit(config.RollupTransactionConditionalRateLimit), params.TransactionConditionalMaxCost), } } diff --git a/miner/miner_test.go b/miner/miner_test.go index 0a0bdaf792..88a9c58b02 100644 --- a/miner/miner_test.go +++ b/miner/miner_test.go @@ -142,7 +142,7 @@ func createMiner(t *testing.T) *Miner { // Create Ethash config config := Config{ PendingFeeRecipient: common.HexToAddress("123456789"), - RollupTransactionConditionalBurstRate: params.TransactionConditionalMaxCost, + RollupTransactionConditionalRateLimit: params.TransactionConditionalMaxCost, } // Create chainConfig chainDB := rawdb.NewMemoryDatabase() From 4612601f79970e9b74b1932a14d4af82e9493eed Mon Sep 17 00:00:00 2001 From: Hamdi Allam Date: Wed, 11 Sep 2024 19:28:47 +0200 Subject: [PATCH 15/16] feedback --- core/txpool/legacypool/legacypool.go | 2 +- internal/ethapi/api.go | 5 +++++ internal/sequencerapi/api.go | 24 +++++++++++++++++------- 3 files changed, 23 insertions(+), 8 deletions(-) diff --git a/core/txpool/legacypool/legacypool.go b/core/txpool/legacypool/legacypool.go index 92466dc579..5b43f97415 100644 --- a/core/txpool/legacypool/legacypool.go +++ b/core/txpool/legacypool/legacypool.go @@ -1739,8 +1739,8 @@ func (pool *LegacyPool) demoteUnexecutables() { }) for _, tx := range rejectedDrops { hash := tx.Hash() - log.Trace("Removed rejected transaction by the miner", "hash", hash) pool.all.Remove(hash) + log.Trace("Removed rejected transaction", "hash", hash) } for _, tx := range invalids { diff --git a/internal/ethapi/api.go b/internal/ethapi/api.go index f6d8cef15f..fa652d0cdd 100644 --- a/internal/ethapi/api.go +++ b/internal/ethapi/api.go @@ -2415,3 +2415,8 @@ func checkTxFee(gasPrice *big.Int, gas uint64, cap float64) error { } return nil } + +// CheckTxFee exports a helper function used to check whether the fee is reasonable +func CheckTxFee(gasPrice *big.Int, gas uint64, cap float64) error { + return checkTxFee(gasPrice, gas, cap) +} diff --git a/internal/sequencerapi/api.go b/internal/sequencerapi/api.go index 232bd47f1d..7d262590ac 100644 --- a/internal/sequencerapi/api.go +++ b/internal/sequencerapi/api.go @@ -2,6 +2,7 @@ package sequencerapi import ( "context" + "errors" "fmt" "time" @@ -52,7 +53,7 @@ func (s *sendRawTxCond) SendRawTransactionConditional(ctx context.Context, txByt } } - state, header, err := s.b.StateAndHeaderByNumber(context.Background(), rpc.LatestBlockNumber) + state, header, err := s.b.StateAndHeaderByNumber(ctx, rpc.LatestBlockNumber) if err != nil { return common.Hash{}, err } @@ -71,7 +72,7 @@ func (s *sendRawTxCond) SendRawTransactionConditional(ctx context.Context, txByt // State is checked against an older block to remove the MEV incentive for this endpoint compared with sendRawTransaction parentBlock := rpc.BlockNumberOrHash{BlockHash: &header.ParentHash} - parentState, _, err := s.b.StateAndHeaderByNumberOrHash(context.Background(), parentBlock) + parentState, _, err := s.b.StateAndHeaderByNumberOrHash(ctx, parentBlock) if err != nil { return common.Hash{}, err } @@ -82,17 +83,26 @@ func (s *sendRawTxCond) SendRawTransactionConditional(ctx context.Context, txByt } } + tx := new(types.Transaction) + if err := tx.UnmarshalBinary(txBytes); err != nil { + return common.Hash{}, err + } + // forward if seqRPC is set, otherwise submit the tx if s.seqRPC != nil { + // Some precondition checks done by `ethapi.SubmitTransaction` that are good to also check here + if err := ethapi.CheckTxFee(tx.GasPrice(), tx.Gas(), s.b.RPCTxFeeCap()); err != nil { + return common.Hash{}, err + } + if !s.b.UnprotectedAllowed() && !tx.Protected() { + // Ensure only eip155 signed transactions are submitted if EIP155Required is set. + return common.Hash{}, errors.New("only replay-protected (EIP-155) transactions allowed over RPC") + } + var hash common.Hash err := s.seqRPC.CallContext(ctx, &hash, "eth_sendRawTransactionConditional", txBytes, cond) return hash, err } else { - tx := new(types.Transaction) - if err := tx.UnmarshalBinary(txBytes); err != nil { - return common.Hash{}, err - } - // Set out-of-consensus internal tx fields tx.SetTime(time.Now()) tx.SetConditional(&cond) From fcae200dd8694d62038b0df6f70ce3da38992f21 Mon Sep 17 00:00:00 2001 From: Hamdi Allam Date: Thu, 12 Sep 2024 01:26:48 +0200 Subject: [PATCH 16/16] small log on enabling sendRawTransactionConditional --- eth/backend.go | 1 + 1 file changed, 1 insertion(+) diff --git a/eth/backend.go b/eth/backend.go index ebe2dfb9ba..d0dd93ec4a 100644 --- a/eth/backend.go +++ b/eth/backend.go @@ -377,6 +377,7 @@ func (s *Ethereum) APIs() []rpc.API { // Append any Sequencer APIs as enabled if s.config.RollupSequencerEnableTxConditional { + log.Info("Enabling eth_sendRawTransactionConditional endpoint support") apis = append(apis, sequencerapi.GetSendRawTxConditionalAPI(s.APIBackend, s.seqRPCService)) }