-
Notifications
You must be signed in to change notification settings - Fork 18
Throttling Tx Ingress #89
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
base: main
Are you sure you want to change the base?
Conversation
spalladino
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just checked the code and the flow when receive a tx is:
- Validate it (of course)
- Add it to the local mempool
- Evict txs from the mempool if needed
- Broadcast it via p2p
So if we (as a node) receive a tx that we add and evict immediately, we still broadcast it. In other words, if we receive a tx that we ourselves would not keep, we are still pushing it to the world.
I think an easy fix to prevent diverging mempools and flooding the network with useless txs would be to only re-broadcast a tx if we'd accept it (and keep it) in our own mempool.
|
|
||
| The only way to moderate the flow of transactions around the network is to control their ingress at source. Once a tx is being propagated it can't be impeded, to do so would exacerbate the problem of divergent mempools. | ||
|
|
||
| So the proposed changes are to track the average rate of transaction delivery at the p2p network, say over the last `N` seconds where `N` is configurable. If this goes beyond a configured threshold, transactions will be rejected at the RPC layer until it reduces. No newline at end of file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't understand what's tracked exactly here. By transaction delivery, does it mean the number of txs received via JSON RPC API that this node injected into the p2p network? Or all txs that were broadcasted, including the ones received via p2p?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would see it as ('Transactions I received over the P2P network' + 'Transactions I introduced to the P2P network' / 'N'. My observed TPS of the network.
This doesn't seem right to me. Just because a transaction isn't of interest to me, or doesn't fit in my local store, doesn't mean it isn't of interest to you, so I shouldn't withhold it. The ideal situation in my view is that nodes don't evict transactions at all until they become expired. Now of course presumably there needs to be some limit, but hopefully it could be huge. This is just part of the responsibility of contributing to tx availability which I think everyone running a node needs to do. Lets say the expectation was that nodes store up to 1 million pending transactions. That a little more than 1 day's worth of backlog at 10TPS. Max transaction expiry is 24 hours. That's about 60GB assuming compressed proofs. Which doesn't seem so bad. But maybe that could lead to problems with people spamming low value transactions so need more thought. |
|
Disagree. I think that we should strive to minimize divergence across node tx pools, and not broadcasting txs that are interesting to me is a good way to do it, while at the same time minimizing low-value traffic on the p2p network. Even if the limits for expiry are huge, they are still there, so we cannot assume that node tx pools will always have enough room. FWIW I asked Claude what geth does, by looking at the codebase. It seems Geth only broadcasts txs that were accepted in its own tx pool. No, if a transaction doesn't fit in the mempool because it's full and there's no lower-priority tx to evict, the new transaction does NOT get broadcasted. Transaction Flow from JSON-RPC to Broadcasting1. JSON-RPC SubmissionLocation: internal/ethapi/api.go:1560 When you call 2. TxPool AdmissionLocation: eth/api_backend.go:323 The transaction is added to the pool: b.eth.txPool.Add([]*types.Transaction{signedTx}, false)[0]3. Legacy Pool ProcessingLocation: core/txpool/legacypool/legacypool.go:935-983 The pool calls Critical check at lines 713-738: if uint64(pool.all.Slots()+numSlots(tx)) > pool.config.GlobalSlots+pool.config.GlobalQueue {
// If the new transaction is underpriced, don't accept it
if pool.priced.Underpriced(tx) {
return false, txpool.ErrUnderpriced
}
// Try to evict lower-priced transactions...
drop, success := pool.priced.Discard(...)
if !success {
return false, ErrTxPoolOverflow // ← REJECTED HERE
}
}4. Broadcasting MechanismLocation: core/txpool/legacypool/legacypool.go:1287-1331
Why Rejected Transactions Don't Get BroadcastedThe key insight is the separation between admission and broadcasting: Admission Happens FirstAt core/txpool/legacypool/legacypool.go:713-738, when the pool is full, the transaction is rejected with Broadcasting Happens LaterOnly after a transaction is:
Does it trigger a The Transaction Never Reaches the Broadcast StageIf rejected during admission, it never enters the pool, never gets promoted, and never triggers the broadcast event. Special Case: Local TransactionsLocation: eth/api_backend.go:327-341 // If the local transaction tracker is not configured, returns whatever
// returned from the txpool.
if b.eth.localTxTracker == nil {
return err
}
// No error will be returned to user if the transaction fails with a temporary
// error and might be accepted later (e.g., the transaction pool is full).
// Locally submitted transactions will be resubmitted later via the local tracker.
b.eth.localTxTracker.Track(signedTx)
return nilFor transactions submitted locally via JSON-RPC, even if rejected with SummaryA transaction that doesn't fit in the mempool due to being full (and unable to evict lower-priority transactions) is:
Key Architectural PointThe broadcasting mechanism in geth is pull-based from the txpool, not push-based from the API layer. Only transactions that successfully enter the pool and become executable trigger broadcast events. |
Very interesting |
No description provided.