Miden Bank Tutorial Feedback

[PhilippGackstatter]

Content is mine, summarized and consolidated by AI.

General

• Reviewed the 0.13 version.
• Some feedback is not related to the tutorial but the overall dev experience.

In general, cool tutorial and a good choice for introducing users to Miden's architecture (accounts, notes, etc.).

Structural suggestion: each part should follow the pattern: implement something -> get an explanation -> test that specific part.

---

Tests that don't compile or are incorrect

1. Part 2 - MockChain test is misleading (account-components#try-it-verify-your-code)

   • The text says "Let's write a MockChain test" but the test does not actually use a mock chain.

2. Part 2 - Deposit-without-init test doesn't test what it claims (constants-constraints#try-it-verify-constraints-work)

   • Text says "This test verifies that depositing without initialization fails" but the test never calls deposit.

3. Part 3 - Test does not compile (asset-management#try-it-verify-deposits-work)

   • Fails because the deposit-note does not yet exist at this point in the tutorial.

4. Part 4 - Test does not compile (note-scripts#note-scripts-vs-account-components)

   • Fails because the tx script has not been created yet.

5. Part 6 - Test does not compile (transaction-scripts#try-it-verify-initialization-works)

   • Compiler error: no field 'dep' on type '&mut Account'.

---

Outdated / incorrect information

1. "Compiler auto-assigns slot numbers" is outdated (project-setup#key-takeaways, account-components#step-1)

   • Storage slots are no longer accessed by index; the "auto-assigns slot numbers based on field order" language is outdated. Appears in multiple places.

2. Named slots terminology (account-components)

   • Slot order shouldn't matter to the user; remove that info. Use "slot name" instead of "named slot identifier".

3. StorageMap::get return type is wrong (account-components)

   • Text claims StorageMap::get() returns a single Felt, but let x: Word = self.balances.get(&key) compiles fine. get can return anything convertible from a Word.

4. Note Storage terminology outdated for 0.14 (note-scripts#note-scripts-vs-account-components)

   • NoteInputs were renamed to NoteStorage, so notes now have storage. Suggest "persistent storage" (accounts) vs "ephemeral storage" (notes).
   • Same section: "Called by other contracts" can be refined to "Called by notes or other accounts".

---

Security concerns

1. Overflow/underflow in deposit and withdraw (asset-management#step-1, asset-management#step-2)

   • current_balance + deposit_amount should check for overflow (e.g. against FungibleAsset::MAX_AMOUNT).
   • Withdraw should use checked_sub (once it exists; see Comparisons and overflow/underflow on Felt #202) instead of a manual >= check.
   • Ideally, an AssetAmount wrapper type (see protocol#2532) would handle this safely via Add/Sub impls.

2. No validation that asset is fungible (asset-management)

   • deposit_asset.inner[0] directly accesses the raw field without validating the asset is fungible. Ideally, this would use a FungibleAsset wrapper or similar safe API, but these do not exist in miden.

3. Withdraw note passes sender explicitly - security risk (output-notes)

   • The withdraw note passes the sender's account ID explicitly to bank_account::withdraw. The account should instead call active_note::get_sender itself.
   • As-is, an attacker could write a note script that passes a victim's account ID to withdraw, draining their balance into notes.

---

Tutorial structure / ordering issues

1. require_initialized introduced too early (account-components)

   • At this stage require_initialized is unused. Consider moving it to the part where it's actually used.

2. Part 5 explains cross-component calls after Part 4 already uses them (cross-component-calls)

   • Part 4 writes a note script calling bank_account::deposit, but the explanation of how that works comes in Part 5.
   • Maybe this is intentional, but it felt off to me - ideally this would come in the same part.

3. balances StorageMap already present from Part 0 (account-components#step-1)

   • Part 2 instructs adding a StorageMap for balance tracking, but it already exists from Part 0.

4. Account deployment pattern will need rethinking (transaction-scripts#account-deployment-pattern)

   • Once fees are enabled, the pattern of mutating a storage slot for initial deployment will no longer be necessary. For testing, consider MockChainBuilder::add_account_from_builder with AccountState::Exists instead (though showing how to actually deploy the account is fine as-is, using the initialize call).

---

Developer experience / workflow issues

1. No IDE support for contracts (project-setup)

   • Contracts are excluded from the Cargo workspace, so there's no IDE support when opening the top-level project. Users must open individual contract directories.
   • Not sure if this can be fixed or where, but would be nice to.

2. Building contracts requires changing directories

   • Must cd contracts/bank-account && miden build --release, then cd ../.. for testing. miden should support building from the top-level directory.

3. Cargo.toml updates are no-ops (transaction-scripts#step-3, output-notes#update-workspace)

   • Parts 6 and 7 instruct updating the workspace Cargo.toml, but the contracts are excluded anyway, making these steps pointless.

4. Part 0 Cargo.toml already up to date (project-setup#step-4)

   • The Cargo.toml already looks like the target state; the step is redundant.

---

Nits

1. Typo: "from outside" -> "from the outside" (account-components#public-vs-private-methods)

2. Storage map key layout should use little-endian (account-components, asset-management#step-1)

   • Key uses [prefix, suffix, 0, 0] but the canonical layout is [0, 0, suffix, prefix] (little-endian). Nudge users toward correct layout for 0.14 compatibility.
   • See also AccountIdKey.
   • The main reason to change this is to nudge users into the "correct" direction of using little-endian layouts (suffix, prefix) rather than big-endian (prefix, suffix). This will generally make their lives easier starting from 0.14 (where layouts in Rust and in MASM / on the stack are little-endian).
   • Same goes for the "Balance Key" (see link).

3. Part 5 bindings visualization unclear (cross-component-calls#building-on-part-4)

   • The arrow from use crate::bindings::miden::bank_account::bank_account; has an unclear target.

cc @Keinberger