fix: stop a reset transfer from writing over the next one - #154
Open
JspIIV wants to merge 1 commit into
Open
Conversation
reset() cleared currentStep, logs and error but left executeTransfer running. retrieveAttestation polls in a `while (true)` loop with no timeout and no cancellation, so the abandoned run kept polling every five seconds and kept calling addLog and setCurrentStep. Pressing Reset hid the transfer rather than ending it: log lines reappeared on the cleared screen and the step advanced on its own to minting and then completed. Starting a second transfer made it worse, because both runs wrote to the same state. Their log lines interleaved and whichever finished first set the step, so the first transfer could display "Bridge completed successfully" while the second was still burning. The hook now owns an AbortController per run. reset() and unmount abort it, executeTransfer abandons any previous run before starting, the signal is threaded through the attestation poll and the mint retry loop, and an aborted run is treated as abandoned rather than failed so it does not paint an error over a screen the user already reset. Two smaller fixes in the same paths: - The attestation wait is bounded. A message that never attests polled forever; it now gives up after three hours and says the burn succeeded so the mint can be completed later. - "Waiting for attestation..." was appended every poll, leaving roughly 700 identical lines after an hour. It is logged once a minute. - The gas line reported a gas limit as a price: formatUnits(gas, 9) printed a 180,000 gas limit as "0.00018 Gwei". It now logs the limit as a number, and the real receipt.gasUsed once the mint lands.
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.
Fixes #153.
The problem
reset()clearedcurrentStep,logsanderrorbut leftexecuteTransferrunning.retrieveAttestationpolls in awhile (true)loop with no timeout and no cancellation, so an abandoned run kept polling every five seconds and kept callingaddLogandsetCurrentStep.Pressing Reset during Waiting for attestation therefore hid the transfer rather than ending it — log lines reappeared on the cleared screen and the step advanced on its own to minting and then completed.
Starting a second transfer made it worse: both runs wrote to the same state, their log lines interleaved, and whichever finished first set the step. The first transfer could display "Bridge completed successfully" while the second was still burning.
The change
The hook now owns an
AbortControllerfor the run in flight.executeTransferaborts any previous run before starting, so two transfers can never write to the same state.reset()aborts before clearing, and an unmount effect aborts too, so a run cannot outlive the page.fetch.wait()helper replaces the baresetTimeoutsleeps so a pending delay gives up immediately instead of holding the run alive for another five seconds.Also in the same paths
The attestation wait is bounded. A message that never attests polled forever. It now gives up after three hours — chosen to sit well clear of standard-finality times — and the error says the burn succeeded so the mint can be completed later.
The waiting log no longer grows without bound.
Waiting for attestation...was appended on every poll: roughly 700 identical lines after an hour. It is now logged once a minute.The gas line reported a gas limit as a price.
gasWithBufferis a count of gas units, so dividing by 1e9 and labelling it Gwei printed a 180,000 gas limit as0.00018 Gwei. It was also called Gas Used while being a pre-flight estimate. The limit is now logged as a number, and the realreceipt.gasUsedis logged once the mint lands.Verification
npx tsc --noEmit,npm run lint,npm run buildandprettier --checkall pass. Prettier touched only the line this PR adds; the rest of the file is unchanged.I have not been able to exercise this end to end against a live bridge — that needs funded wallets on both chains and a real attestation round trip. The change is deliberately confined to cancellation plumbing and two log lines, with no change to the approve, burn, fee or mint transaction paths. Reviewers who can run a real transfer may want to confirm the reset-mid-attestation case directly.
Happy to adjust the three-hour ceiling, the log interval, or split the gas-line fix into its own PR if you would rather keep this one to the cancellation change.