## Why this should be merged
The `temporary.WithTempRegisteredExtras()` global lock introduced in
#234 wasn't fit for purpose when used in `coreth` as it required central
coordination of registration, types, and usage of the payload accessor.
## How this works
Instead of a central registration point, the new
`libevm.WithTemporaryExtrasLock()` function takes out a global lock and
provides the caller with a handle that proves the lock is held. All of
the override functions, e.g. `params.WithTempRegisteredExtras()` now
require a current lock, which will be propagated by the respective
`coreth` functions.
See https://github.com/ava-labs/coreth/pull/1328 for intended usage in
`coreth` and `subnet-evm`. A consumer of both of these can then safely
do the following:
```go
import (
"github.com/ava-labs/libevm/libevm"
coreth "github.com/ava-labs/coreth/plugin/evm"
subnet "github.com/ava-labs/subnet-evm/plugin/evm"
)
// asCChain calls `fn` while emulating `coreth`. It is safe for concurrent usage with [asSubnetEVM].
func asCChain(fn func() error) error {
return libevm.WithTemporaryExtrasLock(func(l libevm.ExtrasLock) error {
return coreth.WithTempRegisteredLibEVMExtras(l, fn)
})
}
// asSubnetEVM calls `fn` while emulating `subnet-evm`. It is safe for concurrent usage with [asCChain].
func asSubnetEVM(fn func() error) error {
return libevm.WithTemporaryExtrasLock(func(l libevm.ExtrasLock) error {
return subnet.WithTempRegisteredLibEVMExtras(l, fn)
})
}
```
## How this was tested
Unit test of the new function plus existing integration tests of all
modified code.
## Why this should be merged
Registration of extras requires runtime enforcement of payload types. In
production, only a single set of types can be registered and any
attempts at re-registration will panic (by design). Although this makes
production usage safe, it doesn't allow downstream consumers (e.g. data
indexers) to use extras from different chains (e.g. `coreth` _and_
`subnet-evm`) at the same time.
## How this works
1. The `libevm/register.AtMostOnce` type can now be overridden
temporarily.
2. `params`, `core/types`, `core/vm`, and `state` packages introduce
`WithTempRegisteredExtras()` functions.
3. `libevm/temporary.WithRegisteredExtras()` provides "atomic" override
of all extras.
In all cases, the scope of the override is limited to the life of a
single function call.
## How this was tested
Relative to numbered list above:
1. Unit test of new and existing functionality.
2. Integration tests of both packages, demonstrating both payload and
behavioural override.
## Why this should be merged
`ava-labs/coreth` has a partitioned state-address space, achieved by
setting or clearing a specific bit in the hash used to key the space.
This change allows such behaviour to be achieved with pure `libevm`
instead of the `StateDB` wrapping that `coreth` currently uses.
## How this works
Introduction of `state.StateDBHooks` interface, including a
`TransformStateKey()` method that allows for arbitrary change of state
key. If registered, this hook will be honoured by
`StateDB.{Get,GetCommitted,State}Key()` methods unless they receive a
`stateconf.SkipStateKeyTransformation` option.
## How this was tested
Unit test of `SetState() -> GetState() + GetCommittedState()` round trip
with and without options to skip.
## Why this should be merged
Coreth and Subnet-EVM require access to the current tx hash in the Warp
precompile (for selecting the correct predicate results).
This can not be exposed as a wrapper through the `OverrideEVMResetArgs`
because `SetTxContext` is called _prior_ to resetting the EVM instance.
Therefore, the `SetTxContext` function on any DB wrapper would never be
called.
## How this works
Exposes the existing `txHash` through a `TxHash()` function.
## How this was tested
Added a trivial unit test.
## Why this should be merged
`goheader` linter apparently hasn't been running but now decided to and
is blocking another PR. It's not worth the time to investigatie why
(probably a version update).
## How this works
Update header comments as required.
## How this was tested
N/A
## Why this should be merged
To allow more thorough handling of duplicate state roots (or any other
info other users would like), additional information can be provided to
a call of `statedb.Commit`. This change allows arbitrary types to be
sent to `triedb` as well as the `SnapshotTree`. However, this is a
breaking change for those using the functionality already, since the
snapshot commit option is wrapped with another call.
## How this works
See the edited libevm test for usage.
## How this was tested
Edited test case to include `TrieDBUpdateOption` and ensures the payload
is sent.
## Why this should be merged
Support for configurable `core/types.Block` with RLP encoding, including
interplay with `Body`.
## How this works
`Block` doesn't export most of its fields so relies on an internal type,
`extblock`, for RLP encoding. This type is modified to implement the
`rlp.Encoder` and `Decoder` methods as a point to inject hooks using
`rlp.Fields` (as in #120 for `Body`).
`Block` shares the same registered extra type as `Body`. Unlike
`Header`, which has its own field in a `Block`, the fields in `Body` are
promoted to be carried directly. This suggests that (at least for pure
data payloads) the modifications might be equivalent (and
`ava-labs/coreth` evidences this). Should different payloads be
absolutely required in the future, we can split the types—the
`RegisterExtras` signature is already too verbose though 😢.
## How this was tested
Explicit inclusion of a backwards-compatibility test for
`NOOPBlockBodyHooks` + implicit testing via the multiple upstream tests
in `block_test.go`. Re implicit testing: default behaviour is now to use
the noop hooks even when no registration is performed, but if we change
this then the tests in `block_test.go` can still be called as subtests
from a test that explicitly registers noops.
---------
Signed-off-by: Arran Schlosberg <519948+ARR4N@users.noreply.github.com>
Co-authored-by: Quentin McGaw <quentin.mcgaw@avalabs.org>
## Why this should be merged
The `types.Header` fields of both
[`coreth`](https://pkg.go.dev/github.com/ava-labs/coreth/core/types#Header)
and
[`subnet-evm`](https://pkg.go.dev/github.com/ava-labs/subnet-evm/core/types#Header)
have been modified such that their RLP encodings (i.e. block hashes)
aren't compatible with vanilla `geth` nor each other. This PR adds
support for arbitrary RLP encoding coupled with type-safe extra
payloads.
## How this works
Equivalent to #1 (`params`) and #44 (`types.StateAccount`) registration
of pseudo-generic payloads. The only major difference is the guarantee
of a non-nil payload pointer, which means that the payload hooks are
never called on nil pointers as this would make it difficult to decode
RLP into them.
## How this was tested
Round-trip RLP {en,de}coding via a registered stub hook.
---------
Signed-off-by: Arran Schlosberg <519948+ARR4N@users.noreply.github.com>
Co-authored-by: Quentin McGaw <quentin.mcgaw@gmail.com>
## Why this should be merged
There are 3 places at which we perform the same, sensitive logic to
access registered payloads and as we modify more types this is likely to
expand. (e.g. `types.Header`).
## How this works
Introduces `pseudo.Accessor` to abstract the reusable code.
## How this was tested
Existing unit tests. Note that the `types.StateAccount` tests needed a
minor refactor to provide the assertions with access to the
`ExtraPayloads[T]` without introducing generic types anywhere.
## Why this should be merged
Provides access to extra payloads registered on `types.SlimAccount`, not
just on regular `types.StateAccount`. This is a breaking syntax change
to have the method reflect the behaviour.
## How this works
Modify existing method to accept the `ExtraPayloadCarrier` interface
instead of `*StateAccount`. The interface is implemented by both
`*StateAccount` and `*SlimAccount`.
## How this was tested
Covered by existing testing.
## Why this should be merged
Allows for a drop-in replacement of `snapshot.Tree` (i.e. one that uses
block hashes instead of state roots). This is intended as a temporary
solution while we investigate having the state root affected by the
block hash to remove path ambiguity.
## How this works
Introduction of:
1. `state.SnapshotTree` interface to match methods required on
`snapshot.Tree` as used by `state.StateDB`; and
2. `stateconf` package for variadic options plumbed by
`StateDB.Commit()` through to `SnapshotTree.Update()`.
Although variadic (to maintain function call-signature compatibility)
only the `stateconf.WithUpdatePayload(any)` is expected to be used.
Recipients of the options can access the payload with
`stateconf.ExtractUpdatePayload()`.
## How this was tested
Unit test demonstrating propagation of `stateconf.UpdateOption` payload.
## Why this should be merged
Performs trie prefetching concurrently, required for equivalent
performance with `coreth` / `subnet-evm` implementations.
## How this works
`StateDB.StartPrefetcher()` accepts variadic options (for backwards
compatibility of function signatures). An option to specify a
`WorkerPool` is provided which, if present, is used to call
`Trie.Get{Account,Storage}()`; the pool is responsible for concurrency
but does not need to be able to wait on the work as that is handled by
this change.
## How this was tested
Unit test demonstrating hand-off of work to a `WorkerPool` as well as
API-guaranteed ordering of events.
## Why this should be merged
Fixes invariant broken by introduction of `types.StateAccountExtra`.
## How this works
`state.stateObject.empty()` now also requires that the underlying
account's `Extra` field carries the zero value if a type is registered,
or is itself nil.
## How this was tested
New unit test.
---------
Co-authored-by: Darioush Jalali <darioush.jalali@avalabs.org>
* feat: `state.{Get,Set}Extra[SA any](*StateDB,types.ExtraPayloads,...)`
* test: `GetExtra()` at each point in `CreateAccount()` + `SetExtra()` lifecycle
* test: reverting extras to snapshot
* test: `GetExtra()` after `StateDB.Copy()` and writes to original
Some of the changes in the full commit history were merged into `libevm` as part of #43 in `336a289` and then merged back into this branch as `5b15698`. Cherry-picking commits was not possible as some touched both halves of the changes; the squash-merges will, however, make this convoluted history irrelevant.
* feat: `types.StateAccount` pseudo-generic payload
* feat: registration of `StateAccount` payload type
* chore: mark `eth/tracers/logger` flaky
* chore: copyright header + `gci`
* test: lock default `types.SlimAccount` RLP encoding
* feat: `vm.SlimAccount.Extra` from `StateAccount` equiv
* chore: placate the linter
* test: `pseudo.Type.EncodeRLP()`
* test: `pseudo.Type.DecodeRLP()`
* fix: `pseudo.Type.DecodeRLP()` with non-pointer type
* feat: `pseudo.Type.IsZero()` and `Type.Equal(*Type)`
* feat: `types.StateAccountExtra.DecodeRLP()`
* fix: remove unnecessary `StateAccountExtra.clone()`
* refactor: readability
* feat: `pseudo.Type.Format()` implements `fmt.Formatter`
This change switches from using the `Hasher` interface to add/query the bloomfilter to implementing it as methods.
This significantly reduces the allocations for Search and Rebloom.
This change makes use of uin256 to represent balance in state. It touches primarily upon statedb, stateobject and state processing, trying to avoid changes in transaction pools, core types, rpc and tracers.
The dump after state-test didn't work, the problem was an error, "Already committed", which was silently ignored.
This change re-initialises the state, so the dumping works again.
This PR replaces Geth's logger package (a fork of [log15](https://github.com/inconshreveable/log15)) with an implementation using slog, a logging library included as part of the Go standard library as of Go1.21.
Main changes are as follows:
* removes any log handlers that were unused in the Geth codebase.
* Json, logfmt, and terminal formatters are now slog handlers.
* Verbosity level constants are changed to match slog constant values. Internal translation is done to make this opaque to the user and backwards compatible with existing `--verbosity` and `--vmodule` options.
* `--log.backtraceat` and `--log.debug` are removed.
The external-facing API is largely the same as the existing Geth logger. Logger method signatures remain unchanged.
A small semantic difference is that a `Handler` can only be set once per `Logger` and not changed dynamically. This just means that a new logger must be instantiated every time the handler of the root logger is changed.
----
For users of the `go-ethereum/log` module. If you were using this module for your own project, you will need to change the initialization. If you previously did
```golang
log.Root().SetHandler(log.LvlFilterHandler(log.LvlInfo, log.StreamHandler(os.Stderr, log.TerminalFormat(true))))
```
You now instead need to do
```golang
log.SetDefault(log.NewLogger(log.NewTerminalHandlerWithLevel(os.Stderr, log.LevelInfo, true)))
```
See more about reasoning here: https://github.com/ethereum/go-ethereum/issues/28558#issuecomment-1820606613
There were several problems related to dumping state.
- If a preimage was missing, even if we had set the `OnlyWithAddresses` to `false`, to export them anyway, the way the mapping was constructed (using `common.Address` as key) made the entries get lost anyway. Concerns both state- and blockchain tests.
- Blockchain test execution was not configured to store preimages.
This changes makes it so that the block test executor takes a callback, just like the state test executor already does. This callback can be used to examine the post-execution state, e.g. to aid debugging of test failures.
* rpc: make subscription test faster
reduces time for TestClientSubscriptionChannelClose
from 25 sec to < 1 sec.
* trie: cache trie nodes for faster sanity check
This reduces the time spent on TestIncompleteSyncHash
from ~25s to ~16s.
* core/forkid: speed up validation test
This takes the validation test from > 5s to sub 1 sec
* core/state: improve snapshot test run
brings the time for TestSnapshotRandom from 13s down to 6s
* accounts/keystore: improve keyfile test
This removes some unnecessary waits and reduces the
runtime of TestUpdatedKeyfileContents from 5 to 3 seconds
* trie: remove resolver
* trie: only check ~5% of all trie nodes
This change allows the creation of a genesis block for verkle testnets. This makes for a chunk of code that is easier to review and still touches many discussion points.
This PR removes panics from stacktrie (mostly), and makes the Update return errors instead. While adding tests for this, I also found that one case of possible corruption was not caught, which is now fixed.
This change enhances the stacktrie constructor by introducing an option struct. It also simplifies the `Hash` and `Commit` operations, getting rid of the special handling round root node.
During snap-sync, we request ranges of values: either a range of accounts or a range of storage values. For any large trie, e.g. the main account trie or a large storage trie, we cannot fetch everything at once.
Short version; we split it up and request in multiple stages. To do so, we use an origin field, to say "Give me all storage key/values where key > 0x20000000000000000". When the server fulfils this, the server provides the first key after origin, let's say 0x2e030000000000000 -- never providing the exact origin. However, the client-side needs to be able to verify that the 0x2e03.. indeed is the first one after 0x2000.., and therefore the attached proof concerns the origin, not the first key.
So, short-short version: the left-hand side of the proof relates to the origin, and is free-standing from the first leaf.
On the other hand, (pun intended), the right-hand side, there's no such 'gap' between "along what path does the proof walk" and the last provided leaf. The proof must prove the last element (unless there are no elements).
Therefore, we can simplify the semantics for trie.VerifyRangeProof by removing an argument. This doesn't make much difference in practice, but makes it so that we can remove some tests. The reason I am raising this is that the upcoming stacktrie-based verifier does not support such fancy features as standalone right-hand borders.
This change
- Removes the owner-notion from a stacktrie; the owner is only ever needed for comitting to the database, but the commit-function, the `writeFn` is provided by the caller, so the caller can just set the owner into the `writeFn` instead of having it passed through the stacktrie.
- Removes the `encoding.BinaryMarshaler`/`encoding.BinaryUnmarshaler` interface from stacktrie. We're not using it, and it is doubtful whether anyone downstream is either.
This change includes a lot of things, listed below.
### Split up interfaces, write vs read
The interfaces have been split up into one write-interface and one read-interface, with `Snapshot` being the gateway from write to read. This simplifies the semantics _a lot_.
Example of splitting up an interface into one readonly 'snapshot' part, and one updatable writeonly part:
```golang
type MeterSnapshot interface {
Count() int64
Rate1() float64
Rate5() float64
Rate15() float64
RateMean() float64
}
// Meters count events to produce exponentially-weighted moving average rates
// at one-, five-, and fifteen-minutes and a mean rate.
type Meter interface {
Mark(int64)
Snapshot() MeterSnapshot
Stop()
}
```
### A note about concurrency
This PR makes the concurrency model clearer. We have actual meters and snapshot of meters. The `meter` is the thing which can be accessed from the registry, and updates can be made to it.
- For all `meters`, (`Gauge`, `Timer` etc), it is assumed that they are accessed by different threads, making updates. Therefore, all `meters` update-methods (`Inc`, `Add`, `Update`, `Clear` etc) need to be concurrency-safe.
- All `meters` have a `Snapshot()` method. This method is _usually_ called from one thread, a backend-exporter. But it's fully possible to have several exporters simultaneously: therefore this method should also be concurrency-safe.
TLDR: `meter`s are accessible via registry, all their methods must be concurrency-safe.
For all `Snapshot`s, it is assumed that an individual exporter-thread has obtained a `meter` from the registry, and called the `Snapshot` method to obtain a readonly snapshot. This snapshot is _not_ guaranteed to be concurrency-safe. There's no need for a snapshot to be concurrency-safe, since exporters should not share snapshots.
Note, though: that by happenstance a lot of the snapshots _are_ concurrency-safe, being unmutable minimal representations of a value. Only the more complex ones are _not_ threadsafe, those that lazily calculate things like `Variance()`, `Mean()`.
Example of how a background exporter typically works, obtaining the snapshot and sequentially accessing the non-threadsafe methods in it:
```golang
ms := metric.Snapshot()
...
fields := map[string]interface{}{
"count": ms.Count(),
"max": ms.Max(),
"mean": ms.Mean(),
"min": ms.Min(),
"stddev": ms.StdDev(),
"variance": ms.Variance(),
```
TLDR: `snapshots` are not guaranteed to be concurrency-safe (but often are).
### Sample changes
I also changed the `Sample` type: previously, it iterated the samples fully every time `Mean()`,`Sum()`, `Min()` or `Max()` was invoked. Since we now have readonly base data, we can just iterate it once, in the constructor, and set all four values at once.
The same thing has been done for runtimehistogram.
### ResettingTimer API
Back when ResettingTImer was implemented, as part of https://github.com/ethereum/go-ethereum/pull/15910, Anton implemented a `Percentiles` on the new type. However, the method did not conform to the other existing types which also had a `Percentiles`.
1. The existing ones, on input, took `0.5` to mean `50%`. Anton used `50` to mean `50%`.
2. The existing ones returned `float64` outputs, thus interpolating between values. A value-set of `0, 10`, at `50%` would return `5`, whereas Anton's would return either `0` or `10`.
This PR removes the 'new' version, and uses only the 'legacy' percentiles, also for the ResettingTimer type.
The resetting timer snapshot was also defined so that it would expose the internal values. This has been removed, and getters for `Max, Min, Mean` have been added instead.
### Unexport types
A lot of types were exported, but do not need to be. This PR unexports quite a lot of them.
