Repository navigation
feat(contract): allow payee-initiated refunds - #169
Conversation
|
@damilare0813 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! 🚀 |
|
This PR has merge conflicts with |
|
@damilare0813 Thanks for the PR. It can't be merged yet. To unblock it: Merge conflicts: this branch conflicts with Push to this same branch and CI will re-run. Once it's green and conflict-free, it'll be reviewed for merge. |
# Conflicts: # ARCHITECTURE_ESSENTIALS.md # ROADMAP.md # contracts/escrow/src/lib.rs # contracts/escrow/tests/release_refund.rs
|
@afeezorobsco-cyber I resolved the conflicts with main and pushed the update. The escrow contract test suite passes. Please review and merge when ready. |
|
@damilare0813 This PR now has merge conflicts with 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-leaseThe usual culprits are shared files like Automated message. It won't repeat unless the PR conflicts again after your next push. |
|
@afeezorobsco-cyber I resolved the conflicts against current main and verified the payee-initiated refund tests pass. The PR description has a standalone |
# Conflicts: # ARCHITECTURE_ESSENTIALS.md # README.md # ROADMAP.md # contracts/escrow/src/lib.rs # docs/threat-model.md
|
@afeezorobsco-cyber I updated this branch with the latest main and resolved the conflicts. The payee refund behavior remains covered, and the full escrow Rust test suite and formatting check pass. Please review and merge when ready. |
|
@afeezorobsco-cyber This branch now includes the latest main and is conflict-free. The full escrow Rust test suite and formatting check pass. Please review and merge when ready. |
|
@damilare0813 This PR now has merge conflicts with 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-leaseThe usual culprits are shared files like Automated message. It won't repeat unless the PR conflicts again after your next push. |
|
@damilare0813 CI is failing on this branch against the latest SDK bindings match contract WASM
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( This PR also conflicts with |
|
@afeezorobsco-cyber I resolved the merge conflicts against the latest main, preserved this PR’s feature, regenerated the contract bindings, and updated the affected test snapshots. The escrow contract tests pass. Could you please review the update and merge when ready? |
|
@damilare0813 This PR now has merge conflicts with 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-leaseThe usual culprits are shared docs like Automated message. It won't repeat unless the PR conflicts again after your next push. |
|
@damilare0813 CI is failing on this branch against the latest SDK bindings match contract WASM
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( This PR also conflicts with |
|
@damilare0813 CI is failing on this branch against the latest Soroban contract (test + wasm build) Your change alters what the committed snapshots record. Run Your change alters contract behaviour that the committed Soroban snapshots record. Run |
|
@damilare0813 CI is failing on this branch against the latest Soroban contract (test + wasm build) The |
Resolve conflicts with main's Phase E5 and E6 work: - contracts/escrow/src/lib.rs: keep PayeeRefunded next to main's pooled and batch types, and keep main's reject_pooled guard at the top of refund() so the payee path cannot touch pooled escrows. - ARCHITECTURE_ESSENTIALS.md: keep main's SDK surface lines and the refund line describing the payee-initiated path. - sdk/src/generated/contract-client.ts: regenerated with the pinned stellar CLI 28.1.0 from the merged WASM (fixes the failing "SDK bindings match contract WASM" check). Also add the TLA+ PayeeRefund action (Funded, caller is the stored payee, no threshold or timeout check), a spec README coverage line, and a changelog.d fragment as AGENTS.md requires.
|
Note for maintainers: (1) docs/adr/0010-payee-initiated-refund.md clashes with the existing docs/adr/0010-payer-cancel-before-payee-accepts.md. Suggest renumbering to 0012. (2) The TLA+ PayeeRefund action in spec/Escrow.tla has not been checked with TLC, because Java was not available in the review environment. (3) The PR title on GitHub differs from the one in the review list. Please confirm the intended title. |
|
@damilare0813 CI is failing at the "Fail if committed test snapshots changed" step: Your change alters contract behaviour that the snapshots record. Run |
|
@damilare0813 This PR now has merge conflicts with 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-leaseThe usual culprits are shared docs like Automated message. It won't repeat unless the PR conflicts again after your next push. |
|
@afeezorobsco-cyber I refreshed the committed snapshot for the voluntary-refund test; the targeted test passes locally. The updated head passes the malware scan. GitHub still reports a merge conflict against current |
|
@damilare0813 Heads-up: #268 just changed how contract PRs work in this repo. Your PR changes
This should also make your next merge from |
# Conflicts: # contracts/escrow/src/lib.rs # sdk/src/generated/contract-client.ts
|
Synced this branch with current main and resolved the merge conflict. Kept CONTRACT_VERSION at 2.3.0, added the required minor contract-change fragment for issue #40, and regenerated the SDK binding. Local cargo tests, the contract-change check, and generated-binding check pass. GitHub reports mergeable=true; Malware scan passed. @afeezorobsco-cyber, please recheck when convenient. |
afeezorobsco-cyber
left a comment
There was a problem hiding this comment.
Sweep: malware scan clean, CI green.
The last merge of main (DocForum#169/DocForum#199) left the branch uncompilable and its tests failing: - `Error` lost `Paused`, `AlreadyPaused` and `NotPaused` while `lib.rs` still returns them (E0599). Re-added with the provisional codes 3500-3502 that contract-changes.d/README.md asks for, since main already spent 35 (`InvalidFee`) and 36 (`Undeliverable`); the maintainer fold assigns the final numbers. README, ADR 0011, ROADMAP, the threat model and the changelog fragment no longer claim 36-38. - `use soroban_sdk::{...}` was spliced into itself, so every imported name was defined twice (E0252). - main's newer `tests/transfer_payee.rs` and `tests/undeliverable.rs` still registered the contract with the old one-argument constructor, so all 14 of their tests panicked ("invalid number of input arguments: 2 expected, got 1"). They now pass a pause admin and their snapshots are regenerated. - Added the required `contract-changes.d/35-emergency-pause.md` fragment (`bump: minor`); `CONTRACT_VERSION` stays at main's 2.3.0, as the new CI rule requires. `cargo test` passes for every suite in contracts/escrow (pause 23, create_escrow 7, release_refund 8, timeout_refund 17, top_up 3, cancel 18, payee_acceptance 3, pooled 19, settle_many 13, liabilities 14, protocol_fee 17, quorum 19, transfer_payee 12, undeliverable 2, token_allowlist 2, token_edge_cases 1, malicious_token 7, sponsored_refund 16, releaser_rotation 3, release_callback 6, time_locked_release 5, model 4). Generated with Codebuff 🤖 Co-Authored-By: Codebuff <noreply@codebuff.com>
* feat(contract): add a deposit-only emergency pause (ADR 0005) There was no on-chain way to slow down a live bug: the only defence was telling integrators to stop. This adds a narrow lever instead — a single custodian set by the contract's constructor can block `create_escrow` and nothing else, so new exposure stops while every existing escrow can still be released, refunded, or timeout-refunded. Blocking outbound paths would have turned a possible loss into a certain lock, which is why the pause deliberately does not read the flag there (and the 16 new tests prove it). Records the decision in ADR 0005, adds the role to the threat model (T11, invariant I6), and regenerates/bumps the SDK to 0.4.0 for the new pause/unpause/is_paused/get_pause_admin surface. Closes #35 🤖 Generated with Codebuff Co-Authored-By: Codebuff <noreply@codebuff.com> * feat(contract): add a deposit-only emergency pause (ADR 0011) Adds a constructor-set custodian that can pause every deposit path (create_escrow and its variants, plus top_up) and nothing else, so new exposure stops while existing escrows can still be released, refunded or timeout-refunded. Pause/unpause are custodian-only, non-idempotent, and emit Paused/Unpaused events; is_paused/get_pause_admin are read-only. Rebased onto current main: the constructor is merged with the existing token allowlist (__constructor(allowed_tokens, pause_admin)), top_up is gated as the ADR requires of any deposit path, error codes are 26-28 to avoid colliding with existing codes, and the ADR is renumbered to 0011 so it does not clash with the protocol-fee ADR. * fix: reconcile emergency pause with pooled escrows from main The last merge of main (#257 pooled escrows, #193/#248 SDK work) left the branch broken: - contract: `DataKey::Pooled`/`Contribution` had been spliced into the `Unpaused` event struct; restore them to `DataKey`. - contract: `Paused`/`AlreadyPaused`/`NotPaused` collided with `NotPooled`/`FundingDeadlinePassed`/`FundingTargetReached` (26-28). Main's codes keep their on-chain values; the pause codes are appended as 35/36/37 (docs, ADR 0011, README, threat model, ROADMAP, changelog updated). - contract: gate the new pooled deposit paths (`create_pooled_escrow`, `contribute`) on the pause; `claim_refund` and `release` stay ungated. Three new tests in tests/pause.rs cover this. - tests: main's new tests (cancel, pooled, model, malicious_token) registered the contract with the old one-argument constructor; pass a pause admin. Snapshots regenerated with `cargo test`. - sdk: regenerate src/generated/contract-client.ts with the pinned stellar CLI 28.1.0 (it contained a broken hand merge); close the unterminated `getPauseAdmin` method in src/index.ts; drop the duplicate `test:unit` script key in package.json. - ci: the Go SDK local-network deploy passes `--pause_admin`. * ci(sdk-go): pin stellar/quickstart to the digest main last passed with The 2026-10-26 quickstart:latest (core/RPC v30) fails the pinned Stellar CLI 28.1.0's first simulation, `stellar contract asset deploy --asset native`, with HostError: Error(Context, InternalError). It fails the same way on unrelated PRs, before the escrow contract (and its constructor args, which match __constructor(allowed_tokens, pause_admin)) is touched. Pin to the digest from main's last green run (39607326690). * test(fuzz): seed lifecycle corpus with the pause-admin constructor crash CI's cargo-fuzz crash (input AQEAAAgBAA==, [1, 1, 0, 0, 8, 1, 0]) was the harness registering the escrow with main's one-argument constructor; every input hit 'invalid number of input arguments: 2 expected, got 1'. The harness now passes (allowed_tokens, pause_admin). Keeping the input in the corpus replays it on every PR and nightly run. * chore: bump CONTRACT_VERSION to 2.1.0 Emergency pause is a backward-compatible feature on top of 2.0.0 (#209). Satisfies the CI rule requiring a CONTRACT_VERSION bump whenever contracts/escrow/src/lib.rs changes. * chore: bump CONTRACT_VERSION to 2.2.0 main is now at 2.1.0 (#242), so the PR head must move past it for the 'Require a contract version bump when lib.rs changes' CI check. * fix(sdk-go): send fee_recipient/fee_bps on create_escrow and create_escrow_release_time #209 added two optional trailing args (fee_recipient, fee_bps) to create_escrow and create_escrow_release_time. The Go SDK still sent the old argument count, so the integration job failed with Error(WasmVm, UnexpectedSize) / MismatchingParameterLen. Send None for both (no protocol fee) until the Go SDK exposes the fee; create_escrow_with_releasers is unchanged. * test(releaser-account): refresh snapshot for per-token liability entry (#242) * feat(contract): settle_many is never blocked by the emergency pause; bump to 2.3.0 Follow-up to the upstream main merge (#254). Per ADR 0011 the pause stops new deposits only and never blocks outbound paths, so settle_many's release and refund ops ignore the pause flag exactly like single release/refund (invariant I8). Documents this in lib.rs, README and the threat model, adds pause tests showing settle_many release, refund and a mixed batch all succeed while paused (with their snapshots), bumps CONTRACT_VERSION to 2.3.0 (main is 2.2.0) and applies cargo fmt. * fix(contract): repair the merge of main that broke the emergency pause The last merge of main (#169/#199) left the branch uncompilable and its tests failing: - `Error` lost `Paused`, `AlreadyPaused` and `NotPaused` while `lib.rs` still returns them (E0599). Re-added with the provisional codes 3500-3502 that contract-changes.d/README.md asks for, since main already spent 35 (`InvalidFee`) and 36 (`Undeliverable`); the maintainer fold assigns the final numbers. README, ADR 0011, ROADMAP, the threat model and the changelog fragment no longer claim 36-38. - `use soroban_sdk::{...}` was spliced into itself, so every imported name was defined twice (E0252). - main's newer `tests/transfer_payee.rs` and `tests/undeliverable.rs` still registered the contract with the old one-argument constructor, so all 14 of their tests panicked ("invalid number of input arguments: 2 expected, got 1"). They now pass a pause admin and their snapshots are regenerated. - Added the required `contract-changes.d/35-emergency-pause.md` fragment (`bump: minor`); `CONTRACT_VERSION` stays at main's 2.3.0, as the new CI rule requires. `cargo test` passes for every suite in contracts/escrow (pause 23, create_escrow 7, release_refund 8, timeout_refund 17, top_up 3, cancel 18, payee_acceptance 3, pooled 19, settle_many 13, liabilities 14, protocol_fee 17, quorum 19, transfer_payee 12, undeliverable 2, token_allowlist 2, token_edge_cases 1, malicious_token 7, sponsored_refund 16, releaser_rotation 3, release_callback 6, time_locked_release 5, model 4). Generated with Codebuff 🤖 Co-Authored-By: Codebuff <noreply@codebuff.com> * fix(sdk,contract): regenerate bindings and pass a pause admin in events.rs The merge of main (#238, lifecycle events) left two jobs failing: - `sdk/src/generated/contract-client.ts` was hand-mangled: the `/**` that opens the `transfer_payee` doc block was lost (TS1131), and the file predated the provisional pause error codes. It is now exactly what the pinned stellar CLI (28.1.0, scripts/stellar-cli-version) generates from the current contract WASM, so "SDK bindings match contract WASM" passes too. Never hand-edited. - main's `contracts/escrow/tests/events.rs` still registered the contract with the one-argument constructor, so its 13 tests panicked with `Error(WasmVm, InvalidAction)` ("2 expected, got 1"). All five `env.register` sites now pass a pause admin, and their snapshots are regenerated for the `PauseAdmin`/`Paused` instance entries. `cargo test` passes for every suite in contracts/escrow (events 13, pause 23, create_escrow 7, release_refund 8, timeout_refund 17, top_up 3, cancel 18, payee_acceptance 3, pooled 19, settle_many 13, liabilities 14, protocol_fee 17, quorum 19, transfer_payee 12, undeliverable 2, token_allowlist 2, token_edge_cases 1, malicious_token 7, sponsored_refund 16, releaser_rotation 3, release_callback 6, time_locked_release 5, model 4). Generated with Codebuff 🤖 Co-Authored-By: Codebuff <noreply@codebuff.com> * fix(sdk): regenerate bindings with the pinned Rust toolchain The bindings committed with the merge were generated from a WASM built with the sandbox's stable Rust (1.99), but the repo pins 1.96 in rust-toolchain.toml and CI builds with it. The compiler changes the order of the ContractSpec entries in the WASM, so the committed file had the PayeeRefunded event spec in a different slot than CI's regeneration and "SDK bindings match contract WASM" failed. Rebuilt the WASM with the pinned 1.96.0 toolchain and regenerated with stellar CLI 28.1.0 as scripts/check-generated-bindings.sh does, so the diff is only that one entry moving; the script now reports OK. Generated with Codebuff 🤖 Co-Authored-By: Codebuff <noreply@codebuff.com> --------- Co-authored-by: Codebuff Agent <noreply@codebuff.com> Co-authored-by: Azintar <Azintar@users.noreply.github.com> Co-authored-by: Azintar <342361479+Azintar@users.noreply.github.com> Co-authored-by: afeez <afeezorobsco@gmail.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Summary
Closes #40
Verification
cargo test --manifest-path contracts/escrow/Cargo.toml -- --test-threads=1passes.cargo fmt --manifest-path contracts/escrow/Cargo.toml --checkpasses.