Skip to content

fix(v2): enter reentrancy guard at start of entrypoints (#132) - #151

Merged
collinsezedike merged 5 commits into
drydocs:mainfrom
xtep103:feat/reentrancy-guard-early-check
Sep 4, 2026
Merged

fix(v2): enter reentrancy guard at start of entrypoints (#132)#151
collinsezedike merged 5 commits into
drydocs:mainfrom
xtep103:feat/reentrancy-guard-early-check

Conversation

@xtep103

@xtep103 xtep103 commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Overview

Hoists enter_reentrancy_guard to the start of assert_outcome, dispute, register, and withdraw immediately after require_auth(), closing potential reentrancy windows during early validation and id/state setup.

Related Issue

Closes #132

Changes

Contracts (tholos-v2)

  • [MODIFY] contracts/tholos-v2/src/lib.rs
    • Hoist enter_reentrancy_guard to the entry of assert_outcome, dispute, and register immediately after require_auth().
    • In withdraw, replace check_reentrancy_guard with enter_reentrancy_guard at entry.
    • Remove redundant late enter_reentrancy_guard invocations before token transfers.
  • [MODIFY] contracts/tholos-v2/src/test.rs
    • Update comment in test_reentrancy_guard_blocks_calls_while_held.
    • Add regression test test_reentrancy_guard_blocks_calls_before_validation verifying ReentrancyGuardActive is returned even when arguments would otherwise fail later validation.
  • [ADD] contracts/tholos-v2/test_snapshots/test/test_reentrancy_guard_blocks_calls_before_validation.1.json
    • Test snapshot for the new regression test.

Verification Results

$ cargo fmt --check
$ cargo clippy --workspace --all-targets --locked -- -D warnings
Finished `dev` profile [unoptimized + debuginfo] target(s) in 24.65s

$ cargo test --workspace --locked
test result: ok. 69 passed; 0 failed; finished in 24.03s (tholos)
test result: ok. 108 passed; 0 failed; finished in 14.44s (tholos-v2)
Acceptance Criteria Status
Guard entered immediately after require_auth() in assert_outcome, dispute, register, and withdraw ✅ Implemented
Existing reentrancy tests and full workspace test suite pass ✅ 108/108 tholos-v2 tests pass, full workspace passes
Regression test covering reentrant call during early validation window ✅ Added test_reentrancy_guard_blocks_calls_before_validation
Code formatting and linter checks pass cleanly cargo fmt --check and cargo clippy passed

@collinsezedike collinsezedike left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

enter_reentrancy_guard's doc comment (contracts/tholos-v2/src/lib.rs:856) still says every guarded function calls it immediately before the transfer, after writing state. That's no longer true after this PR: assert_outcome, dispute, register, and withdraw all call it at the top of the function, right after require_auth, before any validation or state writes. This line isn't part of the diff so I can't anchor a comment to it directly, please update it to describe the new ordering, otherwise a future contributor adding a token-moving entrypoint will copy the stale pattern and reintroduce the exact fragility this issue fixed.

Comment thread contracts/tholos-v2/src/test.rs
@xtep103
xtep103 force-pushed the feat/reentrancy-guard-early-check branch from ca9ca5a to e4544b2 Compare September 3, 2026 17:47
@xtep103

xtep103 commented Sep 3, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the review. I've addressed the requested changes, including updating the reentrancy guard documentation and improving the regression test coverage. The changes have been pushed to this PR for review.

@collinsezedike collinsezedike left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@xtep103 thank you for the contribution. This looks good, there is nothing to flag.

@collinsezedike
collinsezedike merged commit e850f79 into drydocs:main Sep 4, 2026
3 checks 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.

[Bug] Reentrancy guard checked late in four tholos-v2 entrypoints

2 participants