Skip to content

feat(contract): extend refund deadline by mutual consent - #157

Open
BashBurna26 wants to merge 9 commits into
DocForum:mainfrom
BashBurna26:fix/issue-41
Open

BashBurna26 wants to merge 9 commits into
DocForum:mainfrom
BashBurna26:fix/issue-41

Conversation

@BashBurna26

@BashBurna26 BashBurna26 commented Oct 16, 2026 •

Copy link
Copy Markdown

Summary

  • Extend an active refund deadline only with authorization from both the payer and the first releaser.
  • Require an existing, unexpired deadline and a strictly later replacement; reject quorum escrows on this single-releaser path.
  • Emit refund_deadline_extended, expose the new deadline through get_escrow, and add the SDK method.
  • Integrate with current main and regenerate the SDK client.

Verification

  • cargo test --manifest-path contracts/escrow/Cargo.toml -- --test-threads=1 passed.
  • ./scripts/check-generated-bindings.sh passed.
  • git diff --check passed.

Closes #41

@drips-wave

drips-wave Bot commented Oct 16, 2026

Copy link
Copy Markdown

@BashBurna26 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

@BashBurna26

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

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

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
# Conflicts:
#	ARCHITECTURE_ESSENTIALS.md
#	README.md
#	ROADMAP.md
#	contracts/escrow/src/lib.rs
#	sdk/package.json
#	sdk/src/generated/contract-client.ts
#	sdk/src/index.ts
@BashBurna26

Copy link
Copy Markdown
Author

@afeezorobsco-cyber I resolved the conflicts with main, retained the current quorum and top-up support, and pushed the consented deadline extension update. The focused deadline-extension contract tests pass. Please review and merge when ready.

@afeezorobsco-cyber

Copy link
Copy Markdown
Contributor

@BashBurna26 Thanks for pushing. The malware scan passed, but this PR still has merge conflicts with main, so it can't be merged or reviewed yet. Several escrow PRs merged recently (#183 release callback, #203 M-of-N quorum), so please:

git fetch upstream
git rebase upstream/main
# resolve the conflicts (usually contracts/escrow/src/lib.rs, the create_escrow
# call sites in tests, ROADMAP.md and ARCHITECTURE_ESSENTIALS.md), then:
cargo test                              # commit any updated test_snapshots/
./scripts/check-generated-bindings.sh   # regenerate the SDK bindings if it fails
git push --force-with-lease

Once CI is green and the conflicts are gone, it can be merged.

@BashBurna26

Copy link
Copy Markdown
Author

@afeezorobsco-cyber I resolved the merge conflict against current main and preserved refund deadline extension alongside the latest contract changes. The full contract suite and generated SDK binding check pass, and the description has a standalone Closes #41 line. Please review and merge when ready.

@github-actions

Copy link
Copy Markdown

@BashBurna26 This PR now has merge conflicts with main, most likely because #196 (feat(sdk): fee estimation and fee-bump sponsorship helpers) was just merged, so it can't be merged until it's rebased:

git fetch upstream            # upstream = https://github.com/DocForum/docforum-escrow.git
git rebase upstream/main
# resolve the conflicts, re-run the tests, then:
git push --force-with-lease

The usual culprits are shared files like ROADMAP.md and ARCHITECTURE*.md. CI re-runs when you push, and the needs-rebase label is removed automatically once the conflicts are resolved.

Automated message. It won't repeat unless the PR conflicts again after your next push.

# Conflicts:
#	contracts/escrow/src/lib.rs
#	sdk/src/generated/contract-client.ts
#	sdk/src/index.ts
@BashBurna26

Copy link
Copy Markdown
Author

The branch now includes the latest main and the merge conflicts are resolved; the refund extension and regenerated contract bindings are retained. The PR description still includes Closes #41. @afeezorobsco-cyber, please review the updated PR and merge it once the checks pass.

@github-actions

Copy link
Copy Markdown

@BashBurna26 This PR now has merge conflicts with main, most likely because #158 (feat(contract): sponsored funding: a funder distinct from the refund …) was just merged, so it can't be merged until it's rebased:

git fetch upstream            # upstream = https://github.com/DocForum/docforum-escrow.git
git rebase upstream/main
# resolve the conflicts, re-run the tests, then:
git push --force-with-lease

The usual culprits are shared files like ROADMAP.md and ARCHITECTURE*.md. CI re-runs when you push, and the needs-rebase label is removed automatically once the conflicts are resolved.

Automated message. It won't repeat unless the PR conflicts again after your next push.

@afeezorobsco-cyber

Copy link
Copy Markdown
Contributor

@BashBurna26 CI is failing on this branch against the latest main, so it can't be merged yet. The errors from the latest run:

Soroban contract (test + wasm build)

cargo test changed committed Soroban snapshots under contracts/escrow/test_snapshots.

TypeScript SDK (Chromium), TypeScript SDK (Node 24)

src/index.ts(527,9): error TS1361: 'Api' cannot be used as a value because it was imported using 'import type'.

Also failing: TypeScript SDK (Node 20), SDK bindings match contract WASM, TypeScript SDK (Deno), TypeScript SDK (Bun), TypeScript SDK (Node 22).

Your change alters contract behaviour that the committed Soroban snapshots record. Run cargo test in contracts/escrow, review the diff under contracts/escrow/test_snapshots/, and commit the updated snapshots.

This PR also conflicts with main again, so please rebase first, then fix the above.

@BashBurna26

Copy link
Copy Markdown
Author

Rebased onto current main (28ec877) and fixed the CI failures:

  • Merge conflicts: resolved in contracts/escrow/src/lib.rs (both Error
    enum and the extend_refund_after impl block), sdk/src/index.ts (import
    list, EscrowEventType, the method body), and
    sdk/src/generated/contract-client.ts (regenerated fresh rather than
    hand-merged). Kept main's new InvalidReleaser/pooled-escrow/
    InvalidFundingDeadline error variants at their existing discriminants and
    appended this PR's InvalidRefundExtension at the next free value — 29 →
    35
    — since 29 was reused by main's FundingIncomplete in the meantime.
    Updated ADR 0004 and the InvalidRefundExtension doc comment to say 35.
  • TS1361: was already resolved by taking main's import style during the
    merge (import { Api, Server as RpcServer } from "@stellar/stellar-sdk/rpc",
    Api used as a value for Api.isSimulationError/Api.SendTransactionResponse
    etc.). Rewrote extendRefundAfter to use that Api plus the existing
    invoke()/EscrowSigner helpers instead of the old KeypairSigner-based
    draft, so it now accepts any EscrowSigner (keypair or SEP-43 wallet) for
    both payer and releaser, consistent with the rest of the SDK.
  • create_escrow test fixture: tests/refund_extension.rs needed a 9th
    arg (refund_to: Option<Address>) after merging main's sponsored-refund
    change; added &None.
  • Bindings: regenerated sdk/src/generated/contract-client.ts with the
    pinned stellar CLI 28.1.0 from the rebuilt WASM;
    ./scripts/check-generated-bindings.sh now passes clean.
  • Snapshots: ran cargo test, reviewed the test_snapshots diff (mostly
    new files from main's merged branches — pooled, cancel, etc. — plus the 5
    new refund_extension snapshots), committed it, and re-ran cargo test to
    confirm a byte-identical, deterministic re-run with zero diff.
  • Docs/changelog: updated ADR 0004's extension section (error code 35,
    EscrowSigner instead of raw keypairs), added
    changelog.d/41-refund-deadline-extension.md (per the new per-PR changelog
    convention — did not touch ROADMAP.md's Changelog section), and a
    .changeset/refund-deadline-extension.md minor bump entry for the SDK's
    new public method, per CONTRIBUTING.md.

Commands run (contracts/escrow, Rust 1.96.0 via rustup,
wasm32v1-none target):

  • cargo fmt --check — clean for everything this PR touches; pre-existing
    drift in benches/resource_profile.rs, tests/cancel.rs,
    tests/malicious_token.rs, tests/model.rs, tests/pooled.rs, and a few
    unrelated spots in src/lib.rs already exists on main (confirmed via
    git show origin/main:contracts/escrow/src/lib.rs | rustfmt --check), not
    introduced here.
  • cargo clippy --all-targets — no errors; pre-existing warnings only
    (needless_borrow in tests/release_callback.rs, a couple of
    nonminimal_bool lints), none touching this PR's new code.
  • cargo test — 18 test binaries, all green (122 tests across the suite);
    re-run confirms snapshots are deterministic.
  • cargo build --target wasm32v1-none --release — wasm artifact produced.
  • cargo test/cargo build --release for contracts/releaser-account (the
    custom-account CI step) — both green.
  • ./scripts/check-generated-bindings.sh — passes against stellar CLI
    28.1.0.

SDK (Node 24, matching sdk-node/sdk-bindings CI jobs; npm ci --ignore-scripts):

  • npm run typecheck — clean, no TS1361 or other errors.
  • npm run build — clean.
  • npm run test:offline — 9 files / 64 tests passed.
  • npm run test:milestones — 19 tests passed.
  • npm run test:unit — 56 tests passed.
  • npm run test:runtime — smoke test passed.
    (Did not run npm run test:browser/Bun/Deno variants or the live-testnet
    npm test here — happy to if you want them re-verified too.)

Pre-existing, unrelated to this PR: cargo test --manifest-path ../releaser-account/Cargo.toml rewrites
contracts/releaser-account/test_snapshots/custom_account_can_be_configured_as_escrow_releaser.1.json
on plain origin/main too (verified in a clean worktree) — main's storage
optimisation (#98) added a persistent Status write that this older
snapshot predates. Left it alone since it's not gated by CI's snapshot
check (that only covers contracts/escrow/test_snapshots) and isn't this
PR's to fix.

Diff vs origin/main is now exactly this PR's contribution: the
extend_refund_after contract function + RefundDeadlineExtended event,
its 5 new tests + their snapshots, the ADR update, the SDK's
extendRefundAfter wrapper, regenerated bindings, and the new changelog/
changeset fragments — no stray files, no unrelated changes pulled in by the
rebase.

SHAs:

  • Base: origin/main @ 28ec8774160902c62769d606e4cfcfe4646c3d0a
  • Merge commit: 2b597dac69cf5f1ba9bf58622cdba00d240dfb59
    ("Merge remote-tracking branch 'origin/main' into fix/issue-41")

@BashBurna26

Copy link
Copy Markdown
Author

@afeezorobsco-cyber this is ready for a look whenever you have a moment.

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

Copy link
Copy Markdown
Contributor

@BashBurna26 After updating this branch with main, CI fails at the new "Require a contract version bump when lib.rs changes" step (added by #153, which just merged):

contracts/escrow/src/lib.rs changed without a CONTRACT_VERSION bump

Please bump pub const CONTRACT_VERSION: &str = "..."; in contracts/escrow/src/lib.rs, regenerate anything that embeds it (snapshots and SDK bindings, if they change), and push.

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

Copy link
Copy Markdown

@BashBurna26 This PR now has merge conflicts with main, most likely because #209 (feat(contract): optional protocol fee taken at release (recipient and…) was just merged, so it can't be merged until it's rebased:

git fetch upstream            # upstream = https://github.com/DocForum/docforum-escrow.git
git rebase upstream/main
# resolve the conflicts, re-run the tests, then:
git push --force-with-lease

The usual culprits are shared docs like ROADMAP.md and ARCHITECTURE*.md. If the conflict is in the ROADMAP.md changelog, move your entry into a new file under changelog.d/ (see changelog.d/README.md) rather than resolving it in place, so it can't conflict again. CI re-runs when you push, and the needs-rebase label is removed automatically once the conflicts are resolved.

Automated message. It won't repeat unless the PR conflicts again after your next push.

@zeemscript

Copy link
Copy Markdown

Hi @afeezorobsco-cyber — I updated the branch onto current main, resolved the conflicts, bumped CONTRACT_VERSION to 2.2.0, and regenerated the contract bindings. The focused Rust suite passes (5/5) and scripts/check-generated-bindings.sh passes; GitHub reports the branch mergeable and the malware scan passes. Please rerun/approve the full CI checks when available.

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

Copy link
Copy Markdown
Contributor

@BashBurna26 Thanks for the version bump. Two follow-ups:

Soroban contract (cargo test): the version test still expects the old value:

thread 'version_reports_the_contract_semver' panicked at tests/create_escrow.rs:27:5:
assertion `left == right` failed
  left: String(2.2.0)
 right: String(2.1.0)

Update the expected version in tests/create_escrow.rs to 2.2.0.

SDK bindings match contract WASM: sdk/src/generated/contract-client.ts is out of date with the contract. Regenerate it with stellar CLI 28.1.0:

cd contracts/escrow && cargo build --target wasm32v1-none --release
stellar contract bindings typescript --wasm target/wasm32v1-none/release/docforum_escrow.wasm --output-dir /tmp/escrow-bindings --overwrite
cp /tmp/escrow-bindings/src/index.ts ../../sdk/src/generated/contract-client.ts

Then push.

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

Copy link
Copy Markdown

@BashBurna26 This PR now has merge conflicts with main, most likely because #254 (feat(contract): batch release and refund for fast settlement of many escrows) was just merged, so it can't be merged until it's rebased:

git fetch upstream            # upstream = https://github.com/DocForum/docforum-escrow.git
git rebase upstream/main
# resolve the conflicts, re-run the tests, then:
git push --force-with-lease

The usual culprits are shared docs like ROADMAP.md and ARCHITECTURE*.md. If the conflict is in the ROADMAP.md changelog, move your entry into a new file under changelog.d/ (see changelog.d/README.md) rather than resolving it in place, so it can't conflict again. CI re-runs when you push, and the needs-rebase label is removed automatically once the conflicts are resolved.

Automated message. It won't repeat unless the PR conflicts again after your next push.

@afeezorobsco-cyber

Copy link
Copy Markdown
Contributor

@BashBurna26 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. Revert your CONTRACT_VERSION change. This branch sets 2.2.0. Leave the line exactly as it is on main (currently 2.3.0). A maintainer bumps it once per batch now, so nobody picks the same "next" version any more.

  2. Add a fragment, e.g. contract-changes.d/41-feat-contract-extend-refund-deadline-by.md:

    bump: minor
    ---
    Issue #41: 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 InvalidRefundExtension = 36; use InvalidRefundExtension = 4100. 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.

# Conflicts:
#	contracts/escrow/src/lib.rs
#	sdk/src/generated/contract-client.ts
@BashBurna26

Copy link
Copy Markdown
Author

Synced this branch with current main and resolved the merge conflict. Kept CONTRACT_VERSION at 2.3.0, added the required issue #41 minor fragment and provisional InvalidRefundExtension code 4100, and regenerated the SDK binding. Full Rust tests, the contract-change check, and generated-binding check pass. GitHub reports mergeable=true; Malware scan passed. @afeezorobsco-cyber, please recheck when convenient.

@github-actions

Copy link
Copy Markdown

@BashBurna26 This PR now has merge conflicts with main, most likely because #199 (feat(contract): payee rights transfer: payee reassigns future payout to a new address) was just merged, so it can't be merged until it's rebased:

git fetch upstream            # upstream = https://github.com/DocForum/docforum-escrow.git
git rebase upstream/main
# resolve the conflicts, re-run the tests, then:
git push --force-with-lease

The usual culprits are shared docs like ROADMAP.md and ARCHITECTURE*.md. If the conflict is in the ROADMAP.md changelog, move your entry into a new file under changelog.d/ (see changelog.d/README.md) rather than resolving it in place, so it can't conflict again. CI re-runs when you push, and the needs-rebase label is removed automatically once the conflicts are resolved.

Automated message. It won't repeat unless the PR conflicts again after your next push.

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] Extend refund_after by mutual consent of payer and releaser

3 participants