Skip to content

eth/consensus : implement eccpow consensus engine#10

Open
mmingyeomm wants to merge 3518 commits into
cryptoecc:worldlandfrom
ethereum:master
Open

eth/consensus : implement eccpow consensus engine#10
mmingyeomm wants to merge 3518 commits into
cryptoecc:worldlandfrom
ethereum:master

Conversation

@mmingyeomm

Copy link
Copy Markdown

implements eccpow consensus engine for Worldland Network

CPerezz and others added 30 commits April 10, 2026 19:23
`BinaryTrie.DeleteAccount` was a no-op, silently ignoring the caller's
deletion request and leaving the old `BasicData` and `CodeHash` in the
trie.

Co-authored-by: Guillaume Ballet <3272758+gballet@users.noreply.github.com>
…before returning values (#34690)

Fix `GetAccount` returning **wrong account data** for non-existent
addresses when the trie root is a `StemNode` (single-account trie) — the
`StemNode` branch returned `r.Values` without verifying the queried
address's stem matches.

Co-authored-by: Guillaume Ballet <3272758+gballet@users.noreply.github.com>
Pre-refactor PR to get 8037 upstreamed in chunks

---------

Co-authored-by: Gary Rong <garyrong0905@gmail.com>
The spec has been changed during SIC #49, the offset is encoded as a
big-endian number.
Two fixes for `testing_buildBlockV1`:

1. Add `omitempty` to `SlotNumber` in `ExecutableData` so it is omitted
for pre-Amsterdam payloads. The spec defines the response as
`ExecutionPayloadV3` which does not include `slotNumber`.

2. Pass `res.fees` instead of `new(big.Int)` in `BuildTestingPayload` so
`blockValue` reflects actual priority fees instead of always being zero.

Corresponding fixture update: ethereum/execution-apis#783
The comment formula showed (i+3) but the code multiplies by 9 (Lsh 3 +
add = 8+1).
This was a error when porting from upstream golang.org/x/crypto/bn256
where ξ=i+3.
Go-ethereum changed the constant to ξ=i+9 but forgot to update the inner
formula.
TestUpdatedKeyfileContents was intermittently failing with:

- Emptying account file failed
- wasn't notified of new accounts

Root cause: waitForAccounts required the account list match and an
immediately readable ks.changes notification in the same instant,
creating a timing race between cache update visibility and channel
delivery.

This change keeps the same timeout window but waits until both
conditions are observed, which preserves test intent while removing the
flaky timing dependency.

Validation:
- go test ./accounts/keystore -run '^TestUpdatedKeyfileContents$'
-count=100
Return ErrInvalidOpCode with the executing opcode and offending
immediate for forbidden DUPN, SWAPN, and EXCHANGE operands. Extend
TestEIP8024_Execution to assert both opcode and operand for all
invalid-immediate paths.
# Summary

Replaces the inline `errors.New("event signature mismatch")` in
generated `UnpackXxxEvent` methods with per-event package-level sentinel
errors (e.g. `ErrTransferSignatureMismatch`,
`ErrApprovalSignatureMismatch`), allowing callers to reliably
distinguish a topic mismatch from a genuine decoding failure via
`errors.Is`.

Each event gets its own sentinel, generated via the abigen template:

```go
var ErrTransferSignatureMismatch = errors.New("event signature mismatch")
```

This scoping is intentional — it allows callers to be precise about
*which* event was mismatched, which is useful when routing logs across
multiple unpackers.

# Motivation
Previously, all errors returned from `UnpackXxxEvent` were
indistinguishable without string matching. This is especially
problematic when processing logs sourced from `eth_getBlockReceipts`,
where a caller receives the full set of logs for a block across all
contracts and event types. In that context, a signature mismatch is
expected and should be skipped, while any other error (malformed data,
topic parsing failure) indicates something is genuinely wrong and should
halt execution:

```go
for _, log := range blockLogs {
    event, err := contract.UnpackTransferEvent(log)
    if errors.Is(err, gen.ErrTransferSignatureMismatch) {
        continue // not our event, expected
    }
    if err != nil {
        return fmt.Errorf("unexpected decode failure: %w", err) // alert
    }
    // process event
}
```

**Changes:**
- `abigen` template: generates a `ErrXxxSignatureMismatch` sentinel per
event and returns it on topic mismatch instead of an inline error
- Existing generated bindings & testdata: regenerated to reflect the
update

Implements #34075
This fixes a truncation bug that results in an invalid serialization of
empty EIP712.

For example:

```json
{
    "method": "eth_signTypedData_v4",
    "request": {
        "types": {
            "EIP712Domain": [
                {
                    "name": "version",
                    "type": "string"
                }
            ],
            "Empty": []
        },
        "primaryType": "Empty",
        "domain": {
            "version": "0"
        },
        "message": {}
    }
}
``` 

When calculating the type-hash for the stuct-hash, it will incorrectly
use `Empty)` instead of `Empty()`
runtime.setDefaults was unconditionally assigning cfg.Random =
&common.Hash{}, which silently overwrote any caller-provided Random
value. This made it impossible to simulate a specific PREVRANDAO and
also forced post-merge rules whenever London was active, regardless of
the intended environment.

This change only initializes cfg.Random when it is nil, matching how
other fields in Config are defaulted. Existing callers that did not set
Random keep the same behavior (a non-nil zero hash still enables
post-merge semantics), while callers that explicitly set Random now get
their value respected.
Fixes #34108

The UDPv5 test harness (`newUDPV5Test`) uses the default `PingInterval`
of 3 seconds. When tests like `TestUDPv5_findnodeHandling` insert nodes
into the routing table via `fillTable`, the table's revalidation loop
may schedule PING packets for those nodes. Under the race detector or on
slow CI runners, the test runs long enough for revalidation to fire,
causing background pings to be written to the test pipe. The `close()`
method then finds these as unmatched packets and fails.

The fix sets `PingInterval` to a very large value in the test harness so
revalidation never fires during tests.

Verified locally: 100 iterations with `-race -count=100` pass reliably,
where previously the test would fail within ~50 iterations.
…#34043)

This fixes the remaining Hive discv5/FindnodeResults failures in the
cmd/devp2p/internal/v5test fixture.

The issue was in the simulator-side bystander behavior, not in
production discovery logic. The existing fixture could get bystanders
inserted into the remote table, but under current geth behavior they
were not stable enough to remain valid FINDNODE results. In
particular, the fixture still had a few protocol/behavior mismatches:

- incomplete WHOAREYOU recovery
- replies not consistently following the UDP envelope source
- incorrect endpoint echoing in PONG
- fixture-originated PING using the wrong ENR sequence
- bystanders answering background FINDNODE with empty NODES

That last point was important because current lookup accounting can
treat repeatedly unhelpful FINDNODE interactions as failures. As a
result, a bystander could become live via PING/PONG and still later be
dropped from the table before the final FindnodeResults assertion.
This change updates the fixture so that bystanders behave more like
stable discv5 peers:

- perform one explicit initial handshake, then switch to passive response handling
- resend the exact challenged packet when handling WHOAREYOU
- reply to the actual UDP packet source and mirror that source in PONG.ToIP / PONG.ToPort
- use the bystander’s own ENR sequence in fixture-originated PING
- prefill each bystander with the bystander ENR set and answer FINDNODE from that set

The result is that the fixture now forms a small self-consistent lookup
environment instead of a set of peers that are live but systematically
poor lookup participants.
This is meant to be run daily, in order to verify the FreeBSD build
wasn't broken like last time.
Adds config to add Prague prune point for the hoodi testnet.
Optimizes the transient storage. Turns it from a map of maps into a single map keyed by <account,slot>.
This PR simplifies the implementation of EIP-7610 by eliminating the
need to check storage emptiness during contract deployment.

EIP-7610 specifies that contract creation must be rejected if the
destination account has a non-zero nonce, non-empty runtime code, or 
**non-empty storage**.

After EIP-161, all newly deployed contracts are initialized with a nonce
of one. As a result, such accounts are no longer eligible as deployment 
targets unless they are explicitly cleared.

However, prior to EIP-161, contracts were initialized with a nonce of
zero. This made it possible to end up with accounts that have:

- zero nonce
- empty runtime code
- non-empty storage (created during constructor execution)
- non-zero balance

These edge-case accounts complicate the storage emptiness check.

In practice, contract addresses are derived using one of the following
formulas:
- `Keccak256(rlp({sender, nonce}))[12:]`
- `Keccak256([]byte{0xff}, sender, salt[:], initHash)[12:]`

As such, an existing address is not selected as a deployment target
unless a collision occurs, which is extremely unlikely.

---

Previously, verifying storage emptiness relied on GetStorageRoot.
However, with the transition to the block-based access list (BAL), 
the storage root is no longer available, as computing it would require 
reconstructing the full storage trie from all mutations of preceding 
transactions.

To address this, this PR introduces a simplified approach: it hardcodes
the set of known accounts that have zero nonce, empty runtime code, 
but non-empty storage and non-zero balance. During contract deployment, 
if the destination address belongs to this set, the deployment is
rejected.

This check is applied retroactively back to genesis. Since no address
collision events have occurred in Ethereum’s history, this change does
not
alter existing behavior. Instead, it serves as a safeguard for future
state
transitions.
… `testing_buildBlockV1` (#34722)

This is a copy of #34721 but against `master` (rather than
`bal-devnet-3`), as requested by @jwasinger, since the slotnum logic now
exists on `master` as well.
#34723)

StateDB.Commit first commits all storage changes into the storage trie,
then updates the account metadata with the new storage root into the 
account trie.

Within StateDB.Commit, the new storage trie root has already been
computed and applied as the storage root. This PR explicitly skips the 
redundant storage trie root assignment for readability.
Auto-enable logic for `StatelessSelfValidation` was reading CLI flag
directly via `ctx.Bool()`, bypassing the merged `cfg.EnableWitnessStats`
value. Now uses `cfg.EnableWitnessStats` so config file settings trigger
the same auto-enable behavior as CLI flags.
Changes the log handler to check for vmodule level overrides
even for messages above the current level. This enables the user to selectively
hide messages from certain packages, among other things.

Also fixes a bug where handler instances created by WithAttr would not follow
the level setting anymore. The WithAttrs method is calledd by slog.Logger.With,
which we also use in go-ethereum to create context specific loggers with
pre-filled attributes. Under the previous implementation of WithAttrs, if the
application created a long-lived logger (for example, for a specific peer), then
that logger would not be affected by later level changes done on the top-level
logger, leading to potentially missed events.

Closes: #30717

---------

Co-authored-by: Marius van der Wijden <m.vanderwijden@live.de>
Co-authored-by: Felix Lange <fjl@twurst.com>
`trace.noreturndata` is documented as "enable return data output" but
the flag name/value imply it disables return data. This is confusing for
users and likely inverted wording. Update the Usage string to reflect
the actual behavior (disable return data output).
…34700)

This Pr implements some prerequisite changes for #34004 : split the
`CachingDB` into a `MerkleDB` and a `UBTDB`, so that very different
behaviors don't clash as much.

The transition isn't handled by this PR, but after talking to Gary we
agreed that `UBTDB` should receive another `triedb`, which will only be
loaded if the `Ended` flag is set to false in the conversion contract.
If this is too hard to achieve, it makes sense to load it regardless,
and then loading can be prevented at a later stage by adding a
`UBTTransitionFinalizationTime` in `ChainConfig`.

---------

Co-authored-by: Gary Rong <garyrong0905@gmail.com>
Adds a 'code' exporter to 'geth db export' that iterates over all
contract bytecode entries (CodePrefix + code_hash -> bytecode).

Usage: geth --datadir <dir> db export code code.rlp

This enables exporting contract bytecode.
…entity (#34733)

`timedExec` compares errors by direct interface inequality (haveErr !=
err). If execFunc returns newly constructed errors with the same message
each run, this will panic even though behavior is equivalent.
… amplification (#34754)

## Problem

`BinaryTrie.Commit` unconditionally walked every resolved in-memory node
and flushed it into the `NodeSet`, producing one Pebble write per
resolved internal + stem node on every block — even when the node's
on-disk blob was bitwise identical to the previous commit. On a warm
400M-state workload this meant tens of thousands of redundant 65-byte
writes per block, compounding Pebble compaction pressure on every
commit.

The existing `mustRecompute` flag tracks *hash* staleness, not
*disk-blob* staleness: after `Hash()` completes, `mustRecompute` is
cleared even though the fresh blob has not been persisted. It is
therefore insufficient for a skip-flush optimization.

## Fix

Mirror the MPT committer pattern (`trie/committer.go:51-56`) by adding a
`dirty` flag on `InternalNode` and `StemNode` with the semantics *the
on-disk blob is stale*. The flag is:

- set to `true` wherever the node is created or structurally modified
(the same call sites that already set `mustRecompute = true`);
- set to `false` only after the node has been passed to the `flushfn`
inside `CollectNodes`;
- left `false` on nodes produced by `DeserializeNodeWithHash`, matching
the *loaded from disk, already persisted* semantics.

`CollectNodes` short-circuits on `!dirty` subtrees. The propagation
invariant (an ancestor of any dirty node is itself dirty) is already
maintained by the existing `InsertValuesAtStem` / `Insert` paths, which
now mirror every `mustRecompute = true` setter with a `dirty = true`
setter.

## Benchmark

New `BenchmarkCollectNodes_SparseWrite` measures commit cost when only
one leaf changes between blocks — the common case for state updates.
10,000-stem trie, one-leaf modification + Commit per iteration, Apple M4
Pro:

| | before | after | delta |
|---|---|---|---|
| time / op | 12,653,000 ns | 7,336 ns | **~1,725×** |
| bytes / op | 107,224,740 B | 37,774 B | **~2,839×** |
| allocs / op | 80,953 | 134 | **~604×** |

End-to-end impact on a real workload depends on the
resolved-footprint-to-dirty-path ratio; the new
`TestBinaryTrieCommitIncremental` provides a structural regression guard
(asserts that a Commit following a single-leaf modification flushes a
root-to-leaf path, not the whole tree).

---

Found all of this stuff while bloating my #34706 DB to make some
benchmarks. And saw we were spending A LOT OF TIME on hashing.
Hope this helps the perf a bit. Will rebase the flat-state PR on top of
this once merged.
cuiweixie and others added 30 commits June 12, 2026 23:09
This PR fixes an issue that when peers legitimately lack a requested
BAL, empty (0x80) is delivered and this BAL entry will be refetched 
over and over again. 

A `refused` tracker is added and catchUp will fail if this BAL is
unavailable against the entire peerset.
`TestTracingHTTPTimeout` still flakes in CI after #35101, failing at the
POST:

    --- FAIL: TestTracingHTTPTimeout (0.26s)
        tracing_test.go:633: request: Post "http://127.0.0.1:43497": EOF

The test sets a short server `WriteTimeout` and posts a blocking call.
`ContextRequestTimeout` leaves a fixed 100ms for the server to write its
timeout response before the HTTP write deadline cuts the connection.

I can't repro it locally, but my theory is that under load that write
can miss the window, so the connection is dropped and the client POST
returns `EOF`, failing the test before it inspects the span. This is the
only test exposed to it because it is the only one that configures a
`WriteTimeout`.

The EOF is benign: the server sets the timeout error on the SERVER span
before attempting the write, independent of whether the client receives
the response. Since that span status is all the test asserts,
`tryPostJSONRPC` tolerates the transport error instead of failing on it.
The stack primitives pop by value: pop() returns the 32-byte value
itself, so every popped operand is copied out of the stack arena before
it is used. The result side was already in place, peek returns a pointer
and binary ops write into the new stack top. This PR fixes the operand
side: pointer-returning primitives (popPtr, popPtrPeek, etc), with the
handlers rewritten to read operands directly from their arena slots.
Every popped operand paid the copy, whatever the op went on to do with
it, so this optimization covers the arithmetic and comparison ops as
much as JUMP, MSTORE, SSTORE and RETURN.

The copy is visible in the assembly. On arm64, master's opLt spends four
instructions moving the popped value through the frame, and the
comparison then reads it back from there:

LDP (R5), (R6, R7) ; load words 0 and 1 of the popped value from the
arena
    LDP  16(R5), (R5, R8)            ; load words 2 and 3
STP (R6, R7), vm.~r0-64(SP) ; store words 0 and 1 into a frame slot
    STP  (R5, R8), vm.~r0-48(SP)     ; store words 2 and 3

With popPtrPeek those four instructions are gone, the frame shrinks from
locals=0x58 to locals=0x18, and the function from 336 to 288 bytes. The
compiler cannot remove the copy itself: uint256.Int is a four-element
array, and Go's SSA does not promote arrays longer than one element to
registers, so a by-value pop pays this round trip no matter how far
inlining gets, for LT exactly as for ADD.

The CALL and CREATE families are deliberately not converted: a child
frame reuses the same stack arena, so parent pointers into popped slots
die when the child pushes. The rule is recorded on the primitives:
pointers stay valid until the next push or any sub call. Converting the
call family safely means materializing scalars before the child call,
left for later work with a call-heavy benchmark to justify it.

### Benchmarks

Measured with the benchmark suite from #35144 (the evm-bench contract
workloads and the block import benchmark), which is not part of this
PR's diff. Apple M4 Max, fixed iteration counts, n=10, all p=0.000. B/op
and allocs/op are statistically identical on every benchmark:

| benchmark | master | PR | vs master |
|---|---|---|---|
| Snailtracer | 60.0 ms | 54.1 ms | -9.8% |
| TenThousandHashes | 13.2 ms | 12.2 ms | -7.8% |
| ERC20Transfer | 11.7 ms | 11.0 ms | -5.5% |
| ERC20Mint | 7.49 ms | 7.02 ms | -6.2% |
| ERC20ApprovalTransfer | 8.92 ms | 8.44 ms | -5.4% |

This PR is independent of #35144 but plays nicely with it: the generated
dispatch there splices these handler bodies, so the in-place forms land
in its fast path too, where they measure larger.

### Testing

The rewritten handlers run on the interpreter's only execution path, so
correctness rests on references outside the change:

- **Consensus fixtures.** The full tests package passes: state tests,
the execution-spec families, blockchain tests.
- **Opcode testcases.** The JSON testcases compare individual opcode
results against committed expected values.
- **Tracer fixtures.** The tracetest reference files pin exact log and
return data shapes, covering the rewritten LOG and RETURN paths.
- **Cross-build differential.** A goevmlab campaign running this
branch's evm against master's evm over generated state tests across four
forks (Prague, Cancun, London, Osaka) with full trace comparison:
160,566 tests, zero divergences.

---------

Co-authored-by: MariusVanDerWijden <m.vanderwijden@live.de>
…#35170)

sendInvalidTxs's *eth.TransactionsPacket case iterated `txs` — the
locally-sent invalid transactions, every one of which is in `invalids`
by construction — instead of the transactions actually carried by the
received packet. As a result the loop returned "received bad tx" on the
very first TransactionsPacket the peer sent, regardless of its contents,
and never inspected what was really propagated.

Iterate msg.Items() (the decoded contents of the received packet) so the
"node must not propagate invalid txs" conformance check tests the real
condition instead of producing a false negative.

---------

Co-authored-by: Bosul Mun <bsbs8645@snu.ac.kr>
This PR improves the slot reservation logic in the context of snap/2.

Geth has the mechanism to reserve roughly half the peer slots for peers 
supporting the snap protocol if snap syncing is needed by local node.

With the context of snap/2, this mechanism should be changed that:
we reserve the slot for the "usable snap peer", not blindly for peer
with snap extension enabled (such as legacy snap/1, which can't serve
the snap/2).
)

This PR introduces a new condition that if the local node falls behind
too much and the required BAL for catching up is very likely to be
unavailable, the entire snap sync will be restarting from scratch.

As the defined BAL retention window is weak-subjective-period which is
calculated dynamically. A more conservative threshold is used (90K
blocks) for robustness.

Apart from that, the BAL catchup will be divided into several spans and
apply one by one. It's essential to prevent the potential out-of-memory
panic of placing the entire BAL set in memory.
This PR does two things:

- Expose snap/2 specific sync progress fields
- Seed the sync progress after `loadSyncStatus `
This PR fixes an issue where flat states are continuously persisted
during downloadState, while the sync journal is only persisted at the
end of Sync.

As a result, an unclean shutdown can leave the on-disk flat state ahead
of the journal markers. Some persisted entries may be stale (storage
slots that should have been deleted), and these dangling entries are not
detected or fixed by subsequent state downloads.

To address this, this PR introduces a cleanup step before state
downloading begins. It removes all state entries that are not covered by
the persisted journal markers.
Adds `testing_commitBlockV1`. It is the write companion of `testing_buildBlockV1`:
it builds a block from the provided payload attributes and transactions on
top of the current canonical head, inserts it, and sets it as the new
head, returning the new head hash.

---------

Co-authored-by: MariusVanDerWijden <m.vanderwijden@live.de>
Since go 1.18 reflect has `reflect.Pointer` which replaces
`reflect.Ptr`. Newer versions of `govet` will alert. 
See also: https://pkg.go.dev/reflect#pkg-constants
The timer should wait the remaining time, not the elapsed time.
This PR drops support for v0 blob sidecar in blobpool. Since the
osaka fork activation time has passed, these code paths are
now unused. It is assumed that only v1 transactions exist in
the blobpool.
This PR inlines the gas deduction by getting rid of the tracer and use
`chargeRegularOnly` for the non-state opcode.

It fixes a performance regression introduced by EIP-8037 PR.

```

throughput MGas/s | 184.4 (±0.3%) | 193.1 (±1.0%) | +4.7% ▲
-- | -- | -- | --
mean newPayload | 164.2 ms (±0.3%) | 156.9 ms (±1.0%) | -4.5% ▲
p50 newPayload | 154.6 ms (±0.1%) | 147.6 ms (±0.7%) | -4.5% ▲
p95 newPayload | 273.3 ms (±2.3%) | 261.6 ms (±2.4%) | -4.3% ≈ noise
p99 newPayload | 403.6 ms (±4.4%) | 380.9 ms (±4.0%) | -5.6% ≈ noise

```
Mirror the guard applied to (*UDPv4).Dial in #34916: when the target
node has no usable UDP endpoint, return errNoUDPEndpoint instead of
silently sending the ENRRequest to an invalid AddrPort and waiting for a
timeout.

The other UDPEndpoint-using request paths in this file already do this:

  ping        v4_udp.go:215   errNoUDPEndpoint
  Ping        v4_udp.go:228   errNoUDPEndpoint
  newLookup   v4_udp.go:309   errNoUDPEndpoint
  RequestENR  v4_udp.go:358   addr, _ := n.UDPEndpoint()  <-- outlier

RequestENR is reachable from external callers like cmd/devp2p/crawl.go,
which feeds in arbitrary nodes that may not have a UDP port set. Before
this change, such nodes burn one full RPC timeout; after it, the caller
gets a clean error immediately.

The added test fails on master with "RPC timeout" and the trace logs
"PING/v4 addr=invalid AddrPort", confirming packets are being written to
an unspecified address; with the fix it returns errNoUDPEndpoint without
doing any I/O.
This PR adds the support of Pebble v2, details as below:

- Pebble V2 will be used if database is empty
- Pebble V1 will be used if database is not empty and the format is old
- Upgrade command (geth db pebble-upgrade) is provided to upgrade the
format to v2 offline
When ancient history is pruned, geth serves old block bodies and receipts
back from era files on disk. Until now that fallback only worked for .era1
files (pre-merge), so requests for post-merge blocks backed by .ere files
failed even though the data was present. This PR generalizes the era store
to open both formats.

---------

Co-authored-by: Sina Mahmoodi <itz.s1na@gmail.com>
…es (#34772)

Replace 1-byte-per-bit path encoding with bit-packed `BitArray`,
reducing DB key size by 8x

Benchmark (sparse single-leaf write, M3 Pro):
```
                           │  Before (1B/bit)  │  After (BitArray)  │
                           │      sec/op       │  sec/op   vs base  │
CollectNodesSparseWrite-11        10.50µ ± 1%   9.78µ ± 1%  -6.86%
                           │       B/op        │   B/op    vs base  │
CollectNodesSparseWrite-11       5.50Ki ± 0%   5.09Ki ± 0%  -7.38%
                           │    allocs/op      │ allocs   vs base   │
CollectNodesSparseWrite-11          67 ± 0%       58 ± 0%  -13.43%
```

---------

Co-authored-by: Guillaume Ballet <3272758+gballet@users.noreply.github.com>
Implements spec change ethereum/EIPs#11807

This PR resolves the conflict between the EIP-7928 and EIP-8037.
Specifically in contract deployment, EIP-7928 requires to not resolve 
the deployed account until it's accessed, while in EIP-8037, the early 
access is required to determine if the account-creation should
be charged or not.

This PR addresses this conflict by changing the EIP-8037 a bit,
unconditionally charge the account creation in CREATE Family 
(CreateTx, Create/Create2 opcode) and refunds the associated 
gas cost if the account creation doesn't happen ultimately.

Checkout https://hackmd.io/@bFEBbZiVSAO0IURh9qzEFg/BJmFYqCeGl for more
details

What's more, now the LIFO mechanism is used for refilling the state cost
in frame revert, frame halt, state opcode refunds.
This PR improves the block download used by snap sync. Specifically,
blocks and their associated data (receipts and canonical hash mappings)
are now written directly to the database without checking existence.

The current implementation could fail in cases where the block header
and body were already present (has.Block returns true), but the
corresponding canonical hash mapping was missing. One possible scenario
is when a newPayload event is processed without a subsequent
forkChoiceUpdate.

It is still unclear why Geth may re-enter snap sync after Engine API
events have been processed after the sync. Anyway, bypassing the
existence is a reasonable change.

What's more, in the downloader, the presence of canonical hash is also
considered for deciding the range of blocks to be downloaded.
Specifically:

- in the full sync, the block with header and body available but
canonical hash missing will be re-inserted;
- in the snap sync, the block with header, body and receipt available
but canonical hash missing will be re-inserted;
This PR addresses the panic in tests. As the eventLoop is spun up when
the downloader was closed, the sub will be nil and make the panic
happens.

```
  goroutine 421 [running]:
  github.com/ethereum/go-ethereum/eth/downloader.(*DownloaderAPI).eventLoop(0xcb0e4d0)
      /opt/actions-runner/_work/go-ethereum/go-ethereum/eth/downloader/api.go:91 +0x127
  created by github.com/ethereum/go-ethereum/eth/downloader.NewDownloaderAPI in goroutine 352
      /opt/actions-runner/_work/go-ethereum/go-ethereum/eth/downloader/api.go:50 +0xf2
```
## Summary

- Release the storage iterator after iterating slots in `geth snapshot
dump`, matching the existing account iterator cleanup.

## Test plan

- [x] `go build ./cmd/geth/...`
- [ ] Manual: run `geth snapshot dump` on a node with storage data and
verify output is unchanged
Implements https://eips.ethereum.org/EIPS/eip-2780

---------

Co-authored-by: Marius van der Wijden <m.vanderwijden@live.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.