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 addresses an issue in snap sync, specifically when the entire sync process can be halted due to an encountered empty storage range.
Currently, on the snap sync client side, the response to an empty (partial) storage range is discarded as a non-delivery. However, this response can be a valid response, when the particular range requested does not contain any slots.
For instance, consider a large contract where the entire key space is divided into 16 chunks, and there are no available slots in the last chunk [0xf] -> [end]. When the node receives a request for this particular range, the response includes:
The proof with origin [0xf]
A nil storage slot set
If we simply discard this response, the finalization of the last range will be skipped, halting the entire sync process indefinitely. The test case TestSyncWithUnevenStorage can reproduce the scenario described above.
In addition, this change also defines the common variables MaxAddress and MaxHash.
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 is a minor refactor in preparation of changes to range verifier. This PR contains no intentional functional changes but moves (and renames) the light.NodeSet
This change refactors stacktrie to separate the stacktrie itself from the
internal representation of nodes: a stacktrie is not a recursive structure
of stacktries, rather, a framework for representing and operating upon a set of nodes.
---------
Co-authored-by: Gary Rong <garyrong0905@gmail.com>
This change fixes the bug in a benchmark, where the input to the trie is reused in a way which is not correct.
---------
Co-authored-by: Martin Holst Swende <martin@swende.se>
The Go authors updated golang/x/ext to change the function signature of the slices sort method.
It's an entire shitshow now because x/ext is not tagged, so everyone's codebase just
picked a new version that some other dep depends on, causing our code to fail building.
This PR updates the dep on our code too and does all the refactorings to follow upstream...
* all: implement path-based state scheme
* all: edits from review
* core/rawdb, trie/triedb/pathdb: review changes
* core, light, trie, eth, tests: reimplement pbss history
* core, trie/triedb/pathdb: track block number in state history
* trie/triedb/pathdb: add history documentation
* core, trie/triedb/pathdb: address comments from Peter's review
Important changes to list:
- Cache trie nodes by path in clean cache
- Remove root->id mappings when history is truncated
* trie/triedb/pathdb: fallback to disk if unexpect node in clean cache
* core/rawdb: fix tests
* trie/triedb/pathdb: rename metrics, change clean cache key
* trie/triedb: manage the clean cache inside of disk layer
* trie/triedb/pathdb: move journal function
* trie/triedb/path: fix tests
* trie/triedb/pathdb: fix journal
* trie/triedb/pathdb: fix history
* trie/triedb/pathdb: try to fix tests on windows
* core, trie: address comments
* trie/triedb/pathdb: fix test issues
---------
Co-authored-by: Felix Lange <fjl@twurst.com>
Co-authored-by: Martin Holst Swende <martin@swende.se>
This change makes the StateDB track the state key value diff of a block transition.
We already tracked current account and storage values for the purpose of updating
the state snapshot. With this PR, we now also track the original (pre-transition) values
of accounts and storage slots.
The clean trie cache is persisted periodically, therefore Geth can
quickly warmup the cache in next restart.
However it will reduce the robustness of system. The assumption is
held in Geth that if the parent trie node is present, then the entire
sub-trie associated with the parent are all prensent.
Imagine the scenario that Geth rewinds itself to a past block and
restart, but Geth finds the root node of "future state" in clean
cache then regard this state is present in disk, while is not in fact.
Another example is offline pruning tool. Whenever an offline pruning
is performed, the clean cache file has to be removed to aviod hitting
the root node of "deleted states" in clean cache.
All in all, compare with the minor performance gain, system robustness
is something we care more.
* core/state, light, les: make signature of ContractCode hash-independent
* push current state for feedback
* les: fix unit test
* core, les, light: fix les unittests
* core/state, trie, les, light: fix state iterator
* core, les: address comments
* les: fix lint
---------
Co-authored-by: Gary Rong <garyrong0905@gmail.com>
Verkle trees store the code inside the trie. This PR changes the interface to pass the code, as well as the dirty flag to tell the trie package if the code is dirty and needs to be updated. This is a no-op for the MPT and the odr trie.
The state availability is checked during the creation of a state reader.
- In hash-based database, if the specified root node does not exist on disk disk, then
the state reader won't be created and an error will be returned.
- In path-based database, if the specified state layer is not available, then the
state reader won't be created and an error will be returned.
This change also contains a stricter semantics regarding the `Commit` operation: once it has been performed, the trie is no longer usable, and certain operations will return an error.
This removes the feature where top nodes of the proof can be elided.
It was intended to be used by the LES server, to save bandwidth
when the client had already fetched parts of the state and only needed
some extra nodes to complete the proof. Alas, it never got implemented
in the client.
Continuing with a series of PRs to make the Trie interface more generic, this PR moves
the RLP encoding of storage slots inside the StateTrie and light.Trie implementations,
as other types of tries don't use RLP.
* trie: add node type common package
In trie/types package, a few node wrappers are defined, which will be used
in both trie package, trie/snap package, etc. Therefore, a standalone common
package is created to put these stuffs.
* trie: rename trie/types to trie/trienode
In this PR, all TryXXX(e.g. TryGet) APIs of trie are renamed to XXX(e.g. Get) with an error returned.
The original XXX(e.g. Get) APIs are renamed to MustXXX(e.g. MustGet) and does not return any error -- they print a log output. A future PR will change the behaviour to panic on errorrs.
This change renames StateTrie methods to remove the Try* prefix.
We added the Trie methods with prefix 'Try' a long time ago, working
around the problem that most existing methods of Trie did not return the
database error. This weird naming convention has persisted until now.
Co-authored-by: Gary Rong <garyrong0905@gmail.com>
This changes the Trie interface to add the plain account address as a
parameter to all storage-related methods.
After the introduction of the TryAccount* functions, TryGet, TryUpdate and
TryDelete are now only meant to read an account's storage. In their current
form, they assume that an account storage is stored in a separate trie, and
that the hashing of the slot is independent of its account's address.
The proposed structure for a stateless storage breaks these two
assumptions: the hashing of a slot key requires the address and all slots
and accounts are stored in a single trie.
This PR therefore adds an address parameter to the interface. It is ignored
in the MPT version, so this change has no functional impact, however it
will reduce the diff size when merging verkle trees.
The EmptyRootHash and EmptyCodeHash are defined everywhere in the codebase, this PR replaces all of them with unified one defined in core/types package, and also defines constants for TxRoot, WithdrawalsRoot and UncleRoot
This PR contains a small portion of the full pbss PR, namely
Remove the tracer from trie (and comitter), and instead using an accessList.
Related changes to the Nodeset.
---------
Co-authored-by: Gary Rong <garyrong0905@gmail.com>
This PR is a (superior) alternative to https://github.com/ethereum/go-ethereum/pull/26708, it handles deprecation, primarily two specific cases.
`rand.Seed` is typically used in two ways
- `rand.Seed(time.Now().UnixNano())` -- we seed it, just to be sure to get some random, and not always get the same thing on every run. This is not needed, with global seeding, so those are just removed.
- `rand.Seed(1)` this is typically done to ensure we have a stable test. If we rely on this, we need to fix up the tests to use a deterministic prng-source. A few occurrences like this has been replaced with a proper custom source.
`rand.Read` has been replaced by `crypto/rand`.`Read` in this PR.
* common, core, eth, les, trie: make prque generic
* les/vflux/server: fixed issues in priorityPool
* common, core, eth, les, trie: make priority also generic in prque
* les/flowcontrol: add test case for priority accumulator overflow
* les/flowcontrol: avoid priority value overflow
* common/prque: use int priority in some tests
No need to convert to int64 when we can just change the type used by the
queue.
* common/prque: remove comment about int64 range
---------
Co-authored-by: Zsolt Felfoldi <zsfelfoldi@gmail.com>
Co-authored-by: Felix Lange <fjl@twurst.com>
This change ports some changes from the main PBSS PR:
- get rid of callback function in `trie.Database.Commit` which is not required anymore
- rework the `nodeResolver` in `trie.Iterator` to make it compatible with multiple state scheme
- some other shallow changes in tests and typo-fixes
This PR moves some trie-related db accessor methods to a different file, and also removes the schema type. Instead of the schema type, a string is used to distinguish between hashbased/pathbased db accessors.
This also moves some code from trie package to rawdb package.
This PR is intended to be a no-functionality-change prep PR for #25963 .
---------
Co-authored-by: Gary Rong <garyrong0905@gmail.com>
This PR fixes an error in trie commit. If the trie.root is nil, it can be two possible scenarios:
- The trie was empty, and no change happens
- The trie was non-empty and all nodes are dropped
For the latter one, we should collect the deletions and apply them into database(e.g. in PBSS).
This PR introduces a node scheme abstraction. The interface is only implemented by `hashScheme` at the moment, but will be extended by `pathScheme` very soon.
Apart from that, a few changes are also included which is worth mentioning:
- port the changes in the stacktrie, tracking the path prefix of nodes during commit
- use ethdb.Database for constructing trie.Database. This is not necessary right now, but it is required for path-based used to open reverse diff freezer
This PR ports a few changes from PBSS:
- Fix the snapshot generator waiter in case the generation is not even initialized
- Refactor db inspector for ancient store
This changes the CI / release builds to use the latest Go version. It also
upgrades golangci-lint to a newer version compatible with Go 1.19.
In Go 1.19, godoc has gained official support for links and lists. The
syntax for code blocks in doc comments has changed and now requires a
leading tab character. gofmt adapts comments to the new syntax
automatically, so there are a lot of comment re-formatting changes in this
PR. We need to apply the new format in order to pass the CI lint stage with
Go 1.19.
With the linter upgrade, I have decided to disable 'gosec' - it produces
too many false-positive warnings. The 'deadcode' and 'varcheck' linters
have also been removed because golangci-lint warns about them being
unmaintained. 'unused' provides similar coverage and we already have it
enabled, so we don't lose much with this change.
This PR includes minor updates to comments in trie/committer that reference insertion to the db, and adds an err != nil check for the return value of preimages.commit.
It's a trivial PR to hide the error log when the trie node is not found in the database. The idea for this change is for all TryXXX functions, the error is already returned and we don't need to fire a log explicitly.
Recently there are a few tickets #25613#25589 reporting that the trie nodes are missing because of debug.SetHead. The root cause is after resetting, the chain rewinds to a historical point and re-imports the blocks on top.
Since the node is already synced and started to accept transactions previously, these transactions are still kept in the txpool and verified by txpool with a live state. This live state is constructed based on the live trie database, which is changed fast by node referencing and de-referencing.
Unfortunately, when we construct a live state(like the state in txpool), we don't reference the state we have. The blockchain will garbage collect the intermediate version nodes in another thread which leads the broken live state.
The best solution for this is to forcibly obtain a reference for all live states we create and call release function once it's used up. But it might end up with more junks persisted into disk. Will try to find an elegant solution later in the following PR.
This avoids copying the input []byte while decoding trie nodes. In most
cases, particularly when the input slice is provided by the underlying
database, this optimization is safe to use.
For cases where the origin of the input slice is unclear, the copying version
is retained. The new code performs better even when the input must be
copied, because it is now only copied once in decodeNode.
* core, trie: flush preimages to db on database close
Co-authored-by: rjl493456442 <garyrong0905@gmail.com>
* rename Close to CommitPreimages for clarity
* core, trie: nitpick fixes
Co-authored-by: rjl493456442 <garyrong0905@gmail.com>
Co-authored-by: Péter Szilágyi <peterke@gmail.com>
* core: use TryGetAccount to read where TryUpdateAccount has been used to write
* Gary's review feedback
* implement Gary's suggestion
* fix bug + rename NewSecure into NewStateTrie
* trie: add backwards-compatibility aliases for SecureTrie
* Update database.go
* make the linter happy
Co-authored-by: Felix Lange <fjl@twurst.com>
Co-authored-by: rjl493456442 <garyrong0905@gmail.com>
This enables the following linters
- typecheck
- unused
- staticcheck
- bidichk
- durationcheck
- exportloopref
- gosec
WIth a few exceptions.
- We use a deprecated protobuf in trezor. I didn't want to mess with that, since I cannot meaningfully test any changes there.
- The deprecated TypeMux is used in a few places still, so the warning for it is silenced for now.
- Using string type in context.WithValue is apparently wrong, one should use a custom type, to prevent collisions between different places in the hierarchy of callers. That should be fixed at some point, but may require some attention.
- The warnings for using weak random generator are squashed, since we use a lot of random without need for cryptographic guarantees.
This commit replaces ioutil.TempDir with t.TempDir in tests. The
directory created by t.TempDir is automatically removed when the test
and all its subtests complete.
Prior to this commit, temporary directory created using ioutil.TempDir
had to be removed manually by calling os.RemoveAll, which is omitted in
some tests. The error handling boilerplate e.g.
defer func() {
if err := os.RemoveAll(dir); err != nil {
t.Fatal(err)
}
}
is also tedious, but t.TempDir handles this for us nicely.
Reference: https://pkg.go.dev/testing#T.TempDir
Signed-off-by: Eng Zer Jun <engzerjun@gmail.com>
Trie tracer is an auxiliary tool to capture all deleted nodes
which can't be captured by trie.Committer. The deleted nodes
can be removed from the disk later.
* trie: fix memory leak in trie iterator
In the trie iterator, live nodes are tracked in a stack while iterating.
Popped node states should be explictly set to nil in order to get
garbage-collected.
* trie: fix empty trie iterator
This change speeds up trie hashing and all other activities that require
RLP encoding of trie nodes by approximately 20%. The speedup is achieved by
avoiding reflection overhead during node encoding.
The interface type trie.node now contains a method 'encode' that works with
rlp.EncoderBuffer. Management of EncoderBuffers is left to calling code.
trie.hasher, which is pooled to avoid allocations, now maintains an
EncoderBuffer. This means memory resources related to trie node encoding
are tied to the hasher pool.
Co-authored-by: Felix Lange <fjl@twurst.com>
This PR adds an addtional API called `NewBatchWithSize` for db
batcher. It turns out that leveldb batch memory allocation is
super inefficient. The main reason is the allocation step of
leveldb Batch is too small when the batch size is large. It can
take a few second to build a leveldb batch with 100MB size.
Luckily, leveldb also offers another API called MakeBatch which can
pre-allocate the memory area. So if the approximate size of batch is
known in advance, this API can be used in this case.
It's needed in new state scheme PR which needs to commit a batch of
trie nodes in a single batch. Implement the feature in a seperate PR.
This functionality is needed in new path-based storage scheme, but
can be implemented in a seperate PR though.
When an account is deleted, then all the storage slots should be
nuked out from the disk as well. In hash-based storage scheme they
are still left in the disk but in new scheme, they will be iterated
and marked as deleted.
But why the NodeBlob API is needed in this scenario? Because when
the node is marked deleted, the previous value is also required to
be recorded to construct the reverse diff.
* trie/proof: edge case for VerifyRangeProof
* more consistency with other tests in the file
* trie: fix test todo
Co-authored-by: Martin Holst Swende <martin@swende.se>
