Skip to content

Fix/492 vesting paused wrong error#519

Merged
Austinaminu2 merged 2 commits into
Austinaminu2:mainfrom
observerr411:fix/492-vesting-paused-wrong-error
Apr 27, 2026
Merged

Fix/492 vesting paused wrong error#519
Austinaminu2 merged 2 commits into
Austinaminu2:mainfrom
observerr411:fix/492-vesting-paused-wrong-error

Conversation

@observerr411
Copy link
Copy Markdown
Contributor

@observerr411 observerr411 commented Apr 26, 2026

Summary

[Provide a clear and concise summary of the changes made in this PR]
closes #492

Related issue

[Link to the issue, e.g., #123]

Related Issue

[Link to the issue this PR addresses, e.g. Closes #123]

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update

Testing

Tests Performed

[Describe the tests you ran to verify your changes. Include:

  • Unit tests run
  • Integration tests performed
  • Manual testing steps
  • Any specific scenarios tested]

Test Results

[All tests should pass. Include any test output or screenshots if relevant]

Checklist

  • My code follows the project's style guidelines
  • I have run cargo fmt to format the code
  • I have run cargo clippy to lint the code
  • All new and existing tests pass locally
  • I have added necessary documentation (if applicable)
  • I have updated the CHANGELOG.md (if applicable)
  • I have labeled this PR appropriately (e.g., 'good first issue', 'documentation', 'bug')
  • I have requested review from the appropriate maintainers

Additional Notes

[Any additional context, screenshots, or information that reviewers should be aware of]

…n not paused

Closes Austinaminu2#493

unpause() was returning Err(VestingError::Common(CommonError::NotInitialized))
when the schedule was not paused. This is misleading: callers receiving
NotInitialized would assume the contract was never set up, when the actual
problem is simply that the schedule is not in a paused state.

Replace with Err(VestingError::NotPaused), which is the dedicated variant
(error code 10) defined for exactly this purpose.

Also fix pre-existing syntax bugs in the test module:
- Duplicate mod test/mod tests declaration collapsed to a single mod tests
- Broken get_config duplicate definition (incomplete first copy removed)
- Nested function definitions in test_double_cancel_fails and
  test_claim_after_cancel_fails separated into proper top-level tests

Add test:
- test_unpause_when_not_paused_returns_not_paused: verifies unpause()
  returns VestingError::NotPaused on an active (non-paused) schedule.
…laim(), and pause()

Closes Austinaminu2#492

Three functions were returning Err(VestingError::Common(CommonError::Unauthorized))
when the schedule was paused. CommonError::Unauthorized means the caller lacks
permission, but a paused schedule is a state condition, not an authorization
failure. This forces integrators to inspect raw error codes to distinguish
the two cases, making error handling fragile.

Replace all three occurrences with Err(VestingError::Paused), which is the
dedicated variant (error code 9) defined for exactly this purpose:
- claim(): paused check now returns VestingError::Paused
- cancel_and_claim(): paused check now returns VestingError::Paused
- pause(): already-paused check now returns VestingError::Paused

Add tests:
- test_cancel_and_claim_while_paused_returns_paused: verifies cancel_and_claim()
  returns VestingError::Paused when the schedule is paused.
- test_pause_when_already_paused_returns_paused: verifies pause() returns
  VestingError::Paused when called on an already-paused schedule.
@drips-wave
Copy link
Copy Markdown

drips-wave Bot commented Apr 26, 2026

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

@Austinaminu2 Austinaminu2 merged commit 75d22ec into Austinaminu2:main Apr 27, 2026
1 check failed
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.

forge-vesting claim(), cancel_and_claim(), and pause() return CommonError::Unauthorized when schedule is paused — should return VestingError::Paused

2 participants