Miden Bank Tutorial Feedback

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](https://docs.miden.xyz/builder/tutorials/rust-compiler/miden-bank/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](https://docs.miden.xyz/builder/tutorials/rust-compiler/miden-bank/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](https://docs.miden.xyz/builder/tutorials/rust-compiler/miden-bank/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](https://docs.miden.xyz/builder/tutorials/rust-compiler/miden-bank/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](https://docs.miden.xyz/builder/tutorials/rust-compiler/miden-bank/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](https://docs.miden.xyz/builder/tutorials/rust-compiler/miden-bank/project-setup#key-takeaways), [account-components#step-1](https://docs.miden.xyz/builder/tutorials/rust-compiler/miden-bank/account-components#step-1-add-the-balances-storage-map))
- 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](https://docs.miden.xyz/builder/tutorials/rust-compiler/miden-bank/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](https://docs.miden.xyz/builder/tutorials/rust-compiler/miden-bank/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](https://docs.miden.xyz/builder/tutorials/rust-compiler/miden-bank/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](https://docs.miden.xyz/builder/tutorials/rust-compiler/miden-bank/asset-management#step-1-complete-the-deposit-function), [asset-management#step-2](https://docs.miden.xyz/builder/tutorials/rust-compiler/miden-bank/asset-management#step-2-add-the-withdraw-method-skeleton))
- `current_balance + deposit_amount` should check for overflow (e.g. against `FungibleAsset::MAX_AMOUNT`).
- Withdraw should use `checked_sub` (once it exists; see https://github.com/0xMiden/docs/issues/202) instead of a manual `>=` check.
- Ideally, an `AssetAmount` wrapper type (see [protocol#2532](https://github.com/0xMiden/protocol/issues/2532)) would handle this safely via `Add`/`Sub` impls.

2. **No validation that asset is fungible** ([asset-management](https://docs.miden.xyz/builder/tutorials/rust-compiler/miden-bank/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](https://docs.miden.xyz/builder/tutorials/rust-compiler/miden-bank/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](https://docs.miden.xyz/builder/tutorials/rust-compiler/miden-bank/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](https://docs.miden.xyz/builder/tutorials/rust-compiler/miden-bank/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](https://docs.miden.xyz/builder/tutorials/rust-compiler/miden-bank/account-components#step-1-add-the-balances-storage-map))
- 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](https://docs.miden.xyz/builder/tutorials/rust-compiler/miden-bank/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](https://docs.miden.xyz/builder/tutorials/rust-compiler/miden-bank/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](https://docs.miden.xyz/builder/tutorials/rust-compiler/miden-bank/transaction-scripts#step-3-add-to-workspace), [output-notes#update-workspace](https://docs.miden.xyz/builder/tutorials/rust-compiler/miden-bank/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](https://docs.miden.xyz/builder/tutorials/rust-compiler/miden-bank/project-setup#step-4-update-the-workspace-configuration))
- 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](https://docs.miden.xyz/builder/tutorials/rust-compiler/miden-bank/account-components#public-vs-private-methods))

2. **Storage map key layout should use little-endian** ([account-components](https://docs.miden.xyz/builder/tutorials/rust-compiler/miden-bank/account-components), [asset-management#step-1](https://docs.miden.xyz/builder/tutorials/rust-compiler/miden-bank/asset-management#step-1-complete-the-deposit-function))
- 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](https://github.com/0xMiden/protocol/blob/ab4f4efbaf434bc768dd48853ae1fb39837288a5/crates/miden-protocol/src/block/account_tree/account_id_key.rs#L8-L15).
- 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](https://docs.miden.xyz/builder/tutorials/rust-compiler/miden-bank/cross-component-calls/#building-on-part-4))
- The arrow from `use crate::bindings::miden::bank_account::bank_account;` has an unclear target.

cc @Keinberger