Skip to content
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.

Sign up for GitHub

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

Jump to bottom

fault_proving(global_roots): Initial test suite for merklized storage updates #2598

Merged
merged 15 commits into from
Jan 31, 2025
Merged

fault_proving(global_roots): Initial test suite for merklized storage updates #2598

merged 15 commits into from
Jan 31, 2025

Conversation

netrome
Copy link
Contributor

@netrome netrome commented Jan 17, 2025 •
edited
Loading

Closes #2582

Description

This PR adds an initial test suite for the merkelized storage update functionality. These tests ensure that the insertion and deletion behavior for coins (except message coins, to be added in #2597) and contract UTXOs is correct.

@netrome netrome self-assigned this Jan 17, 2025
@netrome netrome changed the title tests: Initial test suite for merklized storage updates fault_proving(global_roots): Initial test suite for merklized storage updates Jan 17, 2025
@netrome netrome force-pushed the 2552-feature-flag-for-global-state-roots-+-first-onchain-table-implementation branch from 83f9cc9 to d83cd6f Compare January 21, 2025 14:34
@netrome netrome force-pushed the 2582-fault_provingglobal_roots-initial-test-suite-for-storage-updates branch from b0be2e5 to c45ab47 Compare January 27, 2025 20:54
@netrome netrome force-pushed the 2552-feature-flag-for-global-state-roots-+-first-onchain-table-implementation branch 2 times, most recently from d2cdb38 to 81b6582 Compare January 28, 2025 13:12
Base automatically changed from 2552-feature-flag-for-global-state-roots-+-first-onchain-table-implementation to master January 28, 2025 16:56
@netrome netrome force-pushed the 2582-fault_provingglobal_roots-initial-test-suite-for-storage-updates branch from c45ab47 to ce34251 Compare January 28, 2025 18:09
@netrome netrome mentioned this pull request Jan 28, 2025
Open
12 tasks
@netrome netrome requested review from acerone85 and a team January 28, 2025 18:27
@netrome netrome linked an issue Jan 28, 2025 that may be closed by this pull request
fault_proving(global_roots): Initial test suite for storage updates #2582
Closed
@netrome netrome marked this pull request as ready for review January 28, 2025 18:27
@netrome netrome force-pushed the 2582-fault_provingglobal_roots-initial-test-suite-for-storage-updates branch 2 times, most recently from 928ac03 to b9bdd5e Compare January 29, 2025 20:32
@netrome netrome requested a review from rymnc as a code owner January 29, 2025 20:32
@netrome netrome mentioned this pull request Jan 29, 2025
Open
acerone85
acerone85 previously approved these changes Jan 30, 2025
View reviewed changes
Copy link
Contributor

@acerone85 acerone85 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My comments are minor, and proposed changes can be tackled in a follow up if accepted.

On a side note, we should start thinking about testing the whole process_block function, e.g. how to generate a list of blocks contianing transactions of all types.

@netrome
Copy link
Contributor Author

netrome commented Jan 30, 2025

My comments are minor, and proposed changes can be tackled in a follow up if accepted.

On a side note, we should start thinking about testing the whole process_block function, e.g. how to generate a list of blocks contianing transactions of all types.

Thanks, and yes - though I think that's going to be more relevant when we integration test the binary so we might not need to cross that bridge just yet.

acerone85
acerone85 previously approved these changes Jan 30, 2025
View reviewed changes
acerone85
acerone85 reviewed Jan 30, 2025
View reviewed changes
crates/fraud_proofs/global_merkle_root/storage/src/update.rs
contract_id
}

trait ConstructUpdateMerkleizedTablesTransactionForTests<'a>: Sized + 'a {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would it not be better to leverage the BuilderPattern in this case?

E.g. something like

UpdateMerkleizedTablesTransactionBuilder::new().with_storage(storage).with_chain_id(&chain_id)

But probably best to do it in a follow-up and merge this one already :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes and yes :) But again this is only for tests, so I'm not sure if it's worth it. I'd happily approve a PR if you implement it as a follow-up.

rymnc
rymnc previously approved these changes Jan 31, 2025
View reviewed changes
Copy link
Member

@rymnc rymnc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, non blocking comment

crates/fraud_proofs/global_merkle_root/storage/Cargo.toml Outdated Show resolved Hide resolved
acerone85
acerone85 previously approved these changes Jan 31, 2025
View reviewed changes
netrome added 15 commits January 31, 2025 21:50
@netrome netrome dismissed stale reviews from acerone85 and xgreenx via bbf6361 January 31, 2025 20:51
@netrome netrome force-pushed the 2582-fault_provingglobal_roots-initial-test-suite-for-storage-updates branch from 2457f06 to bbf6361 Compare January 31, 2025 20:51
@netrome netrome merged commit 1e852a3 into master Jan 31, 2025
33 checks passed
@netrome netrome deleted the 2582-fault_provingglobal_roots-initial-test-suite-for-storage-updates branch January 31, 2025 21:11
@AurelienFT AurelienFT mentioned this pull request Feb 3, 2025
Merged
AurelienFT added a commit that referenced this pull request Feb 3, 2025
@AurelienFT
Release/v0.41.5 (#2663)
ea46e9f 
## Version 0.41.5

### Changed
- [2387](#2387): Update
description `tx-max-depth` flag.
- [2630](#2630): Removed some
noisy `tracing::info!` logs
- [2643](#2643): Before this
fix when tip is zero, transactions that use 30M have the same priority
as transactions with 1M gas. Now they are correctly ordered.

### Added
- [2617](#2617): Add
integration skeleton of parallel-executor.
- [2553](#2553): Scaffold
global merkle root storage crate.
- [2598](#2598): Add initial
test suite for global merkle root storage updates.
- [2635](#2635): Add metrics
to gas price service

### Fixed
- [2632](#2632): Improved
performance of certain async trait impls in the gas price service.
- [2662](#2662): Fix balances
query endpoint cost without indexation and behavior coins to spend with
one parameter at zero.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rymnc rymnc rymnc approved these changes

@MitchTurner MitchTurner MitchTurner approved these changes

@Dentosal Dentosal Awaiting requested review from Dentosal Dentosal is a code owner

@rafal-ch rafal-ch Awaiting requested review from rafal-ch rafal-ch is a code owner

@AurelienFT AurelienFT Awaiting requested review from AurelienFT AurelienFT is a code owner

@acerone85 acerone85 Awaiting requested review from acerone85 acerone85 is a code owner

@xgreenx xgreenx Awaiting requested review from xgreenx xgreenx is a code owner

Assignees

@netrome netrome

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

fault_proving(global_roots): Initial test suite for storage updates
5 participants
@netrome @MitchTurner @xgreenx @acerone85 @rymnc