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 da223b29..0092de16 100644 --- a/contracts/README.md +++ b/contracts/README.md @@ -50,6 +50,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 | @@ -58,6 +72,11 @@ create_commitment ──► fund_escrow ──► release (matured: p | `create_commitment(owner, asset, amount, risk, duration_days, penalty_bps)` | Create an unfunded commitment with explicit penalty; returns its `id`. | | `create_commitment_with_default_penalty(owner, asset, amount, risk, duration_days)` | Create an unfunded commitment using the default penalty for the risk profile; returns its `id`. | | `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. | + | `deposit_yield_pool(admin, amount)` | Admin-only deposit of yield tokens into the contract yield pool. | | `get_yield_pool_balance()` | Read the yield pool balance available for matured release payouts. | | `release(commitment_id, caller)` | Return principal plus accrued yield to owner once matured (`Funded → Released`). | diff --git a/contracts/escrow/src/lib.rs b/contracts/escrow/src/lib.rs index e53897d3..78425abf 100644 --- a/contracts/escrow/src/lib.rs +++ b/contracts/escrow/src/lib.rs @@ -744,6 +744,51 @@ 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 attestation history for a commitment id. pub fn get_attestations(env: Env, commitment_id: u64) -> Vec { env.storage() @@ -886,6 +931,30 @@ 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 yield_pool_balance(env: &Env) -> i128 { env.storage() .instance() diff --git a/contracts/escrow/src/test.rs b/contracts/escrow/src/test.rs index cb5e0e4e..630df571 100644 --- a/contracts/escrow/src/test.rs +++ b/contracts/escrow/src/test.rs @@ -408,6 +408,53 @@ fn owner_index_tracks_commitments() { } #[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() { fn dispute_categorizes_value_mismatch() { let f = setup(); let owner = Address::generate(&f.env);