Skip to content

CLI typed call output: followups #2512

Description

@juan518munoz

#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
(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
(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
(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
(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
(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.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    cliRelated to the CLI

    Type

    Fields

    Priority

    None yet

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions