Skip to content
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

upstream: merge geth date 1210 #2799

Merged
merged 45 commits into from
Dec 13, 2024
Merged

upstream: merge geth date 1210 #2799

merged 45 commits into from
Dec 13, 2024

Conversation

buddh0
Copy link
Collaborator

@buddh0 buddh0 commented Dec 12, 2024

Description

upstream: merge geth date 1210

Rationale

prepare for prague hardfork

Example

add an example CLI or API response...

Changes

Notable changes:
core, eth, internal, cmd: rework EVM constructor (#30745)
now one evm instance can handle all transactions(except system transactions), so evm pool and interpreter pool in bsc both removed
core, triedb: remove destruct flag in state snapshot (#30752) 
a deep clear up
all: exclude empty outputs in requests commitment (#30670)
have affect on bep-466 in bsc
cmd/bootnode: remove bootnode utility (#30813)
revert, bsc still need it
triedb/pathdb: track flat state changes in pathdb (snapshot integration pt 2)(#30643)
to align with it, rewrite asyncnodebuffer in bsc
eth/downloader: move SyncMode to package eth/ethconfig (#30847)
this lead bsc into import cycle issue, introduce miner/minerconfig module to solve it

holiman and others added 30 commits November 19, 2024 14:51
In many cases, there is a need to create somewhat nontrivial bytecode. A
recent example is the verkle statetests, where we want a `CREATE2`- op
to create a contract, which can then be invoked, and when invoked does a
selfdestruct-to-self.

It is overkill to go full solidity, but it is also a bit tricky do
assemble this by concatenating bytes. This PR takes an approach that
has been used in in goevmlab for several years.

Using this utility, the case can be expressed as: 
```golang
	// Some runtime code
	runtime := program.New().Ops(vm.ADDRESS, vm.SELFDESTRUCT).Bytecode()
	// A constructor returning the runtime code
	initcode := program.New().ReturnData(runtime).Bytecode()
	// A factory invoking the constructor
	outer := program.New().Create2AndCall(initcode, nil).Bytecode()
```

We have a lot of places in the codebase where we concatenate bytes, cast
from `vm.OpCode` . By taking tihs approach instead, thos places can be made a
bit more maintainable/robust.
This pull request refactors the EVM constructor by removing the
TxContext parameter.

The EVM object is frequently overused. Ideally, only a single EVM
instance should be created and reused throughout the entire state
transition of a block, with the transaction context switched as needed
by calling evm.SetTxContext.

Unfortunately, in some parts of the code, the EVM object is repeatedly
created, resulting in unnecessary complexity. This pull request is the
first step towards gradually improving and simplifying this setup.

---------

Co-authored-by: Martin Holst Swende <[email protected]>
Follow-up to #30745 , this change removes some unnecessary parameters.
`ActivePrecompiledContracts()` clones the precompiled contract map, thus
its callsite does not need to clone it
in `trie`
- Replace custom logic with `bytes.HasPrefix`
- Remove unnecessary code in `GetNode`
This pull request removes the destruct flag from the state snapshot to
simplify the code.

Previously, this flag indicated that an account was removed during a
state transition, making all associated storage slots inaccessible.
Because storage deletion can involve a large number of slots, the actual
deletion is deferred until the end of the process, where it is handled
in batches.

With the deprecation of self-destruct in the Cancun fork, storage
deletions are no longer expected. Historically, the largest storage
deletion event in Ethereum was around 15 megabytes—manageable in memory.

In this pull request, the single destruct flag is replaced by a set of
deletion markers for individual storage slots. Each deleted storage slot
will now appear in the Storage set with a nil value.

This change will simplify a lot logics, such as storage accessing,
storage flushing, storage iteration and so on.
…30796)

This change fixes a bug on the `DirectoryFlag` and the `BigFlag`, which would trigger a `panic` with the message "flag redefined" in case an alias was added to such a flag.
This PR improves the output of the markdown logger a bit.

- Remove `RStack` field, 
- Move `Stack` last, since it may have very large vertical expansion
- Make the pre- and post-exec  metadata structured into a bullet-list
#28764 updated `func MakeTopics` to support negative `*big.Int`s.
However, it also changed the behavior of the function from just
_reading_ the input `*big.Int` via `Bytes()`, to leveraging
`big.U256Bytes` which is documented as being _destructive_:

This change updates `MakeTopics` to not mutate the original, and 
also applies the same change in signer/core/apitypes.
This PR is purely for improved readability; I was doing work involving
the file and think this may help others who are trying to understand
what's going on.

1. `snapshot.Tree.Rebuild()` now returns a function that blocks until
regeneration is complete, allowing `Tree.waitBuild()` to be removed
entirely as all it did was search for the `done` channel behind this new
function.
2. Its usage inside `New()` is also simplified by (a) only waiting if
`!AsyncBuild`; and (b) avoiding the double negative of `if !NoBuild`.

---------

Co-authored-by: Martin HS <[email protected]>
This updates the message you get when trying to initialize Geth with
genesis.json that doesn't have `terminalTotalDifficulty`. The previous
message was a bit obscure, I had to check the code to find out what the
problem was.
This PR extends the tracing.StateDB interface by adding a GetCodeHash function.
…810)

This reverts commit 2380012.

The original pull request introduces a bug and some flaky tests are
detected because of this flaw.

```
--- FAIL: TestRecoverSnapshotFromWipingCrash (0.27s)
    blockchain_snapshot_test.go:158: The disk layer is not integrated snapshot is not constructed
{"pc":0,"op":88,"gas":"0x7148","gasCost":"0x2","memSize":0,"stack":[],"depth":1,"refund":0,"opName":"PC"}
{"pc":1,"op":255,"gas":"0x7146","gasCost":"0x1db0","memSize":0,"stack":["0x0"],"depth":1,"refund":0,"opName":"SELFDESTRUCT"}
{"output":"","gasUsed":"0x0"}
{"output":"","gasUsed":"0x1db2"}
{"pc":0,"op":116,"gas":"0x13498","gasCost":"0x3","memSize":0,"stack":[],"depth":1,"refund":0,"opName":"PUSH21"}
```

Before the original PR, the snapshot would block the function until the
disk layer
was fully generated under the following conditions:

(a) explicitly required by users with `AsyncBuild = false`.
(b) the snapshot was being fully rebuilt or *the disk layer generation
had resumed*.

Unfortunately, with the changes introduced in that PR, the snapshot no
longer waits
for disk layer generation to complete if the generation is resumed. It
brings lots of
uncertainty and breaks this tiny debug feature.
Reusing state between benchmark iterations resulted in inconsistent
results across runs, which surfaced in ethereum/go-ethereum#30778 .

If these errors are triggered again, they will now trigger panic. 

---------

Co-authored-by: Martin HS <[email protected]>
Add a comment for error return of nil

Signed-off-by: wangjingcun <[email protected]>
This workaround is meant to minimize the possibility for snapshot generation
once the geth node upgrades to new version (specifically #30752 )

In #30752, the journal format in state snapshot is modified by removing
the destruct set. Therefore, the existing old format (version = 0) will be
discarded and all in-memory layers will be lost. Unfortunately, the lost 
in-memory layers can't be recovered by some other approaches, and the 
entire state snapshot will be regenerated (it will last about 2.5 hours).

This pull request introduces a workaround to adopt the legacy journal if
the destruct set contained is empty. Since self-destruction has been
deprecated following the cancun fork, the destruct set is expected to be nil for
layers above the fork block. However, an exception occurs during contract 
deployment: pre-funded accounts may self-destruct, causing accounts with 
non-zero balances to be removed from the state. For example,
https://etherscan.io/tx/0xa087333d83f0cd63b96bdafb686462e1622ce25f40bd499e03efb1051f31fe49).


For nodes with a fully synced state, the legacy journal is likely compatible with
the updated definition, eliminating the need for regeneration. Unfortunately,
nodes performing a full sync of historical chain segments or encountering 
pre-funded account deletions may face incompatibilities, leading to automatic 
snapshot regeneration.
Small optimization. It's guaranteed that `len(keys)` == `len(values)`,
so we can combine the checks in a single loop rather than 2 separate
loops.
Since we don't really support custom networks anymore, we don't need the
bootnode utility. In case a discovery-only node is wanted, it can still be run using cmd/devp2p.
The existing implementation is correct when building and verifying
blocks, since we will only collect non-empty requests into the block
requests list.

But it isn't correct for cases where a requests list containing empty
items is sent by the consensus layer on the engine API. We want to
ensure that empty requests do not cause a difference in validation
there, so the commitment computation should explicitly skip them.
…on pt 2) (#30643)

This pull request ports some changes from the main state snapshot
integration one, specifically introducing the flat state tracking in
pathdb.

Note, the tracked flat state changes are only held in memory and won't
be persisted in the disk. Meanwhile, the correspoding state retrieval in
persistent state is also not supported yet. The states management in
disk is more complicated and will be implemented in a separate pull
request.

Part 1: ethereum/go-ethereum#30752
This PR introduces a `ContractCodeReader` interface with functions defined:

type ContractCodeReader interface {
	Code(addr common.Address, codeHash common.Hash) ([]byte, error)
	CodeSize(addr common.Address, codeHash common.Hash) (int, error)
}

This interface can be implemented in various ways. Although the codebase
currently includes only one implementation, additional implementations
could be created for different purposes and scenarios, such as a code
reader designed for the Verkle tree approach or one that reads code from
the witness.

*Notably, this interface modifies the function’s semantics. If the
contract code is not found, no error will be returned. An error should
only be returned in the event of an unexpected issue, primarily for
future implementations.*

The original state.Reader interface is extended with ContractCodeReader
methods, it gives us more flexibility to manipulate the reader with additional
logic on top, e.g. Hooks.

type Reader interface {
	ContractCodeReader
	StateReader
}

---------

Co-authored-by: Felix Lange <[email protected]>
This change relocates the EVM tx context switching to the ApplyMessage function.
With this change, we can remove a lot of EVM.SetTxContext calls before
message execution.

### Tracing API changes

- This PR replaces the `GasPrice` field of the `VMContext` struct with
  `BaseFee`. Users may instead take the effective gas price from
  `tx.EffectiveGasTipValue(env.BaseFee)`.

---------

Co-authored-by: Sina Mahmoodi <[email protected]>
When a tx/block was being traced through the API the state hooks weren't
being called as they should. This is due to #30745 moving the hooked
statedb one level up in the state processor. This PR fixes that.

---------

Co-authored-by: Martin HS <[email protected]>
Co-authored-by: Gary Rong <[email protected]>
* unify `staterunner` and `blockrunner` CLI flags, especially around
tracing
* added support for struct logger or json logging (although having issue
#30658)
* new --cross-check flag to validate the stateless witness collection
  / execution matches stateful
* adds support for tracing the stateless execution when a tracer is set
  (to more easily debug differences)
* --human for more readable test summary
* directory or file input, so if you pass tests/spec-tests/fixtures/blockchain_tests it will execute all
blockchain tests
holiman and others added 15 commits December 2, 2024 15:43
The fuzzer added recenly to fuzz the eth handler doesn't
build on oss-fuzz, because it also has dependencies in the peer_test.go.

This change fixes it, I hope, by adding that file also for preprocessing.
This flag is very rarely needed, so it's OK for it to have a verbose
name. The name --trace also conflicts with the concept of EVM tracing,
which is much more heavily used.
Lots of packages depend on eth/downloader just for the SyncMode type.
Since we have a dedicated package for eth protocol configuration, it
makes more sense to define SyncMode there, turning eth/downloader into
more of a leaf package.
…0806)

This PR refactors the structlog a bit, making it so that it can be used
in a streaming mode.

-------------

OBS: this PR makes a change in the input `config` config, the third
input-parem field to `debug.traceCall`. Previously, seteting it to e.g.
` {"enableMemory": true, "limit": 1024}` would mean that the response
was limited to `1024` items. Since an 'item' may include both memory and
storage, the actual size of the response was undertermined.
After this change, the response will be limited to `1024` __`bytes`__
(or thereabouts).



-----------


The commandline usage of structlog now uses the streaming mode, leaving
the non-streaming mode of operation for the eth_Call.

There are two benefits of streaming mode 
1. Not have to maintain a long list of operations, 
2. Not have to duplicate / n-plicate data, e.g. memory / stack /
returndata so that each entry has their own private slice.


---------

Co-authored-by: Gary Rong <[email protected]>
This PR extends the Hooks interface with a new method,
`OnSystemCallStartV2`, which takes `VMContext` as its parameter.

Motivation

By including `VMContext` as a parameter, the `OnSystemCallStartV2` hook
achieves parity with the `OnTxStart` hook in terms of provided insights.
This alignment simplifies the inner tracer logic, enabling consistent
handling of state changes and internal calls within the same framework.

---------

Co-authored-by: Sina Mahmoodi <[email protected]>
…n7 (#30856)

This imports the following fixes:

 - update gnark to 1.1.0
 - update go-verkle to 0.2.2
 - fix: main storage offset bug (gballet/go-ethereum#329)
 - fix: tree key generation (gballet/go-ethereum#401)

---------

Signed-off-by: Guillaume Ballet <[email protected]>
Co-authored-by: Ignacio Hagopian <[email protected]>
… 3) (#30650)

It's a pull request based on ethereum/go-ethereum#30643

In this pull request, the partial functional state reader is enabled if **legacy snapshot
is not enabled**. The tracked flat states in pathdb will be used to serve the state
retrievals, as the second implementation to fasten the state access.

This pull request should be a noop change in normal cases.
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 <[email protected]>
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 <[email protected]>
This PR adds the error fragments to `func (abi ABI) getArguments` which
allows typed decoding of errors.
@buddh0 buddh0 changed the base branch from master to develop December 12, 2024 08:39
@buddh0 buddh0 marked this pull request as ready for review December 12, 2024 09:00
@zzzckck zzzckck merged commit 75365d1 into develop Dec 13, 2024
5 of 7 checks passed
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.