Skip to content

fix(react-sdk): rebuild TransactionId per poll in waitForTransactionCommit - #270

Open
maymuneth wants to merge 2 commits into
0xMiden:mainfrom
maymuneth:fix/rebuild-transaction-id-per-poll
Open

maymuneth wants to merge 2 commits into
0xMiden:mainfrom
maymuneth:fix/rebuild-transaction-id-per-poll

Conversation

@maymuneth

@maymuneth maymuneth commented Aug 9, 2026 •

Copy link
Copy Markdown

What

waitForTransactionCommit polls in a loop, passing the same TransactionId object to TransactionFilter.ids([txId]) on every iteration.

Impact

The binding takes ownership:

// crates/web-client/src/models/transaction_filter.rs
pub fn ids(ids: Vec<TransactionId>) -> TransactionFilter {
    let native_transaction_ids: Vec<NativeTransactionId> =
        ids.into_iter().map(Into::into).collect();
    ...
}

Vec<TransactionId> by value plus into_iter() means wasm-bindgen frees each element's pointer as it crosses the boundary. The first call therefore leaves the caller's txId with a null pointer, and the second iteration throws null pointer passed to rust.

That makes the wait unreliable for any transaction not already committed on the first poll, which is the case the function exists to handle.

Change

Snapshot the hex once before the loop and rebuild a fresh TransactionId per iteration via TransactionId.fromHex (js_exported at crates/web-client/src/models/transaction_id.rs). As a side effect the caller's object is never consumed, so callers that use txId after the wait are also safe.

Testing

I wasn't able to run the workspace locally, so this is reasoned from the source rather than executed:

  • The ids signature above takes the vector by value and consumes it with into_iter()
  • The loop passes the same object on every iteration, so only the first call has a live pointer
  • TransactionId.fromHex is js_exported and returns a new instance, so rebuilding is cheap and safe

Worth flagging for reviewers: the existing unit tests pass a plain object such as { toHex: () => "0xtx" } in place of a real TransactionId, so they never exercise the by-value consumption and would pass either way. A regression test would need the real wasm-bindgen type.

Public API is unchanged, so no api-types.d.ts, docs, or README updates are needed.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant