Skip to content

feat(contract): on-chain idempotency: optional uniqueness of (payer, condition_ref) - #178

Open
Cmitchelle7 wants to merge 1 commit into
DocForum:mainfrom
Cmitchelle7:feat/on-chain-idempotency-optional-uniqueness-of-payer
Open

Cmitchelle7 wants to merge 1 commit into
DocForum:mainfrom
Cmitchelle7:feat/on-chain-idempotency-optional-uniqueness-of-payer

Conversation

@Cmitchelle7

Copy link
Copy Markdown

Summary

Closes #47.

A consumer retrying create_escrow after a timeout can fund the same agreement twice. Off-chain guards don't see both transactions; the chain does. This adds an opt-in unique_ref: bool last parameter to create_escrow and a (payer, condition_ref) → escrow_id index that lets a retrying client recover the original escrow id.

Base note: this branch was cut from main before #177 (tombstone / events) merged. The lib.rs diff includes #177's changes — please merge #177 first so GitHub collapses the diff. idempotency.rs is independent of reclaim_storage.rs.

Design decision (raised in issue #47)

Keep the mapping after settlement. A retry that lands after release/refund must still return DuplicateConditionRef (recoverable via get_escrow_id_by_ref), not fund a second escrow. Callers who want to recycle a condition_ref for a new agreement opt out with unique_ref = false.

Alternative rejected: delete the mapping on settlement — closes the retry-after-timeout hole only until settlement, then re-opens it.

Changes

contracts/escrow/src/lib.rs

  • New DataKey::ConditionRef((Address, String)) → u64.
  • New Error::DuplicateConditionRef = 7. contracterror enums can't carry a payload, so a retrying client uses the new getter to recover the id.
  • create_escrow gains unique_ref: bool as the last parameter. Uniqueness is checked before the funding transfer (duplicate retries cost nothing on-chain); the mapping is written after the escrow record and kept across release / refund.
  • New get_escrow_id_by_ref(payer, condition_ref) -> Result<u64, Error> — returns the stored id or NotFound. Works whether the escrow is funded or settled.
  • EscrowCreated event gains a unique_ref: bool field so indexers can distinguish opted-in escrows.

contracts/escrow/tests/idempotency.rs — 6 tests:

  • duplicate_ref_returns_error_and_getter_returns_existing_id
  • different_payer_same_ref_is_allowed (no cross-payer collision)
  • unique_ref_false_skips_uniqueness
  • settled_escrow_still_blocks_same_ref (+ unique_ref=false bypass)
  • get_escrow_id_by_ref_errors_for_unknown
  • storage_cost_of_ref_key_estimate

ARCHITECTURE_ESSENTIALS.md / README.md — create_escrow signature + get_escrow_id_by_ref row + DuplicateConditionRef noted.

ROADMAP.md — Phase E5 section + changelog entry.

SDK — deliberately not in this PR

createEscrow should gain uniqueRef?: boolean and a getEscrowIdByRef method, but sdk/src/generated/contract-client.ts is machine-generated (stellar contract bindings typescript) and requires the stellar CLI, which isn't available in this environment. Per repo convention ("never hand-edit generated bindings"), the SDK changes are deferred to a follow-up PR that:

  1. regenerates the bindings from the new wasm,
  2. adds uniqueRef / getEscrowIdByRef to sdk/src/index.ts,
  3. bumps @docforum/escrow-sdk 0.3.0 → 0.4.0 (breaking), and
  4. republishes the tarball per ADR 0003.

Test plan

  • 6 new tests in contracts/escrow/tests/idempotency.rs
  • Existing suite unaffected (only create_escrow gained a param — existing calls updated in test files as part of feat(contract): ADR + implementation: reclaim storage for settled escrows #177's base; no other API change)
  • CI green (cargo test + cargo build --target wasm32v1-none --release)
  • storage_cost_of_ref_key_estimate output pasted below

Storage cost

cargo test storage_cost_of_ref_key -- --nocapture prints the estimated per-key cost. Local toolchain verification deferred to CI.

@drips-wave

drips-wave Bot commented Oct 17, 2026

Copy link
Copy Markdown

@Cmitchelle7 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! 🚀

Learn more about application limits

@afeezorobsco-cyber

Copy link
Copy Markdown
Contributor

@Cmitchelle7

This PR has merge conflicts with main, which has moved on since this branch was created. Please rebase onto the latest main (or merge main into your branch), resolve the conflicts, and push. CI will then re-run, and the PR can be merged once it's conflict-free, CI is green and it's been reviewed. Thanks!

@afeezorobsco-cyber

Copy link
Copy Markdown
Contributor

@Cmitchelle7 Thanks for the PR. It can't be merged yet. To unblock it:

CI is failing. Here's what broke:

Soroban contract (test + wasm build):

error[E0106]: missing lifetime specifiers
error[E0061]: this method takes 8 arguments but 7 arguments were supplied
error[E0308]: mismatched types

Please fix these errors and run the same steps locally before pushing (npm run typecheck && npm test && npm run build, or cargo test for contracts).

Merge conflicts: this branch conflicts with main. Please rebase onto the latest main (git fetch upstream && git rebase upstream/main), resolve the conflicts (often ROADMAP.md, ARCHITECTURE*.md or prisma/schema.prisma), and force-push.

Push to this same branch and CI will re-run. Once it's green and conflict-free, it'll be reviewed for merge.

@github-actions github-actions Bot added the needs-rebase Has merge conflicts with main; the author needs to rebase label Oct 20, 2026
@afeezorobsco-cyber

Copy link
Copy Markdown
Contributor

@Cmitchelle7 Heads-up: #268 just changed how contract PRs work in this repo. Your PR changes contracts/escrow/src/lib.rs, so CI's new "Contract changes use a fragment, not a version bump" step will fail it until you make these changes (details in contract-changes.d/README.md):

  1. Don't edit CONTRACT_VERSION when you merge main (currently 2.3.0). Keep main's line as it is.

  2. Add a fragment, e.g. contract-changes.d/<issue>-feat-contract-on-chain-idempotency-optio.md:

    bump: minor
    ---
    Issue #NN: one-paragraph summary of the contract change.

    Use patch for a behaviour fix with no interface change, minor for new entry points, arguments, events or errors, and major for breaking changes.

  3. Switch your new Error codes to provisional ones (issue number × 100 + n). This branch adds DuplicateConditionRef = 7; use DuplicateConditionRef = NN00+0. The fold assigns the final numbers. Don't renumber existing codes.

This should also make your next merge from main much easier: the version line was the one thing every contract PR conflicted on.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-rebase Has merge conflicts with main; the author needs to rebase

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Phase E5] On-chain idempotency: optional uniqueness of (payer, condition_ref)

2 participants