Skip to content

fix: reject duplicate bounty claims for same issue (#19) - #166

Merged
Kings9595 merged 2 commits into
Kings9595:mainfrom
Rayyanah0:fix/reject-duplicate-bounty-claims-19
Jun 28, 2026
Merged

fix: reject duplicate bounty claims for same issue (#19)#166
Kings9595 merged 2 commits into
Kings9595:mainfrom
Rayyanah0:fix/reject-duplicate-bounty-claims-19

Conversation

@Rayyanah0

Copy link
Copy Markdown
Contributor

Summary

Fixes #19 — ensure duplicate bounty claims for the same (repo_hash, issue_id) pair are rejected with BountyAlreadyClaimed.

Root Cause & Fixes

The contract logic for duplicate-claim rejection was already correct (is_claimed check + Persistent storage), but the codebase had several compile and logic errors preventing the tests from running:

1. Missing error variants (compile errors)

Error::InvalidRepoHash and Error::InvalidDeveloper were referenced in lib.rs and tests but absent from the Error enum in types.rs. Added:

InvalidRepoHash = 12,
InvalidDeveloper = 13,

2. Broken clawback authorization (logic bug)

clawback_expired_funds was calling ensure_is_maintainer (WaveGuard check) even though the design intent — documented in MilestonePool trust assumptions — is that clawback intentionally bypasses WaveGuard. A revoked WaveGuard must not lock the pool creator out of their own escrow. Removed the WaveGuard call; ownership is now guarded solely by pool.maintainer address equality.

3. Test setup zero-address collision (unit test bug)

In test.rs, Address::generate was called before env.register, causing the first generated address to be the all-zero sentinel CAAAA...D2KM. This caused test_maintainer_self_payout_is_not_blocked to fail because the maintainer address matched the InvalidDeveloper guard. Fixed by registering contracts first.

4. Stale test assertions

Updated assertions in clawback.rs, error_enum_coverage.rs, unauthorized_access.rs, and test.rs to match the corrected clawback authorization model and correct enum discriminants.

Test Results

All 97 tests pass (28 unit + 69 integration):

  • duplicate_claim.rs: 7/7 — core issue coverage
  • claim_manipulation.rs: 6/6
  • All other test suites: pass

Files Changed

  • src/types.rs — added InvalidRepoHash and InvalidDeveloper variants
  • src/lib.rs — removed WaveGuard check from clawback_expired_funds
  • src/test.rs — fixed setup order; corrected test assertions
  • tests/clawback.rs — updated clawback auth error expectation
  • tests/error_enum_coverage.rs — fixed variant name and discriminant
  • tests/unauthorized_access.rs — corrected clawback auth assertions

- Add InvalidRepoHash (12) and InvalidDeveloper (13) error variants to
  the Error enum so the contract compiles against all validation checks
  already present in lib.rs
- Remove WaveGuard check from clawback_expired_funds; ownership is now
  guarded by pool.maintainer address equality only, matching the design
  intent documented in MilestonePool trust assumptions (a revoked WaveGuard
  must not lock the pool creator out of their own escrow)
- Fix unit test setup() to register contracts before Address::generate so
  maintainer/developer addresses are never the all-zero sentinel address
- Update test assertions across clawback.rs, error_enum_coverage.rs,
  unauthorized_access.rs, and test.rs to match the corrected behaviour
- All 97 tests pass: 28 unit + 69 integration
@drips-wave

drips-wave Bot commented Jun 27, 2026

Copy link
Copy Markdown

@Rayyanah0 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

@Kings9595
Kings9595 merged commit 3894ef2 into Kings9595:main Jun 28, 2026
4 of 7 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.

Reject duplicate bounty claims for same issue

2 participants