refactor(juno): integrate common interface with rawDB as trieDB engine #3187
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…erfaces Signed-off-by: MaksymMalicki <[email protected]>
…om/NethermindEth/juno into maksym/integrate-common-interfaces
…erfaces Signed-off-by: MaksymMalicki <[email protected]>
…Eth/juno into maksym/integrate-common-interfaces
infrmtcs
reviewed
Oct 21, 2025
infrmtcs
reviewed
Oct 21, 2025
Comment on lines
-281
to
-283
| root, err := chain.StateCommitment() | ||
| require.NoError(t, err) | ||
| assert.Equal(t, stateUpdate1.NewRoot, &root) |
Contributor
There was a problem hiding this comment.
Why do we remove this test?
infrmtcs
reviewed
Oct 21, 2025
infrmtcs
reviewed
Oct 21, 2025
infrmtcs
reviewed
Oct 21, 2025
infrmtcs
reviewed
Oct 21, 2025
infrmtcs
reviewed
Oct 21, 2025
infrmtcs
reviewed
Oct 21, 2025
blockchain/blockchain_test.go
Outdated
| } | ||
|
|
||
| func fetchStateUpdatesAndBlocks(samples int) ([]*core.StateUpdate, []*core.Block, error) { | ||
| client := feeder.NewClient(utils.Mainnet.FeederURL) |
Contributor
There was a problem hiding this comment.
nit: We should have a definition var network = utils.Mainnet and use network.FeederURL here, because:
- Guarantee that the used network here and in the test functions are the same.
- It's easy to change the test network if necessary.
infrmtcs
reviewed
Oct 21, 2025
infrmtcs
reviewed
Oct 21, 2025
infrmtcs
reviewed
Oct 21, 2025
| return err | ||
| } | ||
|
|
||
| return b.trieDB.Journal(stateUpdate.NewRoot) |
Contributor
There was a problem hiding this comment.
Question: What happens if synchronizer writes a new block in the middle of this process?
infrmtcs
reviewed
Oct 21, 2025
| // storeBlockData persists all block-related data to the database | ||
| func (b *Blockchain) storeBlockData( | ||
| txn db.IndexedBatch, | ||
| w db.KeyValueWriter, |
Contributor
There was a problem hiding this comment.
I think we should create a separate small PR to replace this db.IndexedBatch with either db.KeyValueWriter or db.KeyValueReader. Because db.IndexedBatch implements both of them, I think it should be quite easy to do.
MaksymMalicki
changed the base branch from
maksym/trie-interface
to
maksym/rawdb
Signed-off-by: MaksymMalicki <[email protected]>
Signed-off-by: MaksymMalicki <[email protected]>
MaksymMalicki
changed the base branch from
maksym/rawdb
to
maksym/integrate-common-interfaces-rawdb
…ntegrate-common-interface-rawdb
…ntegrate-common-interface-rawdb, linter
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR should be reviewed after refactor(juno): Use common interfaces in Juno #3283
It integrates the common interfaces across the project, including in utilities and tests. This PR uses
rawDB- direct pebbleDB reads/writes - as a trieDB engine. New components of the pathDB engine will be introduced in the follow-up PRs.Some parts of the logic weren’t straightforward to unify due to the different ways the old and new states interact with PebbleDB. The old state is tightly coupled with
IndexedBatchfor reads and writes, while the new state is designed to read using raw DB/snapshotsand write using regular PebbleDBBatch. As a result, the following methods were "duplicated" and tailored to the way the respectivestateandtriefunctionalities are meant to interact with the DB:blockchain.Storeblockchain.GetReverseStateDiffblockchain.FinaliseThe adapting had to be also done for the prove logic inside of
rpc/v8/storage.go,rpc/v8/storage_test.go,rpc/v9/storage.go,rpc/v9/storage_test.goThe experimental --new-state boolean CLI flag is now fully integrated. It can be used via the command line and in the
dbcmdutilities.The new state package includes some minor bug fixes discovered during previous work (#2912) while debugging sync, RPC, and E2E tests.
A new command was added to the
Makefile, to run the unit tests strictly with the new state and trie implementations in the future CI/CD:Next PR to review: #3188