Skip to content

Enhance VestingManager Functionality and Testing #3

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
May 12, 2025

Conversation

KoxyG
Copy link
Contributor

@KoxyG KoxyG commented May 8, 2025

Changes Made

  1. VestingManager Contract Improvements

    • Changed computeReleasableAmount from public to internal for better encapsulation.
    • Changed initialize function visibility from public to external for upgrade safety
    • Updated our test upgradable contract initializeV2 function in XXXVestingManagerV2 to properly initialize parent contracts
  2. New Test Coverage

    • Added comprehensive tests for the burning mechanism
    • Created and implemented test contract for VestingManager upgrades
    • Added tests for upgrade functionality including:
      • State preservation after upgrade
      • Access control maintenance
      • Double initialization prevention
      • Upgrade authorization checks

Testing

All tests are now passing:

  • 71 tests total
  • All upgrade tests for VestingManager, TokenVault, and XXXToken are successful
  • New burning mechanism tests
  • Comprehensive upgrade testing coverage

Impact

This change ensures:

  1. Contract upgrades are safe and follow OpenZeppelin's upgrade patterns
  2. State is properly preserved during upgrades
  3. Access control and other functionality remain intact after upgrades
  4. Better encapsulation of internal functions
  5. Comprehensive test coverage for all new and existing functionality

Related Issues

Fixes the upgrade safety issues in the VestingManager and TokenVault contracts while adding significant test coverage improvements.

@spikeyrock spikeyrock merged commit 22a4f35 into spikeyrock:main May 12, 2025
1 check 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.

2 participants