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>
It's a pull request based on https://github.com/ethereum/go-ethereum/pull/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 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 <itz.s1na@gmail.com>
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 <garyrong0905@gmail.com>
* 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
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 <itz.s1na@gmail.com>
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 <fjl@twurst.com>
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.
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.
This reverts commit 23800122b3.
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.
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 <martin@swende.se>
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.
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 <martin@swende.se>
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 adds an API method `DropTransactions` to legacy pool, blob pool and
txpool interface. This method removes all txs currently tracked in the
pools.
It modifies the simulated beacon to use the new method in `Rollback`
which removes previous hacky implementation that also erroneously reset
the gas tip to 1 gwei.
---------
Co-authored-by: Felix Lange <fjl@twurst.com>
This change invokes the OnCodeChange hook when selfdestruct operation is performed, and a contract is removed. This is an event which can be consumed by tracers.
This PR fixes some issues with benchmarks
- [x] Removes log output from a log-test
- [x] Avoids a `nil`-defer in `triedb/pathdb`
- [x] Fixes some crashes re tracers
- [x] Refactors a very resource-expensive benchmark for blobpol.
**NOTE**: this rewrite touches live production code (a little bit), as
it makes the validator-function used by the blobpool configurable.
- [x] Switch some benches over to use pebble over leveldb
- [x] reduce mem overhead in the setup-phase of some tests
- [x] Marks some tests with a long setup-phase to be skipped if `-short`
is specified (where long is on the order of tens of seconds). Ideally,
in my opinion, one should be able to run with `-benchtime 10ms -short`
and sanity-check all tests very quickly.
- [x] Drops some metrics-bechmark which times the speed of `copy`.
---------
Co-authored-by: Sina Mahmoodi <itz.s1na@gmail.com>
Tests that are crucial to for verifying the verkle testnet functions properly.
---------
Signed-off-by: Guillaume Ballet <3272758+gballet@users.noreply.github.com>
Co-authored-by: Ignacio Hagopian <jsign.uy@gmail.com>
Co-authored-by: Gary Rong <garyrong0905@gmail.com>
Co-authored-by: Martin HS <martin@swende.se>
I think the core code should generally be agnostic about the witness and
the statedb layer should determine what elements need to be included in
the witness. Because code is accessed via `GetCode`, and
`GetCodeLength`, the statedb will always know when it needs to add that
code into the witness.
The edge case is block hashes, so we continue to add them manually in
the implementation of `BLOCKHASH`.
It probably makes sense to refactor statedb so we have a wrapped
implementation that accumulates the witness, but this is a simpler
change that makes #30078 less aggressive.
Looking at the cpu profile of a burntpix benchmark, I noticed that a lot
of time was spent in gas-used, in the interpreter loop. It's an actual
call (not inlined), which explicitly wants to be ignored by tracing
("tracing.GasChangeIgnored"), so it can be safely and simply inlined.
The other change is in `pushX`. These also do a call to
`common.RightPadBytes`. I replaced that by a doing a corresponding `Lsh`
on the `u256` if needed. Note: it's needed only to make the stack output
look right, for fuzzers. It technically doesn't matter what we put
there: if code ends on a pushdata immediate, nothing will consume the
stack element. We could just as well just ignore it, if we didn't care
about fuzzers (which I do).
Seems quite a lot faster on burntpix, according to my runs.
This PR:
```
EVM gas used: 5642735088
execution time: 34.84609475s
allocations: 915683
allocated bytes: 175334088
```
```
EVM gas used: 5642735088
execution time: 36.671958278s
allocations: 915701
allocated bytes: 175340528
```
Master
```
EVM gas used: 5642735088
execution time: 49.349209526s
allocations: 915684
allocated bytes: 175333368
```
```
EVM gas used: 5642735088
execution time: 46.581006598s
allocations: 915681
allocated bytes: 175330728
```
---------
Co-authored-by: Sina M <1591639+s1na@users.noreply.github.com>
Co-authored-by: Felix Lange <fjl@twurst.com>
This PR adds `DeleteRange` to `ethdb.KeyValueWriter`. While range
deletion using an iterator can be really slow, `DeleteRange` is natively
supported by pebble and apparently runs in O(1) time (typically 20-30ms
in my tests for removing hundreds of millions of keys and gigabytes of
data). For leveldb and memorydb an iterator based fallback is
implemented. Note that since the iterator method can be slow and a
database function should not unexpectedly block for a very long time,
the number of deleted keys is limited at 10000 which should ensure that
it does not block for more than a second. ErrTooManyKeys is returned if
the range has only been partially deleted. In this case the caller can
repeat the call until it finally succeeds.
rebased https://github.com/ethereum/go-ethereum/pull/29766 . The
downstream branch appears to have been deleted and I don't have perms to
push to that fork.
`TerminalTotalDifficultyPassed` is removed. `TerminalTotalDifficulty`
must now be non-nil, and it is expected that networks are already
merged: we can only import PoW/Clique chains, not produce blocks on
them.
---------
Co-authored-by: stevemilk <wangpeculiar@gmail.com>
This PR moves the logging/tracing-facilities out of `*state.StateDB`,
in to a wrapping struct which implements `vm.StateDB` instead.
In most places, it is a pretty straight-forward change:
- First, hoisting the invocations from state objects up to the statedb.
- Then making the mutation-methods simply return the previous value, so
that the external logging layer could log everything.
Some internal code uses the direct object-accessors to mutate the state,
particularly in testing and in setting up state overrides, which means
that these changes are unobservable for the hooked layer. Thus, configuring
the overrides are not necessarily part of the API we want to publish.
The trickiest part about the layering is that when the selfdestructs are
finally deleted during `Finalise`, there's the possibility that someone
sent some ether to it, which is burnt at that point, and thus needs to
be logged. The hooked layer reaches into the inner layer to figure out
these events.
In package `vm`, the conversion from `state.StateDB + hooks` into a
hooked `vm.StateDB` is performed where needed.
---------
Co-authored-by: Gary Rong <garyrong0905@gmail.com>
Way back we've added `common.math.BigMin` and `common.math.BigMax`.
These were kind of cute helpers, but unfortunate ones, because package
all over out codebase added dependencies to this package just to avoid
having to write out 3 lines of code.
Because of this, we've also started having package name clashes with the
stdlib `math`, which got solves even more badly by moving some helpers
over ***from*** the stdlib into our custom lib (e.g. MaxUint64). The
latter ones were nuked out in a previous PR and this PR nukes out BigMin
and BigMax, inlining them at all call sites.
As we're transitioning to uint256, if need be, we can add a min and max
to that.
Breaking changes:
- The ChainConfig was exposed to tracers via VMContext passed in
`OnTxStart`. This is unnecessary specially looking through the lens of
live tracers as chain config remains the same throughout the lifetime of
the program. It was there so that native API-invoked tracers could
access it. So instead we moved it to the constructor of API tracers.
Non-breaking:
- Change the default config of the tracers to be `{}` instead of nil.
This way an extra nil check can be avoided.
Refactoring:
- Rename `supply` struct to `supplyTracer`.
- Un-export some hook definitions.
~~Opening this as a draft to have a discussion.~~ Pressed the wrong
button
I had [a previous PR
](https://github.com/ethereum/go-ethereum/pull/24616)a long time ago
which reduced the peak memory used during reorgs by not accumulating all
transactions and logs.
This PR reduces the peak memory further by not storing the blocks in
memory.
However this means we need to pull the blocks back up from storage
multiple times during the reorg.
I collected the following numbers on peak memory usage:
// Master: BenchmarkReorg-8 10000 899591 ns/op 820154 B/op 1440
allocs/op 1549443072 bytes of heap used
// WithoutOldChain: BenchmarkReorg-8 10000 1147281 ns/op 943163 B/op
1564 allocs/op 1163870208 bytes of heap used
// WithoutNewChain: BenchmarkReorg-8 10000 1018922 ns/op 943580 B/op
1564 allocs/op 1171890176 bytes of heap used
Each block contains a transaction with ~50k bytes and we're doing a 10k
block reorg, so the chain should be ~500MB in size
---------
Co-authored-by: Péter Szilágyi <peterke@gmail.com>
gitglorythegreat
Martin HS
Zheyuan He
rjl493456442
steven
Guillaume Ballet
Nebojsa Urosevic
lightclient
Felix Lange
wangjingcun
Arran Schlosberg
jwasinger
bitcoin-lightning
Karol Chojnowski
piersy
Marius van der Wijden
Péter Szilágyi
Felföldi Zsolt
Sina M