[Soroban/Escrow] Add merchant cancellation before funding

[chiemezie1]

[Soroban/Escrow] Add Admin Getter for Current Arbiter

Description

• Added AdminView contract type to lib.rs
• Implemented get_admin_view getter to return:
  • current primary admin
  • optional pending admin transfer
• Added integration tests in integration_tests.rs for:
  • initialized admin view
  • pending admin transfer state
  • failure before contract initialization
    Closes: [Soroban/Escrow] Add Admin Getter for Current Arbiter #94

Summary by CodeRabbit

• New Features

  • Added an escrow lifecycle that supports creating an escrow first, funding it later, and cancelling it before funding.
  • Added a new cancelled escrow state and a cancellation event that includes the cancelling address and a provided reason.

• Bug Fixes

  • Enforced that cancelling is only allowed while an escrow is still pending; cancellation after funding is rejected.

• Tests

  • Updated integration tests to decode event payloads and added coverage for successful pending cancellation and failed post-funding cancellation.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: da39f706-1d2a-473b-b47c-ebea2c434076

📥 Commits

Reviewing files that changed from the base of the PR and between 6d07fee and 01f566e.

📒 Files selected for processing (2)

• contracts/escrow/src/integration_tests.rs
• contracts/escrow/src/lib.rs

🚧 Files skipped from review as they are similar to previous changes (2)

• contracts/escrow/src/integration_tests.rs
• contracts/escrow/src/lib.rs

---

📝 Walkthrough

Walkthrough

The escrow contract adds a cancelled lifecycle state, a cancellation event, and new entrypoints for creating, funding, and canceling pending escrows. Integration tests cover merchant cancellation before funding and rejection after funding.

Changes

Escrow lifecycle extension

Layer / File(s)	Summary
Status and event shape contracts/escrow/src/lib.rs	EscrowStatus adds Cancelled, and EscrowCancelledEvent adds cancellation fields.
Escrow entrypoints contracts/escrow/src/lib.rs	create_escrow, fund_escrow, and cancel_before_funding add the new escrow state transitions and event emission.
Integration coverage contracts/escrow/src/integration_tests.rs	Tests add TryIntoVal, create pending escrows, and assert the cancellation success and invalid-status failure paths.

Sequence Diagram(s)

sequenceDiagram
  participant Seller
  participant Buyer
  participant Merchant
  participant EscrowContract

  Seller->>EscrowContract: create_escrow(...)
  EscrowContract-->>Seller: escrow_id

  Buyer->>EscrowContract: fund_escrow(escrow_id, buyer)
  EscrowContract-->>Buyer: status = Funded

  Merchant->>EscrowContract: cancel_before_funding(escrow_id, reason)
  alt EscrowStatus::Created
    EscrowContract-->>Merchant: status = Cancelled + EscrowCancelledEvent
  else EscrowStatus::Funded
    EscrowContract-->>Merchant: EscrowError::InvalidStatus
  end

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related issues

• [Soroban/Escrow] Add Merchant Cancellation Before Funding #93 — It matches the new merchant-cancellation-before-funding flow, including EscrowCancelledEvent and the success/failure paths.

Possibly related PRs

• DelegoLabs/Delego#144: Modifies the same escrow lifecycle and integration test surfaces in contracts/escrow/src/lib.rs and contracts/escrow/src/integration_tests.rs.
• DelegoLabs/Delego#81: Adds create_escrow-related lifecycle logic that this PR extends with pending creation and cancellation.
• DelegoLabs/Delego#145: Shares the escrow token-whitelist validation path used by the new create_escrow flow.

Suggested labels

size/M

Poem

🐇 I hop through escrows, light and bright,
with Cancelled stars and event-sparked light.
If funding comes, I nibble slow,
if not, I twirl and let it go.
The ledger hums; my ears stand tall —
a cheerful hop for one and all!

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Linked Issues check	⚠️ Warning	The PR implements cancellation/funding flows, but issue #94 requires an AdminView getter with admin/pending_admin support.	Implement the AdminView getter and its success/failure tests for initialized and transfer-pending states, or retarget this PR to the correct issue.
Out of Scope Changes check	⚠️ Warning	The cancellation/funding escrow changes do not match the linked admin-getter scope, so they appear unrelated to issue #94.	Move these escrow lifecycle changes to a separate PR or update the linked issue if this work was meant to replace the admin-getter task.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly describes the merchant cancellation-before-funding feature.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@contracts/escrow/src/integration_tests.rs`:
- Around line 137-141: The escrow integration tests use `symbol_short!` with the
reason string `merchant_cancelled`, which exceeds the short-symbol length limit
and will fail to compile. Update the affected `cancel_before_funding` call sites
in `integration_tests.rs` to use `Symbol::new(&t.env, ...)` for the full reason
string, or shorten the reason to a valid short symbol consistently at all three
occurrences so the test code compiles.

In `@contracts/escrow/src/lib.rs`:
- Around line 75-78: The EscrowCancelledEvent schema is inconsistent with the
other escrow events because escrow_id uses BytesN<32> instead of the standard
u64. Update EscrowCancelledEvent to match the existing event schema used by the
other escrow event structs and ensure any construction/emission sites in the
escrow module use the same escrow_id type and value shape so downstream
consumers can treat cancellation events uniformly.
- Around line 525-526: The pending escrow timeout is being fixed too early in
create_escrow, so a buyer can later fund an already-expired escrow and
immediately hit the refund path. Update the escrow flow around create_escrow and
deposit so the timeout is anchored when funds are actually deposited, or
validate and reject expired pending escrows before any transfer happens. Use the
existing EscrowRecord, timeout_ledger, and deposit logic to keep the current
behavior while preventing stale pending escrows from being funded.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: acc6ed60-c772-482e-b014-d06a9fe604ff

📥 Commits

Reviewing files that changed from the base of the PR and between b3d63c6 and 6d07fee.

📒 Files selected for processing (2)

• contracts/escrow/src/integration_tests.rs
• contracts/escrow/src/lib.rs

[ScriptedBro]

make the ci checks pass and fix all critical and major issues from code rabit

[ScriptedBro]

Fix the failing ci

[ScriptedBro]

Please resolve conflicts