Skip to content

fix(contracts): reject delay_seconds >= PROPOSAL_TTL_SECONDS in multi… - #1697

Open
Aj-Kayvee wants to merge 5 commits into
LabsCrypt:mainfrom
Aj-Kayvee:fix/1092-max-timelock-bound
Open

fix(contracts): reject delay_seconds >= PROPOSAL_TTL_SECONDS in multi…#1697
Aj-Kayvee wants to merge 5 commits into
LabsCrypt:mainfrom
Aj-Kayvee:fix/1092-max-timelock-bound

Conversation

@Aj-Kayvee

Copy link
Copy Markdown

…sig governance

Add MAX_TIMELOCK_SECONDS constant and DelayTooLong error (4021) to prevent proposals with a timelock delay that would make the finalize window empty. Without this upper bound, setting delay_seconds >= PROPOSAL_TTL_SECONDS silently produces a proposal that can never be finalized because executable_after lands at or past the expiry, causing every finalize call to return ProposalExpired.

  • Add MAX_TIMELOCK_SECONDS = PROPOSAL_TTL_SECONDS - 1 constant
  • Add DelayTooLong = 4021 error variant
  • Reject delay_seconds > MAX_TIMELOCK_SECONDS in propose_admin_transfer
  • Add tests: rejection of delay == TTL, delay > TTL, 7x TTL, and successful finalize at maximum allowed delay

Closes #1092

🤖 Generated with Codebuff

Pull Request Checklist

Please ensure your PR follows these steps, mirroring our CONTRIBUTING.md guidelines.

  • I have read the CONTRIBUTING.md document.
  • My code follows the code style of this project.
  • I have added tests to cover my changes.
  • All new and existing tests passed.
  • I have updated the documentation accordingly.
  • I have verified the changes locally.

@Aj-Kayvee
Aj-Kayvee force-pushed the fix/1092-max-timelock-bound branch from 3239fc0 to aef880b Compare August 30, 2026 20:16
…sig governance

Add MAX_TIMELOCK_SECONDS constant and DelayTooLong error (4021) to prevent
proposals with a timelock delay that would make the finalize window empty.
Without this upper bound, setting delay_seconds >= PROPOSAL_TTL_SECONDS
silently produces a proposal that can never be finalized because
executable_after lands at or past the expiry, causing every finalize call
to return ProposalExpired.

- Add MAX_TIMELOCK_SECONDS = PROPOSAL_TTL_SECONDS - 1 constant
- Add DelayTooLong = 4021 error variant
- Reject delay_seconds > MAX_TIMELOCK_SECONDS in propose_admin_transfer
- Add tests for boundary conditions:
  - Rejection: delay == 0, MIN - 1, TTL, TTL + 1, MAX + 1, 7x TTL
  - Acceptance & finalize: MIN_TIMELOCK_SECONDS, MAX_TIMELOCK_SECONDS
  - Finalize edge cases: 1s before executable, at expiry, 1s before expiry
  - executable_after arithmetic correctness

Closes LabsCrypt#1092

🤖 Generated with Codebuff
Co-Authored-By: Codebuff <[email protected]>
@Aj-Kayvee
Aj-Kayvee force-pushed the fix/1092-max-timelock-bound branch from aef880b to e35a2ef Compare August 30, 2026 20:23
Benjamin O. Ajayi added 4 commits August 31, 2026 17:00
Set all required env vars before validating so tests don't depend on
ambient CI environment, preventing spurious process.exit(1) failures.
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.

[Contracts] MultisigGovernance has no upper bound on delay_seconds, so any timelock >= PROPOSAL_TTL makes the proposal permanently un-finalizable

1 participant