fix(contracts): enforce CEI in withdraw and cancel_stream to close reentrancy surface#947
Open
patopatrish wants to merge 1 commit into
Open
Conversation
…e reentrancy surface (LabsCrypt#789) Both `withdraw` and `cancel_stream` previously executed token transfers before persisting stream state, violating Checks-Effects-Interactions. A token contract with a transfer hook could re-enter either function while storage still held the pre-update state, enabling a double payout. Changes: - Rename `transfer_and_update_stream` → `apply_withdrawal`, which now follows CEI: update stream fields → save_stream (effects committed) → token transfer. - `withdraw` delegates to `apply_withdrawal`; the separate `save_stream` call is removed since persistence is now handled inside the helper. - `cancel_stream` is restructured so all state mutations and `save_stream` happen before either token transfer (recipient payout + sender refund). - Two regression tests added: one for `withdraw` and one for `cancel_stream`, each asserting that a second call at the same timestamp fails because committed state already reflects the first operation. Closes LabsCrypt#789 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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
1 participant
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.
Closes #789
Problem
withdrawandcancel_streamboth transferred tokens before committing updated stream state to storage, violating the Checks-Effects-Interactions (CEI) pattern.token_addressis a caller-supplied contract. A token with a transfer hook could re-enterwithdraworcancel_streamwhile persistent storage still reflected the pre-update state (stalewithdrawn_amount,is_active = true), enabling a double payout.Affected code paths
transfer_and_update_stream(called bywithdraw)token_client.transferfired before updatingwithdrawn_amount/is_active, andsave_streamran even later, after the helper returnedcancel_streamtoken_client.transfercalls fired beforesave_streamFix
apply_withdrawal(replacestransfer_and_update_stream) — new helper that enforces CEI:withdrawn_amount, setlast_update_time, flipis_active/statusif fully drainedsave_stream— state is now durable before any external calltoken_client.transferwithdraw— delegates toapply_withdrawal; the standalonesave_streamcall is removed (now inside the helper).cancel_stream— restructured so all state mutations +save_streamprecede both token transfers (recipient payout and sender refund).Regression tests added
test_withdraw_state_committed_before_transfer_prevents_double_payout— after a successful withdraw, a second call at the same ledger timestamp (simulating a re-entrant hook) returnsInvalidAmountbecause committed state already reflects the withdrawal.test_cancel_state_committed_before_transfers_prevents_double_cancel— aftercancel_stream, a second cancel attempt returnsStreamInactivebecause the stream was marked inactive before either transfer fired.Files changed
contracts/stream_contract/src/lib.rscontracts/stream_contract/src/test.rs