feat(mempool): Equivalent recheck and execution ordering #944
Conversation
mattac21
changed the title
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| } | ||
|
|
||
| // time the duration of rechecking the pending pool | ||
| defer func(start time.Time) { pendingRecheckDurationTimer.UpdateSince(start) }(time.Now()) |
Check warning
Code scanning / CodeQL
Calling the system time Warning
| for addr, txs := range txs { | ||
| ordered[addr] = txs.Flatten() | ||
| } |
Check warning
Code scanning / CodeQL
Iteration over map Warning
| // Returns error in case of a negative effective miner gasTipCap. | ||
| func newTxWithMinerFee(tx *txpool.LazyTransaction, from common.Address, baseFee *uint256.Int) (*txWithMinerFee, error) { | ||
| tip := new(uint256.Int).Set(tx.GasTipCap) | ||
| func newTxWithMinerFee(tx *types.Transaction, from common.Address, baseFee *uint256.Int) (*txWithMinerFee, error) { |
There was a problem hiding this comment.
this is doing the same thing as previously, the chain from txpool.LazyTransaction to types.Transaction made this have to change slightly with how we are getting values.
| // LazyTransaction contains a small subset of the transaction properties that is | ||
| // enough for the miner and other APIs to handle large batches of transactions; | ||
| // and supports pulling up the entire transaction when really needed. | ||
| type LazyTransaction struct { |
There was a problem hiding this comment.
Nuked the LazyTransaction. This wasn't used for anything (we always just accessed the internal tx directly from this.
| // Pending retrieves all currently processable transactions, in the order | ||
| // that they should be executed in, along with each transactions fees. | ||
| Pending(ctx context.Context, height *big.Int) []TxWithFees |
There was a problem hiding this comment.
Instead of directly returning the contents of the subpool in a copy of a map, we return a slice of []TxWihtFees now that is in the exact order that the txs should be proposed with. i.e. no caller that is using this Pending result for a proposal should modify the list
| // consistent ordering of recheck txs <-> execution ordering. We need | ||
| // to defer the removal of txs to only be during reset, before promote | ||
| // + demote. | ||
| pool.validPendingTxs.RemoveTx(tx) |
| // been reached by the collector, it will wait until the context times out or | ||
| // the height is reached. | ||
| func (c *txCollector) Collect(ctx context.Context, height *big.Int, filter txpool.PendingFilter) map[common.Address][]*txpool.LazyTransaction { | ||
| func (c *txCollector) Collect(ctx context.Context, height *big.Int) []txpool.TxWithFees { |
There was a problem hiding this comment.
instead of returning a map of txs, we now simply return a list of txs with their associated fee. the order of this list is the exact order that txs should be proposed in.
| return nil | ||
| } | ||
|
|
||
| func (t *txs) Get() []txpool.TxWithFees { |
There was a problem hiding this comment.
this no longer does any filtering or modifications to the txs internal list of transactions. we do still need to do the minTip filtering that was happening in here. This has now moved inside of the demoteUnexecutables check.
| // TODO: this needs to change, this will change the execution order. | ||
| // we actually cant remove from this at all, as that will also have effects | ||
| // on the recheck. we need to defer removing until after a block has been | ||
| // processed, and then we process the removals, then we rebuild the txs set | ||
| // from scratch. |
| // encounter a tx out of nonce order where this condition is true for | ||
| // that tx, but a tx that we encounter in the future is able to be | ||
| // executed | ||
| if tx.EffectiveGasTipIntCmp(pool.config.MinTip, baseFee) < 0 { |
There was a problem hiding this comment.
emulates the previous behavior that we had in txCollector.Get. If this txs effective gas tip cap is not above the base fee, we will not remove the tx from the mempool, but we will not return it to be included in a proposal.
mattac21
requested review from
Eric-Warehime,
swift1337 and
technicallyty
and removed request for
swift1337
| // and overrides it with values from appOpts if they exist and are non-zero. | ||
| func (app *EVMD) createMempoolConfig(appOpts servertypes.AppOptions, logger log.Logger) (*evmmempool.EVMMempoolConfig, error) { | ||
| legacyPoolConfig := server.GetLegacyPoolConfig(appOpts, logger) | ||
| // TODO: change where mintip is read in from |
Description
Rechecks txs during
demoteUnexecutablesin the order that they should be proposed in (ordered by ascending nonce and decreasing price), and returns them fromtxpool.Pendingin this order.I did some other small changes to make this a bit easier, like removing the
LazyTransactionstruct that was basically unused (replaced withTxWithFeeto cache the txs fee).Previously we filtered txs by effective tip cap
tip - baseFeein the call totxpool.Pending -> txcollector.Collectand excluded them from thePendingresult if they did not satisfy the validators configuredMinTIp. Now, we must not recheck these txs since they will not be included in the execution, so new filtering was added todemoteUnexcutablesto ensure we skip these low fee txs.Closes: #STACK-1965
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
mainbranch