trie: remove prover parameter 'fromLevel' #27512#1165
trie: remove prover parameter 'fromLevel' #27512#1165AnilChinchawale merged 1 commit intoXinFinOrg:dev-upgradefrom
Conversation
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.
gzliudan
force-pushed
the
simplify-trie-prover
branch
from
99bd8b0 to
a244527
Compare
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
gzliudan
changed the title
gzliudan
requested review from
AnilChinchawale,
anunay-xin,
benjamin202410,
liam-lai and
wanwiset25
There was a problem hiding this comment.
Pull request overview
This PR removes the now-unused fromLevel parameter from the core trie proof API and updates all direct call sites to the new signature. The goal is to simplify trie proof generation while keeping existing behavior the same (all previous callers used fromLevel = 0).
Changes:
- Update
(*trie.Trie).Proveand(*trie.StateTrie).Proveto drop thefromLevelargument and simplify the proof construction loop. - Adjust the core
state.Trieinterface and all internal callers (tests, benchmarks, andethapicode) to use the newProve(key []byte, proofDb ethdb.KeyValueWriter)signature. - Update XDCx/XDCxlending trie wrappers to call the new core
Provesignature, though they still expose a deprecatedfromLevelparameter in their own APIs.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| trie/trie_test.go | Updates randomized trie tests to call Prove without the fromLevel parameter. |
| trie/tracer_test.go | Adjusts tracer-related tests to use the new Prove signature. |
| trie/proof_test.go | Updates proof and range-proof tests and benchmarks to remove the fromLevel argument when calling Prove. |
| trie/proof.go | Removes the fromLevel parameter from Trie.Prove and StateTrie.Prove, simplifying the proof node iteration and updating comments. |
| internal/ethapi/trie_proof_test.go | Aligns transaction and receipt proof tests with the new Prove API. |
| internal/ethapi/api.go | Updates GetTransactionAndReceiptProof to use Prove(key, proofDb) for both transaction and receipt tries. |
| core/state/database.go | Updates the core state.Trie interface so Prove now takes only key and proofDb, matching the new trie API. |
| XDCxlending/lendingstate/XDCx_trie.go | Updates the XDCx lending trie wrapper to delegate to the new core Prove signature, but leaves a now-unused fromLevel parameter in its own method. |
| XDCx/tradingstate/XDCx_trie.go | Updates the XDCx trading trie wrapper to call the new core Prove signature while still exposing an unused fromLevel parameter. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -216,5 +216,5 @@ func (t *XDCXTrie) getSecKeyCache() map[string][]byte { | |||
| // nodes of the longest existing prefix of the key (at least the root node), ending | |||
| // with the node that proves the absence of the key. | |||
| func (t *XDCXTrie) Prove(key []byte, fromLevel uint, proofDb ethdb.KeyValueWriter) error { | |||
There was a problem hiding this comment.
XDCXTrie.Prove still exposes a fromLevel parameter even though the underlying trie.Trie.Prove method (and the core state.Trie interface) no longer take it, and the argument is ignored in the body. To fully remove the now-dead fromLevel concept and keep the APIs consistent, consider dropping this parameter from XDCXTrie.Prove and updating the corresponding Trie interface in lendingstate accordingly.
| func (t *XDCXTrie) Prove(key []byte, fromLevel uint, proofDb ethdb.KeyValueWriter) error { | |
| func (t *XDCXTrie) Prove(key []byte, proofDb ethdb.KeyValueWriter) error { |
There was a problem hiding this comment.
Shouldn't this unused fromLevel be also removed? @gzliudan
| @@ -220,5 +220,5 @@ func (t *XDCXTrie) getSecKeyCache() map[string][]byte { | |||
| // nodes of the longest existing prefix of the key (at least the root node), ending | |||
| // with the node that proves the absence of the key. | |||
| func (t *XDCXTrie) Prove(key []byte, fromLevel uint, proofDb ethdb.KeyValueWriter) error { | |||
There was a problem hiding this comment.
XDCXTrie.Prove in the tradingstate package still has a fromLevel parameter that is no longer supported by the underlying trie.Trie.Prove implementation and is effectively ignored here. To avoid confusion and keep the local Trie interface aligned with the core trie API, it would be better to remove this parameter from the method signature and from the tradingstate Trie interface.
| func (t *XDCXTrie) Prove(key []byte, fromLevel uint, proofDb ethdb.KeyValueWriter) error { | |
| func (t *XDCXTrie) Prove(key []byte, proofDb ethdb.KeyValueWriter) error { |
Proposed changes
Ref: ethereum#27512
Types of changes
What types of changes does your code introduce to XDC network?
Put an
✅in the boxes that applyImpacted Components
Which part of the codebase this PR will touch base on,
Put an
✅in the boxes that applyChecklist
Put an
✅in the boxes once you have confirmed below actions (or provide reasons on not doing so) that