Skip to content

feat: add tests for sparse and merkleized blueprint proof generation#2914

Merged
netrome merged 43 commits intomasterfrom
feature/clnt-1451-add-tests-for-sparse-and-merkleized-blueprint-proof
Apr 9, 2025
Merged

feat: add tests for sparse and merkleized blueprint proof generation#2914
netrome merged 43 commits intomasterfrom
feature/clnt-1451-add-tests-for-sparse-and-merkleized-blueprint-proof

Conversation

@netrome
Copy link
Copy Markdown
Contributor

@netrome netrome commented Mar 31, 2025

Closes #2907

This PR does two things:

  1. The bulk of this PR is a pure refactor of the basic_merkleized_storage_tests and the root_storage_tests macros. Instead of having the tests defined within the macros, the tests themselves are broken out to a separate helper trait (one for each module). This provides better type safety and makes it easier to maintain and develop the tests.
  2. Secondly, this PR adds tests for proof generation and verification for the merkleized and SMT blueprints. See the test_can_generate_and_validate_proofs tests in the merklized.rs and sparse.rs blueprint impls. These tests provide examples of how proof generation and validation works for the respective merkle tree implementations.

@netrome netrome self-assigned this Mar 31, 2025
@netrome netrome changed the title Feature/clnt 1451 add tests for sparse and merkleized blueprint proof feat: add tests for sparse and merkleized blueprint proof Mar 31, 2025
@netrome netrome force-pushed the feature/clnt-1451-add-tests-for-sparse-and-merkleized-blueprint-proof branch from da31042 to a09615e Compare March 31, 2025 19:44
Copy link
Copy Markdown
Contributor

@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.

looks good

@netrome
Copy link
Copy Markdown
Contributor Author

netrome commented Apr 1, 2025

looks good

Nice, I'll proceed with the SMT tests then ☺️

@netrome
Copy link
Copy Markdown
Contributor Author

netrome commented Apr 2, 2025

After chatting with @xgreenx about this, we agreed that it would be less intrusive to only modify the tests for the refactor and not modify the TableWithBlueprint impl blocks to use blanket implementations through wrapper traits.

@netrome netrome changed the title feat: add tests for sparse and merkleized blueprint proof feat: add tests for sparse and merkleized blueprint proof generation Apr 3, 2025
@netrome netrome force-pushed the feature/clnt-1451-add-tests-for-sparse-and-merkleized-blueprint-proof branch 3 times, most recently from b9f541c to bd3fb1b Compare April 7, 2025 18:36
@netrome netrome marked this pull request as ready for review April 7, 2025 18:39
@netrome netrome requested review from a team, Dentosal, MitchTurner and xgreenx as code owners April 7, 2025 18:39
@netrome netrome requested a review from rymnc April 7, 2025 18:39
@@ -0,0 +1 @@
Add tests for sparse and merkleized blueprint proofs.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

not sure this pr warrants a changelog, or maybe it should be phrased better if there is a change to the public api

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sure, I think this warrants a changelog. The tests themselves are debatable so this entry could be skipped - but the refactor is changing the api for downstream crates reusing these tests (which we'd like to do in SRS but couldn't due to the macros making implicit assumptions that didn't work for all tables - such as assuming keys always being sized)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated the formulations in 1fad862. Let me know what you think.

};

/// Provides root storage tests for SMT storage tables.
pub trait RootStorageTests: SmtTableWithBlueprint
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Why do we need a default implementation within the RootStorageTests trait when we could have a blanket implementation?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Not sure I follow what you mean. Do you mean why we provide default implementations of the tests in the trait definition, as opposed to implementing them in the blanket implementation on line 890?

I think this is cleaner, since redefining the methods in the blanket implementation would cause us to have to repeat them additionally one time. And I don't see any benefit of doing it the other way around - although I have nothing against doing that way either.

Is this what you were referring to or did I misunderstand something?

Comment on lines +392 to +383
fn key() -> Box<Self::Key>;

#[test]
fn insert() {
let mut storage = InMemoryStorage::default();
let mut storage_transaction = storage.write_transaction();
let key = $key;
/// Returns a random key for testing
fn random_key(rng: &mut StdRng) -> Box<Self::Key>;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Why do we need to return Box? I think you should be able to put a Sized constraint on the Key and Value. Also, you can use OwnedKey or OwnedValue instead, if it is not possible because of Rust limitation

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes this was actually a problem with the old macros. We have tables where the keys aren't sized, so putting the sized constraints would break it. I don't remember if there was any issues using OwnedKey and OwnedValue - I don't think so - so I can definitely change to use it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Okay looked deeper into this and the problem here is that Mappable requires that we should be able to construct and OwnedKey from a Key, but not the other way around.

pub trait Mappable {
    /// The key type is used during interaction with the storage. In most cases, it is the
    /// same as `Self::OwnedKey`.
    type Key: ?Sized + ToOwned;
    /// The owned type of the `Key` retrieving from the storage.
    type OwnedKey: From<<Self::Key as ToOwned>::Owned> + Clone;
    /// The value type is used while setting the value to the storage. In most cases, it
    /// is the same as `Self::OwnedValue`, but it is without restriction and can be
    /// used for performance optimizations.
    type Value: ?Sized + ToOwned;
    /// The owned type of the `Value` retrieving from the storage.
    type OwnedValue: From<<Self::Value as ToOwned>::Owned> + Clone;
}

And since we need references to Self::Key for e.g. insert, we'd need something like this constraint to make it work:Self::OwnedKey: AsRef<Self::Key>. I think this should hold for most tables, but not necessarily for all of them - so it is more correct to stick with the boxes here. Also while I don't like unnecessary heap allocations, these boxes are only for tests and doesn't affect the rest of the code negatively in any way.

@netrome netrome requested review from rymnc and xgreenx April 8, 2025 08:08
@netrome netrome force-pushed the feature/clnt-1451-add-tests-for-sparse-and-merkleized-blueprint-proof branch from 9deb5f1 to bd88d74 Compare April 9, 2025 15:00
xgreenx
xgreenx previously approved these changes Apr 9, 2025
xgreenx and others added 3 commits April 9, 2025 11:11
…re/clnt-1451-add-tests-for-sparse-and-merkleized-blueprint-proof
…re/clnt-1451-add-tests-for-sparse-and-merkleized-blueprint-proof
rymnc
rymnc previously approved these changes Apr 9, 2025
Base automatically changed from feature/reject-transactions-for-spent-coins to master April 9, 2025 19:19
@xgreenx xgreenx dismissed stale reviews from rymnc and themself April 9, 2025 19:19

The base branch was changed.

@xgreenx xgreenx requested a review from AurelienFT as a code owner April 9, 2025 19:19
@netrome netrome requested a review from rymnc April 9, 2025 19:21
@netrome netrome merged commit 49dc165 into master Apr 9, 2025
34 checks passed
@netrome netrome deleted the feature/clnt-1451-add-tests-for-sparse-and-merkleized-blueprint-proof branch April 9, 2025 20:09
@github-actions github-actions bot mentioned this pull request Apr 9, 2025
xgreenx pushed a commit that referenced this pull request Apr 9, 2025
## Version 0.43.0

### Breaking
- [2882](#2882): Changed the
type of the `resolved_outputs` for pre-confirmations. Now it also
includes `Utxoid`. `resolved_outputs` field contains only `Change` and
`Variable` outputs, so the `UtxoId` for them could be hard to derive, if
transaction has known inputs. This information should help to create
dependent transactions more easily.
- [2900](#2900): Get rid of
`Deref` impl on `ImportResult` by introducing wrapper type.
- [2909](#2909): Compressed
block headers now include a merkle root of the temporal registry after
compression was performed.
- [2931](#2931): In
`fuel-core-compression`, the `compress` function now takes a reference
to `Config` instead of the value.

### Added
- [2848](#2848): Link all
components of preconfirmations and add E2E tests.
- [2882](#2882): Listen to tx
status update from `TxStatusManager` in `TxPool`. Added logic to clean
up transactions from the pool if received squeezed out pre
confirmations. Added logic to promote transactions on sentry nodes when
receive pre-confirmation.
- [2885](#2885): Notify P2P
from `TxStatusManager` in case of bad preconfirmation message.
- [2901](#2901): New query
`dryRunRecordStorageReads` which works like `dryRun` but also returns
storage reads, allowing use of execution tracer or local debugger
- [2912](#2912): Add the
`allow_partial` parameter to the `coinsToSpend` query. The default value
of this parameters is `false` to preserve the old behavior. If set to
`true`, the query returns available coins instead of failing when the
requested amount is unavailable.
- [2914](#2914): Tests
ensuring the proof generation and validation of tables with sparse and
merklized blueprints work.

### Changed
- [2859](#2859): Swap out
off-chain worker compression for dedicated compression service in
`fuel-core-bin`.
- [2914](#2914): Break out
test logic to trait methods for `root_storage_tests` and
`basic_merkleized_storage_tests` test macros.
- [2925](#2925): Make
preconfirmation optional on API endpoints.

### Fixed
- [2918](#2918): Only cancel
background work if primary RocksDB instance is dropped
- [2935](#2935): The change
rejects transactions immediately, if they use spent coins. `TxPool` has
a SpentInputs LRU cache, storing all spent coins.

### Removed
- [2859](#2859): Removed DA
compression from off-chain worker in favor of dedicated compression
service in `fuel-core-bin`.

## What's Changed
* Rework `TxStatusManager` tests by @rafal-ch in
#2871
* fix(storage): custom impl of EnumCount for MerkleizedColumn by @rymnc
in #2875
* Update network versions on README by @github-actions in
#2889
* fix(version-compatibility): pin async-graphql to 7.0.15 by @rymnc in
#2891
* Fix proptest for tx status manager by @rafal-ch in
#2893
* fault_proving(compression): glue code for integ into fuel-core by
@rymnc in #2859
* Link and activate preconfirmations and add integrations tests by
@AurelienFT in #2848
* refactor: Get rid of `Deref` impl on `ImportResult` by introducing
wrapper type. by @netrome in
#2900
* fix(compression_service): post merge reviews by @rymnc in
#2895
* Listen status updates from in TxStatusManager in TxPool by @AurelienFT
in #2882
* chore(compression): include registry root in compressed block header
by @rymnc in #2909
* Improve TxStatusManager tests coverage by @AurelienFT in
#2911
* ci(benchmarks): run nightly benchmark and create PR by @rymnc in
#2915
* fix: Only cancel background work if primary RocksDB instance is
dropped by @netrome in #2918
* fix(ci): nightly benchmark by @rymnc in
#2922
* fix(ci): explicitly push before creating PR by @rymnc in
#2926
* Add allow_partial parameter to the "coins to spend" query by
@AurelienFT in #2912
* fix(compression): improve robustness on startup and shutdown by @rymnc
in #2923
* chore(compression): metrics for compression service by @rymnc in
#2920
* chore(compression): pass compression config by reference by @rymnc in
#2931
* Dry run execution tracing by @Dentosal in
#2901
* Make preconfirmation optional on API endpoints by @AurelienFT in
#2925
* Notify P2P in case of bad preconfirmation message by @AurelienFT in
#2885
* chore(1.85): 1.85.0 readiness by @rymnc in
#2937
* Reject transactions for already spent coins by @xgreenx in
#2935
* feat: add tests for sparse and merkleized blueprint proof generation
by @netrome in #2914


**Full Changelog**:
v0.42.0...v0.43.0

---------

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
ChapmaBeerbohm added a commit to ChapmaBeerbohm/holo-kit that referenced this pull request Sep 26, 2025
## Version 0.43.0

### Breaking
- [2882](FuelLabs/fuel-core#2882): Changed the
type of the `resolved_outputs` for pre-confirmations. Now it also
includes `Utxoid`. `resolved_outputs` field contains only `Change` and
`Variable` outputs, so the `UtxoId` for them could be hard to derive, if
transaction has known inputs. This information should help to create
dependent transactions more easily.
- [2900](FuelLabs/fuel-core#2900): Get rid of
`Deref` impl on `ImportResult` by introducing wrapper type.
- [2909](FuelLabs/fuel-core#2909): Compressed
block headers now include a merkle root of the temporal registry after
compression was performed.
- [2931](FuelLabs/fuel-core#2931): In
`fuel-core-compression`, the `compress` function now takes a reference
to `Config` instead of the value.

### Added
- [2848](FuelLabs/fuel-core#2848): Link all
components of preconfirmations and add E2E tests.
- [2882](FuelLabs/fuel-core#2882): Listen to tx
status update from `TxStatusManager` in `TxPool`. Added logic to clean
up transactions from the pool if received squeezed out pre
confirmations. Added logic to promote transactions on sentry nodes when
receive pre-confirmation.
- [2885](FuelLabs/fuel-core#2885): Notify P2P
from `TxStatusManager` in case of bad preconfirmation message.
- [2901](FuelLabs/fuel-core#2901): New query
`dryRunRecordStorageReads` which works like `dryRun` but also returns
storage reads, allowing use of execution tracer or local debugger
- [2912](FuelLabs/fuel-core#2912): Add the
`allow_partial` parameter to the `coinsToSpend` query. The default value
of this parameters is `false` to preserve the old behavior. If set to
`true`, the query returns available coins instead of failing when the
requested amount is unavailable.
- [2914](FuelLabs/fuel-core#2914): Tests
ensuring the proof generation and validation of tables with sparse and
merklized blueprints work.

### Changed
- [2859](FuelLabs/fuel-core#2859): Swap out
off-chain worker compression for dedicated compression service in
`fuel-core-bin`.
- [2914](FuelLabs/fuel-core#2914): Break out
test logic to trait methods for `root_storage_tests` and
`basic_merkleized_storage_tests` test macros.
- [2925](FuelLabs/fuel-core#2925): Make
preconfirmation optional on API endpoints.

### Fixed
- [2918](FuelLabs/fuel-core#2918): Only cancel
background work if primary RocksDB instance is dropped
- [2935](FuelLabs/fuel-core#2935): The change
rejects transactions immediately, if they use spent coins. `TxPool` has
a SpentInputs LRU cache, storing all spent coins.

### Removed
- [2859](FuelLabs/fuel-core#2859): Removed DA
compression from off-chain worker in favor of dedicated compression
service in `fuel-core-bin`.

## What's Changed
* Rework `TxStatusManager` tests by @rafal-ch in
FuelLabs/fuel-core#2871
* fix(storage): custom impl of EnumCount for MerkleizedColumn by @rymnc
in FuelLabs/fuel-core#2875
* Update network versions on README by @github-actions in
FuelLabs/fuel-core#2889
* fix(version-compatibility): pin async-graphql to 7.0.15 by @rymnc in
FuelLabs/fuel-core#2891
* Fix proptest for tx status manager by @rafal-ch in
FuelLabs/fuel-core#2893
* fault_proving(compression): glue code for integ into fuel-core by
@rymnc in FuelLabs/fuel-core#2859
* Link and activate preconfirmations and add integrations tests by
@AurelienFT in FuelLabs/fuel-core#2848
* refactor: Get rid of `Deref` impl on `ImportResult` by introducing
wrapper type. by @netrome in
FuelLabs/fuel-core#2900
* fix(compression_service): post merge reviews by @rymnc in
FuelLabs/fuel-core#2895
* Listen status updates from in TxStatusManager in TxPool by @AurelienFT
in FuelLabs/fuel-core#2882
* chore(compression): include registry root in compressed block header
by @rymnc in FuelLabs/fuel-core#2909
* Improve TxStatusManager tests coverage by @AurelienFT in
FuelLabs/fuel-core#2911
* ci(benchmarks): run nightly benchmark and create PR by @rymnc in
FuelLabs/fuel-core#2915
* fix: Only cancel background work if primary RocksDB instance is
dropped by @netrome in FuelLabs/fuel-core#2918
* fix(ci): nightly benchmark by @rymnc in
FuelLabs/fuel-core#2922
* fix(ci): explicitly push before creating PR by @rymnc in
FuelLabs/fuel-core#2926
* Add allow_partial parameter to the "coins to spend" query by
@AurelienFT in FuelLabs/fuel-core#2912
* fix(compression): improve robustness on startup and shutdown by @rymnc
in FuelLabs/fuel-core#2923
* chore(compression): metrics for compression service by @rymnc in
FuelLabs/fuel-core#2920
* chore(compression): pass compression config by reference by @rymnc in
FuelLabs/fuel-core#2931
* Dry run execution tracing by @Dentosal in
FuelLabs/fuel-core#2901
* Make preconfirmation optional on API endpoints by @AurelienFT in
FuelLabs/fuel-core#2925
* Notify P2P in case of bad preconfirmation message by @AurelienFT in
FuelLabs/fuel-core#2885
* chore(1.85): 1.85.0 readiness by @rymnc in
FuelLabs/fuel-core#2937
* Reject transactions for already spent coins by @xgreenx in
FuelLabs/fuel-core#2935
* feat: add tests for sparse and merkleized blueprint proof generation
by @netrome in FuelLabs/fuel-core#2914


**Full Changelog**:
FuelLabs/fuel-core@v0.42.0...v0.43.0

---------

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
GeorgeBake added a commit to GeorgeBake/pipeline-cli that referenced this pull request Sep 29, 2025
## Version 0.43.0

### Breaking
- [2882](FuelLabs/fuel-core#2882): Changed the
type of the `resolved_outputs` for pre-confirmations. Now it also
includes `Utxoid`. `resolved_outputs` field contains only `Change` and
`Variable` outputs, so the `UtxoId` for them could be hard to derive, if
transaction has known inputs. This information should help to create
dependent transactions more easily.
- [2900](FuelLabs/fuel-core#2900): Get rid of
`Deref` impl on `ImportResult` by introducing wrapper type.
- [2909](FuelLabs/fuel-core#2909): Compressed
block headers now include a merkle root of the temporal registry after
compression was performed.
- [2931](FuelLabs/fuel-core#2931): In
`fuel-core-compression`, the `compress` function now takes a reference
to `Config` instead of the value.

### Added
- [2848](FuelLabs/fuel-core#2848): Link all
components of preconfirmations and add E2E tests.
- [2882](FuelLabs/fuel-core#2882): Listen to tx
status update from `TxStatusManager` in `TxPool`. Added logic to clean
up transactions from the pool if received squeezed out pre
confirmations. Added logic to promote transactions on sentry nodes when
receive pre-confirmation.
- [2885](FuelLabs/fuel-core#2885): Notify P2P
from `TxStatusManager` in case of bad preconfirmation message.
- [2901](FuelLabs/fuel-core#2901): New query
`dryRunRecordStorageReads` which works like `dryRun` but also returns
storage reads, allowing use of execution tracer or local debugger
- [2912](FuelLabs/fuel-core#2912): Add the
`allow_partial` parameter to the `coinsToSpend` query. The default value
of this parameters is `false` to preserve the old behavior. If set to
`true`, the query returns available coins instead of failing when the
requested amount is unavailable.
- [2914](FuelLabs/fuel-core#2914): Tests
ensuring the proof generation and validation of tables with sparse and
merklized blueprints work.

### Changed
- [2859](FuelLabs/fuel-core#2859): Swap out
off-chain worker compression for dedicated compression service in
`fuel-core-bin`.
- [2914](FuelLabs/fuel-core#2914): Break out
test logic to trait methods for `root_storage_tests` and
`basic_merkleized_storage_tests` test macros.
- [2925](FuelLabs/fuel-core#2925): Make
preconfirmation optional on API endpoints.

### Fixed
- [2918](FuelLabs/fuel-core#2918): Only cancel
background work if primary RocksDB instance is dropped
- [2935](FuelLabs/fuel-core#2935): The change
rejects transactions immediately, if they use spent coins. `TxPool` has
a SpentInputs LRU cache, storing all spent coins.

### Removed
- [2859](FuelLabs/fuel-core#2859): Removed DA
compression from off-chain worker in favor of dedicated compression
service in `fuel-core-bin`.

## What's Changed
* Rework `TxStatusManager` tests by @rafal-ch in
FuelLabs/fuel-core#2871
* fix(storage): custom impl of EnumCount for MerkleizedColumn by @rymnc
in FuelLabs/fuel-core#2875
* Update network versions on README by @github-actions in
FuelLabs/fuel-core#2889
* fix(version-compatibility): pin async-graphql to 7.0.15 by @rymnc in
FuelLabs/fuel-core#2891
* Fix proptest for tx status manager by @rafal-ch in
FuelLabs/fuel-core#2893
* fault_proving(compression): glue code for integ into fuel-core by
@rymnc in FuelLabs/fuel-core#2859
* Link and activate preconfirmations and add integrations tests by
@AurelienFT in FuelLabs/fuel-core#2848
* refactor: Get rid of `Deref` impl on `ImportResult` by introducing
wrapper type. by @netrome in
FuelLabs/fuel-core#2900
* fix(compression_service): post merge reviews by @rymnc in
FuelLabs/fuel-core#2895
* Listen status updates from in TxStatusManager in TxPool by @AurelienFT
in FuelLabs/fuel-core#2882
* chore(compression): include registry root in compressed block header
by @rymnc in FuelLabs/fuel-core#2909
* Improve TxStatusManager tests coverage by @AurelienFT in
FuelLabs/fuel-core#2911
* ci(benchmarks): run nightly benchmark and create PR by @rymnc in
FuelLabs/fuel-core#2915
* fix: Only cancel background work if primary RocksDB instance is
dropped by @netrome in FuelLabs/fuel-core#2918
* fix(ci): nightly benchmark by @rymnc in
FuelLabs/fuel-core#2922
* fix(ci): explicitly push before creating PR by @rymnc in
FuelLabs/fuel-core#2926
* Add allow_partial parameter to the "coins to spend" query by
@AurelienFT in FuelLabs/fuel-core#2912
* fix(compression): improve robustness on startup and shutdown by @rymnc
in FuelLabs/fuel-core#2923
* chore(compression): metrics for compression service by @rymnc in
FuelLabs/fuel-core#2920
* chore(compression): pass compression config by reference by @rymnc in
FuelLabs/fuel-core#2931
* Dry run execution tracing by @Dentosal in
FuelLabs/fuel-core#2901
* Make preconfirmation optional on API endpoints by @AurelienFT in
FuelLabs/fuel-core#2925
* Notify P2P in case of bad preconfirmation message by @AurelienFT in
FuelLabs/fuel-core#2885
* chore(1.85): 1.85.0 readiness by @rymnc in
FuelLabs/fuel-core#2937
* Reject transactions for already spent coins by @xgreenx in
FuelLabs/fuel-core#2935
* feat: add tests for sparse and merkleized blueprint proof generation
by @netrome in FuelLabs/fuel-core#2914


**Full Changelog**:
FuelLabs/fuel-core@v0.42.0...v0.43.0

---------

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add tests for Sparse and Merkleized blueprint proof generation

3 participants