From c264b6ca7560542cc079c5d861b08c8d28171f50 Mon Sep 17 00:00:00 2001 From: Meet-hybrid Date: Wed, 27 May 2026 18:37:20 +0100 Subject: [PATCH] feat: implement transfer ownership functionality for funded commitments --- TODO.md | 29 ++++++++------ contracts/README.md | 16 ++++++++ contracts/escrow/src/lib.rs | 73 ++++++++++++++++++++++++++++++++++++ contracts/escrow/src/test.rs | 46 +++++++++++++++++++++++ 4 files changed, 152 insertions(+), 12 deletions(-) diff --git a/TODO.md b/TODO.md index 4f8403a6..c088882c 100644 --- a/TODO.md +++ b/TODO.md @@ -1,14 +1,19 @@ -# TODO +# TODO - transfer_ownership entrypoint -## Contracts - Fix unchecked arithmetic in maturity calculation in create_commitment - -- [ ] Create branch `bug/checked-maturity-arithmetic` (if not already). -- [x] Update `contracts/escrow/src/lib.rs` to compute `maturity` using `checked_mul` and `checked_add`. -- [x] Return `InvalidDuration` on overflow. -- [x] Add tests in `contracts/escrow/src/test.rs` covering overflow inputs for `duration_days` and timestamp. -- [x] Document behavior in `contracts/README.md`. -- [x] Add comments on the overflow guard. - -- [ ] Run tests / build (requires Rust toolchain / cargo available). -- [ ] Commit with message like `fix: use checked arithmetic for maturity calculation`. +- [x] Inspect escrow contract + tests + docs +- [ ] Implement `transfer_ownership(commitment_id, new_owner)` in `contracts/escrow/src/lib.rs` + - [ ] Gate by current owner auth (`c.owner.require_auth()`) + - [ ] Allow only `Funded` commitments + - [ ] Update `Commitment.owner` + - [ ] Maintain `OwnerIndex` for both old + new owners (remove from old, add to new) + - [ ] Add internal helper(s) for index de-registration + - [ ] Add event + review-oriented comments +- [ ] Add unit tests in `contracts/escrow/src/test.rs` + - [ ] Happy path: funded commitment index updates + - [ ] Fails when commitment not funded + - [ ] Fails when commitment disputed (still requires funded-only) + - [ ] Edge: transfer to self / duplicate handling +- [ ] Document flow in `contracts/README.md` +- [ ] Run `cargo test` and ensure coverage meets requirement +- [ ] Commit changes on branch (feature/transfer-ownership-entrypoint) diff --git a/contracts/README.md b/contracts/README.md index 5e7da5be..f7b860fe 100644 --- a/contracts/README.md +++ b/contracts/README.md @@ -31,6 +31,20 @@ create_commitment ──► fund_escrow ──► release (matured: p └──► dispute ──► resolve_dispute (admin adjudication) ``` +### Marketplace transfer flow (secondary trading) + +`transfer_ownership(commitment_id, new_owner)` updates ownership for a **funded** commitment. + +**Flow** +1. Marketplace buyer proposes `new_owner`. +2. The current commitment owner calls `transfer_ownership` and must authorize via `require_auth()`. +3. The contract verifies the commitment is `Funded` (transfers are blocked for non-funded states). +4. The contract updates: + - `Commitment.owner` + - `OwnerIndex` for both `old_owner` and `new_owner` +5. The commitment is now eligible for subsequent `release` / `refund` / dispute handling under the new owner. + + ### Public functions | Function | Description | @@ -41,9 +55,11 @@ create_commitment ──► fund_escrow ──► release (matured: p Overflow behavior: `duration_days` is converted into an absolute maturity timestamp using checked arithmetic. If the conversion overflows, the call fails with `InvalidDuration`. | | `fund_escrow(commitment_id)` | Transfer `amount` from owner into the contract (`Created → Funded`). | +| `transfer_ownership(commitment_id, new_owner)` | Transfer marketplace ownership for secondary trading (`Funded` only). Current owner must authorize and the contract updates both `Commitment.owner` and `OwnerIndex`. | | `release(commitment_id, caller)` | Return principal to owner once matured (`Funded → Released`). | | `refund(commitment_id)` | Early-exit refund of principal minus `penalty_bps` (`Funded → Refunded`). | | `dispute(commitment_id, caller, reason)` | Freeze a funded commitment pending admin resolution. | + | `resolve_dispute(commitment_id, release_to_owner)` | Admin-only settlement of a disputed commitment. | | `record_attestation(commitment_id, attestor, compliance_score)` | Record a 0–100 compliance score. | | `get_commitment(commitment_id)` | Read a single commitment record. | diff --git a/contracts/escrow/src/lib.rs b/contracts/escrow/src/lib.rs index d14ccdd5..2679a61b 100644 --- a/contracts/escrow/src/lib.rs +++ b/contracts/escrow/src/lib.rs @@ -375,6 +375,53 @@ impl EscrowContract { Self::load(&env, commitment_id) } + /// Transfer marketplace ownership for secondary trading. + /// + /// Preconditions: + /// - Commitment must be in `Funded` state. + /// + /// Authorization: + /// - Current commitment owner must authorize via `require_auth()`. + /// + /// Effects: + /// - Updates `Commitment.owner`. + /// - Maintains `OwnerIndex` for both the old owner and the new owner. + /// - Emits `transfer_ownership`. + pub fn transfer_ownership(env: Env, commitment_id: u64, new_owner: Address) -> Result<(), Error> { + Self::require_init(&env)?; + + let mut c = Self::load(&env, commitment_id)?; + + // Authorization: only the current owner can transfer ownership. + // NOTE: Must remain tied to the stored commitment owner. + c.owner.require_auth(); + + // Only allow transfer of funded commitments. + if c.status != EscrowStatus::Funded { + return Err(Error::InvalidState); + } + + let old_owner = c.owner.clone(); + if old_owner == new_owner { + // No-op transfer. Kept explicit to avoid index churn. + return Ok(()); + } + + // Maintain OwnerIndex for both sides. + Self::deindex_owner(&env, &old_owner, commitment_id); + Self::index_owner(&env, &new_owner, commitment_id); + + c.owner = new_owner.clone(); + Self::save(&env, &c); + + env.events().publish( + (Symbol::new(&env, "transfer_ownership"), old_owner), + (commitment_id, new_owner), + ); + + Ok(()) + } + /// Return the list of commitment ids owned by an address. pub fn get_owner_commitments(env: Env, owner: Address) -> Vec { env.storage() @@ -383,6 +430,7 @@ impl EscrowContract { .unwrap_or_else(|| Vec::new(&env)) } + // ── Internal helpers ──────────────────────────────────────────────────── fn require_init(env: &Env) -> Result<(), Error> { @@ -427,6 +475,31 @@ impl EscrowContract { .set(&DataKey::OwnerIndex(owner.clone()), &ids); } + /// Remove `id` from `owner`'s OwnerIndex list. + fn deindex_owner(env: &Env, owner: &Address, id: u64) { + let mut ids: Vec = env + .storage() + .persistent() + .get(&DataKey::OwnerIndex(owner.clone())) + .unwrap_or_else(|| Vec::new(env)); + + // Vec in soroban-sdk is append-only by default; build a new list. + let mut i: u32 = 0; + let mut out: Vec = Vec::new(env); + while i < ids.len() { + let cur = ids.get(i).unwrap(); + if cur != id { + out.push_back(cur); + } + i += 1; + } + + env.storage() + .persistent() + .set(&DataKey::OwnerIndex(owner.clone()), &out); + } + + fn token_client(env: &Env) -> soroban_sdk::token::Client { let token: Address = env .storage() diff --git a/contracts/escrow/src/test.rs b/contracts/escrow/src/test.rs index 4e47f819..3f173314 100644 --- a/contracts/escrow/src/test.rs +++ b/contracts/escrow/src/test.rs @@ -208,6 +208,52 @@ fn owner_index_tracks_commitments() { assert_eq!(ids.get(1).unwrap(), b); } +#[test] +fn transfer_ownership_updates_commitment_and_indices() { + let f = setup(); + let old_owner = Address::generate(&f.env); + let new_owner = Address::generate(&f.env); + + fund_owner(&f, &old_owner, 1_000); + let id = f + .client + .create_commitment(&old_owner, &f.asset, &1_000, &RiskProfile::Safe, &30, &200); + f.client.fund_escrow(&id); + + let before_old = f.client.get_owner_commitments(&old_owner); + assert_eq!(before_old.len(), 1); + assert_eq!(before_old.get(0).unwrap(), id); + + f.client.transfer_ownership(&id, &new_owner); + + let c = f.client.get_commitment(&id); + assert_eq!(c.owner, new_owner); + + let after_old = f.client.get_owner_commitments(&old_owner); + assert_eq!(after_old.len(), 0); + + let after_new = f.client.get_owner_commitments(&new_owner); + assert_eq!(after_new.len(), 1); + assert_eq!(after_new.get(0).unwrap(), id); +} + +#[test] +fn transfer_ownership_rejects_non_funded_commitments() { + let f = setup(); + let old_owner = Address::generate(&f.env); + let new_owner = Address::generate(&f.env); + + fund_owner(&f, &old_owner, 1_000); + let id = f + .client + .create_commitment(&old_owner, &f.asset, &1_000, &RiskProfile::Safe, &30, &200); + // Intentionally do NOT call fund_escrow; status remains Created. + + let res = f.client.try_transfer_ownership(&id, &new_owner); + assert_eq!(res, Err(Ok(Error::InvalidState))); +} + + #[test] fn create_rejects_duration_seconds_overflow() { let f = setup();