feat(program-escrow): fee-on-transfer audit with regression tests (closes #946)#991
Merged
Jagadeeshftw merged 5 commits intoJagadeeshftw:masterfrom Apr 1, 2026
Conversation
|
@precious-akpan is attempting to deploy a commit to the Jagadeesh B's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
@precious-akpan Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
ba9e383 to
a0c8161
Compare
i128::checked_sub only returns None on arithmetic overflow (e.g.,
i128::MIN - 1), not when the result is negative. safe_sub_zero(5, 10)
was therefore returning -5 instead of 0.
Replace with an explicit guard: if b > a { 0 } else { a - b }.
Fixes test: gas_optimization::tests::test_safe_sub_zero
…from lock_program_funds_from uses token::transfer_from internally, which requires the depositor to have granted an allowance to the contract before the call. The test was minting tokens to the depositor but not calling token.approve(), causing an 'not enough allowance' failure at the token level. Add: token.approve(&depositor, &client.address, &5000, &99999) Fixes test: test_draft_state::test_operations_succeed_after_publish
…gadeeshftw#946) Implements issue Jagadeeshftw#946 requirement: simulate deflationary / inconsistent token transfer behaviour using a purpose-built DeflatToken mock contract that charges a configurable basis-point fee on every transfer() and transfer_from() call. Adds test_fee_on_transfer.rs with 12 integration test scenarios: 1. 10 % inbound fee — actual_received credited, not requested amount 2. 50 % high-fee inbound lock scenario 3. Repeated sequential FoT locks — cumulative balance invariant 4. lock_program_funds_v2 rejects amount > actual on-chain balance (FoT mismatch detection guard) 5. lock_program_funds_v2 accepts amount == actual received after fee 6. Zero-fee token is identical to standard SAC token behaviour 7. remaining_balance never exceeds actual on-chain balance (invariant) 8. FoT lock then payout within credited funds succeeds 9. Over-payout attempt panics (cannot draw more than credited balance) 10. FundsLocked event carries actual_received, not requested amount 11. Multi-depositor sequential FoT locks — cumulative invariant holds 12. Batch payout from FoT-funded escrow — accounting correct Security finding documented in tests: outbound payout transfers via token::transfer ALSO incur FoT fees; recipients receive net-of-fee tokens while escrow debits remaining_balance by the full release amount. This is the expected contract behaviour (cannot be prevented at the escrow layer) and is now explicitly documented in test comments. Also registers the new module in lib.rs (mod test_fee_on_transfer). Test suite: 78 passed, 0 failed.
a0c8161 to
1888ac9
Compare
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
Resolves #946 — Fee-on-transfer (FoT) token handling review for
program-escrow.This PR audits and validates
program-escrowaccounting under deflationary / fee-on-transfer token conditions. All existing tests continue to pass (78 passing, 0 failing).Changes
contracts/program-escrow/src/test_fee_on_transfer.rs[NEW]A purpose-built
DeflatTokenmock contract charges a configurable basis-point fee on everytransfer/transfer_fromcall, faithfully simulating real-world FoT / deflationary tokens. 12 edge-case integration tests:test_fot_lock_10pct_fee_credits_actual_receivedremaining_balancereflectsactual_received, not requested amounttest_fot_lock_50pct_fee_credits_actual_receivedtest_fot_repeated_locks_accumulate_actual_receivedtest_fot_lock_v2_rejects_amount_exceeding_actual_balancelock_program_funds_v2detects FoT mismatch and panicstest_fot_lock_v2_accepts_actual_received_amountlock_program_funds_v2succeeds when amount == actual balancetest_zero_fee_token_behaves_like_standard_sactest_fot_remaining_balance_never_exceeds_on_chain_balancetest_fot_lock_then_payout_succeeds_within_actual_balancetest_fot_lock_then_over_payout_panicstest_fot_funds_locked_event_reflects_actual_receivedFundsLockedevent carriesactual_received, not requested amounttest_fot_cumulative_balance_invariant_across_sequential_lockstest_fot_batch_payout_distributes_correctly_from_credited_fundscontracts/program-escrow/src/lib.rsmod test_fee_on_transferin the test module block.gas_optimization::efficient_math::safe_sub_zero— was usingchecked_subwhich only returnsNoneon arithmetic overflow (not on negative results); replaced with explicitif b > a { 0 } else { a - b }.test_draft_state::test_operations_succeed_after_publish— addedtoken.approve()beforelock_program_funds_from(which usestransfer_from, requiring a pre-approved allowance).Security Notes
lock_program_funds_frommeasuresbalance_before → balance_afterand credits onlyactual_received. FoT fees are transparently reconciled.lock_program_funds_v2assertscontract_balance ≥ requested_amountbefore crediting, catching any FoT shortfall.remaining_balanceby the requested release amount and callstoken::transfer. If the token silently burns a fee on the outbound transfer, the recipient receives less than requested. This is documented but cannot be prevented at the escrow layer — it is inherent to non-standard FoT tokens.Test Results