feat: implement v2 credit withdrawal and reentrancy guard - #88
Merged
Conversation
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
Implements #70: credit withdrawal and its reentrancy guard.
withdraw(owner, id, destination): transfersowner's entire withdrawableCredit(id, owner)balance todestination. Requiresowner's authorization.destinationcan be any address, not necessarilyowneritself, so a token that rejects transfers toownerdirectly can't permanently strand funds there.Resolution's newoutstanding_liability/withdrawn_totalfields updated, before the outgoing transfer. If the transfer fails, the whole call fails and every write above rolls back with it (Soroban reverts all of a failed invocation's storage writes, the same guarantee every other value-moving function here already relies on): the credit is never consumed by a failed transfer.settlenow also maintainsResolution.outstanding_liability, incrementing it by whatever it accrues (a position's own payout, and any dust) in the same call that persistssettled_recipient_weight/settled_reward_total, so there's a single source of truth for what's owed but not yet withdrawn per dispute.enter_reentrancy_guard/exit_reentrancy_guard/check_reentrancy_guard), addressing the issue's second ask (which also applies to registration from [Feature] V2: implement dispute and third-party registration #66, called out there as requested): held for the duration of every external token transfer this contract initiates (assert_outcome,dispute,register,finalize,withdraw), and checked at the entry ofreveal,resolve_outcome, andsettleas well, even though none of those three move tokens themselves. A non-standard token whosetransferimplementation calls back into this contract mid-transfer, instead of a well-behaved SEP-41 token that just updates balances, would otherwise be able to act on state that looks complete (because it's written before the transfer, the existing state-before-external-call pattern) while the tokens backing it haven't actually moved yet.Test plan
cargo fmt --check,cargo clippy --workspace --all-targets -- -D warnings, andcargo testpass locallycargo build -p tholos --target wasm32v1-none --releaseandcargo build --workspace --target wasm32v1-none --releasepass locallymdbook build docspasses locallyCONTRACT.mdupdated if the public interface changed — not applicable,tholos-v2isn't documented there yet (tracked separately)scripts/testnet-smoke.shrun against testnet — not run; this only touchestholos-v2, which isn't deployed yetoutstanding_liability/withdrawn_totalend up consistent). The reentrancy guard is tested by directly setting the guard flag in storage (viaenv.as_contract) to simulate a stuck lock, since triggering real reentrancy would need a separate hostile-token contract, and confirming every guarded entrypoint rejects withReentrancyGuardActivewhile held, then succeeds once released.