feat(drive-abci): gate shielded-pool seeding behind shielded_test_data feature#3774
Conversation
…lop) Picks up the post-merge tip of dashpay/grovedb#752 (snapshot bootstrap API). The grovedb branch was force-rebased onto develop after PR #751 ('perf(commitment-tree): batched append_many_raw — replace _without_frontier API') was squash-merged, leaving the snapshot branch as just the bootstrap surface: * GroveDb::raw_storage() — escape hatch * RocksDbStorage::ingest_subtree_sst() — bulk-load SST * GroveDb::replace_commitment_tree_subtree_root() — patch parent Merk leaf Used by drive-abci's shielded_snapshot module to dump/apply a precomputed shielded-pool subtree at devnet genesis (without paying the per-write WAL + fsync cost of the runtime seeder path). Roundtrip verified via the previously-ignored test: snapshot_dump_apply_preserves_anchor ← now passes 30k notes → 8.6 MB SST → reapplied yields byte-equivalent anchor The standalone seed tests (seeded_pool_count_matches_total_notes etc.) remain green on the new grovedb rev. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The grovedb snapshot bootstrap API (dashpay/grovedb#752 — `ingest_subtree_sst`, `replace_commitment_tree_subtree_root`, `raw_storage`) is now wired in via the rev bump in the previous commit, so the roundtrip test is no longer gated behind a pre-merge marker. Verified locally: 30k notes → 8.6 MB SST → re-applied to a fresh platform yields a byte-equivalent Sinsemilla anchor (df37726e…4c2300). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR updates the grovedb dependency revision across six packages; introduces a new ChangesGrovedb revision bump and shielded test-data gating
🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
✅ Review complete (commit e4b1008) |
|
✅ DashSDKFFI.xcframework built for this PR.
SwiftPM (host the zip at a stable URL, then use): .binaryTarget(
name: "DashSDKFFI",
url: "https://your.cdn.example/DashSDKFFI.xcframework.zip",
checksum: "507d9f857f49d5f07dba03e69ee717dd67fc806ee89f634922cb57f97e375ada"
)Xcode manual integration:
|
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Code Review
PR's in-scope changes are a mechanical grovedb pin bump (5eb7a538 → 67acdbcb) across six Cargo.toml manifests + Cargo.lock and removal of one #[ignore] attribute on snapshot_dump_apply_preserves_anchor. Both changes are minimal, internally consistent, and the un-ignored test matches the new grovedb API (ingest_subtree_sst, replace_commitment_tree_subtree_root) that the rev brings in. No in-scope blocking or suggestion-level issues identified.
Adds a second build-time gate `create_shielded_test_data` nested inside `create_sdk_test_data`, so SDK_TEST_DATA-only devnets (the pre-#3732 shape) skip the 1M-note shielded-pool seeding and the snapshot bake/apply machinery. * `#[cfg(create_shielded_test_data)]` now gates: - the `create_data_for_shielded_pool` call inside `create_sdk_test_data` - the `mod shielded` declaration (in create_genesis_state/test/mod.rs) - the `shielded_snapshot` lib module (bake/apply public API) - the `snapshot-bake` CLI subcommand + everything supporting it * `mod snapshot_bake_main` in main.rs consolidates what used to be three separately-gated items (NoopCoreRPC, its impl block, and the `snapshot_bake` fn) under a single `#[cfg(create_shielded_test_data)]` on the module declaration. * New `.cargo/config-shielded-test-data.toml` profile sets BOTH cfgs (`create_sdk_test_data` + `create_shielded_test_data`); the existing `config-test-sdk-data.toml` continues to set only the outer cfg. * Dockerfile gains a `SHIELDED_TEST_DATA` build arg (forwarded by dashmate, parallel to `SDK_TEST_DATA`). Setting it to `true` selects the dual-cfg cargo profile AND drives the bake stage to produce a `shielded-pool.snap`. SDK-only builds skip the bake (the binary doesn't even contain the `snapshot-bake` subcommand). * `check-cfg` allowlist in `packages/rs-drive-abci/Cargo.toml` updated to recognise the new cfg. Verified locally with `cargo check -p drive-abci` in all three modes: - default (no cfgs) -> 15m 15s, clean - only `create_sdk_test_data` -> 6m 16s, clean - both cfgs -> 18m 41s, clean Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
shumkov
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/rs-drive-abci/src/execution/platform_events/initialization/create_genesis_state/test/mod.rs (1)
10-14: 💤 Low valueGating is consistent; note the implicit cfg dependency.
Shielded seeding only runs when
create_shielded_test_datais set, and the surroundingcreate_sdk_test_datacall increate_genesis_stateonly runs undercreate_sdk_test_data. So a bake produces non-empty shielded data only when both cfgs are active. The.cargo/config-shielded-test-data.tomlprofile sets both, so the Docker path holds — butcreate_shielded_test_dataset alone (e.g. a manual--cfg) would silently bake an empty snapshot. The doc comments capture this; no code change needed.Also applies to: 46-53
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/rs-drive-abci/src/execution/platform_events/initialization/create_genesis_state/test/mod.rs` around lines 10 - 14, Add a clarifying comment above the `mod shielded;` declaration to state that shielded seeding only produces non-empty data when both cfgs are active (the `create_shielded_test_data` flag and the surrounding `create_sdk_test_data` call in `create_genesis_state`), and reference the `.cargo/config-shielded-test-data.toml` profile; this makes the implicit cfg dependency explicit without changing behavior of `mod shielded` or the `create_genesis_state` / `create_sdk_test_data` logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@packages/rs-drive-abci/src/execution/platform_events/initialization/create_genesis_state/test/mod.rs`:
- Around line 10-14: Add a clarifying comment above the `mod shielded;`
declaration to state that shielded seeding only produces non-empty data when
both cfgs are active (the `create_shielded_test_data` flag and the surrounding
`create_sdk_test_data` call in `create_genesis_state`), and reference the
`.cargo/config-shielded-test-data.toml` profile; this makes the implicit cfg
dependency explicit without changing behavior of `mod shielded` or the
`create_genesis_state` / `create_sdk_test_data` logic.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 982960e4-b61f-4ffe-8949-f2d6d955a10a
📒 Files selected for processing (6)
.cargo/config-shielded-test-data.tomlDockerfilepackages/rs-drive-abci/Cargo.tomlpackages/rs-drive-abci/src/execution/platform_events/initialization/create_genesis_state/test/mod.rspackages/rs-drive-abci/src/lib.rspackages/rs-drive-abci/src/main.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/rs-drive-abci/Cargo.toml
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Code Review
Verified the PR's cfg gating against the worktree at bf2efc5. The single nitpick from Claude is real-but-speculative: under the documented invocation contract (.cargo/config-shielded-test-data.toml sets both cfgs; the SHIELDED_TEST_DATA Dockerfile arg implies SDK_TEST_DATA), the asymmetric cfg keys never produce a silent no-op, and the author already documents the invariant in both mod.rs and test/mod.rs. Dropping it as speculative, low-value, and outside the PR's stated goal.
…-dump-load` Picks up the post-refactor tip of dashpay/grovedb#752 (`2e9dff50`), which: 1. Renames `replace_commitment_tree_subtree_root` → `replace_subtree_root` and makes it generic over Element variant (caller builds whichever non-Merk tree element they're snapshot-restoring). 2. Gates all three snapshot-bootstrap entry points (`raw_storage`, `ingest_subtree_sst`, `replace_subtree_root`) behind a new `unsafe-dump-load` Cargo feature, so production grovedb has no compiled access to them. Platform changes: * `drive` exposes a pass-through `unsafe-dump-load` feature that forwards to `grovedb/unsafe-dump-load`. * `drive-abci` exposes a pass-through `unsafe-dump-load` feature that forwards to `drive/unsafe-dump-load`. * `drive-abci`'s `[dev-dependencies]` always enables the feature so `cargo test` can run the snapshot roundtrip test. * Dockerfile passes `--features=unsafe-dump-load` to both `cargo chef cook` and `cargo build` when `SHIELDED_TEST_DATA=true` (composes correctly with any user-supplied `ADDITIONAL_FEATURES`). * Callers in `shielded_snapshot` and the snapshot-bake path now construct `Element::new_commitment_tree(total_count, chunk_power, flags)` locally and pass it to `replace_subtree_root`. Verified locally: * Default build (no SDK_TEST_DATA, no SHIELDED_TEST_DATA): compiles, no snapshot-bootstrap APIs in the binary. * `SDK_TEST_DATA=true` only: compiles, no shielded code or APIs. * `SHIELDED_TEST_DATA=true`: compiles with feature on. * `snapshot_dump_apply_preserves_anchor` test (with feature on) passes — 30k notes dumped + reapplied yields byte-equivalent anchor. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## v3.1-dev #3774 +/- ##
============================================
+ Coverage 87.16% 87.18% +0.02%
============================================
Files 2621 2624 +3
Lines 320743 321014 +271
============================================
+ Hits 279583 279892 +309
+ Misses 41160 41122 -38
🚀 New features to boost your workflow:
|
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Code Review
Latest delta cleanly forwards a new unsafe-dump-load feature through drive-abci → drive → grovedb to gate the snapshot-bootstrap APIs, and adapts call sites to grovedb's renamed replace_subtree_root. Carried-forward prior findings: none (the prior no-finding review at bf2efc5 remains applicable). Two suggestion-level findings remain about build-invariant enforcement: the new create_shielded_test_data cfg and the unsafe-dump-load feature are coupled only by convention/Dockerfile, with no compile-time guard if someone hand-rolls a build.
🟡 2 suggestion(s)
1 additional finding(s) omitted (not in diff).
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `packages/rs-drive-abci/src/main.rs`:
- [SUGGESTION] packages/rs-drive-abci/src/main.rs:83-89: `SnapshotBake` compiles without `create_sdk_test_data`, yielding a silently empty snapshot
`SnapshotBake` is gated only by `#[cfg(create_shielded_test_data)]`, but the genesis hook that actually seeds the shielded pool lives behind `#[cfg(create_sdk_test_data)]` in `create_genesis_state/mod.rs:53-57` (it dispatches to `create_sdk_test_data`, which then runs `create_data_for_shielded_pool` under its own `#[cfg(create_shielded_test_data)]`). The `.cargo/config-shielded-test-data.toml` profile sets both cfgs together, but the invariant exists only in comments — a direct `cargo build --cfg create_shielded_test_data` produces a valid `snapshot-bake` command whose `create_genesis_state()` skips `create_sdk_test_data` entirely, baking a snapshot with no shielded data. Add a compile-time guard so this fails fast.
In `.cargo/config-shielded-test-data.toml`:
- [SUGGESTION] .cargo/config-shielded-test-data.toml:30-38: Shielded-test-data cargo profile activates cfgs without the `unsafe-dump-load` feature they require
This profile injects `create_sdk_test_data` and `create_shielded_test_data` cfgs, which activate `shielded_snapshot/mod.rs` and the seeding path in `test/shielded.rs`. Those modules now call grovedb APIs (`raw_storage`, `ingest_subtree_sst`, `replace_subtree_root`) that are gated behind the new `unsafe-dump-load` feature on drive-abci → drive → grovedb (this commit). The Dockerfile correctly appends `--features unsafe-dump-load` when `SHIELDED_TEST_DATA=true`, but cargo config files cannot enable Cargo features, so a direct `cargo build` against just this profile will fail with method-missing errors with no clear pointer to the missing feature. Either add a `compile_error!` guard when the cfg is set without the feature, or document the required `--features` flag in the profile's header comment.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Code Review
Latest delta 614ae01..e8f3099 is a cargo-fmt-only change (collapses snapshot_bake_main::run signature) with no semantic effect. Both prior findings remain STILL VALID at the current head: SnapshotBake is still gated on create_shielded_test_data alone while the seeder it depends on is reached only under create_sdk_test_data, and .cargo/config-shielded-test-data.toml still activates cfgs without enabling the unsafe-dump-load cargo feature those cfgs require.
🟡 1 suggestion(s)
1 additional finding(s) omitted (not in diff).
1 carried-forward finding(s) already raised on this PR; not re-posting as new inline comments.
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `packages/rs-drive-abci/src/main.rs`:
- [SUGGESTION] packages/rs-drive-abci/src/main.rs:83-89: [Carried-forward — STILL VALID] Gate `SnapshotBake` on both `create_sdk_test_data` and `create_shielded_test_data`
`SnapshotBake` is gated only by `#[cfg(create_shielded_test_data)]` at main.rs:83 (and the dispatch at main.rs:188-189 and config branch at main.rs:209-214). However, the shielded seeder is only reached through `create_sdk_test_data`: `create_genesis_state` calls `self.create_sdk_test_data(...)` under `#[cfg(create_sdk_test_data)]` at packages/rs-drive-abci/src/execution/platform_events/initialization/create_genesis_state/mod.rs:53-57, and that function in turn dispatches to `create_data_for_shielded_pool` under `#[cfg(create_shielded_test_data)]`. A build that sets only `--cfg create_shielded_test_data` (without `create_sdk_test_data`) will compile the `snapshot-bake` CLI, run a full genesis cycle, skip the entire SDK-test-data seeding branch, and then write a snapshot of an unseeded shielded subtree — a silent empty-bake rather than a loud failure. The new cargo profile happens to set both cfgs, but the binary itself doesn't enforce the invariant. Gating `SnapshotBake` (and the surrounding match arm / config branch) on `all(create_sdk_test_data, create_shielded_test_data)` turns the only invalid combination into a compile-time error.
…lop) Repoints the pin from the pre-merge feat branch tip (`2e9dff50455083dc9fbf15b954cea602431acf9d`) to the squash-merge commit on grovedb develop (`a18f7929460ef9c5d814f61ff84d8805b2a1761b`). Same content, canonical address. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Code Review
Latest delta is solely a grovedb dependency rev bump (2e9dff50 → a18f7929) across six Cargo.toml files and Cargo.lock; no code logic changed since the prior reviewed SHA. Both prior structural-robustness suggestions are re-verified STILL VALID at e230766 — SnapshotBake's cfg gate and the shielded cargo config remain decoupled from the genesis-seed gate and the unsafe-dump-load feature respectively. No new defects introduced in this delta; carrying forward both prior suggestions.
🟡 1 suggestion(s)
1 additional finding(s) omitted (not in diff).
1 carried-forward finding(s) already raised on this PR; not re-posting as new inline comments.
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `packages/rs-drive-abci/src/main.rs`:
- [SUGGESTION] packages/rs-drive-abci/src/main.rs:83-89: [Carried-forward — STILL VALID] Gate `SnapshotBake` on both `create_sdk_test_data` and `create_shielded_test_data`
`Commands::SnapshotBake` (main.rs:83-89) is still gated only by `#[cfg(create_shielded_test_data)]`, while the genesis seeding hook lives behind a different cfg: `create_genesis_state/mod.rs:53-57` only calls `self.create_sdk_test_data(...)` under `#[cfg(create_sdk_test_data)]`, and the shielded-pool seeder runs from inside that `create_sdk_test_data` path. The two cfgs are independent, so a build that activates `create_shielded_test_data` alone (e.g. through `RUSTFLAGS` rather than the Docker/cargo profile that today sets both) would expose `snapshot-bake` while compiling the seeder out. In that case the command runs to completion against a fresh genesis with no shielded notes inserted, producing an empty-but-successful snapshot rather than failing. The doc comment on main.rs:80-82 even acknowledges that the seeder is what makes the command meaningful, but the cfg only enforces half of that contract. Tighten the gate to `#[cfg(all(create_sdk_test_data, create_shielded_test_data))]`, or add a `compile_error!` when only one of the two is active, so the misuse is loud at build time.
…elded_test_data Cargo feature
Codex reviewer flagged that the previous design split shielded-pool gating
across two mechanisms (a cfg flag from `.cargo/config-shielded-test-data.toml`
plus a `unsafe-dump-load` Cargo feature for the underlying grovedb APIs),
which made it possible to have the cfg set without the feature — surfacing
as cryptic method-missing compile errors instead of a clear feature-not-enabled
message.
Collapses the two mechanisms into a single Cargo feature chain:
drive-abci/shielded_test_data
└── drive/shielded_test_data
└── grovedb/unsafe-dump-load
The grovedb-side feature keeps its `unsafe-dump-load` name since it
describes the primitive there; in this tree the only use case for those
primitives is shielded-pool test seeding, hence the renamed handle.
All `#[cfg(create_shielded_test_data)]` attributes in drive-abci become
`#[cfg(feature = "shielded_test_data")]`. Production builds carry neither
the seeder nor the bake/apply code paths; `SHIELDED_TEST_DATA=true` in
the Dockerfile adds `--features=shielded_test_data` to the cargo
invocation.
Drops:
* `.cargo/config-shielded-test-data.toml` (no longer needed — the outer
`create_sdk_test_data` cfg keeps its own `config-test-sdk-data.toml`,
matching the pre-PR pattern).
* `create_shielded_test_data` from drive-abci's check-cfg allowlist.
The outer `create_sdk_test_data` gate stays a rustc cfg (unchanged); the
project's existing rationale applies to the SDK-fixture machinery as
before, but doesn't apply to the narrower shielded path add-on.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
shumkov
changed the title
shielded_test_data feature
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Code Review
The cfg→Cargo-feature refactor cleanly resolves the prior cargo-profile/grovedb feature-resolution mismatch by deleting .cargo/config-shielded-test-data.toml and chaining drive-abci/shielded_test_data → drive/shielded_test_data → grovedb/unsafe-dump-load. One prior structural finding remains valid: SnapshotBake is now gated only on the new Cargo feature while the seeder dispatch in create_genesis_state still requires the unchanged cfg(create_sdk_test_data), so a feature-only build can still produce an empty-but-successful snapshot. One new minor doc inconsistency was introduced in the Dockerfile bake-stage comment.
🟡 1 suggestion(s) | 💬 1 nitpick(s)
1 additional finding(s) omitted (not in diff).
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `packages/rs-drive-abci/src/main.rs`:
- [SUGGESTION] packages/rs-drive-abci/src/main.rs:83-89: [Carried-forward — STILL VALID] `SnapshotBake` is exposed under `feature = "shielded_test_data"` without the `create_sdk_test_data` genesis hook it depends on
`Commands::SnapshotBake` (main.rs:83), the dispatch arm (main.rs:188-189), and the `load_config` bypass (main.rs:209-215) are gated only on `#[cfg(feature = "shielded_test_data")]`. The seeder is reached via `Platform::create_sdk_test_data`, but that call is gated on `#[cfg(create_sdk_test_data)]` in `packages/rs-drive-abci/src/execution/platform_events/initialization/create_genesis_state/mod.rs:53-57`, and the shielded seeder inside it is further gated on `#[cfg(feature = "shielded_test_data")]` (`create_genesis_state/test/mod.rs:50-51`). Cargo features cannot enable rustc `--cfg` flags, so an ad-hoc `cargo build -p drive-abci --features shielded_test_data` (or any path that doesn't also install `.cargo/config-test-sdk-data.toml`) compiles the `snapshot-bake` CLI yet `create_genesis_state` never invokes `create_sdk_test_data`, producing an empty-but-successful snapshot that would later be applied by `DRIVE_SHIELDED_SNAPSHOT`. The Dockerfile (Dockerfile:570-574) sets both correctly so production/devnet images are unaffected, and `create_sdk_test_data` runtime-rejects non-Regtest/Devnet networks (test/mod.rs:33-40) so there is no mainnet/testnet exposure. The remaining risk is a latent build-integrity footgun against direct `cargo` invocations and future refactors. Fix by either also gating `SnapshotBake` on `cfg(create_sdk_test_data)`, or adding a `compile_error!` when `feature = "shielded_test_data"` is enabled without the cfg. The docstring at main.rs:78-82 ("Requires the binary to be built with `--features=shielded_test_data`") should also be updated to mention the `create_sdk_test_data` cfg requirement.
| # The drive-abci binary itself only contains the `snapshot-bake` subcommand | ||
| # when built with `--cfg create_shielded_test_data` (selected by the | ||
| # SHIELDED_TEST_DATA build arg in the build stage), so invoking it on an | ||
| # SDK-only binary would fail with "unknown subcommand" anyway. |
There was a problem hiding this comment.
💬 Nitpick: Stale Dockerfile bake-stage comment still references --cfg create_shielded_test_data
The bake-stage comment claims the snapshot-bake subcommand is only present "when built with --cfg create_shielded_test_data", but the build stage above now activates the subcommand via --features=shielded_test_data (Dockerfile:573-574, 584, 619). Update the comment so the gate description matches the actual build invocation; otherwise a future reader patching this stage may set a no-longer-recognized cfg.
| # The drive-abci binary itself only contains the `snapshot-bake` subcommand | |
| # when built with `--cfg create_shielded_test_data` (selected by the | |
| # SHIELDED_TEST_DATA build arg in the build stage), so invoking it on an | |
| # SDK-only binary would fail with "unknown subcommand" anyway. | |
| # The drive-abci binary itself only contains the `snapshot-bake` subcommand | |
| # when built with `--features=shielded_test_data` (selected by the | |
| # SHIELDED_TEST_DATA build arg in the build stage), so invoking it on an | |
| # SDK-only binary would fail with "unknown subcommand" anyway. |
source: ['claude']
3 participants
Summary
Two related changes, both keeping the shielded-pool stress-test path strictly opt-in:
1. Bump grovedb pin to
a18f7929(develop tip post-merge of dashpay/grovedb#752)Picks up the squash-merge of dashpay/grovedb#752 on grovedb develop. Net contribution to platform: three snapshot-bootstrap primitives behind grovedb's
unsafe-dump-loadfeature:GroveDb::raw_storage()— borrow underlyingRocksDbStorageRocksDbStorage::ingest_subtree_sst()— bulk-load an SST into a CFGroveDb::replace_subtree_root()— patch a parent Merk leaf with a caller-provided child hash + elementUn-ignores
snapshot_dump_apply_preserves_anchorin drive-abci (was gated on the unmerged grovedb branch).2. Gate shielded-pool seeding behind a new
shielded_test_dataCargo featureThe 1M-note shielded-pool seeding (and the bake/apply machinery added in #3732) used to ride along with every
SDK_TEST_DATA=truebuild. That's expensive and undesirable for most devnet setups — they just want the base SDK fixtures (addresses, group token queries, token direct prices) like before #3732.Single feature chain controls both the in-tree code paths AND the underlying grovedb primitives:
drive-abci/shielded_test_datacompiles inmod shielded,mod shielded_snapshot, and thesnapshot-bakeCLI subcommand.drive/shielded_test_data→grovedb/unsafe-dump-loadso the grovedb APIs above are unlocked through the dep chain.create_sdk_test_datacfg gate is unchanged (still rustc cfg, still activated by.cargo/config-test-sdk-data.toml). The shielded gate is purely additive inside it.Build matrix:
SDK_TEST_DATASHIELDED_TEST_DATAtruetruetrueDockerfile
SHIELDED_TEST_DATA=trueadds--features=shielded_test_datato bothcargo chef cookandcargo build. Bake stage produces/artifacts/shielded-pool.snap; runtime stage wiresDRIVE_SHIELDED_SNAPSHOTinto the binary's.envonly when that file exists. SDK-only builds (SDK_TEST_DATA=truewithoutSHIELDED_TEST_DATA) skip the feature, the bake, and the env wiring entirely.SHIELDED_TEST_DATA=trueimpliesSDK_TEST_DATA=true— the shielded seeder runs insidecreate_sdk_test_data.Why a feature, not another cfg flag?
First iteration of this PR used a second cfg (
create_shielded_test_data) paired with a.cargo/config-shielded-test-data.tomlprofile. The codex review on the original commit pointed out the obvious risk: cargo config files can't enable Cargo features, so the cfg could be set without the grovedb feature being enabled, which would surface as cryptic "method not found" compile errors rather than a clear opt-in message.The simplification: drop the second cfg, use a single Cargo feature, let cargo's normal feature resolution handle the in-tree gating AND the cross-crate API availability. The outer
create_sdk_test_datastays a cfg because its project-stated rationale ("don't activate this withcargo test --all-features") was load-bearing for the SDK-fixture path; the shielded add-on doesn't need that protection since the feature itself is the opt-in.Verified locally
cargo check -p drive-abci--features shielded_test_datacargo fmt --check --allsnapshot_dump_apply_preserves_anchor(debug, 8 min): 30k notes → 8.6 MB SST → applied to fresh platform → byte-equivalent Sinsemilla anchordf37726e…4c2300. The 6 existing shielded seed tests stay green.Notes
unsafe-dump-loadfeature name — the name describes the primitive correctly there. The drive-abci/drive features are namedshielded_test_databecause in this tree the only consumer of those primitives is shielded-pool test seeding.unsafe_dump_load_subtree_roundtrip_preserves_root_hash), plus consumer-level viasnapshot_dump_apply_preserves_anchorhere.Test plan
SHIELDED_TEST_DATA=trueproduces a runtime image with/opt/dashmate/snapshots/shielded-pool.snapand theDRIVE_SHIELDED_SNAPSHOTenv var set.SDK_TEST_DATA=true(noSHIELDED_TEST_DATA) produces a working drive-abci with the base SDK fixtures and no shielded snapshot.🤖 Generated with Claude Code