fix(react-sdk): rebuild by-value WASM handles per poll/refetch iteration - #296
Open
emreyurur wants to merge 1 commit into
Open
fix(react-sdk): rebuild by-value WASM handles per poll/refetch iteration#296emreyurur wants to merge 1 commit into
emreyurur wants to merge 1 commit into
Conversation
getConsumableNotes and TransactionFilter.ids take their arguments by value, so wasm-bindgen moves the handle into WASM and zeroes the JS wrapper's pointer. Four hooks held one such object across poll/refetch iterations and threw 'null pointer passed to rust' on every use after the first. Part of 0xMiden#278.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Part of #278.
getConsumableNotestakesOption<AccountId>andTransactionFilter::idstakesVec<TransactionId>by value, so wasm-bindgen calls__destroy_into_raw()and zeroes the JS wrapper's pointer on the first call. Four hooks kept one such object in a variable and handed it over again on later iterations, throwingnull pointer passed to ruston every use after the first.The fix everywhere is the shape #270 established: snapshot the stable value (a hex string, or the original account reference) once, and rebuild a fresh handle per iteration. It's also the convention the package already follows elsewhere —
useMultiSendrebuildsiterSenderId/iterAssetIdper iteration anduseSessionAccountrebuildsfreshAccountId.Changes
useWaitForCommit.ts—TransactionFilter.ids([txId])consumed the caller'sTransactionIdon the first poll.targetHexwas already snapshotted, so the id is now rebuilt from it each iteration.useTransactionHistory.ts— the memoisedTransactionIds were handed toTransactionFilter.idsagain on every refetch, so the second refetch (e.g. the one a sync tick drives) threw. Ids are now rebuilt from the existingidsHexsnapshot.useTransaction.ts—txId.toHex()was read afterwaitForTransactionCommithad consumed the handle. The hex is now snapshotted before the call.useWaitForNotes.ts— not in React SDK reuses by-value WASM objects across poll/refetch iterations ("null pointer passed to rust") #278's list; found while tracing the others. A singleAccountIdwas built before the loop and passed togetConsumableNoteson every poll, sowaitForConsumableNotesonly ever worked when the notes were already present and failed whenever it actually had to wait. A freshAccountIdis built per poll; the reference is still parsed once up front so a malformed id rejects before the first sync, exactly as before.CHANGELOG.md—[FIX][react]entry underUnreleased.Not included
utils/transactions.ts:30— fix(react-sdk): rebuild TransactionId per poll in waitForTransactionCommit #270 already covers it, left untouched to avoid conflicting.useTransactionHistory.ts:134(caller-suppliedfilter) — this one can't be rebuilt from a snapshot, since the hook only receives an opaqueTransactionFilterinstance. Fixing it needs an API decision: accept a factory (filter?: TransactionFilter | (() => TransactionFilter)) and call it per refetch, or document the instance as single-use. Happy to do either in a follow-up — didn't want to slip an API change into a bugfix PR. React SDK reuses by-value WASM objects across poll/refetch iterations ("null pointer passed to rust") #278 should stay open for it.Testing
New
packages/react-sdk/src/__tests__/hooks/issue278-wasm-by-value-reuse.test.tsx, plus a case added touseWaitForNotes.test.tsx.The existing suite couldn't catch any of this:
setup.tsmocksTransactionFilter.idsas a plain object factory and models none of wasm-bindgen's move semantics, so a consumed handle stays perfectly usable in tests. The new tests install a mock that does model it — marking each id consumed and throwingnull pointer passed to ruston reuse, mirroring howsetup.tsalready does this forsendPrivateNote.Before the fix, against
main:After: full package suite green — 63 files, 848 tests (
vitest run). ESLint clean on every changed file.Two existing assertions had to be updated:
useWaitForCommit.test.tsxanduseTransactionHistory.test.tsxasserted thatTransactionFilter.idswas called with the caller's exact object, which pinned the buggy behaviour. They now assert on the id's hex instead of object identity.useTransaction.tshas no dedicated regression test — its change is a pure snapshot-before-use and the existinguseTransactionsuite covers the surrounding flow. Happy to add a targeted one if you'd like it.Verified in a clean fork clone with a full
pnpm install, not just in isolation.