feat: upgrade miden dependencies to v0.16.x#331
Open
zeljkoX wants to merge 4 commits into
Open
Conversation
WalkthroughThe repository is upgraded to Miden 0.16 prerelease dependencies and Rust 1.96.1. Account deltas now use patch-based reconstruction, RPC bindings are generated during builds, MASM scripts use updated entry syntax, SDK procedure roots and payment APIs are refreshed, and compatibility documentation and package metadata are updated. ChangesMiden 0.16 migration
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant MidenRpcClient
participant MidenAccountInspector
participant guardian_shared_account_delta
participant Account
MidenRpcClient->>MidenAccountInspector: receive account delta
MidenAccountInspector->>guardian_shared_account_delta: reconstruct account with storage patch
guardian_shared_account_delta->>Account: apply AccountPatch
Account-->>MidenRpcClient: updated account state
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Codecov Report❌ Patch coverage is ❌ Your patch status has failed because the patch coverage (80.73%) is below the target coverage (90.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #331 +/- ##
==========================================
+ Coverage 77.76% 77.88% +0.11%
==========================================
Files 167 168 +1
Lines 30778 31086 +308
==========================================
+ Hits 23934 24210 +276
- Misses 6844 6876 +32 Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Pull request overview
Upgrades Guardian’s Rust workspace, TypeScript SDKs, and examples to the Miden v0.16 pre-release line, including the related protocol/API migrations (delta→patch, MASM authoring updates) and synchronized regenerated artifacts (procedure roots, kernel commitment, fixtures).
Changes:
- Bump Rust toolchain/MSRV and pin Rust Miden dependencies to exact
0.16.0-alpha.*versions. - Migrate core runtime paths (server/network, multisig SDK, contracts) to Miden 0.16 semantics (AccountPatch, new tx-script/MASM patterns, updated storage patch handling).
- Update TS packages/examples to
@miden-sdk/miden-sdk 0.16.0-alpha.1, refresh procedure roots/MASM, and update docs/specs for the new baseline.
Reviewed changes
Copilot reviewed 108 out of 117 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| speckit/features/329-upgrade-miden-016/spec.md | Migration feature spec (0.16 baseline, scenarios/requirements). |
| speckit/features/329-upgrade-miden-016/release-notes-draft.md | Draft breaking-change notes for 0.16 line. |
| speckit/features/329-upgrade-miden-016/quickstart.md | Execution/runbook for the migration + validation gates. |
| speckit/features/329-upgrade-miden-016/plan.md | Implementation plan and propagation checklist. |
| speckit/features/329-upgrade-miden-016/data-model.md | Pin matrix + artifact/state inventory for 0.16. |
| speckit/features/329-upgrade-miden-016/contracts/compatibility-contract.md | Compatibility guarantees for wire/artifact/version behavior. |
| speckit/features/329-upgrade-miden-016/checklists/requirements.md | Spec quality checklist for the feature. |
| rust-toolchain.toml | Bump toolchain channel to 1.96.1. |
| README.md | Document new Miden 0.16 baseline + interoperability note. |
| packages/miden-multisig-client/src/procedures.ts | Update procedure roots + add create_note. |
| packages/miden-multisig-client/src/account/masm/auth.ts | MASM updates for 0.16 (imports, account procedures). |
| packages/miden-multisig-client/src/account/masm/account-components/auth.ts | MASM pub use syntax updates for 0.16. |
| packages/miden-multisig-client/README.md | Document TS SDK now targeting Miden 0.16 pre-releases. |
| packages/miden-multisig-client/package.json | Bump package version + pin @miden-sdk/miden-sdk 0.16 alpha. |
| packages/miden-multisig-client/package-lock.json | Lockfile refresh for new TS dependency pins. |
| packages/miden-multisig-client/masm/auth/multisig.masm | Vendored MASM updated to 0.16 syntax/ops. |
| packages/miden-multisig-client/masm/auth/multisig_ecdsa.masm | Vendored MASM updated to 0.16 syntax/ops. |
| packages/miden-multisig-client/masm/auth/guardian.masm | Vendored MASM updated to 0.16 syntax/ops. |
| packages/miden-multisig-client/masm/auth/guardian_ecdsa.masm | Vendored MASM updated to 0.16 syntax/ops. |
| packages/miden-multisig-client/masm/account_components/auth/multisig.masm | Vendored account-components MASM syntax update. |
| packages/miden-multisig-client/masm/account_components/auth/multisig_guardian.masm | Vendored account-components MASM syntax update. |
| packages/miden-multisig-client/masm/account_components/auth/multisig_guardian_ecdsa.masm | Vendored account-components MASM syntax update. |
| packages/miden-multisig-client/masm/account_components/auth/multisig_ecdsa.masm | Vendored account-components MASM syntax update. |
| packages/guardian-operator-client/package.json | Bump package version to 0.16.0. |
| packages/guardian-operator-client/package-lock.json | Lockfile version bump for operator client. |
| packages/guardian-evm-client/package.json | Bump package version to 0.16.0. |
| packages/guardian-evm-client/package-lock.json | Lockfile version bump for EVM client. |
| packages/guardian-client/package.json | Bump package version to 0.16.0. |
| packages/guardian-client/package-lock.json | Lockfile version bump for guardian client. |
| examples/web/package.json | Pin @miden-sdk/* to 0.16 alpha + overrides for Para deps. |
| examples/web/package-lock.json | Lockfile refresh for web example dependency graph. |
| examples/smoke-web/package.json | Pin @miden-sdk/* to 0.16 alpha + overrides for Para deps. |
| examples/smoke-web/package-lock.json | Lockfile refresh for smoke-web dependency graph. |
| examples/rust/src/multisig.rs | Update tx script syntax to 0.16 form. |
| examples/rust/src/main.rs | Remove debug mode + switch to account_patch nonce output. |
| examples/rust/rust-toolchain.toml | Align example toolchain to 1.96.1. |
| examples/operator-smoke-web/package.json | Pin @miden-sdk/miden-sdk to 0.16 alpha. |
| examples/operator-smoke-web/package-lock.json | Lockfile refresh for operator smoke example. |
| examples/demo/src/actions/proposal_management.rs | Update build_transfer_asset call signature. |
| examples/_shared/multisig-browser/package.json | Pin @miden-sdk/* to 0.16 alpha + overrides for Para deps. |
| examples/_shared/multisig-browser/package-lock.json | Lockfile refresh for shared browser package. |
| docs/TROUBLESHOOTING.md | Add 0.15↔0.16 mismatch/reset troubleshooting entries. |
| docs/QUICKSTART.md | Update quickstart notes for 0.16 baseline + resets. |
| docs/openapi.json | Bump OpenAPI doc version to 0.16.0. |
| docs/openapi-evm.json | Bump OpenAPI doc version to 0.16.0. |
| docs/openapi-dashboard.json | Bump OpenAPI doc version to 0.16.0. |
| docs/openapi-client.json | Bump OpenAPI doc version to 0.16.0. |
| docs/MULTISIG_SDK.md | Update install/version matrix + tagging for 0.16 line. |
| docs/LOCAL_DEV.md | Note node/version-line compatibility requirement. |
| docs/guides/postgres-tls/.env.example | Add 0.16 baseline/version mismatch note. |
| docs/guides/observability/.env.example | Add 0.16 baseline/version mismatch note. |
| docs/guides/miden-dashboard/.env.example | Add 0.16 baseline/version mismatch note. |
| docs/guides/horizontal-scaling/.env.example | Add 0.16 baseline/version mismatch note. |
| docs/guides/aws-signers/.env.example | Add 0.16 baseline/version mismatch note. |
| Dockerfile | Bump Rust base image to 1.96.1. |
| crates/shared/src/lookup_auth_message.rs | Update AccountId dummy ctor for new callback flag param. |
| crates/shared/src/lib.rs | Export new shared delta/patch helper module + auth API updates. |
| crates/shared/src/auth_request_message.rs | Update AccountId dummy ctor for new callback flag param. |
| crates/shared/src/account_delta.rs | New helper to apply AccountDelta via AccountPatch in 0.16. |
| crates/shared/Cargo.toml | Use workspace rand dependency. |
| crates/server/src/testing/integration/lookup_helpers.rs | Update AccountId dummy ctor signature. |
| crates/server/src/testing/helpers.rs | Update delta construction to AccountStoragePatch + code param. |
| crates/server/src/testing/generate_fixtures.rs | Update fixture generation to patches + shared delta applier. |
| crates/server/src/testing/fixtures/delta_1.json | Regenerated 0.16 fixture values. |
| crates/server/src/testing/fixtures/delta_2.json | Regenerated 0.16 fixture values. |
| crates/server/src/testing/fixtures/delta_3.json | Regenerated 0.16 fixture values. |
| crates/server/src/testing/fixtures/commitments.json | Regenerated 0.16 fixture commitments. |
| crates/server/src/testing/e2e/switch_guardian_canonicalization.rs | Update tx-script syntax + apply_patch usage. |
| crates/server/src/storage/filesystem.rs | Sort deltas using Reverse key (style change). |
| crates/server/src/services/dashboard_account_snapshot.rs | Use NF asset id (not vault_key) for dashboard snapshot. |
| crates/server/src/network/miden/mod.rs | Apply deltas via patch model + custom delta-merge helper. |
| crates/server/src/network/miden/account_inspector.rs | Update storage map key access API usage. |
| crates/server/src/metadata/auth/miden_falcon_rpo.rs | Update AccountId dummy ctor signature. |
| crates/server/src/metadata/auth/miden_ecdsa.rs | Update AccountId dummy ctor signature. |
| crates/server/src/evm/session.rs | rand trait import adjustment for token generation. |
| crates/server/src/delta_summary/projection.rs | Update note/tag handling + storage/vault projection for 0.16. |
| crates/server/src/delta_summary/build.rs | Update note creation pattern for 0.16 builder API. |
| crates/server/src/dashboard/cursor.rs | rand trait import adjustment for cursor secret generation. |
| crates/server/Cargo.toml | Move miden-protocol testing feature to dev-deps; workspace rand. |
| crates/server/bench/loadgen/src/scenarios.rs | Update delta construction for AccountStoragePatch + code param. |
| crates/miden-rpc-client/src/lib.rs | Switch to miden-node-proto-build-generated bindings include!. |
| crates/miden-rpc-client/Cargo.toml | Add build-deps for proto codegen; remove direct proto dep. |
| crates/miden-rpc-client/build.rs | Generate tonic bindings wrapper for stable include path. |
| crates/miden-multisig-client/src/transaction/payment.rs | Update P2ID note/send-script building for 0.16 APIs. |
| crates/miden-multisig-client/src/transaction/mod.rs | Update salt RNG call + kernel commitment constant. |
| crates/miden-multisig-client/src/transaction/guardian.rs | Update tx-script syntax to 0.16 form. |
| crates/miden-multisig-client/src/transaction/configuration/config.rs | Update tx-script syntax to 0.16 form. |
| crates/miden-multisig-client/src/transaction/builder.rs | Update asset builder signature + delta constructors in tests. |
| crates/miden-multisig-client/src/proposal.rs | Update delta constructors in tests. |
| crates/miden-multisig-client/src/procedures.rs | Add CreateNote procedure + updated roots and tests. |
| crates/miden-multisig-client/src/export.rs | Update delta constructors in tests. |
| crates/miden-multisig-client/src/execution.rs | Update asset transfer helper for 0.16 callback semantics. |
| crates/miden-multisig-client/src/client/proposals.rs | Update delta constructors in tests. |
| crates/miden-multisig-client/src/client/helpers.rs | Switch local account rebuild to account_patch apply path. |
| crates/miden-multisig-client/src/client/account.rs | Update RNG fill + delta apply via shared helper. |
| crates/miden-multisig-client/src/builder.rs | Remove debug-mode usage in client builder. |
| crates/miden-multisig-client/src/account.rs | Update storage map key API usage. |
| crates/miden-multisig-client/examples/procedure_roots.rs | Include create_note root mapping in generator output. |
| crates/miden-multisig-client/Cargo.toml | Bump internal crate versions to =0.16.0. |
| crates/miden-keystore/src/keystore.rs | Update rand trait bounds/imports for new rand line. |
| crates/miden-keystore/Cargo.toml | Use workspace rand dependency. |
| crates/contracts/tests/auth/multisig.rs | Update tx-script syntax + patch APIs in contract tests. |
| crates/contracts/src/multisig_guardian.rs | Update parity assertions to new 0.16-derived identities. |
| crates/contracts/src/masm_builder.rs | Migrate MASM compilation/linking to 0.16 assembler/code builder APIs. |
| crates/contracts/masm/auth/multisig.masm | MASM 0.16 import/op updates + account procedure annotations. |
| crates/contracts/masm/auth/multisig_ecdsa.masm | MASM 0.16 import/op updates + account procedure annotations. |
| crates/contracts/masm/auth/guardian.masm | MASM 0.16 import/op updates + account procedure annotations. |
| crates/contracts/masm/auth/guardian_ecdsa.masm | MASM 0.16 import/op updates + account procedure annotations. |
| crates/contracts/masm/account_components/auth/multisig.masm | MASM 0.16 pub use syntax update. |
| crates/contracts/masm/account_components/auth/multisig_guardian.masm | MASM 0.16 pub use syntax update. |
| crates/contracts/masm/account_components/auth/multisig_guardian_ecdsa.masm | MASM 0.16 pub use syntax update. |
| crates/contracts/masm/account_components/auth/multisig_ecdsa.masm | MASM 0.16 pub use syntax update. |
| crates/contracts/Cargo.toml | Bump guardian-shared version pin to 0.16.0. |
| crates/client/src/transaction.rs | Update delta constructor signature in tests. |
| crates/client/Cargo.toml | Bump guardian-shared version pin to 0.16.0. |
| Cargo.toml | Bump workspace version/MSRV + exact-pin Miden 0.16 alphas + rand 0.10. |
| benchmarks/prod-server/src/operations.rs | Update delta construction for AccountStoragePatch + code param. |
| AGENTS.md | Update repo baseline statement to Miden 0.16 pre-release line. |
| .github/workflows/ci.yml | Bump CI toolchain to 1.96.1 + add shared crate check. |
| .agents/skills/smoke-test-ts-multisig-sdk/SKILL.md | Update pinned versions for 0.16 testing. |
| .agents/skills/smoke-test-rust-multisig-sdk/SKILL.md | Update pinned versions for 0.16 testing. |
Files not reviewed (8)
- examples/_shared/multisig-browser/package-lock.json: Generated file
- examples/operator-smoke-web/package-lock.json: Generated file
- examples/smoke-web/package-lock.json: Generated file
- examples/web/package-lock.json: Generated file
- packages/guardian-client/package-lock.json: Generated file
- packages/guardian-evm-client/package-lock.json: Generated file
- packages/guardian-operator-client/package-lock.json: Generated file
- packages/miden-multisig-client/package-lock.json: Generated file
Comments suppressed due to low confidence (2)
packages/miden-multisig-client/package.json:34
@openzeppelin/miden-multisig-clientis bumped to 0.16.0, but it still depends on@openzeppelin/guardian-clientas^0.15.0. This allows installing an older guardian-client alongside the 0.16 multisig client, which is very likely incompatible with the new baseline/versioning policy. Align the dependency range with the 0.16 line.
packages/miden-multisig-client/package-lock.json:16- The lockfile still records
@openzeppelin/guardian-clientas^0.15.0, which is inconsistent with this package being versioned to 0.16.0 and will allow/record installing an older guardian-client. After aligningpackage.json, regenerate/update the lockfile entry to match the 0.16 line.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+481
to
+488
| fn merge_account_deltas( | ||
| base: miden_protocol::account::AccountDelta, | ||
| next: miden_protocol::account::AccountDelta, | ||
| ) -> Result<miden_protocol::account::AccountDelta, String> { | ||
| let account_id = base.id(); | ||
| let (mut storage, mut vault, code, nonce_delta) = base.into_parts(); | ||
| let (next_storage, next_vault, next_code, next_nonce_delta) = next.into_parts(); | ||
|
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
crates/shared/src/account_delta.rs (1)
66-94: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd test coverage for the nonce-zero-to-one reconstruction branch.
account_patch_from_deltaspecial-cases the transition fromFelt::ZEROtoFelt::ONE(first transaction on an undeployed account) by rebuilding a full-state patch from the account's current slots. Neither existing test (applies_create_update_and_remove_storage_operations,converts_relative_vault_delta_to_absolute_patch) exercises this branch — both start accounts atFelt::ONE. Given this path feeds directly into server/client commitment reconstruction, it's worth a dedicated test.🤖 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 `@crates/shared/src/account_delta.rs` around lines 66 - 94, Add a focused test for account_patch_from_delta covering an account with nonce Felt::ZERO and a delta nonce of Felt::ONE. Verify the reconstructed patch includes the account’s existing storage and vault state along with the delta changes, and confirms the resulting nonce is Felt::ONE; leave the existing tests unchanged.crates/miden-multisig-client/src/procedures.rs (1)
189-193: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd ECDSA root coverage to this test.
The test validates ordering and roots only for
MultisigGuardianBuilder; it cannot catch scheme-specific root mismatches in ECDSA accounts. Add an ECDSA account assertion before relying on these constants.🤖 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 `@crates/miden-multisig-client/src/procedures.rs` around lines 189 - 193, Add ECDSA account root coverage to the test around the existing MultisigGuardianBuilder assertions, using the appropriate ECDSA account builder or procedure-root assertion before validating the hard-coded roots. Keep the existing ordering checks intact and ensure the new assertion verifies the scheme-specific root against the expected ECDSA value.crates/miden-multisig-client/src/client/helpers.rs (1)
375-387: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd regression coverage for the Miden 0.16 state and asset migrations.
crates/miden-multisig-client/src/client/helpers.rs#L375-L387: cover both full-state reconstruction and incrementalapply_patch(...), including an upstream finalize flow.crates/miden-multisig-client/src/execution.rs#L88-L94: replace the removed callback tests with a P2ID asset construction/execution test and invalid-input coverage.Based on learnings, update tests in the layer where behavior changes and at least one upstream consumer; run targeted tests before the broader suite.
🤖 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 `@crates/miden-multisig-client/src/client/helpers.rs` around lines 375 - 387, Update tests in crates/miden-multisig-client/src/client/helpers.rs around the account reconstruction flow to cover both full-state Account reconstruction and incremental apply_patch, including an upstream finalize flow. In crates/miden-multisig-client/src/execution.rs, replace the removed callback tests with coverage for P2ID asset construction and execution plus invalid-input cases, then run the targeted tests before the broader suite.Source: Learnings
🤖 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.
Inline comments:
In `@crates/contracts/src/masm_builder.rs`:
- Around line 137-154: Cache the auth-support library assembled by
assemble_with_auth_support, including the parsed auth modules, so repeated
get_multisig_library, get_multisig_ecdsa_library, and get_guardian_library calls
do not re-read and re-parse every file under auth_dir(). Use OnceLock or Lazy at
the shared library/support boundary, preserve the existing assembly result and
error behavior, and ensure each target-library accessor reuses the cached value
across transaction-script builds.
In `@crates/server/src/services/dashboard_account_snapshot.rs`:
- Line 138: Add a value-level assertion to the snapshot test around the key_word
assignment, constructing a known non-fungible asset and verifying that vault_key
equals the hexadecimal representation of a.id().to_word(). Ensure the test
executes with at least one NFT so it would fail under the previous vault_key
behavior.
In `@crates/server/src/testing/e2e/switch_guardian_canonicalization.rs`:
- Around line 205-208: Update the expect message on the apply_patch call for
executed_account so it says “executed patch applies” instead of referring to an
executed delta.
---
Nitpick comments:
In `@crates/miden-multisig-client/src/client/helpers.rs`:
- Around line 375-387: Update tests in
crates/miden-multisig-client/src/client/helpers.rs around the account
reconstruction flow to cover both full-state Account reconstruction and
incremental apply_patch, including an upstream finalize flow. In
crates/miden-multisig-client/src/execution.rs, replace the removed callback
tests with coverage for P2ID asset construction and execution plus invalid-input
cases, then run the targeted tests before the broader suite.
In `@crates/miden-multisig-client/src/procedures.rs`:
- Around line 189-193: Add ECDSA account root coverage to the test around the
existing MultisigGuardianBuilder assertions, using the appropriate ECDSA account
builder or procedure-root assertion before validating the hard-coded roots. Keep
the existing ordering checks intact and ensure the new assertion verifies the
scheme-specific root against the expected ECDSA value.
In `@crates/shared/src/account_delta.rs`:
- Around line 66-94: Add a focused test for account_patch_from_delta covering an
account with nonce Felt::ZERO and a delta nonce of Felt::ONE. Verify the
reconstructed patch includes the account’s existing storage and vault state
along with the delta changes, and confirms the resulting nonce is Felt::ONE;
leave the existing tests unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 11a8b5ba-e204-404d-9bfe-66ef565034bd
⛔ Files ignored due to path filters (9)
Cargo.lockis excluded by!**/*.lockexamples/_shared/multisig-browser/package-lock.jsonis excluded by!**/package-lock.jsonexamples/operator-smoke-web/package-lock.jsonis excluded by!**/package-lock.jsonexamples/smoke-web/package-lock.jsonis excluded by!**/package-lock.jsonexamples/web/package-lock.jsonis excluded by!**/package-lock.jsonpackages/guardian-client/package-lock.jsonis excluded by!**/package-lock.jsonpackages/guardian-evm-client/package-lock.jsonis excluded by!**/package-lock.jsonpackages/guardian-operator-client/package-lock.jsonis excluded by!**/package-lock.jsonpackages/miden-multisig-client/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (108)
.agents/skills/smoke-test-rust-multisig-sdk/SKILL.md.agents/skills/smoke-test-ts-multisig-sdk/SKILL.md.github/workflows/ci.ymlAGENTS.mdCargo.tomlDockerfileREADME.mdbenchmarks/prod-server/src/operations.rscrates/client/Cargo.tomlcrates/client/src/transaction.rscrates/contracts/Cargo.tomlcrates/contracts/masm/account_components/auth/multisig.masmcrates/contracts/masm/account_components/auth/multisig_ecdsa.masmcrates/contracts/masm/account_components/auth/multisig_guardian.masmcrates/contracts/masm/account_components/auth/multisig_guardian_ecdsa.masmcrates/contracts/masm/auth/guardian.masmcrates/contracts/masm/auth/guardian_ecdsa.masmcrates/contracts/masm/auth/multisig.masmcrates/contracts/masm/auth/multisig_ecdsa.masmcrates/contracts/src/masm_builder.rscrates/contracts/src/multisig_guardian.rscrates/contracts/tests/auth/multisig.rscrates/miden-keystore/Cargo.tomlcrates/miden-keystore/src/keystore.rscrates/miden-multisig-client/Cargo.tomlcrates/miden-multisig-client/examples/procedure_roots.rscrates/miden-multisig-client/src/account.rscrates/miden-multisig-client/src/builder.rscrates/miden-multisig-client/src/client/account.rscrates/miden-multisig-client/src/client/helpers.rscrates/miden-multisig-client/src/client/proposals.rscrates/miden-multisig-client/src/execution.rscrates/miden-multisig-client/src/export.rscrates/miden-multisig-client/src/procedures.rscrates/miden-multisig-client/src/proposal.rscrates/miden-multisig-client/src/transaction/builder.rscrates/miden-multisig-client/src/transaction/configuration/config.rscrates/miden-multisig-client/src/transaction/guardian.rscrates/miden-multisig-client/src/transaction/mod.rscrates/miden-multisig-client/src/transaction/payment.rscrates/miden-rpc-client/Cargo.tomlcrates/miden-rpc-client/build.rscrates/miden-rpc-client/src/lib.rscrates/server/Cargo.tomlcrates/server/bench/loadgen/src/scenarios.rscrates/server/src/dashboard/cursor.rscrates/server/src/delta_summary/build.rscrates/server/src/delta_summary/projection.rscrates/server/src/evm/session.rscrates/server/src/metadata/auth/miden_ecdsa.rscrates/server/src/metadata/auth/miden_falcon_rpo.rscrates/server/src/network/miden/account_inspector.rscrates/server/src/network/miden/mod.rscrates/server/src/services/dashboard_account_snapshot.rscrates/server/src/storage/filesystem.rscrates/server/src/testing/e2e/switch_guardian_canonicalization.rscrates/server/src/testing/fixtures/account.jsoncrates/server/src/testing/fixtures/commitments.jsoncrates/server/src/testing/fixtures/delta_1.jsoncrates/server/src/testing/fixtures/delta_2.jsoncrates/server/src/testing/fixtures/delta_3.jsoncrates/server/src/testing/fixtures/keys.jsoncrates/server/src/testing/generate_fixtures.rscrates/server/src/testing/helpers.rscrates/server/src/testing/integration/lookup_helpers.rscrates/shared/Cargo.tomlcrates/shared/src/account_delta.rscrates/shared/src/auth_request_message.rscrates/shared/src/lib.rscrates/shared/src/lookup_auth_message.rsdocs/LOCAL_DEV.mddocs/MULTISIG_SDK.mddocs/QUICKSTART.mddocs/TROUBLESHOOTING.mddocs/guides/aws-signers/.env.exampledocs/guides/horizontal-scaling/.env.exampledocs/guides/miden-dashboard/.env.exampledocs/guides/observability/.env.exampledocs/guides/postgres-tls/.env.exampledocs/openapi-client.jsondocs/openapi-dashboard.jsondocs/openapi-evm.jsondocs/openapi.jsonexamples/_shared/multisig-browser/package.jsonexamples/demo/src/actions/proposal_management.rsexamples/operator-smoke-web/package.jsonexamples/rust/rust-toolchain.tomlexamples/rust/src/main.rsexamples/rust/src/multisig.rsexamples/smoke-web/package.jsonexamples/web/package.jsonpackages/guardian-client/package.jsonpackages/guardian-evm-client/package.jsonpackages/guardian-operator-client/package.jsonpackages/miden-multisig-client/README.mdpackages/miden-multisig-client/masm/account_components/auth/multisig.masmpackages/miden-multisig-client/masm/account_components/auth/multisig_ecdsa.masmpackages/miden-multisig-client/masm/account_components/auth/multisig_guardian.masmpackages/miden-multisig-client/masm/account_components/auth/multisig_guardian_ecdsa.masmpackages/miden-multisig-client/masm/auth/guardian.masmpackages/miden-multisig-client/masm/auth/guardian_ecdsa.masmpackages/miden-multisig-client/masm/auth/multisig.masmpackages/miden-multisig-client/masm/auth/multisig_ecdsa.masmpackages/miden-multisig-client/package.jsonpackages/miden-multisig-client/src/account/masm/account-components/auth.tspackages/miden-multisig-client/src/account/masm/auth.tspackages/miden-multisig-client/src/procedures.tsrust-toolchain.toml
💤 Files with no reviewable changes (1)
- crates/miden-multisig-client/src/builder.rs
Comment on lines
+137
to
154
| /// Assembles a single-root library with the `openzeppelin::auth` modules available as support. | ||
| fn assemble_with_auth_support( | ||
| package_name: &str, | ||
| root_path: &str, | ||
| code: String, | ||
| ) -> Result<Library> { | ||
| let source_manager: Arc<dyn SourceManagerSync> = Arc::new(DefaultSourceManager::default()); | ||
| let assembler = kernel_assembler(source_manager.clone())?; | ||
|
|
||
| let root = parse_module(root_path, code, source_manager.clone())?; | ||
| let support = parse_auth_modules(source_manager)?; | ||
|
|
||
| let library = assembler | ||
| .clone() | ||
| .assemble_library(modules) | ||
| .map_err(|e| anyhow!("failed to assemble openzeppelin library: {e:?}"))?; | ||
| .assemble_library(package_name, root, support) | ||
| .map_err(|e| anyhow!("failed to assemble {package_name} library: {e:?}"))?; | ||
|
|
||
| Ok((*library).clone()) | ||
| Ok(*library) | ||
| } |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win
Every get_*_library() call now re-parses all auth-dir MASM files, not just the target one.
assemble_with_auth_support calls parse_auth_modules, which re-reads and re-parses every file under auth_dir() as "support" on each invocation. get_multisig_library/get_multisig_ecdsa_library/get_guardian_library are called from the multisig client's transaction-script builders (e.g. build_update_signers_script in crates/miden-multisig-client/src/transaction/configuration/config.rs), which run once per transaction request — a real per-transaction cost, whereas previously only the single requested module was parsed. Consider caching the parsed/assembled libraries (e.g. via OnceLock/Lazy) rather than reassembling from disk on every call.
Also applies to: 251-271
🤖 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 `@crates/contracts/src/masm_builder.rs` around lines 137 - 154, Cache the
auth-support library assembled by assemble_with_auth_support, including the
parsed auth modules, so repeated get_multisig_library,
get_multisig_ecdsa_library, and get_guardian_library calls do not re-read and
re-parse every file under auth_dir(). Use OnceLock or Lazy at the shared
library/support boundary, preserve the existing assembly result and error
behavior, and ensure each target-library accessor reuses the cached value across
transaction-script builds.
| }), | ||
| Asset::NonFungible(a) => { | ||
| let key_word = a.vault_key().to_word(); | ||
| let key_word = a.id().to_word(); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Add a value-level NFT identifier test.
The current snapshot test only checks formatting and may iterate zero NFTs, so it would pass with the old vault_key() behavior. Build a known non-fungible asset and assert vault_key equals a.id().to_word() hex.
🤖 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 `@crates/server/src/services/dashboard_account_snapshot.rs` at line 138, Add a
value-level assertion to the snapshot test around the key_word assignment,
constructing a known non-fungible asset and verifying that vault_key equals the
hexadecimal representation of a.id().to_word(). Ensure the test executes with at
least one NFT so it would fail under the previous vault_key behavior.
Source: Learnings
Comment on lines
205
to
208
| let mut executed_account = multisig_account.clone(); | ||
| executed_account | ||
| .apply_delta(executed_tx.account_delta()) | ||
| .apply_patch(executed_tx.account_patch()) | ||
| .expect("executed delta applies"); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Correct the stale patch-application error message.
Line 207 applies an account patch, but Line 208 still reports “executed delta applies,” which will misdirect debugging when the patch application fails. Rename it to “executed patch applies.”
Proposed fix
executed_account
.apply_patch(executed_tx.account_patch())
- .expect("executed delta applies");
+ .expect("executed patch applies");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| let mut executed_account = multisig_account.clone(); | |
| executed_account | |
| .apply_delta(executed_tx.account_delta()) | |
| .apply_patch(executed_tx.account_patch()) | |
| .expect("executed delta applies"); | |
| let mut executed_account = multisig_account.clone(); | |
| executed_account | |
| .apply_patch(executed_tx.account_patch()) | |
| .expect("executed patch applies"); |
🤖 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 `@crates/server/src/testing/e2e/switch_guardian_canonicalization.rs` around
lines 205 - 208, Update the expect message on the apply_patch call for
executed_account so it says “executed patch applies” instead of referring to an
executed delta.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
create_noteprocedure support across the multisig SDKs.Bug Fixes
Documentation