fix(#132): add MAX_MIN_LOCK_PERIOD ceiling to farming-pool and factory - #135
Open
devkryssie wants to merge 1 commit into
Open
fix(#132): add MAX_MIN_LOCK_PERIOD ceiling to farming-pool and factory#135devkryssie wants to merge 1 commit into
devkryssie wants to merge 1 commit into
Conversation
…ol and factory min_lock_period accepted the full u32 range (up to ~681 years at 5 s/ledger) in both FarmingPool::initialize and set_min_lock_period, with no upper-bound validation — unlike global_multiplier and credit_rate which gained explicit ceilings in SmartDropLabs#89. Any user who locks after an admin sets an extreme value has their principal effectively trapped permanently. Changes: - farming-pool: add MAX_MIN_LOCK_PERIOD = 12_614_400 (~2 years at 5 s/ledger) with a worked derivation comment mirroring SmartDropLabs#89's style - farming-pool: enforce ceiling in initialize and set_min_lock_period, returning PoolError::MinLockPeriodAboveCeiling (= 10) on violation - factory: mirror the same ceiling in create_pool before deploying/initializing a pool, returning FactoryError::MinLockPeriodAboveCeiling (= 9) on violation - tests: add test_set_min_lock_period_rejects_above_ceiling, test_set_min_lock_period_accepts_exactly_the_ceiling, test_initialize_rejects_min_lock_period_above_ceiling, test_initialize_accepts_min_lock_period_exactly_at_ceiling (farming-pool) and test_create_pool_rejects_min_lock_period_above_ceiling, test_create_pool_accepts_min_lock_period_exactly_at_ceiling (factory) All tests pass (104 farming-pool + factory, 20 vesting-wallet). Closes SmartDropLabs#132
✅ Deploy Preview for sdcontracts ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #132.
min_lock_periodaccepted the fullu32range (up to ~681 years at 5 s/ledger) in bothFarmingPool::initializeandset_min_lock_period, with no upper-bound validation — unlikeglobal_multiplierandcredit_ratewhich gained explicit ceilings in #89. Any user who locks after an admin sets an extreme value has their principal effectively trapped permanently, with no recovery path other than the admin-gatedemergency_withdraw.Changes
farming-pool
MAX_MIN_LOCK_PERIOD = 12_614_400(~2 years at 5 s/ledger) with a worked derivation comment mirroring farming-pool: set_global_multiplier and set_credit_rate accept unbounded values with no sanity ceiling, guaranteeing compute_credits overflow at scale #89's styleinitializenow rejectsmin_lock_period > MAX_MIN_LOCK_PERIODwithPoolError::MinLockPeriodAboveCeiling(= 10)set_min_lock_periodnow rejects values above the ceiling with the same errorfactory
create_poolbefore deploying/initializing a poolFactoryError::MinLockPeriodAboveCeiling(= 9) on violation — checked after the existingMinLockPeriodOutOfRangeu32-fit check, so both remain distinct and testableTests
test_set_min_lock_period_rejects_above_ceilingtest_set_min_lock_period_accepts_exactly_the_ceilingtest_initialize_rejects_min_lock_period_above_ceilingtest_initialize_accepts_min_lock_period_exactly_at_ceilingtest_create_pool_rejects_min_lock_period_above_ceilingtest_create_pool_accepts_min_lock_period_exactly_at_ceilingCeiling derivation
Two years covers any realistic campaign-style lock window. The companion
vesting-walletcontract in this workspace is the intended vehicle for pure vesting use cases, sofarming-poollocks are campaign-oriented by design — multi-year horizons beyond this ceiling have no credible business justification in this contract's role.Tested
All tests pass: 104 farming-pool + factory unit tests, 20 vesting-wallet tests.