CLI typed call output: followups
- 主要言語
- Rust
- スター
- 78
- フォーク
- 129
- 平均マージ
- 4日 14時間
- マージ済み PR(30日)
- 52
説明
[#2179](https://github.com/0xMiden/rust-sdk/pull/2179) merged with five review comments still open.
---
### 1. `result_felts` on `CallCode` duplicates `TypedProcInfo::output_felt_count()`
> `result_felts` is always exactly `typed.output_felt_count()`.
[discussion_r3884146667](https://github.com/0xMiden/rust-sdk/pull/2179#discussion_r3884146667)
(`bin/miden-cli/src/commands/call.rs:269`)
The field is set from `typed` in the only branch that populates it and left `None` everywhere
else, so it can be dropped and read off `typed` at the single use site.
---
### 2. An unrenderable result aborts `call` before the state-delta phase
> A result the typed decoder cannot render aborts here before the state-delta transaction runs, so
> the user loses both the delta and any view of the returned felts.
[discussion_r3884145663](https://github.com/0xMiden/rust-sdk/pull/2179#discussion_r3884145663)
(`bin/miden-cli/src/commands/call.rs:352`)
`print_call_result(&output_stack, typed.as_ref())?` propagates the decode error out of
`run_local_call`, so step 2 never runs. Making it infallible (print a notice plus a raw felt dump,
then continue) matches `report_failed_delta`, which already never fails the command.
---
### 3. The untyped-fallback comment does not describe the branch it sits on
> The comment just above says the fallback is for hand-written MASM that carries no signature, but
> the code never checks `proc.signature.is_none()`. Any export whose bare name matches and whose
> ABI is not `ComponentModel` lands here, and is then driven down the `encode_raw_args` path in
> `resolve_from_package`.
[discussion_r3884145767](https://github.com/0xMiden/rust-sdk/pull/2179#discussion_r3884145767)
(`bin/miden-cli/src/commands/call.rs:498`)
Comment only, the behaviour is intended: a procedure with no WIT signature must stay callable. The
`Fast` and `C` ABI cases land in the same bucket and the comment should say so.
---
### 4. `account-id` arguments accept raw hex only, while the call target accepts bech32
> This accepts only raw hex, while the target of the same command goes through `parse_account_id`,
> which also takes a bech32 address or a partial ID. So `miden-client call
> mm1abc...:take_account_id mm1def...` resolves the target but rejects the argument, and the user
> has to convert one of the two spellings by hand inside a single command line.
[discussion_r3884146485](https://github.com/0xMiden/rust-sdk/pull/2179#discussion_r3884146485)
(`bin/miden-cli/src/codecs/account_id.rs:32`)
`AccountIdCodec::encode` calls `AccountId::from_hex` directly. A shared hex-or-bech32 token parser
used by both the `account-id` codec and the faucet half of the `asset` codec closes the gap.
Partial-prefix IDs stay out of scope: resolving one reads the store, and a codec only ever sees a
single argument's text.
---
### 5. Redundant unit tests in `call.rs`
> Some of this tests look redundant, I'd favor a shortened diff over overly specific tests.
[discussion_r3864601118](https://github.com/0xMiden/rust-sdk/pull/2179#discussion_r3864601118)
(`bin/miden-cli/src/commands/call.rs:596`)
The three untyped-export cases, the two raw-arg rejection cases, and the lowered-name case that
re-asserts a message another test already pins can fold into table-driven tests without losing
coverage.
コントリビューションガイド
調査の方向性
The work is in bin/miden-cli/src/commands/call.rs and bin/miden-cli/src/codecs/account_id.rs. Start by reading the linked PR #2179 to understand the context. For each follow-up item, locate the mentioned lines and understand the current behavior. The changes involve removing a redundant field, making error handling infallible, updating a comment, unifying account ID parsing, and refactoring tests. Run existing tests in call.rs to ensure they pass after changes.
索引モデルが issue の本文から書いたものです。
評価
- 技術スタック
- rust
- 領域
- cli
- issue の種類
- リファクタリング
- 難易度
- 3/5
- 見積もり時間
- 1〜2日
- 活発さ
- 活発
- 明瞭さ
- 明確に書かれている
- 初心者へのやさしさ
- 55/100