refactor(juno): integrate common interface with rawDB as trieDB engine #3187
Conversation
Codecov Report❌ Patch coverage is ❌ Your patch check has failed because the patch coverage (29.43%) is below the target coverage (60.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## maksym/integrate-common-interfaces-rawdb #3187 +/- ##
============================================================================
- Coverage 76.30% 75.63% -0.68%
============================================================================
Files 327 327
Lines 32335 32704 +369
============================================================================
+ Hits 24673 24735 +62
- Misses 5889 6175 +286
- Partials 1773 1794 +21 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| root, err := chain.StateCommitment() | ||
| require.NoError(t, err) | ||
| assert.Equal(t, stateUpdate1.NewRoot, &root) |
There was a problem hiding this comment.
Why do we remove this test?
blockchain/blockchain_test.go
Outdated
| } | ||
|
|
||
| func fetchStateUpdatesAndBlocks(samples int) ([]*core.StateUpdate, []*core.Block, error) { | ||
| client := feeder.NewClient(utils.Mainnet.FeederURL) |
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.
| return err | ||
| } | ||
|
|
||
| return b.trieDB.Journal(stateUpdate.NewRoot) |
There was a problem hiding this comment.
Question: What happens if synchronizer writes a new block in the middle of this process?
| // storeBlockData persists all block-related data to the database | ||
| func (b *Blockchain) storeBlockData( | ||
| txn db.IndexedBatch, | ||
| w db.KeyValueWriter, |
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
MaksymMalicki
changed the base branch from
maksym/rawdb
to
maksym/integrate-common-interfaces-rawdb
MaksymMalicki
force-pushed
the
maksym/integrate-common-interface-rawdb
branch
from
33d81d5 to
dc23482
Compare
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