0xMiden / 0xMiden/docs

Miden Bank Tutorial Feedback

Đang mở
#203 1 bình luận 2 reaction 0 người được giao Xem trên GitHub
documentation
Ngôn ngữ chính
TypeScript
Star
10
Fork
50
Merge trung bình
22 ngày 23 giờ
Pull request đã merge (30 ngày)
1

Mô tả

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

Hướng dẫn đóng góp

Mở hướng dẫn đóng góp

Hướng nghiên cứu

The issue lists specific broken tests and outdated information across the Miden Bank tutorial. Start by reviewing the linked sections in the docs repository to understand the current state. For each broken test, locate the corresponding test file in the tutorial codebase and verify the compilation errors. For outdated information, check the Miden protocol changes (e.g., storage slots, NoteInputs rename) to update the tutorial text. Focus on one part at a time, ensuring fixes align with the latest Miden compiler version.

Do mô hình lập chỉ mục viết ra từ nội dung của issue.

Đánh giá

Công nghệ
rust, typescript
Lĩnh vực
developer-experience, documentation
Loại issue
Tài liệu
Độ khó
3/5
Thời gian dự kiến
1-2 ngày
Mức độ hoạt động
Đình trệ
Độ rõ ràng
Đặc tả rõ ràng
Mức phù hợp với người mới
45/100

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.