You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
#16 ("Extract compute_split into a shared crate with proof of behavioral equivalence") is scoped, by its own title and body, specifically to compute_split — it doesn't mention the other pieces of logic that are equally, byte-for-byte triplicated across all three contracts. Confirmed by direct comparison:
extend_ttl is defined identically in all three files: env.storage().persistent().extend_ttl(key, 100_000, 500_000) (contracts/escrow/src/lib.rs:332-334, contracts/milestones/src/lib.rs:313-315, contracts/maintenance-pool/src/lib.rs:181-183) — same constants, same body, differing only in the DataKey enum type each is generic over (which itself differs per contract, so the extraction needs to handle that genericity, not just copy the function).
require_admin is defined identically in all three files: env.storage().instance().get(&DataKey::Admin).ok_or(Error::NotInitialized) (contracts/escrow/src/lib.rs:322-327, contracts/milestones/src/lib.rs:306-311, contracts/maintenance-pool/src/lib.rs:174-179).
This matters beyond ordinary code-hygiene, for a reason specific to this batch of issues: this repository's audit batch (this issue included) identifies several fixes that need to be applied to extend_ttl/require_admin-adjacent logic identically across all three contracts — the instance-storage-TTL-never-extended issue, the admin/treasury-rotation issues, and #11's fixed-threshold concern all require touching this exact triplicated code, in three independently-maintained copies, for every one of those fixes. Leaving the triplication in place while implementing those fixes means each one has to be applied three times, correctly, by hand — exactly the kind of situation where one contract silently receives an incomplete or inconsistent version of a fix relative to the other two, a risk this repo's own docs/access-control-audit.md implicitly demonstrates is real (it found initialize's missing require_auth() was consistent across all three contracts by luck of them being copy-pasted together, but there's no structural guarantee that stays true for the next fix applied to this logic).
This issue is scoped as the direct structural prerequisite for cleanly implementing the TTL and admin-rotation fixes elsewhere in this batch — extracting extend_ttl/require_admin into a shared crate before those land means each fix is written and tested once, then automatically applies to all three contracts; extracting after means redoing the triplicated-fix work now, then refactoring it into a shared form later, twice the effort for the same eventual result.
Sequence this work explicitly relative to the TTL/admin-rotation issues filed elsewhere in this batch — ideally this extraction lands first, so those fixes are written once against the shared crate rather than three times against three copies.
Acceptance Criteria
extend_ttl and require_admin extracted into a shared crate, generic/parameterized appropriately across the three contracts' distinct DataKey types
All three contracts updated to use the shared implementations, with no behavioral change to existing tests (all existing tests in all three test.rs files pass unmodified)
Explicit note in the PR/issue about sequencing relative to the TTL and admin-rotation issues filed in this same batch
cargo test --workspace passes
Additional Notes
Precise references confirmed via direct diff: extend_ttl — contracts/escrow/src/lib.rs:332-334, contracts/milestones/src/lib.rs:313-315, contracts/maintenance-pool/src/lib.rs:181-183; require_admin — contracts/escrow/src/lib.rs:322-327, contracts/milestones/src/lib.rs:306-311, contracts/maintenance-pool/src/lib.rs:174-179. (For reference, compute_split itself was confirmed byte-identical between escrow and milestones via diff during this audit — maintenance-pool doesn't have a compute_split at all, since withdraw is single-recipient, which is why Extract compute_split into a shared crate with proof of behavioral equivalence #16's scope is two contracts while this issue's scope is all three.)
Overview
#16 ("Extract
compute_splitinto a shared crate with proof of behavioral equivalence") is scoped, by its own title and body, specifically tocompute_split— it doesn't mention the other pieces of logic that are equally, byte-for-byte triplicated across all three contracts. Confirmed by direct comparison:extend_ttlis defined identically in all three files:env.storage().persistent().extend_ttl(key, 100_000, 500_000)(contracts/escrow/src/lib.rs:332-334,contracts/milestones/src/lib.rs:313-315,contracts/maintenance-pool/src/lib.rs:181-183) — same constants, same body, differing only in theDataKeyenum type each is generic over (which itself differs per contract, so the extraction needs to handle that genericity, not just copy the function).require_adminis defined identically in all three files:env.storage().instance().get(&DataKey::Admin).ok_or(Error::NotInitialized)(contracts/escrow/src/lib.rs:322-327,contracts/milestones/src/lib.rs:306-311,contracts/maintenance-pool/src/lib.rs:174-179).This matters beyond ordinary code-hygiene, for a reason specific to this batch of issues: this repository's audit batch (this issue included) identifies several fixes that need to be applied to
extend_ttl/require_admin-adjacent logic identically across all three contracts — the instance-storage-TTL-never-extended issue, the admin/treasury-rotation issues, and #11's fixed-threshold concern all require touching this exact triplicated code, in three independently-maintained copies, for every one of those fixes. Leaving the triplication in place while implementing those fixes means each one has to be applied three times, correctly, by hand — exactly the kind of situation where one contract silently receives an incomplete or inconsistent version of a fix relative to the other two, a risk this repo's owndocs/access-control-audit.mdimplicitly demonstrates is real (it foundinitialize's missingrequire_auth()was consistent across all three contracts by luck of them being copy-pasted together, but there's no structural guarantee that stays true for the next fix applied to this logic).This issue is scoped as the direct structural prerequisite for cleanly implementing the TTL and admin-rotation fixes elsewhere in this batch — extracting
extend_ttl/require_admininto a shared crate before those land means each fix is written and tested once, then automatically applies to all three contracts; extracting after means redoing the triplicated-fix work now, then refactoring it into a shared form later, twice the effort for the same eventual result.Requirements
extend_ttlandrequire_admininto a shared, non-contract Rust crate (following the exact precedent Extractcompute_splitinto a shared crate with proof of behavioral equivalence #16 already establishes forcompute_split— same crate, if Extractcompute_splitinto a shared crate with proof of behavioral equivalence #16 lands first, or a new one otherwise), parameterized appropriately over each contract's ownDataKeyenum (likely via a small trait each contract'sDataKeyimplements, exposing just what these two helpers need — e.g. anAdminKeymarker or similar, kept minimal).compute_splitinto a shared crate with proof of behavioral equivalence #16's own title calls for — tests confirming the extracted functions produce identical results to the current triplicated versions for every existing test scenario in all three contracts.Acceptance Criteria
extend_ttlandrequire_adminextracted into a shared crate, generic/parameterized appropriately across the three contracts' distinctDataKeytypestest.rsfiles pass unmodified)cargo test --workspacepassesAdditional Notes
extend_ttl—contracts/escrow/src/lib.rs:332-334,contracts/milestones/src/lib.rs:313-315,contracts/maintenance-pool/src/lib.rs:181-183;require_admin—contracts/escrow/src/lib.rs:322-327,contracts/milestones/src/lib.rs:306-311,contracts/maintenance-pool/src/lib.rs:174-179. (For reference,compute_splititself was confirmed byte-identical betweenescrowandmilestonesviadiffduring this audit —maintenance-pooldoesn't have acompute_splitat all, sincewithdrawis single-recipient, which is why Extractcompute_splitinto a shared crate with proof of behavioral equivalence #16's scope is two contracts while this issue's scope is all three.)compute_splitinto a shared crate with proof of behavioral equivalence #16 (the direct precedent and likely shared home for this extraction); the instance-storage-TTL issue, the admin/treasury-rotation issues (escrow and milestones+maintenance-pool variants), and Fixed TTL extension thresholds risk archival/fund-inaccessibility for long-lived escrows and pools #11, all filed in or referenced by this batch — every one of them touchesextend_ttl/require_admin-adjacent code in all three contracts, making this extraction the single highest-leverage piece of groundwork for implementing the rest of this batch's TTL- and admin-related findings consistently.