Skip to content

feat: enhance client initialization with additional parameters and im…#571

Merged
greatest0fallt1me merged 6 commits into
Predictify-org:masterfrom
Oluwasuyi-Timilehin:task/reentrancy-guard-transfers
May 28, 2026
Merged

feat: enhance client initialization with additional parameters and im…#571
greatest0fallt1me merged 6 commits into
Predictify-org:masterfrom
Oluwasuyi-Timilehin:task/reentrancy-guard-transfers

Conversation

@Oluwasuyi-Timilehin
Copy link
Copy Markdown
Contributor

#closes #557

Summary

This PR adds cross-cutting usage of the contract-level ReentrancyGuard around external token transfers to prevent re-entrancy during withdraw and payout flows. It also fixes several compile/test issues introduced during the change so tests can run.

What changed

  • Wrapped external transfer sites with ReentrancyGuard::with_external_call where implemented:
    • contracts/predictify-hybrid/src/bets.rs
    • contracts/predictify-hybrid/src/balances.rs
    • contracts/predictify-hybrid/src/fees.rs
  • Removed accidental stray top-level statements and orphaned doc comments in contracts/predictify-hybrid/src/lib.rs.
  • Fixed test/signature and import issues across test modules and utilities (queries.rs, oracles.rs, multi_admin_multisig_tests.rs, storage_layout_tests.rs, etc.).
  • Verified the reentrancy_guard unit tests pass locally.

Motivation

  • Protect critical flows (withdraws, payouts, fee transfers) from malicious tokens that attempt to re-enter contract code during transfer callbacks.
  • Ensure the guard releases in success and failure paths and that state is restored on transfer failures.

Security notes

  • CEI preserved: state changes (effects) are applied before external interactions.
  • ReentrancyGuard::with_external_call is used to acquire/release the guard around single external calls.
  • unlock_funds now acquires the guard internally; this is a semantic change and may require caller updates for batch refund code paths.

Tests

  • Ran focused reentrancy tests locally:
cargo test -p predictify-hybrid -- reentrancy
# Result: 12 passed; 0 failed
  • Recommended additional tests (to be added before merging):
    • Integration test using a re-entrant token mock that tries to re-enter withdraw/distribute_payouts and assert no double-spend.
    • End-to-end tests covering batch refunds and any callers that previously managed guard acquisition externally.

How to verify locally

From the repository root run:

# run focused reentrancy tests
cargo test -p predictify-hybrid -- reentrancy

# run full package tests (recommended in CI)
cargo test -p predictify-hybrid

#closes

@drips-wave
Copy link
Copy Markdown

drips-wave Bot commented May 27, 2026

@Oluwasuyi-Timilehin 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! 🚀

Learn more about application limits

Oluwasuyi-Timilehin and others added 4 commits May 27, 2026 17:27
Remove duplicate module and import declarations, add missing Error variants and match arms, and import BetManager in queries.

Co-authored-by: Cursor <cursoragent@cursor.com>
Bring Ledger and Persistent testutils traits into scope for get_ttl and with_mut in storage tests.

Co-authored-by: Cursor <cursoragent@cursor.com>
@greatest0fallt1me greatest0fallt1me merged commit 15ce062 into Predictify-org:master May 28, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add cross-cutting reentrancy_guard wrapping around external SAC transfers in withdraw and distribute_payouts

2 participants