Heartbeats are used to drop non-executable transactions from the queue.
The timeout mechanism was not clearly documented, and it was updates
also when not necessary.
Calling `pool.priced.Removed` is needed to keep is sync with
`pool.all.Remove`.
It was called in other occurances, but not here.
The counter is used for internal heap management. It was working even without this, just not calling reheap at the intended frequency.
Signed-off-by: Csaba Kiraly <csaba.kiraly@gmail.com>
This PR adds metrics that count the number of accounts having transactions
in the txpool. Together with the transaction count this can be used as a
simple indicator of the diversity of transactions in the pool.
Note: as an alternative implementation, we could use a periodic or event
driven update of these Gauges using len.
I've preferred this implementation to match what we have for the pool
sizes.
---------
Signed-off-by: Csaba Kiraly <csaba.kiraly@gmail.com>
Allow the blobpool to accept blobs out of nonce order
Previously, we were dropping blobs that arrived out-of-order. However,
since fetch decisions are done on receiver side,
out-of-order delivery can happen, leading to inefficiencies.
This PR:
- adds an in-memory blob tx storage, similar to the queue in the
legacypool
- a limited number of received txs can be added to this per account
- txs waiting in the gapped queue are not processed further and not
propagated further until they are unblocked by adding the previos nonce
to the blobpool
The size of the in-memory storage is currently limited per account,
following a slow-start logic.
An overall size limit, and a TTL is also enforced for DoS protection.
---------
Signed-off-by: Csaba Kiraly <csaba.kiraly@gmail.com>
Co-authored-by: MariusVanDerWijden <m.vanderwijden@live.de>
Blobs are stored per transaction in the pool, so we need billy to handle
up to the per-tx limit, not to the per-block limit.
The per-block limit was larger than the per-tx limit, so it not a bug,
we just created and handled a few billy files for no reason.
Signed-off-by: Csaba Kiraly <csaba.kiraly@gmail.com>
This PR removes the legacy sidecar conversion logic.
After the Osaka fork, the blobpool will accept only blob sidecar version
1.
Any remaining version 0 blob transactions, if they still exist, will no
longer
be eligible for inclusion.
Note that conversion at the RPC layer is still supported, and version 0
blob
transactions will be automatically converted to version 1 there.
This PR fixes the bug reported in #33365.
The impact of the bug is not catastrophic. After a transaction is
ultimately fetched, validation and propagation will be performed based
on the fetched body, and any response with a mismatched type is treated
as a protocol violation. An attacker could only waste the limited
portion of victim’s bandwidth at most.
However, the reasons for submitting this PR are as follows
1. Fetching a transaction announced with an arbitrary type is a weird
behavior.
2. It aligns with efforts such as EIP-8077 and #33119 to make the
fetcher smarter and reduce bandwidth waste.
Regarding the `FilterType` function, it could potentially be implemented
by modifying the Filter function's parameter itself, but I wasn’t sure
whether changing that function is acceptable, so I left it as is.
This change fixes a stall in the legacy blob sidecar conversion pipeline
where tasks that arrived during an active batch could remain unprocessed
indefinitely after that batch completed, unless a new external event
arrived.
The root cause was that the loop did not restart processing in
the case <-done: branch even when txTasks had accumulated work, relying
instead on a future event to retrigger the scheduler. This behavior is
inconsistent with the billy task pipeline, which immediately chains to
the next task via runNextBillyTask() without requiring an external trigger.
The fix adds a symmetric restart path in `case <-done`: that checks
`len(txTasks) > 0`, clones the accumulated tasks, clears the queue, and
launches a new run with a fresh done and interrupt.
This preserves batching semantics, prevents indefinite blocking of callers
of convert(), and remains safe during shutdown since the quit path
still interrupts and awaits the active batch. No public interfaces or logging
were changed.
Previously, the journal writer is nil until the first time rejournal
(default 1h), which means during this period, txs submitted to this node
are not written into journal file (transactions.rlp). If this node is
shutdown before the first time rejournal, then txs in pending or queue
will get lost.
Here, this PR initializes the journal writer soon after launch to solve
this issue.
---------
Co-authored-by: Gary Rong <garyrong0905@gmail.com>
This pr addresses a few issues brought by the #32270
- Add updates to pricedList after dropping transactions.
- Remove redundant deletions in queue.evictList, since
pool.removeTx(hash, true, true) already performs the removal.
- Prevent duplicate addresses during promotion when Reset is not nil.
This PR move the queue out of the main transaction pool.
For now there should be no functional changes.
I see this as a first step to refactor the legacypool and make the queue
a fully separate concept from the main pending pool.
---------
Signed-off-by: Csaba Kiraly <csaba.kiraly@gmail.com>
Co-authored-by: Csaba Kiraly <csaba.kiraly@gmail.com>
invalidTxMeter was counting txs, while validTxMeter was counting
accounts. Better make the two comparable.
---------
Signed-off-by: Csaba Kiraly <csaba.kiraly@gmail.com>
The TxPool.signer field was never read and each subpool (legacy/blob)
maintains its own signer instance. This field remained after txpool
refactoring into subpools and is dead code. Removing it reduces
confusion and simplifies the constructor.
This is a small improvement on #32656 in case Add was called with
multiple type 3 transactions, adding transactions to the pool one-by-one
as they are converted.
Announcement to peers is still done in a batch.
Signed-off-by: Csaba Kiraly <csaba.kiraly@gmail.com>
This implements the conversion of existing blob transactions to the new proof
version. Conversion is triggered at the Osaka fork boundary. The conversion is
designed to be idempotent, and may be triggered multiple times in case of a reorg
around the fork boundary.
This change is the last missing piece that completes our strategy for the blobpool
conversion. After the Osaka fork,
- new transactions will be converted on-the-fly upon entry to the pool
- reorged transactions will be converted while being reinjected
- (this change) existing transactions will be converted in the background
---------
Co-authored-by: Gary Rong <garyrong0905@gmail.com>
Co-authored-by: lightclient <lightclient@protonmail.com>
This pull request introduces a queue for legacy sidecar conversion to
handle transactions that persist after the Osaka fork. Simply dropping
these transactions would significantly harm the user experience.
To balance usability with system complexity, we have introduced a
conversion time window of two hours post Osaka fork. During this period,
the system will accept legacy blob transactions and convert them in a
background process.
After the window, all legacy transactions will be rejected. Notably, all
the blob transactions will be validated statically before the conversion,
and also all conversion are performed in a single thread, minimize the risk
of being DoS.
We believe this two hour window provides sufficient time to process
in-flight legacy transactions and allows submitters to migrate to the
new format.
---------
Co-authored-by: Felix Lange <fjl@twurst.com>
Implements a migration path for the blobpool slotter
---------
Co-authored-by: lightclient <lightclient@protonmail.com>
Co-authored-by: lightclient <14004106+lightclient@users.noreply.github.com>
Co-authored-by: Gary Rong <garyrong0905@gmail.com>
This PR removes the conversion of legacy sidecars after Osaka and instead rejects them to the pool.
---------
Co-authored-by: lightclient <lightclient@protonmail.com>
Another getBlobs PR on top of
https://github.com/ethereum/go-ethereum/pull/32190 to avoid some minor
regressions.
- bring back more log messages from before
- continue processing also on some internal errors
- ensure v2 complies with spec even if there are internal errors
---------
Signed-off-by: Csaba Kiraly <csaba.kiraly@gmail.com>
Co-authored-by: rjl493456442 <garyrong0905@gmail.com>
This pull request fixes a regression, introduced in #32190
Specifically, in GetBlobsV1 engine API, if any blob is missing, the null
should be placed in
response, unfortunately a behavioral change was introduced in #32190,
returning an error
instead.
What's more, a more comprehensive test suite is added to cover both
`GetBlobsV1` and
`GetBlobsV2` endpoints.
This pull request implements #32235 , constructing blob sidecar in new
format (cell proof)
if the Osaka has been activated.
Apart from that, it introduces a pre-conversion step in the blob pool
before adding the txs.
This mechanism is essential for handling the remote **legacy** blob txs
from the network.
One thing is still missing and probably is worthy being highlighted
here: the blobpool may
contain several legacy blob txs before the Osaka and these txs should be
converted once
Osaka is activated. While the `GetBlob` API in blobpool is capable for
generating cell proofs
at the runtime, converting legacy txs at one time is much cheaper
overall.
---------
Co-authored-by: MariusVanDerWijden <m.vanderwijden@live.de>
Co-authored-by: lightclient <lightclient@protonmail.com>
This PR should reduce overall allocations of a running node by ~10
percent. Since most allocations are coming from the re-heaping of the
transaction pool.
```
(pprof) list EffectiveGasTipCmp
Total: 38197204475
ROUTINE ======================== github.com/ethereum/go-ethereum/core/types.(*Transaction).EffectiveGasTipCmp in github.com/ethereum/go-ethereum/core/types/transaction.go
0 3766837369 (flat, cum) 9.86% of Total
. . 386:func (tx *Transaction) EffectiveGasTipCmp(other *Transaction, baseFee *big.Int) int {
. . 387: if baseFee == nil {
. . 388: return tx.GasTipCapCmp(other)
. . 389: }
. . 390: // Use more efficient internal method.
. . 391: txTip, otherTip := new(big.Int), new(big.Int)
. 1796172553 392: tx.calcEffectiveGasTip(txTip, baseFee)
. 1970664816 393: other.calcEffectiveGasTip(otherTip, baseFee)
. . 394: return txTip.Cmp(otherTip)
. . 395:}
. . 396:
. . 397:// EffectiveGasTipIntCmp compares the effective gasTipCap of a transaction to the given gasTipCap.
. . 398:func (tx *Transaction) EffectiveGasTipIntCmp(other *big.Int, baseFee *big.Int) int {
```
This PR reduces the allocations for comparing two transactions from 2 to
0:
```
goos: linux
goarch: amd64
pkg: github.com/ethereum/go-ethereum/core/types
cpu: Intel(R) Core(TM) Ultra 7 155U
│ /tmp/old.txt │ /tmp/new.txt │
│ sec/op │ sec/op vs base │
EffectiveGasTipCmp/Original-14 64.67n ± 2% 25.13n ± 9% -61.13% (p=0.000 n=10)
│ /tmp/old.txt │ /tmp/new.txt │
│ B/op │ B/op vs base │
EffectiveGasTipCmp/Original-14 16.00 ± 0% 0.00 ± 0% -100.00% (p=0.000 n=10)
│ /tmp/old.txt │ /tmp/new.txt │
│ allocs/op │ allocs/op vs base │
EffectiveGasTipCmp/Original-14 2.000 ± 0% 0.000 ± 0% -100.00% (p=0.000 n=10)
```
It also speeds up the process by ~60%
There are two minor caveats with this PR:
- We change the API for `EffectiveGasTipCmp` and `EffectiveGasTipIntCmp`
(which are probably not used by much)
- We slightly change the behavior of `tx.EffectiveGasTip` when it
returns an error. It would previously return a negative number on error,
now it does not (since uint256 does not allow for negative numbers)
---------
Signed-off-by: Csaba Kiraly <csaba.kiraly@gmail.com>
Co-authored-by: Csaba Kiraly <csaba.kiraly@gmail.com>
