Skip to content

feat: Logic duplication#297

Merged
hman38705 merged 2 commits into
solutions-plug:mainfrom
Macnelson9:logic-duplication
Mar 26, 2026
Merged

feat: Logic duplication#297
hman38705 merged 2 commits into
solutions-plug:mainfrom
Macnelson9:logic-duplication

Conversation

@Macnelson9

Copy link
Copy Markdown
Contributor

📋 Description

Refactors duplicated payout logic in the contract betting module by introducing a shared internal claim helper. Both winner claims and cancellation refunds now use the same secure transfer and cleanup flow, reducing divergence risk and improving maintainability.

🎯 Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Performance improvement
  • Code refactoring
  • Test addition/update
  • CI/CD update

🔗 Related Issues

Closes #148

📝 Changes Made

  • Added private helper internal_claim_amount in bets.rs.
  • Refactored claim_winnings to use the shared helper in bets.rs.
  • Refactored withdraw_refund to use the same helper and aligned transfer security path in bets.rs.
  • Added token consistency validation in refund path to ensure payout token matches market token in bets.rs.

🧪 Testing

Test Coverage

  • Unit tests added/updated
  • Integration tests added/updated
  • All tests passing locally
  • Test coverage maintained/improved

Manual Testing

  • Tested on testnet
  • Tested locally
  • Edge cases tested

Test Results:
Refactor completed with no file-level diagnostics errors on the edited module.
Local full test execution is still pending in shell for this branch.

📸 Screenshots (if applicable)

Not applicable (no UI changes).

✅ Checklist

Code Quality

  • Code follows project style guidelines
  • Self-review completed
  • Comments added for complex code
  • No unnecessary console.log or debug code
  • No commented-out code

Documentation

  • Documentation updated (if needed)
  • API documentation updated (if applicable)
  • CHANGELOG.md updated
  • README updated (if needed)

Testing & Quality

  • All tests pass (cargo test)
  • No linting errors (cargo clippy)
  • Code formatted (cargo fmt)
  • No new warnings introduced
  • Gas benchmarks run (if applicable)

Security

  • Security implications considered
  • No sensitive data exposed
  • Input validation added
  • Authorization checks in place

Git

  • Branch is up to date with main
  • Commits are atomic and well-described
  • Commit messages follow convention
  • No merge conflicts

🔍 Reviewer Notes

Please focus review on:

  • Correctness of shared internal claim helper behavior for both claim and refund paths.
  • Safety and consistency of token transfer path in both flows.
  • Storage cleanup behavior parity after refactor.

📊 Performance Impact

  • No performance impact
  • Performance improved
  • Performance impact acceptable (explain below)

Details:
No additional storage or external calls were introduced; logic was consolidated without changing complexity class.

🚀 Deployment Notes

No migration required.
No config changes required.
Safe as a non-breaking internal refactor.

📚 Additional Context

Primary modified file:

  • bets.rs

Suggested commit message:
refactor: consolidate claim and refund logic into a shared helper


By submitting this PR, I confirm that:

  • I have read and followed the CONTRIBUTING.md guidelines
  • My code follows the project's coding standards
  • I have tested my changes thoroughly
  • I am ready for code review

@drips-wave

drips-wave Bot commented Mar 26, 2026

Copy link
Copy Markdown

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

@hman38705 hman38705 merged commit 566a1b4 into solutions-plug:main Mar 26, 2026
2 of 13 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.

Refactor Logic Duplication in Claim Paths

2 participants