Feat/reentrancy protection - #805
Conversation
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Team Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Fix your conflicts to get PR merged, and don't forget to use https://stellar.fundable.finance for your offramps |
Utilitycoder
left a comment
There was a problem hiding this comment.
Please fix up the merge conflict and update your PR. Kindly ensure you offramp with Fundable at https://stellar.fundable.finance/offramp
a16bd43 to
81d0f44
Compare
|
I just fixed the merge conflicts |
|
Noted, will use the Fundable offramp for payouts. |
…tions (Fundable-Protocol#505) Integrates reentrancy guard decorators using Soroban temporary storage to all state-mutating functions in the payment-stream Soroban contract. - Error::ReentrancyGuard = 36 added to error enum - Per-stream lock on: create_stream, deposit, withdraw, pause_stream, resume_stream, cancel_stream, set_delegate, revoke_delegate - Global lock on: set_protocol_fee_rate, set_fee_collector - 13 new reentrancy tests covering every guarded function - Lock released before token transfers (check-effects-interactions)
81d0f44 to
8434e06
Compare
|
fixed the conflicts, please merge the PR |
|
some of the lint error were pre-existing |
Closes #505
PR: Add reentrancy protection to
payment-streamcontract (#505)Summary
Adds per-stream and global reentrancy guards to every state-mutating function in the
payment-streamSoroban contract, closes issue #505.Motivation
The
payment-streamcontract performs token transfers (viatransfer/transfer_from) inside several functions. Without reentrancy protection a malicious token contract could re-enter any of those functions mid-execution and manipulate stream state (balances, status) before the initial call has written its effects back. This class of bug has caused significant losses in EVM-based streaming protocols; Soroban's synchronous host makes the attack surface different but not absent, especially when cross-contract calls are involved.Design
Guard mechanism
Reentrancy is detected using Soroban temporary storage boolean flags. Temporary storage is:
Lock granularity
(stream_id, Symbol("lock"))create_stream,deposit,withdraw,pause_stream,resume_stream,cancel_stream,set_delegate,revoke_delegateSymbol("g_lock")set_protocol_fee_rate,set_fee_collectorPer-stream locks are preferred over a single global lock so that operations on independent streams do not block each other.
withdraw_maxis not given its own lock; it delegates entirely towithdraw, which holds the lock for the duration.Error code
Error::ReentrancyGuard = 17is added to the existing error enum.Four private helpers
All guarded functions follow check-effects-interactions order: auth checks → state validation → state mutation → lock release → token transfer. The lock is acquired at the very top of each function and released immediately before the token transfer call.
Changes
contracts/payment-stream/src/lib.rsError::ReentrancyGuard = 17added to theErrorenum.acquire_stream_lock,release_stream_lock,acquire_global_lock,release_global_lock.create_stream,deposit,withdraw,pause_stream,resume_stream,cancel_stream,set_delegate,revoke_delegate.set_protocol_fee_rate,set_fee_collector.# Errorssections listing all possible error codes.contracts/payment-stream/src/test.rs13 new tests added (appended to the existing suite):
test_reentrancy_guard_blocks_reentrant_withdrawwithdrawpanics#17test_reentrancy_guard_blocks_reentrant_depositdepositpanics#17test_reentrancy_guard_blocks_reentrant_cancelcancel_streampanics#17test_reentrancy_guard_blocks_reentrant_pausepause_streampanics#17test_reentrancy_guard_blocks_reentrant_resumeresume_streampanics#17test_reentrancy_guard_blocks_reentrant_set_delegateset_delegatepanics#17test_reentrancy_guard_blocks_reentrant_revoke_delegaterevoke_delegatepanics#17test_reentrancy_guard_blocks_global_set_fee_rateg_lock→set_protocol_fee_ratepanics#17test_reentrancy_guard_blocks_global_set_fee_collectorg_lock→set_fee_collectorpanics#17test_reentrancy_lock_released_after_successful_withdrawtest_reentrancy_lock_released_after_successful_canceltest_independent_streams_use_separate_lockswithdrawon stream BTesting
cargo build --lib -p payment-streamis clean with zero warnings.cargo testis blocked by the upstreamsoroban-env-host v22.1.3/rand_coreversion conflict tracked separately in #506; this is not introduced by this PR.Checklist
Error::ReentrancyGuard = 17added to error enumwithdraw_maxcorrectly inherits the lock viawithdrawdelegationcargo build --lib -p payment-streampasses clean# Errorson all public functionsSummary by CodeRabbit
New Features
Bug Fixes
Tests