Debug info encoder decoder

[marijamijailovic]

This PR adds ast::types::TypedProcInfo to miden-assembly-syntax: a typed view over procedure signatures that can print the signature, encode argument into stack felts, and decode result felts back. Callers pass the procedure name and its FunctionType; word and felt are built in, while other types such as account-id are provided by the caller via the WitScalarCodec trait. The miden call command (0xMiden/rust-sdk#2179) uses this to print typed signatures and decode results instead of raw feltst. The module was moved from miden-client into miden-assembly-syntax since the types already live in miden-vm (0xMiden/rust-sdk#2179 (review)).

The relevant issue 0xMiden/rust-sdk#2098, and context and motivation: Main issue #1395, and see the comment in #1943.

[github-actions]

Automated check (CONTRIBUTING.md)

Findings:

• Ensure issue Merge AdviceInputs with AdviceProvider #1943 is assigned to the PR author.
• Add a short Rationale explaining why the change is needed.

Recommendations:

• Consider adding a Test plan or clear review steps.

Next steps:

• Ensure issue Merge AdviceInputs with AdviceProvider #1943 is assigned to you.
• See CONTRIBUTING.md for expectations.
• If this is a false positive, comment: /quality-review.

[huitseeker]

Typed primitive sizing and encoding do not match declared debug primitive widths

[marijamijailovic]

Typed primitive sizing and encoding do not match declared debug primitive widths

Hi @huitseeker, thanks for your comment! I’ve updated the PR to correct the felt sizing in encoding/decoding.

[marijamijailovic]

I’ve rebased this branch and I’m re‑introducing ‎size_in_bytes and ‎size_in_felts on ‎DebugPrimitiveType. These methods were removed in #3221, but the new typed encoder/decoder needs the per‑primitive byte width and felt count in order to lower argument tokens into stack felts and lift return felts back from the stack.

[huitseeker]

Besides the non-canonical bool, this looks much closer now!

[marijamijailovic]

Besides the non-canonical bool, this looks much closer now!

@huitseeker Thanks for your comments. I’ve added strict checks for boolean, and also for integer widths (including rejecting out‑of‑range felts), and expanded the typed debug errors so we get more precise diagnostics

[huitseeker]

Please add a line to the Changelog too.

[marijamijailovic]

Please add a line to the Changelog too.

Hi @huitseeker, thanks again for the review and comments! I’ve added the changelog entry, and the other changes as well (normalizing the ‎Void return type to ‎None, using ‎Package::debug_info, and fixing the doc link)

[huitseeker]

Thanks for addressing the prior bool/trust-path/changelog concerns.

[marijamijailovic]

Thanks for addressing the prior bool/trust-path/changelog concerns.

Thanks again @huitseeker for all the detailed reviews and suggestions

[bitwalker]

Looks good overall, but I have some nits/changes that I think need to be made before we merge this

[marijamijailovic]

Thanks @huitseeker and @bitwalker for all the detailed review comments. I’ve gone through the latest round and addressed the requested changes. I’ve also left a couple of follow‑up questions in the relevant threads.

[huitseeker]

This LGTM! Please rebase.

[marijamijailovic]

This LGTM! Please rebase.

Thanks @huitseeker! I've rebased this PR.

[marijamijailovic]

Hi @huitseeker, I’ve fixed the ‎miden-core-fuzz crate check

[huitseeker]

i'm sorry but this will need a rebase once again. Beware that the Changelog format has changed a bit. You can also click options to let maintainers edit this PR.

[bitwalker]

I think we will need to wait to determine the disposition of #3398 - whether we will call it quits on the v0.25 release series and move everything to v0.26, or land #3398 on main and yank all prior v0.25 releases - either way, it makes significant changes to how debug info is represented that directly affect this PR and the way the miden call command is implemented.

We're planning to make a final call on that tomorrow morning, but just wanted to give you a heads up @marijamijailovic

[marijamijailovic]

I think we will need to wait to determine the disposition of #3398 - whether we will call it quits on the v0.25 release series and move everything to v0.26, or land #3398 on main and yank all prior v0.25 releases - either way, it makes significant changes to how debug info is represented that directly affect this PR and the way the miden call command is implemented.

We're planning to make a final call on that tomorrow morning, but just wanted to give you a heads up @marijamijailovic

I appreciate the heads-up! I’ll wait for the decision on #3398 and then update this PR as needed.

[marijamijailovic]

I think we will need to wait to determine the disposition of #3398 - whether we will call it quits on the v0.25 release series and move everything to v0.26, or land #3398 on main and yank all prior v0.25 releases - either way, it makes significant changes to how debug info is represented that directly affect this PR and the way the miden call command is implemented.

We're planning to make a final call on that tomorrow morning, but just wanted to give you a heads up @marijamijailovic

Hi @bitwalker, is there any update regarding this and the plan for #3398?

[marijamijailovic]

Hi @huitseeker and @bitwalker, I’ve now rebased this on top of #3398.

What changed:

• The module moved from crates/mast-package/src/debug_info/typed/ to crates/mast-package/src/typed/.
• It no longer reads the DEBUG_FUNCTIONS / DEBUG_TYPES sections. It’s now a typed view over the package manifest’s procedure signatures.
• Re‑introducing size_in_bytes / size_in_felts on DebugPrimitiveType is no longer needed, since arity is derived from the manifest type instead (typed/arity.rs).

Since the old review threads now point at code that no longer exists, it’s probably cleanest to review the single new commit. The earlier commits are still reachable through the force‑push compare in this PR’s timeline (f57c182f3 → d6230b541) if you’d like to compare.

Thanks again for all your previous comments and suggestions.

[huitseeker]

The anonymous type comment is the important one.

[marijamijailovic]

Hi @huitseeker, I have a quick question about the CHANGELOG.md format.

I see the latest entry is for version 0.28.0, and below that there is miden-vm 0.25.8. For this PR, where should I add the changelog entry?

[marijamijailovic]

Hi @huitseeker, @bitwalker I’ve rebased this and added the changelog entry under the 0.30.0 release. Could you please take another look and let me know if it’s ready to merge?

[huitseeker]

I still think this works. But TypedProcInfo accepts calling conventions it cannot encode correctly, so the public API promises more than the implementation delivers. The zero-width array decode also lets a small serialized type description consume unbounded time and memory. There's also an edge case paper cut in custom-codec arrays.

I would restrict the API to Component Model signatures for now, bound zero-width decoding, and make the array loop track token consumption.

[huitseeker]

Could we reject or cap zero-width arrays before this loop? ArrayType::len can come from a serialized usize, but felt_count([empty; n]) is zero for any n, so the stack-length check does not bound the work here.

A package declaring [empty; 1_000_000] made decode_result(&[]) spend about 0.67 seconds and 62 MB building the string, and the encoded length can be much larger. This gives a small untrusted manifest a direct memory and CPU denial of service.

[marijamijailovic]

Thanks for this. I was thinking about a cap, but I was not sure about the upper limit, so I reject it with a ZeroWidthArray error instead. The test is an_array_of_elements_that_take_no_felts_is_not_decoded.

[huitseeker]

Could TypedProcInfo reject signatures whose abi is not ComponentModel, unless the other layouts are implemented?

The encoder always flattens aggregate fields separately. That matches the canonical ABI used by the downstream miden call, but not Fast: the Miden ABI packs struct { u8, u8 } into one raw stack part, while this API emits two felts. C and Wasm have different aggregate rules too.

The test helper currently labels every fixture CallConv::Fast, so it hides this mismatch.

[marijamijailovic]

Thanks for this catch! The tests now use ComponentModel, and TypedProcInfo::new refuses any other convention

[huitseeker]

A custom codec for a zero-field struct still consumes one token even though it correctly emits zero felts. For [empty; 2], the count check accepts two tokens, this breaks after the first codec call, and encode_args returns TokenCountMismatch.

Checking token_count or otherwise tracking token progress would preserve the large zero-width-array guard without skipping token-consuming codecs.

[marijamijailovic]

Thanks! The array now skips its elements on token_count == 0, not on an empty chunk, so a codec that reads a token runs once per element and the token count matches.

[marijamijailovic]

Hi @huitseeker @bitwalker, just a quick bump on this PR. Can you look into this, and let me know if there’s anything else blocking merge?

[bitwalker]

I think the only thing I would like to change before this is merged is to move this code into miden-assembly-syntax under the ast::types module, maybe as ast::types::signatures or something to that effect, and re-export TypedProcInfo (or whatever makes the most sense) from ast::types.

Nothing implemented here is relevant to miden-mast-package itself though - the details are all type signature oriented, and should live as close to that code as possible IMO.

Once this is moved into miden-assembly-syntax, it can be merged.

[marijamijailovic]

I think the only thing I would like to change before this is merged is to move this code into miden-assembly-syntax under the ast::types module, maybe as ast::types::signatures or something to that effect, and re-export TypedProcInfo (or whatever makes the most sense) from ast::types.

Nothing implemented here is relevant to miden-mast-package itself though - the details are all type signature oriented, and should live as close to that code as possible IMO.

Once this is moved into miden-assembly-syntax, it can be merged.

Hi @bitwalker, I have moved TypedProfInfo to the miden-assembly-syntax.

[bitwalker]

Thanks @marijamijailovic!

[huitseeker]

@marijamijailovic Can you make sure every commit in this PR is signed?

[marijamijailovic]

@marijamijailovic Can you make sure every commit in this PR is signed?

Hi @huitseeker, I’ve rebased this branch so that every commit is signed