feat(proposal): mandatory timelock between approval and execution (issue #93) - #194
Open
merciiiqode wants to merge 5 commits into
Open
feat(proposal): mandatory timelock between approval and execution (issue #93)#194merciiiqode wants to merge 5 commits into
merciiiqode wants to merge 5 commits into
Conversation
…cel flows The repo was committed in a broken mid-merge state that did not compile (duplicated error enum variants beyond the XDR limit, merged functions, unclosed delimiters, stale helper signatures). Restore it so `cargo test --workspace`, `cargo fmt` and clippy all pass, and finish the escrow work this branch was opened for (issue ASTROIDX556#80). shared: - rewrite Error enum as exactly 50 #[contracterror] variants; add GraceActive / EscrowNotExpired / EscrowAlreadySettled / ReserveViolation - fix safe-arithmetic test expectations (Overflow for i128::MIN-1; mul round-trips below i128::MAX) escrow (issue ASTROIDX556#80): - implement expiration deadline handling: post-deadline/grace refunds (refund, reclaim, refund_timelock, expire) return ESCROW_EXPIRED and graceful errors, plus sender-only clawback cancel with EscrowNotExpired / GraceActive / EscrowAlreadySettled transitions into Cancelled - reconstruct multi-asset escrow (create, timelock, scheduled, milestones, override release, vesting/claiming views, structured events) - reconcile tests to new cancel semantics; fix stale helper signatures policy: - un-merge whitelist/blocklist/category functions corrupted by the merge; repair check_transfer gates (recipient whitelist, dedupe blacklist checks) - update register_policy helper to current 9-arg signature wallet: - Restore lost batch_execute (atomic sub-calls); sub-calls target external contracts (Soroban forbids self re-entry), gate on wallet Active state, surface callee errors via try_invoke_contract - Fix Val (RawVal rename), ContractCall arg encoding, set_reserve_ratio auth multisig: TooManySigners -> InvalidThreshold; budget/treasury: clippy fixes
…tion status Issue ASTROIDX556#82 already had the core chaining machinery (prerequisite ids in storage, acyclicity checks, ensure_dependencies_met enforcing the executed state before execute). This completes the acceptance criteria that were still missing: - Emit structured events whenever a dependency chain is validated: ("proposal", "dep_ok") when every prerequisite has executed, and ("proposal", "dep_fail") with the id of the first unmet prerequisite when execution is blocked. - Add an is_executed view exposing has_executed() (Executed | Closed) so downstream contracts can query a proposal's completion status directly. - Tests: blocked execution emits dep_fail (no dep_ok), satisfied chains emit dep_ok, end-to-end completion via is_executed including a Failed proposal never counting as executed. Closes ASTROIDX556#82
Adds a validated batch path to the wallet alongside the existing raw
batch_execute. Each BatchAction carries the external sub-call plus the
metadata needed to gate it: policy envelope id (asset/recipient/amount are
checked against the wired policy contract) and budget envelope id (consumed
from the wired budget contract). Empty ids skip the corresponding gate.
batch_execute_validated is two-phase:
1. Validate every action and aggregate its value with checked math in a
single iteration pass (one budget consumption per envelope, no
speculative pre-flights).
2. Execute the sub-calls sequentially.
Any failure — policy denial, budget overrun, cumulative overflow, failing
sub-call — reverts the whole transaction, so validation and execution are
atomic. On success an aggregated BatchReceipt (executed, total_amount,
budget_remaining) is returned and ("wallet", "batch_validated") is
published.
Contract-level set_policy/set_budget (admin) wire the gates, mirroring the
treasury convention. Tests cover success, policy denial rollback, budget
insufficiency rollback, cumulative overflow, unwired-gate refusal,
envelope-less execution, mixed-asset aggregation, and the standard empty /
role / frozen gates.
Closes ASTROIDX556#90
…tion Protects against sudden governance takeovers by refusing to execute a freshly-approved proposal until its timelock delay has elapsed. - Proposal gains an approved_at timestamp, recorded once when the threshold is reached (approve transitions to Approved); the created record carries approved_at = 0 until then. - initialize now takes and stores the mandatory per-proposal timelock (seconds) in a new instance Timelock key; 0 disables the delay. - execute checks the timelock after the state gate and before dependency resolution, reusing the protocol-wide TimelockNotExpired constant via the shared require_time_reached helper: a premature attempt is reported as a scheduling error rather than a dependency failure. State transitions and views (dependencies_met, is_executed, fail) are unaffected by the lock. - Tests: approval records the timestamp; refusal inside the window; success at and after the release time; zero timelock preserves the historical immediate-execution behaviour; the lock never blocks non-execution state transitions. Proposal suite is now 34 tests. Closes ASTROIDX556#93
|
@merciiiqode 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! 🚀 |
Contributor
|
resolve conflict on this issue |
Contributor
|
resolve conflict on this issue @merciiiqode |
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
3 participants
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.
Closes #93
Summary
Adds a mandatory minimum timelock between a proposal's approval and its
execution in
contracts/proposal, so a sudden governance takeover cannotexecute freshly-approved proposals before the delay gives honest members time
to act.
What changed
Proposal.approved_at— the ledger timestamp is recorded exactly once,when the threshold is reached and the proposal transitions to
Approved(
0until then, and preserved through the rest of its life).initializenow takes theper-proposal timelock (seconds) and stores it under a new instance
Timelockkey;0disables the delay. Stored on-chain, applied to everyproposal.
execute— after the state gate and before dependencyresolution, execution refuses with
TimelockNotExpireduntilenv.ledger().timestamp() >= approved_at + timelock(computed with checkedmath). Reuses the protocol-wide timelock constant via the shared
require_time_reachedhelper, and the ordering ensures a premature attemptis reported as a scheduling error rather than a (still accurate) dependency
failure.
execute; approval, rejection,cancellation, expiry,
fail,close, and the dependency/completion viewsare untouched.
Acceptance criteria
execute)(
TimelockNotExpired = 91)expire/fail/close and views still work, zero timelock = old behaviour)
Verification
cargo test -p astroid-proposal— 34 tests pass (5 new)cargo test --workspace— 347 tests passcargo clippy --workspace --all-targets -- -D warnings— cleancargo fmt --all --check— cleanNote: branches back to the accumulated repair/feature branch (#189/#192) because
upstream
maindoesn't compile; merge those first.