Architecture: no shared registry prevents the same issue_id from being double-funded via escrow AND milestones simultaneously

[chonilius]

Overview

escrow and milestones are two entirely separate contract crates with entirely independent storage (contracts/escrow/src/types.rs vs contracts/milestones/src/types.rs — no shared DataKey namespace, no shared instance, no cross-contract call between them anywhere in either lib.rs). Both use a bare u64 issue_id as their unit of "which GitHub issue is this for" — escrow::fund(issue_id: u64, ...) (contracts/escrow/src/lib.rs:64-71) and milestones::allocate(milestone_id: u64, issue_id: u64, ...) (contracts/milestones/src/lib.rs:92) — with no shared registry, on-chain or otherwise, that either contract consults to check whether the other contract has already claimed the same issue_id.

Consequence: the exact same GitHub issue can be funded as a standalone bounty via escrow::fund(issue_id=555, ...) and simultaneously allocated a budget within some release milestone via milestones::allocate(milestone_id=X, issue_id=555, ...), with neither contract aware the other exists, let alone that both are backing the same underlying piece of work. Both can independently reach release/release_issue and pay out in full, for what is, off-chain, one single contribution being compensated twice by two different financial instruments on the same platform. Unlike the within-milestones double-allocation issue filed alongside this one (a single contract's storage-scoping gap), this is a system-level architecture gap: no amount of fixing either contract in isolation closes it, because the information needed to detect the collision — "is issue_id 555 already committed anywhere in the system?" — doesn't exist anywhere on-chain; it can currently only ever live in mergefi-backend's own, off-chain database, with the smart contracts providing zero defense-in-depth if that database is ever wrong, out of sync, or bypassed.

This directly touches the tradeoff the README's "Why three contracts instead of one" section explicitly discusses and accepts ("Independent upgrade/audit surface... you can fix and redeploy that contract without touching escrow funds that are mid-flight") without discussing this specific consequence of that independence: the same architectural choice that gives independent-upgrade benefits also means there is no natural place for a cross-contract "who has already claimed this issue" check to live, short of introducing genuine cross-contract calls between all three contracts (which the README elsewhere implies was deliberately avoided — "compute_split is duplicated... rather than shared via a common library crate," i.e. even code sharing was avoided in favor of independence, let alone live cross-contract calls).

Requirements

• Evaluate architectural options and their tradeoffs explicitly:
  • A shared, cross-contract registry contract (a fourth, minimal contract whose only job is "claim issue_id X for contract Y," called by fund/allocate before proceeding) — closes the gap on-chain, but reintroduces cross-contract calls the current design has otherwise avoided, and creates a new single point of failure/upgrade coupling across all three contracts, in tension with the README's stated independence rationale.
  • A shared library crate exposing a common DataKey convention that each contract could, in principle, be extended to check against a common instance if one were designated as the "source of truth" — weaker (doesn't fully close the gap without also adding cross-contract calls) but lower-coupling.
  • Accept the gap on-chain and mitigate entirely at the backend layer, with the contracts' role limited to documenting the assumption plainly so it's a conscious, informed tradeoff rather than a silent one — likely the most consistent choice given the README's explicit preference for contract independence over shared coupling, but this should be a stated decision, not a default by omission.
• Whichever direction is chosen, document it explicitly in the README's "Why three contracts instead of one" section, since that section currently makes the independence tradeoff sound costless and doesn't mention this consequence at all.

Acceptance Criteria

• Architectural decision made and documented with real reasoning, in the README
• If an on-chain registry is implemented: shared claim-checking mechanism added to both escrow::fund and milestones::allocate, with tests proving a cross-contract collision is now rejected
• If the gap is accepted and pushed to the backend layer: the README's "Why three contracts" tradeoff section explicitly names this as a known, accepted limitation of the independent-contracts design, not left implicit
• cargo test --workspace passes (if code changes are made)

Additional Notes

• Confirmed via full read of both contracts/escrow/src/lib.rs and contracts/milestones/src/lib.rs: neither contains any Address/contract-id reference to the other, nor any cross-contract Env::invoke_contract call anywhere in either file — the two contracts are, today, entirely unaware of each other's existence at the code level.
• Test sketch (if an on-chain fix is chosen): test_same_issue_id_rejected_across_escrow_and_milestones — this test would need to live somewhere that can construct both contract instances in one Env (neither existing test file does this today, since each is scoped to its own crate's #[cfg(test)] mod test) — likely requiring a new workspace-level integration test crate/directory that depends on both mergefi-escrow and mergefi-milestones, itself a small but real infrastructure addition worth calling out as part of this issue's scope.
• Cross-references: the within-milestones double-funding issue filed alongside this one (the narrower, single-contract-scoped version of the same underlying concern — fixable independently and should probably land first, since it's self-contained, while this cross-contract issue is a larger, more architectural decision); README "Why three contracts instead of one" (the design rationale this issue's analysis directly extends); Extract compute_split into a shared crate with proof of behavioral equivalence #16 (compute_split shared-crate extraction — precedent for the "how much should these contracts share" question this issue also has to answer, just for a claim-registry instead of split-math logic).

[prodbycorne]

The separate escrow and milestones contracts do not appear to enforce global uniqueness of an issue_id across the two funding mechanisms.

They are independent contract crates with separate storage namespaces and no cross-contract calls or shared registry. Both nevertheless use a bare u64 to identify the GitHub issue: escrow::fund(issue_id, ...) and milestones::allocate(milestone_id, issue_id, ...). Neither contract consults the other before accepting that identifier.

As a result, the same GitHub issue can potentially have an escrow and a milestones allocation created independently. Each contract can therefore treat the issue as valid within its own isolated state while the application has two distinct on-chain funding mechanisms claiming to represent the same real-world bounty.

I have experience reviewing smart-contract systems where identifiers cross trust and contract boundaries. A local uniqueness check is not sufficient when the identifier represents a globally meaningful entity. If these contracts are intended to be mutually exclusive funding models for GitHub issues, the system needs an explicit coordination mechanism—such as a shared registry/coordinator or a clearly enforced application-level invariant—to prevent the same issue_id from being funded through both paths.

[grantfox-oss]

🦊 GrantFox — @prodbycorne has been assigned to this issue as part of the Third Campaign campaign!

Next steps:

1. Open a Pull Request referencing this issue (e.g., Closes #52)
2. Your PR will be reviewed by the MergeFi maintainers

Good luck! Track your progress on GrantFox.