[amanusk]

Problem

parse_multicall only understood the modern Cairo 1 Array<Call> calldata layout, which inlines each call's arguments:

[n, (to, selector, calldata_len, calldata...) * n]

Deprecated (Cairo 0) accounts serialize __execute__ differently — fixed-size 4-felt entries that index into a single flat blob appended after the array:

__execute__(call_array_len, call_array: CallArray*, calldata_len, calldata*)
struct CallArray { to, selector, data_offset, data_len }

[n, (to, selector, data_offset, data_len) * n, calldata_len, blob...]

Read as the modern layout, a legacy multicall whose first entry has data_offset = 0 is misinterpreted as "call 1 takes zero arguments". The parser then advances one felt too few and reads the first entry's data_len as the second call's target address — producing absurd targets like 0x3, with everything after that shifted and the walk collapsing early.

The user-visible symptom is a truncated call list whose first call looks correct (to/selector sit at indices 1/2 in both layouts) followed by one bogus low-integer target. It also triggered pointless get_class_hash_at / class-history lookups for non-existent contracts.

Approach

These accounts are still live and still relay INVOKE v3 transactions, so the layout is a property of the account class, not of the transaction version — there is nothing in the transaction to switch on, and no version gate would work. Resolving it via a class lookup would make a hot, synchronous parser async.

Instead parse_multicall picks whichever layout is internally self-consistent:

1. Modern, strict — all n calls present and consuming the array exactly, nothing trailing.
2. Legacy, strict — the 4-felt entries followed by a calldata_len felt whose value makes the trailing blob fill the array precisely.
3. Lenient walk — previous behaviour, preserved for genuinely truncated calldata (e.g. partial records from an external indexer).

Modern is tried first on risk asymmetry: a false positive there mangles only the rare format, whereas a false positive on the legacy reading would mangle the common one.

Two details worth calling out:

• felt_to_usize_checked (new, in utils): the existing felt_to_u64 keeps the low 64 bits, so a 251-bit value sitting in an offset or length position could truncate to a plausible small number and slip past a bounds check. Both strict parsers reject any such felt outright. It returns usize rather than u64 so no caller needs an as usize that would re-open the same hole where usize is narrower than 64 bits.
• Legacy offsets are bounds-checked, not required to be cumulative. The ABI permits entries pointing at arbitrary — even overlapping — slices of the blob; rejecting those would drop the transaction back to the lenient walk, i.e. back to garbage.

parse_call_array's layout handling is deliberately left alone: its SNIP-9 outside-execution callers are Cairo 1 only. (Its arithmetic is hardened in the follow-up commit below.)

Verification

Against mainnet, not just a clean build:

• Zero false positives on normal traffic. 314 recent INVOKE transactions all match the modern layout; none are ambiguous, and the legacy branch never fires.
• Trace-exact on the legacy layout. Across 60 transactions from an affected legacy account, the new parse reproduces every call derived from starknet_traceTransaction — target, selector and full calldata — for all 53 that expose a trace. Confirmed the same check fails on pre-fix code (2 calls parsed instead of 5), so it is not vacuous.
• TUI. Transaction detail renders the full, correctly-named call list; a modern multi-call transaction still renders unchanged; the address view's endpoint column is correct across all rows.
• 276 tests pass; cargo fmt, cargo clippy and cargo build --release clean.

Tests

Regression tests built from synthetic placeholder felts reproducing the structure only: the legacy layout end-to-end, modern-wins-when-consistent (including the zero-argument first call that triggers the misread), overlapping legacy offsets, rejection of a truncating length felt, and the lenient fallback on truncated input.

Follow-up: overflow panics on untrusted length felts (2nd commit)

Review flagged that the lenient fallback could panic, which turned out to be correct and worth fixing here since this PR is what documents that path as the malformed-input route.

parse_call_array added an untrusted data_len to offset unchecked. That felt can hold up to u64::MAX, so in release builds the add wraps below offset, passes the <= calldata.len() bounds check, and panics slicing backwards:

debug   → attempt to add with overflow           (functions.rs:168)
release → slice index starts at 4 but ends at 3  (functions.rs:169)

Now checked_add for the slice bound, and saturating_add when advancing offset — saturating rather than wrapping matters because callers read an offset past the end as "did not consume cleanly", which a wrapped small value would hide.

The same pattern appeared in outside_execution.rs on an untrusted sig_len: the validation add panicked in debug builds, while in release it wrapped to a value that happened not to equal data.len(), so malformed input was rejected by luck rather than by the check. The extraction add is gated behind that validation and was not reachable on its own; it is made checked too rather than left depending on the gate.

Both faults predate this series — master carries the identical unchecked add in parse_call_array, and the old parse_multicall called straight into it. Regression tests cover all three sites, pass in debug and release, and were each confirmed to fail against the pre-fix code.

Follow-up: checked usize narrowing (3rd commit)

A second review pass noted that felt_to_u64_checked(...)? as usize re-opens the truncation hole the helper exists to close, wherever usize is narrower than 64 bits — 0x1_0000_0003 reads as 3, and the strict bounds and exact-consumption checks then validate against the wrong number.

Not reachable on any target this builds for (usize is 64-bit on x86_64 and aarch64; CI is x86_64 only), so the casts were lossless in practice — but they contradicted the validators' documented intent, and a checked narrowing costs nothing.

Rather than add a second helper, felt_to_u64_checked now returns usize directly and is renamed felt_to_usize_checked, so no caller has a u64 to cast in the first place. The lenient fallback keeps felt_to_u64 + as usize: it is explicitly best-effort, and its arithmetic is already checked/saturating, so a truncated length there shortens a salvaged slice rather than overflowing.

Note for existing users

The parser fix does not rewrite already-persisted rows. Cached address_txs / address_calls entries for a legacy-layout account keep their old derived labels until cleared, at which point they are re-derived correctly.

🤖 Generated with Claude Code

[Copilot]

Pull request overview

This PR updates the calldata decoder to correctly parse Starknet account multicalls that use the legacy Cairo 0 CallArray* ((to, selector, data_offset, data_len)) layout, while preserving the existing Cairo 1 inline Array<Call> decoding behavior and maintaining a best-effort fallback for malformed/truncated inputs.

Changes:

• Add strict parsing for modern inline multicalls and legacy CallArray* multicalls, selecting the first internally self-consistent layout.
• Introduce felt_to_u64_checked to reject non-fitting (truncation-prone) felts in strict parsing paths.
• Add regression tests covering legacy parsing, ambiguity resolution, truncation rejection, overlapping offsets, and fallback behavior.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
src/utils.rs	Adds a non-truncating felt→u64 conversion helper used to harden strict parsing of lengths/offsets.
src/decode/functions.rs	Implements strict modern/legacy multicall parsers with a safe selection strategy, refactors RawCall construction, and adds targeted regression tests.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[amanusk]

Confirmed and fixed in 1169350.

Reproduced with [1, to, sel, 0xFFFFFFFFFFFFFFFF]:

debug   → attempt to add with overflow           (functions.rs:168)
release → slice index starts at 4 but ends at 3  (functions.rs:169)

The release case is the one that matters: 4 + usize::MAX wraps to 3, which passes the <= calldata.len() guard, so it slices [4..3] and panics. Now uses checked_add for the bound, and saturating_add for offset — saturating rather than wrapping matters because callers read an offset past the end as "did not consume cleanly", which a wrapped small value would hide.

Two notes for the record. This predates the CallArray work: master has the identical unchecked add, and the old parse_multicall called straight into it, so the same input panics there too. And while checking, the same pattern turned up in outside_execution.rs on an untrusted sig_len — the validation add panicked in debug and in release wrapped to a value that happened not to equal data.len(), so malformed input was rejected by luck rather than by the check. Both sites are now checked, though the extraction one was gated behind the validation and not reachable on its own.

Regression tests at all three sites, passing in debug and release, each confirmed to fail against the pre-fix code.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Suppressed comments (5)

src/decode/functions.rs:80

• data_len is treated as a strict, validated length here; using as usize can truncate on 32-bit targets and make the strict parser accept malformed calldata. Use a checked conversion to usize and return None if it does not fit.

        let data_len = felt_to_u64_checked(&calldata[offset + 2])? as usize;

src/decode/functions.rs:111

• In the strict legacy parser, num_calls should also be converted to usize with a checked conversion. as usize can truncate on 32-bit builds, undermining the strict length equation checks.

    let num_calls = felt_to_u64_checked(&calldata[0])? as usize;

src/decode/functions.rs:119

• blob_len participates in the strict "exact consumption" equation. Converting via as usize can truncate on 32-bit targets and could make invalid calldata appear self-consistent. Prefer a checked conversion to usize.

    let blob_len = felt_to_u64_checked(&calldata[blob_len_idx])? as usize;

src/decode/functions.rs:129

• data_offset / data_len are treated as strict bounds-checked indices into the blob; as usize can truncate on 32-bit targets and turn large values into plausible small ones. Use checked conversions to usize (returning None on overflow) before doing arithmetic/bounds checks.

        let data_offset = felt_to_u64_checked(&calldata[base + 2])? as usize;
        let data_len = felt_to_u64_checked(&calldata[base + 3])? as usize;

src/decode/functions.rs:69

• felt_to_u64_checked ensures the felt fits in a u64, but as usize can still silently truncate on 32-bit targets. Since this function is intended to be a strict validator, prefer a checked conversion to usize (returning None on overflow) rather than truncation.

This issue also appears in the following locations of the same file:

• line 80
• line 111
• line 119
• line 128

    let num_calls = felt_to_u64_checked(&calldata[0])? as usize;

[amanusk]

Addressed the 5 suppressed comments from the latest review pass (all one concern: as usize truncating on 32-bit) in c79c699.

The point lands on a real inconsistency — felt_to_u64_checked existed precisely to stop a 251-bit felt truncating into a plausible small length, and the trailing as usize re-opened that hole wherever usize is narrower than 64 bits. 0x1_0000_0003 reads as 3, and the strict bounds and exact-consumption checks then validate against the wrong number.

It is not reachable on any target this builds for: usize is 64 bits on x86_64 and aarch64, so the casts were lossless, and CI is x86_64 only with no target matrix. Fixed anyway because the casts contradicted the validators' documented intent and a checked narrowing costs nothing.

Rather than convert each of the 6 sites individually, the helper now returns usize directly and is renamed felt_to_usize_checked — so no caller has a u64 to cast in the first place, and the hole cannot be re-introduced by a future caller.

The lenient fallback deliberately keeps felt_to_u64 + as usize: it is best-effort by design, and its arithmetic is already checked/saturating after the previous commit, so a truncated length there shortens a salvaged slice rather than overflowing.

Verified the refactor did not disturb parsing: the real legacy transaction still decodes trace-exact in both debug and release, and the full suite is 276 passing.