Skip to content

Comments

fix(BUG-8): clarify cooldownDuration units as seconds in NatSpec#433

Open
mtabasco wants to merge 2 commits intossv-stakingfrom
fix/BUG-8-cooldown-duration-units
Open

fix(BUG-8): clarify cooldownDuration units as seconds in NatSpec#433
mtabasco wants to merge 2 commits intossv-stakingfrom
fix/BUG-8-cooldown-duration-units

Conversation

@mtabasco
Copy link
Contributor

BUG-8: Cooldown duration uses block.timestamp but DIP specifies blocks.

The implementation correctly uses block.timestamp (seconds) for cooldown calculations in SSVStaking.requestUnstake() and calculateTotalUnfrozenBalance(). The deploy config already sets 604800 (7 days in seconds). However, the NatSpec documentation was ambiguous about the unit, risking future misconfiguration.

Changes:

  • ISSVDAO.sol: Add 'in seconds' to CooldownDurationUpdated event and setUnstakeCooldownDuration function NatSpec
  • SSVStorageStaking.sol: Add NatSpec comment documenting cooldownDuration field is in seconds
  • SSVNetworkSSVStakingUpgrade.sol: Add NatSpec to initializeSSVStaking documenting cooldownDuration param is in seconds (e.g. 604800 for 7 days)
  • requestUnstake.test.ts: Add BUG-8 regression test verifying unlockTime is derived from block.timestamp (not block.number)

No logic, ABI, or storage layout changes. NatSpec-only + regression test.

BUG-8: Cooldown duration uses block.timestamp but DIP specifies blocks.

The implementation correctly uses block.timestamp (seconds) for cooldown
calculations in SSVStaking.requestUnstake() and calculateTotalUnfrozenBalance().
The deploy config already sets 604800 (7 days in seconds). However, the NatSpec
documentation was ambiguous about the unit, risking future misconfiguration.

Changes:
- ISSVDAO.sol: Add 'in seconds' to CooldownDurationUpdated event and
  setUnstakeCooldownDuration function NatSpec
- SSVStorageStaking.sol: Add NatSpec comment documenting cooldownDuration
  field is in seconds
- SSVNetworkSSVStakingUpgrade.sol: Add NatSpec to initializeSSVStaking
  documenting cooldownDuration param is in seconds (e.g. 604800 for 7 days)
- requestUnstake.test.ts: Add BUG-8 regression test verifying unlockTime
  is derived from block.timestamp (not block.number)

No logic, ABI, or storage layout changes. NatSpec-only + regression test.
@github-actions
Copy link

github-actions bot commented Feb 17, 2026

Gas Usage Report

Commit: 7f6b495
Branch: HEAD
All within limits: No

Exceeded Limits

Operation Limit Actual Over by
CANCEL_OPERATOR_FEE 38,000 34,865 +-8.25%
REMOVE_MULTIPLE_OPERATOR_WHITELIST_10_10 108,000 129,006 +19.45%
REMOVE_OPERATOR 75,000 76,818 +2.42%
REMOVE_OPERATOR_WHITELISTING_CONTRACT 52,000 57,036 +9.68%
REMOVE_OPERATOR_WHITELISTING_CONTRACT_10 109,500 126,292 +15.34%
REMOVE_OPERATOR_WITH_WITHDRAW 74,000 79,860 +7.92%
SET_MULTIPLE_OPERATOR_WHITELIST_10_10 137,000 157,821 +15.20%
SET_OPERATOR_WHITELISTING_CONTRACT 122,500 128,658 +5.03%
SET_OPERATOR_WHITELISTING_CONTRACT_10 316,000 337,000 +6.65%
SET_OPERATORS_PRIVATE_10 51,000 52,976 +3.87%
SET_OPERATORS_PUBLIC_10 29,000 30,915 +6.60%
UPDATE_OPERATOR_WHITELISTING_CONTRACT 79,500 85,799 +7.92%

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.

1 participant