fix(multisig): fix #10 by adding instance storage TTL extensions#40
Conversation
ogazboiz
left a comment
There was a problem hiding this comment.
this is a correct, well-scoped fix, nice work. the bump_instance_ttl helper is called on every write and read path and in read_admin, the constants are sane (threshold ~1 day, bump ~30 days, comfortably above the 7-day proposal lifetime + 1-day timelock), the extend_ttl(threshold, bump_to) signature is right, the new ttl-extension test passes and all 35 multisig tests pass, clippy clean.
one thing blocks CI: cargo fmt --check fails on multisig_governance/src/lib.rs:131 (rustfmt collapses the aligned padding before the // ~30 days comment), and fmt is the very first CI step so it hard-fails there. just run cargo fmt and commit:
cargo fmt && git commit -am "style: rustfmt"
heads up, since this is your first PR here the CI run is gated and shows "no checks reported" until a maintainer approves it. i'll approve the run so you get the signal, but it'll fail on that fmt line until you push the fix above. once fmt is clean it should go green and i'll merge.
if you want to keep contributing, join us on Telegram: https://t.me/+DOylgFv1jyJlNzM0
|
@ogazboiz I have made the simple correction. I ran |
ogazboiz
left a comment
There was a problem hiding this comment.
thanks for the quick turnaround, the rustfmt fix is in and CI is now green (fmt + clippy + tests + wasm all pass). re-confirming the fix itself from the last round: bump_instance_ttl is called on every write and read path and in read_admin, the constants comfortably exceed the max proposal lifetime (7-day PROPOSAL_TTL_SECONDS + 24h delay), the extend_ttl signature is right, and the new test that advances ledgers across the timelock window and finalizes proves the proposal survives archival. that covers all of issue #10's acceptance criteria.
merging. nice work on this one.
if you want to keep contributing, join us on Telegram: https://t.me/+DOylgFv1jyJlNzM0
Closes #10
PR Description
This PR resolves issue #10 by extending the Time-To-Live (TTL) of the instance storage in the
multisig_governancecontract.Previously, the contract stored the pending proposal under instance storage (
KEY_PENDING) but did not extend its TTL. In Soroban, instance storage can be archived by the host if it is not accessed or bumped within its lifetime. For long-lived governance proposals waiting on the 7-day proposal TTL plus the timelock window, this could result in proposal data becoming archived, preventing finalization or approvals.To address this, I added an
INSTANCE_TTL_THRESHOLDof 17,280 ledgers (~1 day) and anINSTANCE_TTL_BUMPof 518,400 ledgers (~30 days). We introduced abump_instance_ttlhelper function that extends the instance storage TTL. This function is called on all entrypoints (both write operations like propose, approve, and finalize, and read-only views) to keep the contract state and proposal data active.Changes Made
INSTANCE_TTL_THRESHOLDandINSTANCE_TTL_BUMPconstants tomultisig_governance/src/lib.rs.bump_instance_ttlhelper function inmultisig_governance/src/lib.rs.multisig_governance/src/lib.rs.test_proposal_ttl_extension_keeps_proposal_active_and_finalizableinmultisig_governance/src/test.rsto verify that advancing sequence numbers across the timelock keeps the proposal finalizable.