Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

core/txpool/legacypool: add support for SetCode transactions #31073

Merged
merged 15 commits into from
Feb 11, 2025
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions core/txpool/blobpool/blobpool_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@ func (bc *testBlockChain) CurrentBlock() *types.Header {
GasLimit: gasLimit,
BaseFee: baseFee,
ExcessBlobGas: &excessBlobGas,
Difficulty: common.Big0,
}
}

Expand Down Expand Up @@ -1565,8 +1566,9 @@ func TestAdd(t *testing.T) {
if tt.block != nil {
// Fake a header for the new set of transactions
header := &types.Header{
Number: big.NewInt(int64(chain.CurrentBlock().Number.Uint64() + 1)),
BaseFee: chain.CurrentBlock().BaseFee, // invalid, but nothing checks it, yolo
Number: big.NewInt(int64(chain.CurrentBlock().Number.Uint64() + 1)),
Difficulty: common.Big0,
BaseFee: chain.CurrentBlock().BaseFee, // invalid, but nothing checks it, yolo
}
// Inject the fake block into the chain
txs := make([]*types.Transaction, len(tt.block))
Expand Down
5 changes: 5 additions & 0 deletions core/txpool/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,4 +60,9 @@ var (
// input transaction of non-blob type when a blob transaction from this sender
// remains pending (and vice-versa).
ErrAlreadyReserved = errors.New("address already reserved")

// ErrAuthorityReserved is returned if a transaction has an authorization
// signed by an address which already has in-flight transactions known to the
// pool.
ErrAuthorityReserved = errors.New("authority already reserved")
)
78 changes: 70 additions & 8 deletions core/txpool/legacypool/legacypool.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"errors"
"math"
"math/big"
"slices"
"sort"
"sync"
"sync/atomic"
Expand Down Expand Up @@ -263,7 +264,7 @@ func New(config Config, chain BlockChain) *LegacyPool {
// pool, specifically, whether it is a Legacy, AccessList or Dynamic transaction.
func (pool *LegacyPool) Filter(tx *types.Transaction) bool {
switch tx.Type() {
case types.LegacyTxType, types.AccessListTxType, types.DynamicFeeTxType:
case types.LegacyTxType, types.AccessListTxType, types.DynamicFeeTxType, types.SetCodeTxType:
return true
default:
return false
Expand Down Expand Up @@ -540,7 +541,8 @@ func (pool *LegacyPool) validateTxBasics(tx *types.Transaction) error {
Accept: 0 |
1<<types.LegacyTxType |
1<<types.AccessListTxType |
1<<types.DynamicFeeTxType,
1<<types.DynamicFeeTxType |
1<<types.SetCodeTxType,
MaxSize: txMaxSize,
MinTip: pool.gasTip.Load().ToBig(),
}
Expand All @@ -565,6 +567,10 @@ func (pool *LegacyPool) validateTx(tx *types.Transaction) error {
if list := pool.queue[addr]; list != nil {
have += list.Len()
}
if pool.currentState.GetCode(addr) != nil {
lightclient marked this conversation as resolved.
Show resolved Hide resolved
// Allow at most one in-flight tx for delegated accounts.
return have, max(0, 1-have)
}
return have, math.MaxInt
},
ExistingExpenditure: func(addr common.Address) *big.Int {
Expand All @@ -581,6 +587,22 @@ func (pool *LegacyPool) validateTx(tx *types.Transaction) error {
}
return nil
},
KnownConflicts: func(from common.Address, auths []common.Address) []common.Address {
var conflicts []common.Address
// The transaction sender cannot have an in-flight authorization.
if _, ok := pool.all.auths[from]; ok {
conflicts = append(conflicts, from)
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I may ask simply out of curiosity, what is the reason for this rule?
It seemed to me that an "in-flight" (meaning "currently in the mempool", right?) authorization is not very different from an authorization already on-chain. At least, not in how it can suddently invalidate some "legacy" transaction from the mempool.
And if I understand this code correctly, at this point the authorizations in pool.all.auths are not even checked to have a valid nonce, so even an invalid authorization in a mempool prevents a "legacy" transaction from entering the mempool.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The attack I was trying to avoid here is many EOA accounts send the max number of pending txs possible and all of those accounts get invalidated with a single 7702 tx. There also doesn't seem to be a good reason for an honest user to have both a pending delegation and an unrelated pending tx originating from their account.

This policy is probably too draconian and can be used to deny service from regular users. Maybe a better thing to do is verify the auths as they enter the pool, then when txs are included on chain, purge all pending txs with that auth.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I ended up relaxing the requirement slightly so accounts with a pending delegation can originate 1 tx. This matches what is offered for accounts with deployed delegations while avoiding the costly state lookup for every account in a 7702 tx.

// Authorities cannot conflict with any pending or queued transactions.
for _, addr := range auths {
if list := pool.pending[addr]; list != nil {
conflicts = append(conflicts, addr)
} else if list := pool.queue[addr]; list != nil {
conflicts = append(conflicts, addr)
}
}
return conflicts
},
}
if err := txpool.ValidateTransactionWithState(tx, pool.signer, opts); err != nil {
return err
Expand Down Expand Up @@ -1334,15 +1356,13 @@ func (pool *LegacyPool) promoteExecutables(accounts []common.Address) []*types.T
// Drop all transactions that are deemed too old (low nonce)
forwards := list.Forward(pool.currentState.GetNonce(addr))
for _, tx := range forwards {
hash := tx.Hash()
pool.all.Remove(hash)
pool.all.Remove(tx.Hash())
}
log.Trace("Removed old queued transactions", "count", len(forwards))
// Drop all transactions that are too costly (low balance or out of gas)
drops, _ := list.Filter(pool.currentState.GetBalance(addr), gasLimit)
for _, tx := range drops {
hash := tx.Hash()
pool.all.Remove(hash)
pool.all.Remove(tx.Hash())
}
log.Trace("Removed unpayable queued transactions", "count", len(drops))
queuedNofundsMeter.Mark(int64(len(drops)))
Expand Down Expand Up @@ -1531,8 +1551,8 @@ func (pool *LegacyPool) demoteUnexecutables() {
drops, invalids := list.Filter(pool.currentState.GetBalance(addr), gasLimit)
for _, tx := range drops {
hash := tx.Hash()
log.Trace("Removed unpayable pending transaction", "hash", hash)
pool.all.Remove(hash)
log.Trace("Removed unpayable pending transaction", "hash", hash)
}
pendingNofundsMeter.Mark(int64(len(drops)))

Expand Down Expand Up @@ -1641,12 +1661,15 @@ type lookup struct {
slots int
lock sync.RWMutex
txs map[common.Hash]*types.Transaction

auths map[common.Address][]*types.Transaction // All accounts with a pooled authorization
}

// newLookup returns a new lookup structure.
func newLookup() *lookup {
return &lookup{
txs: make(map[common.Hash]*types.Transaction),
txs: make(map[common.Hash]*types.Transaction),
auths: make(map[common.Address][]*types.Transaction),
}
}

Expand Down Expand Up @@ -1697,6 +1720,7 @@ func (t *lookup) Add(tx *types.Transaction) {
slotsGauge.Update(int64(t.slots))

t.txs[tx.Hash()] = tx
t.addAuthorities(tx)
}

// Remove removes a transaction from the lookup.
Expand All @@ -1711,6 +1735,7 @@ func (t *lookup) Remove(hash common.Hash) {
}
lightclient marked this conversation as resolved.
Show resolved Hide resolved
t.slots -= numSlots(tx)
slotsGauge.Update(int64(t.slots))
t.removeAuthorities(tx)

delete(t.txs, hash)
}
Expand All @@ -1727,6 +1752,43 @@ func (t *lookup) TxsBelowTip(threshold *big.Int) types.Transactions {
return found
}

// addAuthorities tracks the supplied tx in relation to each authority it
// specifies.
func (t *lookup) addAuthorities(tx *types.Transaction) {
for _, addr := range tx.Authorities() {
list, ok := t.auths[addr]
if !ok {
list = []*types.Transaction{}
}
if slices.Contains(list, tx) {
// Don't add duplicates.
continue
}
list = append(list, tx)
t.auths[addr] = list
}
}

// removeAuthorities stops tracking the supplied tx in relation to its
// authorities.
func (t *lookup) removeAuthorities(tx *types.Transaction) {
for _, addr := range tx.Authorities() {
// Remove tx from tracker.
list := t.auths[addr]
if i := slices.Index(list, tx); i >= 0 {
lightclient marked this conversation as resolved.
Show resolved Hide resolved
list = append(list[:i], list[i+1:]...)
} else {
log.Error("Authority with untracked tx", "addr", addr, "hash", tx.Hash())
}
if len(list) == 0 {
// If list is newly empty, delete it entirely.
delete(t.auths, addr)
continue
}
t.auths[addr] = list
}
}

// numSlots calculates the number of slots needed for a single transaction.
func numSlots(tx *types.Transaction) int {
return int((tx.Size() + txSlotSize - 1) / txSlotSize)
Expand Down
Loading