-
Notifications
You must be signed in to change notification settings - Fork 162
NOMT storage by default in TestRollup #2329
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev
Are you sure you want to change the base?
NOMT storage by default in TestRollup #2329
Conversation
Keep working from there
…tils # Conflicts: # .config/nextest.toml
Greptile SummaryMigrated test infrastructure from JMT to NOMT as the default storage backend by swapping type aliases (
Confidence Score: 5/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant Test as Test Code
participant Utils as sov-test-utils
participant Spec as DefaultNomtSpec
participant StorageMgr as NomtStorageManager
participant NOMT as NomtProverStorage
participant Config as RollupDbConfig
Note over Test,NOMT: Migration from JMT to NOMT Storage
Test->>Utils: Use TestSpec (formerly TestNomtSpec)
Utils->>Spec: Instantiate DefaultNomtSpec
Spec->>Spec: Configure Storage type as NomtProverStorage
Test->>Config: Create RollupDbConfig::default_in_path()
Config->>Config: Set reduced memory settings for tests
Note over Config: hashtable_buckets: 500 (debug) / 1M (release)<br/>page_cache_size: 8, leaf_cache_size: 8<br/>commit_concurrency: 2
Test->>StorageMgr: Initialize NomtStorageManager::new(config)
StorageMgr->>NOMT: Create user_nomt_db
StorageMgr->>NOMT: Create kernel_nomt_db
Test->>StorageMgr: create_storage()
StorageMgr->>NOMT: Return NomtProverStorage instance
Test->>Test: Execute tests with NOMT storage
Note over Test: All tests now use NOMT by default<br/>JMT available as TestJmtSpec if needed
|
Greptile found no issues!From now on, if a review finishes and we haven't found any issues, we will not post anything, but you can confirm that we reviewed your changes in the status check section. This feature can be toggled off in your Code Review Settings by deselecting "Create a status check for each PR". |
* Re-enabled proof tests * Introduce wrapper type
...m/module-implementations/sov-attester-incentives/tests/integration/attestation_processing.rs
Outdated
Show resolved
Hide resolved
…tils # Conflicts: # crates/full-node/sov-stf-runner/proptest-regressions/state_manager/tests.txt
theodorebugnet
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't have proper context of the NOMT proving (so I can't properly review the ignored tests and the open_proof() implementation) but otherwise changes LGTM.
Would it make sense to also have sov_test_utils::storage::SimpleNomtStorageManager -> SimpleStoragemanager and SimpleStorageManager -> SimpleJmtStorageManager the same way?
crates/full-node/sov-sequencer/tests/integration/preferred_end_to_end.rs
Outdated
Show resolved
Hide resolved
…_to_end.rs Co-authored-by: Theodore Bugnet <24320578+theodorebugnet@users.noreply.github.com>
Yep, it also should reduce size of the PR |
Description
NOMT is our main production storage at the moment. More tests run using it, the better.
TestSpec->TestJmtSpecTestNomtSpec->TestSpecSimpleStorageManager->SimpleJmtStorageManagerSimpleNomtStorageManager->SimpleStorageManagerCouple more type aliases add in
sov-test-utilsfor easier usage.Implemented
open_proofwhich is small change actually. But keep in mind, that it only works for live state, and not historical state. Because of that, majority of the tests that needs proofs have been ignored, because it needs to be redesigned.Potential follow ups
ProverStoragetoJmtProverStorageand comment that it is going to be phased out at some pointCHANGELOG.mdwith a new entry if my PR makes any breaking changes or fixes a bug. If my PR removes a feature or changes its behavior, I provide help for users on how to migrate to the new behavior.Cargo.tomlchanges before opening the PRs. (Are all new dependencies necessary? Is any module dependency leaked into the full-node (hint: it shouldn't)?)Testing
All existing tests are passing
Docs
No updates