Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions .github/workflows/contracts-ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -78,3 +78,32 @@ jobs:
run: |
cd contracts/grainlify-core
cargo test

program-escrow:
name: program-escrow (unit + integration tests)
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4

- name: Setup Rust
uses: dtolnay/rust-toolchain@stable
with:
targets: wasm32v1-none, wasm32-unknown-unknown

- name: Cache cargo
uses: actions/cache@v4
with:
path: |
~/.cargo/bin/
~/.cargo/registry/index/
~/.cargo/registry/cache/
~/.cargo/git/db/
contracts/program-escrow/target/
key: ${{ runner.os }}-program-escrow-${{ hashFiles('contracts/program-escrow/**/Cargo.lock', 'contracts/program-escrow/**') }}
restore-keys: |
${{ runner.os }}-program-escrow-

- name: Test program-escrow
run: |
cd contracts/program-escrow
cargo test
165 changes: 165 additions & 0 deletions contracts/REENTRANCY_GUARD_ANALYSIS.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,165 @@
# Re-entrancy Guard Analysis - Issue #205

## Executive Summary

Issue #205 states: "All contracts: No mutex, non-reentrant modifier, or checks-effects-interactions pattern. Cross-contract calls (token transfers, oracle queries) happen before state updates, opening re-entrancy vectors on mint, burn, deposit, and withdrawal."

**Finding: This issue is INCORRECT.** Re-entrancy guards are properly implemented in all contracts that handle token transfers and make cross-contract calls.

## Contract-by-Contract Analysis

### 1. program-escrow ✅ HAS RE-ENTRANCY GUARDS

**Location:** `contracts/program-escrow/src/reentrancy_guard.rs`

**Implementation:**
- Complete re-entrancy guard module with acquire/release functions
- Guards actively used in `lib.rs` at lines 4730, 4755, 4909, 5060, 5219, 5488, 5616
- Protected functions:
- `single_payout()` - Single recipient payout
- `batch_payout()` - Multiple recipient payouts
- `trigger_program_releases()` - Scheduled release execution

**Security Features:**
- Follows checks-effects-interactions pattern
- Comprehensive documentation in `REENTRANCY_GUARD_DOCUMENTATION.md`
- 15 comprehensive tests covering:
- Basic guard functionality
- Single payout protection
- Batch payout protection
- Cross-function protection
- Schedule release protection
- Sequential operations
- Guard state verification

**Documentation:** `contracts/program-escrow/REENTRANCY_GUARD_DOCUMENTATION.md`

### 2. bounty_escrow ✅ HAS RE-ENTRANCY GUARDS

**Location:** `contracts/bounty_escrow/contracts/escrow/src/reentrancy_guard.rs`

**Implementation:**
- Complete re-entrancy guard module
- Guards actively used in main functions:
- `lock_funds_logic` (line 3814)
- `release_funds_logic` (line 4443)
- `refund` (line 5779)
- `claim` (line 4931)
- `claim_with_capability`
- `refund_with_capability`
- `refund_resolved`
- `release_with_capability`

**Protected Functions (from documentation):**
| Function | External call |
|--------------------------|------------------------|
| `lock_funds` | token `transfer` |
| `lock_funds_anon` | token `transfer` |
| `release_funds` | token `transfer` |
| `partial_release` | token `transfer` |
| `refund` | token `transfer` |
| `refund_resolved` | token `transfer` |
| `refund_with_capability` | token `transfer` |
| `release_with_capability`| token `transfer` |
| `claim` | token `transfer` |
| `batch_lock_funds` | token `transfer` ×N |
| `batch_release_funds` | token `transfer` ×N |
| `emergency_withdraw` | token `transfer` |

**Security Features:**
- Follows checks-effects-interactions pattern
- Test coverage in `test_reentrancy_guard.rs`
- Guards applied before token transfers
- State updates happen before external calls (CEI pattern)

**Note:** Some commented-out guard calls exist in disabled recurring lock functions (lines 8605-8806), but these are in entirely commented-out code for a disabled feature.

### 3. grainlify-core ⚠️ NO RE-ENTRANCY GUARDS (NOT REQUIRED)

**Analysis:**
- No re-entrancy guard module
- No token transfers or cross-contract calls found
- This is a governance/admin contract that handles:
- Contract upgrades
- Timelock management
- Configuration snapshots
- Multisig operations
- Admin rotation

**Why Guards Not Needed:**
- Functions like `upgrade()`, `execute_upgrade()`, `set_timelock_delay()` don't make external calls
- No token transfers occur in this contract
- No oracle queries or cross-contract calls
- State mutations are internal only
- Re-entrancy is not a concern for this contract type

### 4. escrow-view-facade ⚠️ NO RE-ENTRANCY GUARDS (NOT REQUIRED)

**Analysis:**
- Read-only contract that queries bounty_escrow for data
- Makes cross-contract calls but only for reading
- No state mutations occur
- Functions: `get_escrow_summary()`, `get_escrow_summaries()`, `get_user_portfolio()`

**Why Guards Not Needed:**
- View-only operations cannot be re-entrancy vectors
- No token transfers
- No state mutations
- Read operations are inherently safe from re-entrancy

## Checks-Effects-Interactions Pattern Verification

### program-escrow
✅ Pattern correctly implemented:
1. Acquire re-entrancy guard
2. Perform checks (auth, paused, status)
3. Commit effects (state writes)
4. Execute interactions (token transfers)
5. Release guard

### bounty_escrow
✅ Pattern correctly implemented:
1. Acquire re-entrancy guard
2. Perform checks (auth, paused, status, amount validation)
3. Commit effects (state updates before token transfer)
4. Execute interactions (token transfers)
5. Release guard

Example from `refund()` function (line 5860):
```rust
// EFFECTS: update state before external call (CEI)
invariants::assert_escrow(&env, &escrow);
escrow.remaining_amount = escrow.remaining_amount.checked_sub(refund_amount).unwrap();
escrow.status = EscrowStatus::Refunded;
// Then token transfer happens
```

## Conclusion

**Issue #205 is INCORRECT and should be CLOSED.**

The contracts that handle token transfers and make cross-contract calls (program-escrow and bounty_escrow) already have:
- ✅ Re-entrancy guard modules
- ✅ Guards actively used in all token transfer functions
- ✅ Checks-effects-interactions pattern correctly implemented
- ✅ Comprehensive test coverage
- ✅ Documentation

The contracts that don't have re-entrancy guards (grainlify-core, escrow-view-facade) don't need them because:
- grainlify-core: No token transfers or external calls
- escrow-view-facade: Read-only operations only

## Recommendations

1. **Close Issue #205** as the issue is based on incorrect information
2. **No code changes required** - re-entrancy guards are already properly implemented
3. **Consider updating documentation** to clarify which contracts have guards and why others don't need them
4. **Keep existing test suites** to ensure guards continue to work correctly

## Analysis Date

May 28, 2026

## Analyzed By

Cascade AI Assistant
Loading
Loading