**Problem:** Including full account code in prestateTracer response
significantly increases response payload size.
**Solution:** Add codeHash field to the response. This will allow
client-side bytecode caching and is a non-breaking change.
**Note:** codeHash for EoAs is excluded to save space.
---------
Co-authored-by: Sina Mahmoodi <itz.s1na@gmail.com>
The GetHeader function was incorrectly returning an error when
encountering nil peers in the peers list, which contradicted the comment
"keep retrying if none are yet available".
Changed the logic to skip nil peers with 'continue' instead of returning
an error, allowing the function to properly iterate through all
available peers and attempt to retrieve the target header from each valid peer.
This ensures the function behaves as intended - trying all available
peers before giving up, rather than failing on the first nil peer encountered.
The previous comment stated that every 3rd block has a tx and every 5th
has an uncle.
The implementation actually adds one transaction to every second block
and does not add uncles.
Updated the comment to reflect the real behavior to avoid confusion when
reading tests.
This PR makes 2 changes to how
[EIP-7825](https://github.com/ethereum/go-ethereum/pull/31824) behaves.
When `eth_estimateGas` or `eth_createAccessList` is called without any
gas limit in the payload, geth will choose the block's gas limit or the
`RPCGasCap`, which can be larger than the `maxTxGas`.
When this happens for `estimateGas`, the gas estimation just errors out
and ends, when it should continue doing binary search to find the lowest
possible gas limit.
This PR will:
- Add a check to see if `hi` is larger than `maxTxGas` and cap it to
`maxTxGas` if it's larger. And add a special case handling for gas
estimation execute when it errs with `ErrGasLimitTooHigh`
---------
Co-authored-by: Gary Rong <garyrong0905@gmail.com>
- If all the `vhashes` are in the same `sidecar`, then it will load the
same blob tx many times. This PR aims to upgrade this.
---------
Co-authored-by: Gary Rong <garyrong0905@gmail.com>
This introduces an error when the filter has both `blockHash` and
`fromBlock`/`toBlock`, since these are mutually exclusive. Seems the
tests were actually returning `not found` error, which went undetected
since there was no check on the actual returned error in the test.
Correct the error message in the ExecuteStatelessPayloadV4 function to
reference newPayloadV4 and the Prague fork, instead of incorrectly
referencing newPayloadV3 and Cancun.
This improves clarity during debugging and aligns the error message with
the actual function and fork being validated. No logic is changed.
---------
Co-authored-by: rjl493456442 <garyrong0905@gmail.com>
The main purpose of this change is to enforce the version setting when
constructing the blobSidecar, avoiding creating sidecar with wrong/default
version tag.
Fixes#32175.
This fixes the scenario where the blockhash opcode would return 0x0
during RPC simulations when using BlockOverrides with a future block
number. The root cause was that BlockOverrides.Apply() only modified the
vm.BlockContext, but GetHashFn() depends on the actual
types.Header.Number to resolve valid historical block hashes. This
caused a mismatch and resulted in incorrect behavior during trace and
call simulations.
---------
Co-authored-by: shantichanal <158101918+shantichanal@users.noreply.github.com>
Co-authored-by: lightclient <lightclient@protonmail.com>
This is something interesting I came across during my benchmarks, we
spent ~3.8% of all allocations allocating the header number on the heap.
```
(pprof) list GetHeaderByHash
Total: 38197204475
ROUTINE ======================== github.com/ethereum/go-ethereum/core.(*BlockChain).GetHeaderByHash in github.com/ethereum/go-ethereum/core/blockchain_reader.go
0 5786566117 (flat, cum) 15.15% of Total
. . 79:func (bc *BlockChain) GetHeaderByHash(hash common.Hash) *types.Header {
. 5786566117 80: return bc.hc.GetHeaderByHash(hash)
. . 81:}
. . 82:
. . 83:// GetHeaderByNumber retrieves a block header from the database by number,
. . 84:// caching it (associated with its hash) if found.
. . 85:func (bc *BlockChain) GetHeaderByNumber(number uint64) *types.Header {
ROUTINE ======================== github.com/ethereum/go-ethereum/core.(*HeaderChain).GetHeaderByHash in github.com/ethereum/go-ethereum/core/headerchain.go
0 5786566117 (flat, cum) 15.15% of Total
. . 404:func (hc *HeaderChain) GetHeaderByHash(hash common.Hash) *types.Header {
. 1471264309 405: number := hc.GetBlockNumber(hash)
. . 406: if number == nil {
. . 407: return nil
. . 408: }
. 4315301808 409: return hc.GetHeader(hash, *number)
. . 410:}
. . 411:
. . 412:// HasHeader checks if a block header is present in the database or not.
. . 413:// In theory, if header is present in the database, all relative components
. . 414:// like td and hash->number should be present too.
(pprof) list GetBlockNumber
Total: 38197204475
ROUTINE ======================== github.com/ethereum/go-ethereum/core.(*HeaderChain).GetBlockNumber in github.com/ethereum/go-ethereum/core/headerchain.go
94438817 1471264309 (flat, cum) 3.85% of Total
. . 100:func (hc *HeaderChain) GetBlockNumber(hash common.Hash) *uint64 {
94438817 94438817 101: if cached, ok := hc.numberCache.Get(hash); ok {
. . 102: return &cached
. . 103: }
. 1376270828 104: number := rawdb.ReadHeaderNumber(hc.chainDb, hash)
. . 105: if number != nil {
. 554664 106: hc.numberCache.Add(hash, *number)
. . 107: }
. . 108: return number
. . 109:}
. . 110:
. . 111:type headerWriteResult struct {
(pprof) list ReadHeaderNumber
Total: 38197204475
ROUTINE ======================== github.com/ethereum/go-ethereum/core/rawdb.ReadHeaderNumber in github.com/ethereum/go-ethereum/core/rawdb/accessors_chain.go
204606513 1376270828 (flat, cum) 3.60% of Total
. . 146:func ReadHeaderNumber(db ethdb.KeyValueReader, hash common.Hash) *uint64 {
109577863 1281242178 147: data, _ := db.Get(headerNumberKey(hash))
. . 148: if len(data) != 8 {
. . 149: return nil
. . 150: }
95028650 95028650 151: number := binary.BigEndian.Uint64(data)
. . 152: return &number
. . 153:}
. . 154:
. . 155:// WriteHeaderNumber stores the hash->number mapping.
. . 156:func WriteHeaderNumber(db ethdb.KeyValueWriter, hash common.Hash, number uint64) {
```
Opening this to discuss the idea, I know that rawdb.EmptyNumber is not a
great name for the variable, open to suggestions
Introduce file-based state journal in path database, fixing
the Pebble restriction when the journal size exceeds 4GB.
---------
Signed-off-by: jsvisa <delweng@gmail.com>
Co-authored-by: Gary Rong <garyrong0905@gmail.com>
This PR fixes an issue in the tx_fetcher DoS prevention logic where the
code keeps the overflow amount (`want - maxTxAnnounces`) instead of the
allowed amount (`maxTxAnnounces - used`). The specific changes are:
- Correct slice indexing in the announcement drop logic
- Extend the overflow test case to cover the inversion scenario
This pull request fixes an issue in disabling direct-ancient mode in
snap sync.
Specifically, if `origin >= frozen && origin != 0`, it implies a part of
chain data has been written into the key-value store, all the following
writes into ancient store scheduled by downloader will be rejected
with error
`ERROR[07-10|03:46:57.924] Error importing chain data to ancients
err="can't add block 1166 hash: the append operation is out-order: have
1166 want 0"`.
This issue is detected by the https://github.com/ethpandaops/kurtosis-sync-test,
which initiates the first snap sync cycle without the finalized header and
implicitly disables the direct-ancient mode. A few seconds later the second
snap sync cycle is initiated with the finalized information and direct-ancient mode
is enabled incorrectly.
alternate approach to https://github.com/ethereum/go-ethereum/pull/31328
suggested by @MariusVanDerWijden . This prevents Geth from outputting a
lot of logs when trying to commit on-demand dev mode blocks while the
client is shutting down.
The issue is hard to reproduce, but I've seen it myself and it is
annoying when it happens. I think this is a reasonable simple solution,
and we can revisit if we find that the output is still too large (i.e.
there is a large delay between initiating shut down and the simulated
beacon receiving the signal, while in this loop).
Co-authored-by: Marius van der Wijden <m.vanderwijden@live.de>
Improves the SSTORE gas calculation a bit. Previously we would pull up
the state object twice. This is okay for existing objects, since they
are cached, however non-existing objects are not cached, thus we needed
to go through all 128 diff layers as well as the disk layer twice, just
for the gas calculation
```
goos: linux
goarch: amd64
pkg: github.com/ethereum/go-ethereum/core/vm
cpu: AMD Ryzen 9 5900X 12-Core Processor
│ /tmp/old.txt │ /tmp/new.txt │
│ sec/op │ sec/op vs base │
Interpreter-24 1118.0n ± 2% 602.8n ± 1% -46.09% (p=0.000 n=10)
```
---------
Co-authored-by: Gary Rong <garyrong0905@gmail.com>
Small update for logs when syncing with blsync. Downgrades the "latest
filled block is not available" to warn.
Co-authored-by: shantichanal <158101918+shantichanal@users.noreply.github.com>
geth cmd: `geth --dev --dev.period 5`
call: `debug.setHead` to rollback several blocks.
If the `debug.setHead` call is delayed, it will trigger a panic with a
small probability, due to using the null point of
`fcResponse.PayloadID`.
---------
Co-authored-by: Marius van der Wijden <m.vanderwijden@live.de>
This pull request refines the filtermap implementation, defining key
APIs for map and
epoch calculations to improve readability.
This pull request doesn't change any logic, it's a pure cleanup.
---------
Co-authored-by: zsfelfoldi <zsfelfoldi@gmail.com>
The address filter was never checked against a maximum limit, which can
be somewhat abusive for API nodes. This PR adds a limit similar to
topics
## Description (AI generated)
This pull request introduces a new validation to enforce a maximum limit
on the number of addresses allowed in filter criteria for Ethereum logs.
It includes updates to the `FilterAPI` and `EventSystem` logic, as well
as corresponding test cases to ensure the new constraint is properly
enforced.
### Core functionality changes:
* **Validation for maximum addresses in filter criteria**:
- Added a new constant, `maxAddresses`, set to 100, to define the
maximum allowable addresses in a filter.
- Introduced a new error, `errExceedMaxAddresses`, to handle cases where
the number of addresses exceeds the limit.
- Updated the `GetLogs` method in `FilterAPI` to validate the number of
addresses against `maxAddresses`.
- Modified the `UnmarshalJSON` method to return an error if the number
of addresses in the input JSON exceeds `maxAddresses`.
- Added similar validation to the `SubscribeLogs` method in
`EventSystem`.
### Test updates:
* **New test cases for address limit validation**:
- Added a test in `TestUnmarshalJSONNewFilterArgs` to verify that
exceeding the maximum number of addresses triggers the
`errExceedMaxAddresses` error.
- Updated `TestInvalidLogFilterCreation` to include a test case for an
invalid filter with more than `maxAddresses` addresses.
- Updated `TestInvalidGetLogsRequest` to test for invalid log requests
with excessive addresses.
These changes ensure that the system enforces a reasonable limit on the
number of addresses in filter criteria, improving robustness and
preventing potential performance issues.
---------
Co-authored-by: zsfelfoldi <zsfelfoldi@gmail.com>
core.BlockChainConfig.VmConfig is not a pointer, so setting the Tracer
on the `vmConfig` object after it was passed to options does *not* apply
it to options.VmConfig
This fixes the issue by setting the value directly inside the `options`
object and removing the confusing `vmConfig` variable to prevent further
mistakes.
This pull request tracks the state indexing progress in eth_syncing
RPC response, i.e. we will return non-null syncing status until indexing
has finished.
This pull request fixes a flaw in PBSS archive mode that significantly
degrades performance when the mode is enabled.
Originally, in hash mode, the dirty trie cache is completely disabled
when archive mode is active, in order to disable the in-memory garbage
collection mechanism. However, the internal logic in path mode differs
significantly, and the dirty trie node cache is essential for maintaining
chain insertion performance. Therefore, the cache is now retained in
path mode.
If Geth is engaged in a long-run block synchronization, such as a full
syncing over a large number of blocks, invoking `debug_setHead` will
cause `downloader.Cancel` to wait for all fetchers to stop first.
This can be time-consuming, particularly for the block processing
thread.
To address this, we manually call `blockchain.StopInsert` to interrupt
the blocking processing thread and allow it to exit immediately, and
after that call `blockchain.ResumeInsert` to resume the block
downloading process.
Additionally, we add a sanity check for the input block number of
`debug_setHead` to ensure its validity.
---------
Signed-off-by: jsvisa <delweng@gmail.com>
Co-authored-by: Gary Rong <garyrong0905@gmail.com>
In this pull request, the original `CacheConfig` has been renamed to `BlockChainConfig`.
Over time, more fields have been added to `CacheConfig` to support
blockchain configuration. Such as `ChainHistoryMode`, which clearly extends
beyond just caching concerns.
Additionally, adding new parameters to the blockchain constructor has
become increasingly complicated, since it’s initialized across multiple
places in the codebase. A natural solution is to consolidate these arguments
into a dedicated configuration struct.
As a result, the existing `CacheConfig` has been redefined as `BlockChainConfig`.
Some parameters, such as `VmConfig`, `TxLookupLimit`, and `ChainOverrides`
have been moved into `BlockChainConfig`. Besides, a few fields in `BlockChainConfig`
were renamed, specifically:
- `TrieCleanNoPrefetch` -> `NoPrefetch`
- `TrieDirtyDisabled` -> `ArchiveMode`
Notably, this change won't affect the command line flags or the toml
configuration file. It's just an internal refactoring and fully backward-compatible.
---------
Co-authored-by: Felix Lange <fjl@twurst.com>
The prestateTracer had the intention of excluding accounts that were
empty prior to execution from the prestate. This was being done only for
created contracts. This PR makes it so all such empty accounts are
excluded. This behavior is configurable using the `includeEmpty: true`
flag introduced in #31855.
---------
Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com>
Co-authored-by: Sina Mahmoodi <itz.s1na@gmail.com>
This PR introduces a flag that enables returning of newly created state
objects in the prestateTracer.
**Rationale**
Having this information is useful because local execution can more
easily distinguish between newly created objects and system contracts.
---------
Co-authored-by: Sina Mahmoodi <itz.s1na@gmail.com>
This pull request introduces a new test suite in workload framework, for
transaction tracing.
**test generation**
`go run . tracegen --trace-tests trace-test.json http://host:8545`
and you can choose to store the trace result in a specific folder
`go run . tracegen --trace-tests trace-test.json --trace-output
./trace-result http://host:8545`
**test run**
`./workload test -run Trace/Transaction --trace-invalid ./trace-invalid
http://host:8545`
The mismatched trace result will be saved in the specific folder for
further investigation.
This PR improves the speed of Disc/v4 and Disc/v5 based discovery by
adding a prefetch buffer to discovery sources, eliminating slowdowns
due to timeouts and rate mismatch between the two processes.
Since we now want to filter the discv4 nodes iterator, it is being removed
from the default discovery mix in p2p.Server. To keep backwards-compatibility,
the default unfiltered discovery iterator will be utilized by the server when
no protocol-specific discovery is configured.
---------
Signed-off-by: Csaba Kiraly <csaba.kiraly@gmail.com>
Co-authored-by: Felix Lange <fjl@twurst.com>
This fixes a regression introduced by #29158 where receipts of empty blocks
were stored into the database as an empty byte array, instead of an RLP empty list.
Fixes#31938
---------
Co-authored-by: Felix Lange <fjl@twurst.com>
Fixes an issue where querying logs for block ranges starting from 0 would fail with an irrelevant
error on a pruned node. Now the correct "history is pruned" error will be returned.
This situation was failing quietly for me recently when I had a partial
data corruption issue. Changing the log level to Error would increase
visibility for me.
This implements a backing store for chain history based on era1 files.
The new store is integrated with the freezer. Queries for blocks and receipts
below the current freezer tail are handled by the era store.
---------
Co-authored-by: Gary Rong <garyrong0905@gmail.com>
Co-authored-by: Felix Lange <fjl@twurst.com>
Co-authored-by: lightclient <lightclient@protonmail.com>
This changes the API backend to return null for not-found blocks. This behavior
is required by the RPC When `BlockByNumberOrHash` always returned an error
for this case ever since being added in https://github.com/ethereum/go-ethereum/pull/19491.
The backend method has a couple of call sites, and all of them handle a `nil`
block result because `BlockByNumber` returns `nil` for not-found.
The only case where this makes a real difference is for `eth_getBlockReceipts`,
which was changed in #31361 to actually forward the error from `BlockByNumberOrHash`
to the caller.
This PR introduces a new native tracer for AA bundlers. Bundlers participating in the alternative
mempool will need to validate userops. This tracer will return sufficient information for them to
decide whether griefing is possible. Resolves#30546
---------
Co-authored-by: Sina M <1591639+s1na@users.noreply.github.com>
This PR implements eth/69. This protocol version drops the bloom filter
from receipts messages, reducing the amount of data needed for a sync
by ~530GB (2.3B txs * 256 byte) uncompressed. Compressed this will
be reduced to ~100GB
The new version also changes the Status message and introduces the
BlockRangeUpdate message to relay information about the available history
range.
---------
Co-authored-by: Felix Lange <fjl@twurst.com>
In this pull request, snapshot generation in pathdb has been ported from
the legacy state snapshot implementation. Additionally, when running in
path mode, legacy state snapshot data is now managed by the pathdb
based snapshot logic.
Note: Existing snapshot data will be re-generated, regardless of whether
it was previously fully constructed.
This PR contains three refactors:
- refactor the latest fork check that we use quite extensively
- refactor the nil checks in NewPayloads
---------
Co-authored-by: lightclient <lightclient@protonmail.com>
This PR introduces an allocation-free version of the
Transaction.EffectiveGasTip method to improve performance by reducing
memory allocations.
## Changes
- Added a new `EffectiveGasTipInto` method that accepts a destination
parameter to avoid memory allocations
- Refactored the existing `EffectiveGasTip` method to use the new
allocation-free implementation
- Updated related methods (`EffectiveGasTipValue`, `EffectiveGasTipCmp`,
`EffectiveGasTipIntCmp`) to use the allocation-free approach
- Added tests and benchmarks to verify correctness and measure
performance improvements
## Motivation
In high-transaction-volume environments, the `EffectiveGasTip` method is
called frequently. Reducing memory allocations in this method decreases
garbage collection pressure and improves overall system performance.
## Benchmark Results
As-Is
BenchmarkEffectiveGasTip/Original-10 42089140 27.45 ns/op 8 B/op 1
allocs/op
To-Be
BenchmarkEffectiveGasTip/IntoMethod-10 72353263 16.73 ns/op 0 B/op 0
allocs/op
## Summary of Improvements
- **Performance**: ~39% faster execution (27.45 ns/op → 16.73 ns/op)
- **Memory**: Eliminated all allocations (8 B/op → 0 B/op)
- **Allocation count**: Reduced from 1 to 0 allocations per operation
This optimization follows the same pattern successfully applied to other
methods in the codebase, maintaining API compatibility while improving
performance.
## Safety & Compatibility
This optimization has no side effects or adverse impacts because:
- It maintains functional equivalence as confirmed by comprehensive
tests
- It preserves API compatibility with existing callers
- It follows clear memory ownership patterns with the destination
parameter
- It maintains thread safety by only modifying the caller-provided
destination parameter
This optimization follows the same pattern successfully applied to other
methods in the codebase, providing better performance without
compromising stability or correctness.
---------
Co-authored-by: lightclient <lightclient@protonmail.com>
Fixes methods debug_standardTraceBlockToFile
and debug_standardTraceBadBlockToFile which were
outputting empty files.
---------
Co-authored-by: maskpp <maskpp266@gmail.com>
Co-authored-by: Sina Mahmoodi <itz.s1na@gmail.com>
Fixes https://github.com/ethereum/go-ethereum/issues/31732.
This logic was removed in the recent refactoring in the txindexer to
handle history cutoff (#31393). It was first introduced in this PR:
https://github.com/ethereum/go-ethereum/pull/28908.
I have tested it and it works as an alternative to #31745.
This PR packs 3 changes to the flow of fetching txs from the API:
- It caches the indexer tail after each run is over to avoid hitting the
db all the time as was done originally in #28908.
- Changes `backend.GetTransaction`. It doesn't return an error anymore
when tx indexer is in progress. It shifts the responsibility to the
caller to check the progress. The reason is that in most cases we anyway
check the txpool for the tx. If it was indeed a pending tx we can avoid
the indexer progress check.
---------
Co-authored-by: Gary Rong <garyrong0905@gmail.com>
This changes the filtermaps to only pull up the raw receipts, not the
derived receipts which saves a lot of allocations.
During normal execution this will reduce the allocations of the whole
geth node by ~15%.
This PR changes the chain view update mechanism of the log filter.
Previously the head updates were all wired through the indexer, even in
unindexed mode. This was both a bit weird and also unsafe as the
indexer's chain view was updates asynchronously with some delay, making
some log related tests flaky. Also, the reorg safety of the indexed
search was integrated with unindexed search in a weird way, relying on
`syncRange.ValidBlocks` in the unindexed case too, with a special
condition added to only consider the head of the valid range but not the
tail in the unindexed case.
In this PR the current chain view is directly accessible through the
filter backend and unindexed search is also chain view based, making it
inherently safe. The matcher sync mechanism is now only used for indexed
search as originally intended, removing a few ugly special conditions.
The PR is currently based on top of
https://github.com/ethereum/go-ethereum/pull/31642
Together they fix https://github.com/ethereum/go-ethereum/issues/31518
and replace https://github.com/ethereum/go-ethereum/pull/31542
---------
Co-authored-by: Gary Rong <garyrong0905@gmail.com>
The API `eth_feeHistory` returns
`{"jsonrpc":"2.0","id":1,"error":{"code":-32603,"message":"json:
unsupported value: NaN"}}`, when we query `eth_feeHistory` with a old
block that without a blob, or when the field
`config.blobSchedule.cancun.max` in genesis.config is 0 (that happens
for some projects fork geth but they don't have blob).
So here we specially handle the case when maxBlobGas == 0 to prevent
this issue from happening.
This pull request improves error handling for local transaction submissions.
Specifically, if a transaction fails with a temporary error but might be
accepted later, the error will not be returned to the user; instead, the
transaction will be tracked locally for resubmission.
However, if the transaction fails with a permanent error (e.g., invalid
transaction or insufficient balance), the error will be propagated to the user.
These errors returned in the legacyPool are regarded as temporary failure:
- `ErrOutOfOrderTxFromDelegated`
- `txpool.ErrInflightTxLimitReached`
- `ErrAuthorityReserved`
- `txpool.ErrUnderpriced`
- `ErrTxPoolOverflow`
- `ErrFutureReplacePending`
Notably, InsufficientBalance is also treated as a permanent error, as
it’s highly unlikely that users will transfer funds into the sender account
after submitting the transaction. Otherwise, users may be confused—seeing
their transaction submitted but unaware that the sender lacks sufficient funds—and
continue waiting for it to be included.
---------
Co-authored-by: lightclient <lightclient@protonmail.com>
closes#31401
---------
Co-authored-by: Marius van der Wijden <m.vanderwijden@live.de>
Co-authored-by: Guillaume Ballet <3272758+gballet@users.noreply.github.com>
Co-authored-by: Felix Lange <fjl@twurst.com>
BroadcastTransactions needs the Sender address to route message flows
from the same Sender address consistently to the same random subset of
peers. It however spent considerable time calculating the Sender
addresses, even if the Sender address was already calculated and cached
in other parts of the code.
Since we only need the mapping, we can use any signer, and the one that
had already been used is a better choice because of cache reuse.
I added the history mode configuration in eth/ethconfig initially, since
it seemed like the logical place. But it turns out we need access to the
intended pruning setting at a deeper level, and it actually needs to be
integrated with the blockchain startup procedure.
With this change applied, if a node previously had its history pruned,
and is subsequently restarted **without** the `--history.chain
postmerge` flag, the `BlockChain` initialization code will now verify
the freezer tail against the known pruning point of the predefined
network and will restore pruning status. Note that this logic is quite
restrictive, we allow non-zero tail only for known networks, and only
for the specific pruning point that is defined.
As of now, Geth disconnects peers only on protocol error or timeout,
meaning once connection slots are filled, the peerset is largely fixed.
As mentioned in https://github.com/ethereum/go-ethereum/issues/31321,
Geth should occasionally disconnect peers to ensure some churn.
What/when to disconnect could depend on:
- the state of geth (e.g. sync or not)
- current number of peers
- peer level metrics
This PR adds a very slow churn using a random drop.
---------
Signed-off-by: Csaba Kiraly <csaba.kiraly@gmail.com>
Co-authored-by: Felix Lange <fjl@twurst.com>
before this changes, this will result in numerous test failures:
```
> go test -run=Eth2AssembleBlock -c
> stress ./catalyst.test
```
The reason is that after creating/inserting the test chain, there is a
race between the txpool head reset and the promotion of txs added from
tests.
Ensuring that the txpool state is up to date with the head of the chain
before proceeding fixes these flaky tests.
Fixes#31169
The TestTransactionForgotten test was flaky due to real time
dependencies. This PR:
- Replaces real time with mock clock for deterministic timing control
- Adds precise state checks at timeout boundaries
- Verifies underpriced cache states and cleanup
- Improves test reliability by controlling transaction timestamps
- Adds checks for transaction re-enqueueing behavior
The changes ensure consistent test behavior without timing-related
flakiness.
---------
Co-authored-by: Zsolt Felfoldi <zsfelfoldi@gmail.com>
This pull request introduces two constraints in the blobPool:
(a) If the sender has a pending authorization or delegation, only one
in-flight
executable transaction can be cached.
(b) If the authority address in a SetCode transaction is already
reserved by
the blobPool, the transaction will be rejected.
These constraints mitigate an attack where an attacker spams the pool
with
numerous blob transactions, evicts other transactions, and then cancels
all
pending blob transactions by draining the sender’s funds if they have a
delegation.
Note, because there is no exclusive lock held between different subpools
when processing transactions, it's totally possible the SetCode
transaction
and blob transactions with conflict sender and authorities are accepted
simultaneously. I think it's acceptable as it's very hard to be
exploited.
---------
Co-authored-by: lightclient <lightclient@protonmail.com>
This pull request introduces new sync logic for pruning mode. The downloader will now skip
insertion of block bodies and receipts before the configured history cutoff point.
Originally, in snap sync, the header chain and other components (bodies and receipts) were
inserted separately. However, in Proof-of-Stake, this separation is unnecessary since the
sync target is already verified by the CL.
To simplify the process, this pull request modifies `InsertReceiptChain` to insert headers
along with block bodies and receipts together. Besides, `InsertReceiptChain` doesn't have
the notion of reorg, as the common ancestor is always be found before the sync and extra
side chain is truncated at the beginning if they fall in the ancient store. The stale
canonical chain flags will always be rewritten by the new chain. Explicit reorg logic is
no longer required in `InsertReceiptChain`.
This is an alternative to #31309
With eth/68, transaction announcement must have transaction type and
size. So in announceTransactions, we need to query the transaction from
transaction pool with its hash. This creates overhead in case of blob
transaction which needs to load data from billy and RLP decode. This
commit creates a lightweight lookup from transaction hash to transaction
size and a function GetMetadata to query transaction type and
transaction size given the transaction hash.
---------
Co-authored-by: Gary Rong <garyrong0905@gmail.com>
This PR adds `rawdb.SafeDeleteRange` and uses it for range deletion in
`core/filtermaps`. This includes deleting the old bloombits database,
resetting the log index database and removing index data for unindexed
tail epochs (which previously weren't properly implemented for the
fallback case).
`SafeDeleteRange` either calls `ethdb.DeleteRange` if the node uses the
new path based state scheme or uses an iterator based fallback method
that safely skips trie nodes in the range if the old hash based state
scheme is used. Note that `ethdb.DeleteRange` also has its own iterator
based fallback implementation in `ethdb/leveldb`. If a path based state
scheme is used and the backing db is pebble (as it is on the majority of
new nodes) then `rawdb.SafeDeleteRange` uses the fast native range
delete.
Also note that `rawdb.SafeDeleteRange` has different semantics from
`ethdb.DeleteRange`, it does not automatically return if the operation
takes a long time. Instead it receives a `stopCallback` that can
interrupt the process if necessary. This is because in the safe mode
potentially a lot of entries are iterated without being deleted (this is
definitely the case when deleting the old bloombits database which has a
single byte prefix) and therefore restarting the process every time a
fixed number of entries have been iterated would result in a quadratic
run time in the number of skipped entries.
When running in safe mode, unindexing an epoch takes about a second,
removing bloombits takes around 10s while resetting a full log index
might take a few minutes. If a range delete operation takes a
significant amount of time then log messages are printed. Also, any
range delete operation can be interrupted by shutdown (tail uinindexing
can also be interrupted by head indexing, similarly to how tail indexing
works). If the last unindexed epoch might have "dirty" index data left
then the indexed map range points to the first valid epoch and
`cleanedEpochsBefore` points to the previous, potentially dirty one. At
startup it is always assumed that the epoch before the first fully
indexed one might be dirty. New tail maps are never rendered and also no
further maps are unindexed before the previous unindexing is properly
cleaned up.
---------
Co-authored-by: Gary Rong <garyrong0905@gmail.com>
Co-authored-by: Felix Lange <fjl@twurst.com>
This PR adds an extra condition to the log indexer initialization in
order to avoid initializing with block 0 as target head. Previously this
caused the indexer to initialize without a checkpoint. Later, when the
real chain head was set, it indexed the entire history, then unindexed
most of it if only the recent history was supposed to be indexed. Now
the init only happens when there is an actual synced chain head and
therefore the index is initialized at the most recent checkpoint and
only the last year is indexed according to the default parameters.
During checkpoint initialization the best available checkpoint is also
checked against the history cutoff point and fails if the indexing would
have to start from a block older than the cutoff. If initialization
fails then the indexer reverts to unindexed mode instead of retrying
because the the failure conditions cannot be expected to recover later.
This pull request fixes a broken unit test
```
=== CONT TestTracingWithOverrides
api_test.go:1012: result: {"gas":21167,"failed":false,"returnValue":"0x0000000000000000000000000000000000000000000000000000000000000002","structLogs":[{"pc":0,"op":"PUSH1","gas":24978860,"gasCost":3,"depth":1,"stack":[]},{"pc":2,"op":"CALLDATALOAD","gas":24978857,"gasCost":3,"depth":1,"stack":["0x0"]},{"pc":3,"op":"PUSH1","gas":24978854,"gasCost":3,"depth":1,"stack":["0x1"]},{"pc":5,"op":"ADD","gas":24978851,"gasCost":3,"depth":1,"stack":["0x1","0x1"]},{"pc":6,"op":"PUSH1","gas":24978848,"gasCost":3,"depth":1,"stack":["0x2"]},{"pc":8,"op":"MSTORE","gas":24978845,"gasCost":6,"depth":1,"stack":["0x2","0x0"]},{"pc":9,"op":"PUSH1","gas":24978839,"gasCost":3,"depth":1,"stack":[]},{"pc":11,"op":"PUSH1","gas":24978836,"gasCost":3,"depth":1,"stack":["0x20"]},{"pc":13,"op":"RETURN","gas":24978833,"gasCost":0,"depth":1,"stack":["0x20","0x0"]}]}
api_test.go:1013: test 10, result mismatch, have
{21167 false 0x0000000000000000000000000000000000000000000000000000000000000002}
, want
{21167 false 0000000000000000000000000000000000000000000000000000000000000002}
api_test.go:1012: result: {"gas":25664,"failed":false,"returnValue":"0x000000000000000000000000c6e93f4c1920eaeaa1e699f76a7a8c18e3056074","structLogs":[]}
api_test.go:1013: test 11, result mismatch, have
{25664 false 0x000000000000000000000000c6e93f4c1920eaeaa1e699f76a7a8c18e3056074}
, want
{25664 false 000000000000000000000000c6e93f4c1920eaeaa1e699f76a7a8c18e3056074}
```
This is a **breaking change** to the opcode tracer. The top-level
`returnValue` field of a trace will be now hex-encoded. If the return
data is empty, this field will contain "0x".
Fixes#31196
Currently, when answering GetPooledTransaction request, txpool.Get() is
used. When the requested hash is blob transaction, blobpool.Get() is
called. This function loads the RLP-encoded transaction from limbo then
decodes and returns. Later, in answerGetPooledTransactions, we need to
RLP encode again. This decode then encode is wasteful. This commit adds
GetRLP to transaction pool interface so that answerGetPooledTransactions
can use the RLP-encoded from limbo directly.
---------
Co-authored-by: Gary Rong <garyrong0905@gmail.com>
this adds 2 features to improve `geth --dev` experience.
1. we don't need to use `dev_SetFeeRecipient` to set initial coinbase
address. it was a pain.
2. we don't need to unlock keystore if we don't use it. we had it
because of clique.
This pull request enhances the unit test, avoiding unnecessary failure
in CI.
```
--- FAIL: TestSimulatedBeaconSendWithdrawals (12.08s)
simulated_beacon_test.go:139: timed out without including all withdrawals/txs
FAIL
```
Here I am adding a config option and geth flag (`--history.chain`) for
configuring history pruning. There are two options available:
- `--history.chain all` is the default and will keep all history like
before.
- `--history.chain postmerge` will configure the history cutoff point to
the merge block.
The option doesn't actually do anything right now, but we need it as a
precursor for other history pruning changes.
This ensures that if we receive a blob transaction announcement where we cannot
link the tx to the sidecar commitments, we will drop the sending peer. This check
is added in the protocol handler for the PooledTransactions message.
Tests for this have also been added in the cross-client "eth" protocol test suite.
---------
Co-authored-by: Felix Lange <fjl@twurst.com>
In transaction-sending APIs such as `eth_sendRawTransaction`, a submitted transaction
failing the configured txpool validation rules (i.e. fee too low) would cause an error to be
returned, even though the transaction was successfully added into the locals tracker.
Once added there, the transaction may even be included into the chain at a later time,
when fee market conditions change.
This change improves on this by performing the validation in the locals tracker, basically
skipping some of the validation rules for local transactions. We still try to add the tx to the
main pool immediately, but an error will only be returned for transactions which are
fundamentally invalid.
---------
Co-authored-by: Gary Rong <garyrong0905@gmail.com>
I ran into this while trying to debug a discv5 thing. I tried to disable
DNS discovery using `--discovery.dns=false`, which doesn't work.
Annoyingly, geth started anyway and discarded the error silently. I
eventually found my mistake, but it took way longer than it should have.
Also including a small change to the error message for invalid DNS URLs
here. The user actually needs to see the URL to make sense of the error.
This PR does a few things including:
- Remove `ContractRef` interface
- Remove `vm.AccountRef` which implements `ContractRef` interface
- Maintain the `jumpDests` struct in EVM for sharing between call frames
- Simplify the delegateCall context initialization
Currently, when calculating block's bloom, we loop through all the
receipt logs to calculate the hash value. However, normally, after going
through applyTransaction, the receipt's bloom is already calculated
based on the receipt log, so the block's bloom can be calculated by just
ORing these receipt's blooms.
```
goos: darwin
goarch: arm64
pkg: github.com/ethereum/go-ethereum/core/types
cpu: Apple M1 Pro
BenchmarkCreateBloom
BenchmarkCreateBloom/small
BenchmarkCreateBloom/small-10 810922 1481 ns/op 104 B/op 5 allocs/op
BenchmarkCreateBloom/large
BenchmarkCreateBloom/large-10 8173 143764 ns/op 9614 B/op 401 allocs/op
BenchmarkCreateBloom/small-mergebloom
BenchmarkCreateBloom/small-mergebloom-10 5178918 232.0 ns/op 0 B/op 0 allocs/op
BenchmarkCreateBloom/large-mergebloom
BenchmarkCreateBloom/large-mergebloom-10 54110 22207 ns/op 0 B/op 0 allocs/op
```
---------
Co-authored-by: Gary Rong <garyrong0905@gmail.com>
Co-authored-by: Zsolt Felfoldi <zsfelfoldi@gmail.com>
This removes the method `TestingTTDBlock` introduced by #30744. It was
added to make the beacon consensus engine aware of the merge block in
tests without relying on the total difficulty. However, tracking the
merge block this way is very annoying. We usually configure forks in the
`ChainConfig`, but the method is on the consensus engine, which isn't
always created in the same place. By sidestepping the `ChainConfig` we
don't get the usual fork-order checking, so it's possible to enable the
merge before the London fork, for example. This in turn can lead to very
hard-to-debug outputs and validation errors.
So here I'm changing the consensus engine to check the
`MergeNetsplitBlock` instead. Alternatively, we assume a network is
merged if it has a `TerminalTotalDifficulty` of zero, which is a very
common configuration in tests.
Here we add some more changes for live tracing API v1.1:
- Hook `OnSystemCallStartV2` was introduced with `VMContext` as parameter.
- Hook `OnBlockHashRead` was introduced.
- `GetCodeHash` was added to the state interface
- The new `WrapWithJournal` construction helps with tracking EVM reverts in the tracer.
---------
Co-authored-by: Felix Lange <fjl@twurst.com>
This PR changes the signature of `CalcExcessBlobGas` to take in just
the header timestamp instead of the whole object. It also adds a sanity
check for the parent->child block order to `VerifyEIP4844Header`.
A clarification was made to EIP-7691 stating that at the fork boundary
it is required to use the target blob count associated with the head
block, rather than the parent as implemented here.
See for more: https://github.com/ethereum/EIPs/pull/9249
Replaces #29297, descendant from #27535
---------
This PR removes `locals` as a concept from transaction pools. Therefore,
the pool now acts as very a good simulation/approximation of how our
peers' pools behave. What this PR does instead, is implement a
locals-tracker, which basically is a little thing which, from time to
time, asks the pool "did you forget this transaction?". If it did, the
tracker resubmits it.
If the txpool _had_ forgotten it, chances are that the peers had also
forgotten it. It will be propagated again.
Doing this change means that we can simplify the pool internals, quite a
lot.
### The semantics of `local`
Historically, there has been two features, or usecases, that has been
combined into the concept of `locals`.
1. "I want my local node to remember this transaction indefinitely, and
resubmit to the network occasionally"
2. "I want this (valid) transaction included to be top-prio for my
miner"
This PR splits these features up, let's call it `1: local` and `2:
prio`. The `prio` is not actually individual transaction, but rather a
set of `address`es to prioritize.
The attribute `local` means it will be tracked, and `prio` means it will
be prioritized by miner.
For `local`: anything transaction received via the RPC is marked as
`local`, and tracked by the tracker.
For `prio`: any transactions from this sender is included first, when
building a block. The existing commandline-flag `--txpool.locals` sets
the set of `prio` addresses.
---------
Co-authored-by: Gary Rong <garyrong0905@gmail.com>
This PR defines the Osaka fork. An easy first step to start our work on
the next hardfork
(This is needed for EOF testing as well)
---------
Co-authored-by: lightclient <14004106+lightclient@users.noreply.github.com>
The total difficulty is the sum of all block difficulties from genesis
to a certain block. This value was used in PoW for deciding which chain
is heavier, and thus which chain to select. Since PoS has a different
fork selection algorithm, all blocks since the merge have a difficulty
of 0, and all total difficulties are the same for the past 2 years.
Whilst the TDs are mostly useless nowadays, there was never really a
reason to mess around removing them since they are so tiny. This
reasoning changes when we go down the path of pruned chain history. In
order to reconstruct any TD, we **must** retrieve all the headers from
chain head to genesis and then iterate all the difficulties to compute
the TD.
In a world where we completely prune past chain segments (bodies,
receipts, headers), it is not possible to reconstruct the TD at all. In
a world where we still keep chain headers and prune only the rest,
reconstructing it possible as long as we process (or download) the chain
forward from genesis, but trying to snap sync the head first and
backfill later hits the same issue, the TD becomes impossible to
calculate until genesis is backfilled.
All in all, the TD is a messy out-of-state, out-of-consensus computed
field that is overall useless nowadays, but code relying on it forces
the client into certain modes of operation and prevents other modes or
other optimizations. This PR completely nukes out the TD from the node.
It doesn't compute it, it doesn't operate on it, it's as if it didn't
even exist.
Caveats:
- Whenever we have APIs that return TD (devp2p handshake, tracer, etc.)
we return a TD of 0.
- For era files, we recompute the TD during export time (fairly quick)
to retain the format content.
- It is not possible to "verify" the merge point (i.e. with TD gone, TTD
is useless). Since we're not verifying PoW any more, just blindly trust
it, not verifying but blindly trusting the many year old merge point
seems just the same trust model.
- Our tests still need to be able to generate pre and post merge blocks,
so they need a new way to split the merge without TTD. The PR introduces
a settable ttdBlock field on the consensus object which is used by tests
as the block where originally the TTD happened. This is not needed for
live nodes, we never want to generate old blocks.
- One merge transition consensus test was disabled. With a
non-operational TD, testing how the client reacts to TTD is useless, it
cannot react.
Questions:
- Should we also drop total terminal difficulty from the genesis json?
It's a number we cannot react on any more, so maybe it would be cleaner
to get rid of even more concepts.
---------
Co-authored-by: Gary Rong <garyrong0905@gmail.com>
This pull request delivers the new version of the state history, where
the raw storage key is used instead of the hash.
Before the cancun fork, it's supported by protocol to destruct a
specific account and therefore, all the storage slot owned by it should
be wiped in the same transition.
Technically, storage wiping should be performed through storage
iteration, and only the storage key hash will be available for traversal
if the state snapshot is not available. Therefore, the storage key hash
is chosen as the identifier in the old version state history.
Fortunately, account self-destruction has been deprecated by the
protocol since the Cancun fork, and there are no empty accounts eligible
for deletion under EIP-158. Therefore, we can conclude that no storage
wiping should occur after the Cancun fork. In this case, it makes no
sense to keep using hash.
Besides, another big reason for making this change is the current format
state history is unusable if verkle is activated. Verkle tree has a
different key derivation scheme (merkle uses keccak256), the preimage of
key hash must be provided in order to make verkle rollback functional.
This pull request is a prerequisite for landing verkle.
Additionally, the raw storage key is more human-friendly for those who
want to manually check the history, even though Solidity already
performs some hashing to derive the storage location.
---
This pull request doesn't bump the database version, as I believe the
database should still be compatible if users degrade from the new geth
version to old one, the only side effect is the persistent new version
state history will be unusable.
---------
Co-authored-by: Zsolt Felfoldi <zsfelfoldi@gmail.com>
Fix the error comparison in tracer to prevent dropping revert reason data
---------
Co-authored-by: Martin <mrscdevel@gmail.com>
Co-authored-by: rjl493456442 <garyrong0905@gmail.com>
This commit makes it so that the struct logger will not emit logs while
system calls are being executed. This will make it consistent with
the JSON and MD loggers. It is as it stands hard to distinguish when
system calls are being processed vs when a tx is being processed.
---------
Co-authored-by: Sina Mahmoodi <itz.s1na@gmail.com>
This PR implements EIP-7702: "Set EOA account code".
Specification: https://eips.ethereum.org/EIPS/eip-7702
> Add a new transaction type that adds a list of `[chain_id, address,
nonce, y_parity, r, s]` authorization tuples. For each tuple, write a
delegation designator `(0xef0100 ++ address)` to the signing account’s
code. All code reading operations must load the code pointed to by the
designator.
---------
Co-authored-by: Mario Vega <marioevz@gmail.com>
Co-authored-by: Martin Holst Swende <martin@swende.se>
Co-authored-by: Felix Lange <fjl@twurst.com>
This PR modifies how the metrics library handles `Enabled`: previously,
the package `init` decided whether to serve real metrics or just
dummy-types.
This has several drawbacks:
- During pkg init, we need to determine whether metrics are enabled or
not. So we first hacked in a check if certain geth-specific
commandline-flags were enabled. Then we added a similar check for
geth-env-vars. Then we almost added a very elaborate check for
toml-config-file, plus toml parsing.
- Using "real" types and dummy types interchangeably means that
everything is hidden behind interfaces. This has a performance penalty,
and also it just adds a lot of code.
This PR removes the interface stuff, uses concrete types, and allows for
the setting of Enabled to happen later. It is still assumed that
`metrics.Enable()` is invoked early on.
The somewhat 'heavy' operations, such as ticking meters and exp-decay,
now checks the enable-flag to prevent resource leak.
The change may be large, but it's mostly pretty trivial, and from the
last time I gutted the metrics, I ensured that we have fairly good test
coverage.
---------
Co-authored-by: Felix Lange <fjl@twurst.com>
This PR attempts to clean up some ambiguities and quirks from recent
changes to evm flag handling.
This PR mainly focuses on `evm run` subcommand, to use the same flags
for configuring tracing/output options as `statetest/blocktest` does.
Additionally, it adds quite a lot of tests for expected outputs of the
various subcommands, to avoid accidental changes.
---------
Co-authored-by: Felix Lange <fjl@twurst.com>
