From b7cdd16389631843a07151e5bc5d975218d38e2d Mon Sep 17 00:00:00 2001 From: tolulopedd26 Date: Mon, 22 Jun 2026 15:39:50 +0100 Subject: [PATCH] fix: add duplicate detection and escrow ID mapping to deposit_partial Closes #13 Co-Authored-By: Claude Sonnet 4.6 --- app/contract/contracts/Folder/src/escrow.rs | 41 +++++++++- .../contracts/Folder/src/escrow_id.rs | 64 ++++++++++++++++ app/contract/contracts/Folder/src/test.rs | 75 +++++++++++++++++++ 3 files changed, 177 insertions(+), 3 deletions(-) diff --git a/app/contract/contracts/Folder/src/escrow.rs b/app/contract/contracts/Folder/src/escrow.rs index 9c5bd7aa8..a01d044af 100644 --- a/app/contract/contracts/Folder/src/escrow.rs +++ b/app/contract/contracts/Folder/src/escrow.rs @@ -352,11 +352,40 @@ pub fn deposit_partial( // INV-3: validated, overflow-safe expiry computation let expires_at = compute_expires_at(env, timeout_secs)?; - let commitment = commitment::create_amount_commitment(env, owner.clone(), amount_due, salt)?; - let now = env.ledger().timestamp(); + // Derive a deterministic escrow_id that includes initial_payment so + // identical retries are detected and idempotently return the existing + // commitment without creating a duplicate entry. + let escrow_id = escrow_id::derive_partial_escrow_id( + env, + &token, + amount_due, + initial_payment, + &owner, + &salt, + timeout_secs, + &arbiter, + )?; + if let Some(existing) = get_escrow_id_mapping(env, &escrow_id) { + return Ok(existing); + } - let token_client = token::Client::new(env, &token); + let (commitment, legacy_commitment) = + commitment::amount_commitment_hashes(env, &owner, amount_due, &salt)?; let commitment_bytes: Bytes = commitment.clone().into(); + + // Reject duplicate commitment to prevent overwriting an existing escrow. + if has_escrow(env, &commitment_bytes) { + return Err( RustAcademyError::CommitmentAlreadyExists); + } + if legacy_commitment != commitment { + let legacy_bytes: Bytes = legacy_commitment.into(); + if has_escrow(env, &legacy_bytes) { + return Err( RustAcademyError::CommitmentAlreadyExists); + } + } + + let now = env.ledger().timestamp(); + let token_client = token::Client::new(env, &token); let entry = EscrowEntry { token, // moved amount_due, @@ -371,6 +400,12 @@ pub fn deposit_partial( }; put_escrow(env, &commitment_bytes, &entry); + // Store forward (escrow_id → commitment) and reverse (commitment → escrow_id) + // mappings so the backend indexer can correlate partial escrow events with + // their stable IDs, and terminal cleanup can drop the dedup mapping. + put_escrow_id_mapping(env, &escrow_id, &commitment); + put_commitment_escrow_id(env, &commitment_bytes, &escrow_id); + token_client.transfer(&owner, env.current_contract_address(), &initial_payment); let token_addr = token_client.address.clone(); diff --git a/app/contract/contracts/Folder/src/escrow_id.rs b/app/contract/contracts/Folder/src/escrow_id.rs index a14797fbc..49a7c1a27 100644 --- a/app/contract/contracts/Folder/src/escrow_id.rs +++ b/app/contract/contracts/Folder/src/escrow_id.rs @@ -135,3 +135,67 @@ pub fn derive_escrow_id( Ok(env.crypto().sha256(&payload).into()) } + +/// Domain separation tag for partial-escrow-id derivation. +/// +/// Distinct from [`ESCROW_ID_DOMAIN_TAG`] to prevent cross-protocol collisions +/// between full-payment and partial-payment escrow IDs. +pub const PARTIAL_ESCROW_ID_DOMAIN_TAG: &[u8] = b" RustAcademy::PARTIAL_ESCROW_ID::v1"; + +/// Derive a deterministic 32-byte escrow id for a partial-payment deposit. +/// +/// Extends the full-payment derivation by committing to both `amount_due` and +/// `initial_payment`, so two partial escrows with the same `amount_due` but +/// different initial payments are treated as distinct and never alias each other. +/// +/// # Errors +/// +/// - [` RustAcademyError::InvalidAmount`] if `amount_due < 0` or `initial_payment < 0`. +/// - [` RustAcademyError::InvalidSalt`] if `salt.len() > 1024`. +pub fn derive_partial_escrow_id( + env: &Env, + token: &Address, + amount_due: i128, + initial_payment: i128, + owner: &Address, + salt: &Bytes, + timeout_secs: u64, + arbiter: &Option
, +) -> Result, RustAcademyError> { + if amount_due < 0 || initial_payment < 0 { + return Err( RustAcademyError::InvalidAmount); + } + if salt.len() > MAX_SALT_LEN { + return Err( RustAcademyError::InvalidSalt); + } + + let mut payload = Bytes::new(env); + + payload.append(&Bytes::from_slice(env, PARTIAL_ESCROW_ID_DOMAIN_TAG)); + + let token_xdr = token.to_xdr(env); + append_len_prefixed(env, &mut payload, &token_xdr); + + payload.append(&Bytes::from_array(env, &amount_due.to_be_bytes())); + payload.append(&Bytes::from_array(env, &initial_payment.to_be_bytes())); + + let owner_xdr = owner.to_xdr(env); + append_len_prefixed(env, &mut payload, &owner_xdr); + + payload.append(&Bytes::from_array(env, &timeout_secs.to_be_bytes())); + + match arbiter { + None => { + payload.append(&Bytes::from_array(env, &[ARBITER_TAG_NONE])); + } + Some(arb) => { + payload.append(&Bytes::from_array(env, &[ARBITER_TAG_SOME])); + let arb_xdr = arb.to_xdr(env); + append_len_prefixed(env, &mut payload, &arb_xdr); + } + } + + append_len_prefixed(env, &mut payload, salt); + + Ok(env.crypto().sha256(&payload).into()) +} diff --git a/app/contract/contracts/Folder/src/test.rs b/app/contract/contracts/Folder/src/test.rs index 4a5e50894..f79a430f8 100644 --- a/app/contract/contracts/Folder/src/test.rs +++ b/app/contract/contracts/Folder/src/test.rs @@ -3784,3 +3784,78 @@ fn test_single_arbiter_still_works() { ); assert_eq!(token_client.balance(&recipient), amount); } + +// ============================================================================ +// deposit_partial duplicate detection and escrow ID mapping (Issue #13) +// ============================================================================ + +#[test] +fn test_deposit_partial_identical_retry_returns_existing_commitment() { + let (env, client) = setup(); + let token = create_test_token(&env); + let owner = Address::generate(&env); + let amount_due: i128 = 1000; + let initial: i128 = 300; + let salt = Bytes::from_slice(&env, b"idempotent_salt_partial"); + + let token_client = token::StellarAssetClient::new(&env, &token); + token_client.mint(&owner, &(initial * 2)); + + let c1 = client.deposit_partial(&token, &amount_due, &initial, &owner, &salt, &0, &None); + // Identical retry must return the same commitment, not create a duplicate. + let c2 = client.deposit_partial(&token, &amount_due, &initial, &owner, &salt, &0, &None); + assert_eq!(c1, c2); +} + +#[test] +fn test_deposit_partial_duplicate_commitment_rejected() { + // Two partial deposits with the same (amount_due, salt) must not overwrite each other. + let (env, client) = setup(); + let token = create_test_token(&env); + let owner = Address::generate(&env); + let amount_due: i128 = 1000; + let salt = Bytes::from_slice(&env, b"dup_check_salt"); + + let token_client = token::StellarAssetClient::new(&env, &token); + token_client.mint(&owner, &2000); + + client.deposit_partial(&token, &amount_due, &200, &owner, &salt, &0, &None); + // Changing initial_payment but same (amount_due, salt) key → CommitmentAlreadyExists. + let result = client.try_deposit_partial(&token, &amount_due, &300, &owner, &salt, &0, &None); + assert!(result.is_err()); + assert_eq!( + result.unwrap_err().unwrap(), + crate::errors:: RustAcademyError::CommitmentAlreadyExists + ); +} + +#[test] +fn test_deposit_partial_has_escrow_id_mapping() { + let (env, client) = setup(); + let token = create_test_token(&env); + let owner = Address::generate(&env); + let amount_due: i128 = 2000; + let initial: i128 = 500; + let salt = Bytes::from_slice(&env, b"id_map_salt"); + + let token_client = token::StellarAssetClient::new(&env, &token); + token_client.mint(&owner, &initial); + + let commitment = client.deposit_partial(&token, &amount_due, &initial, &owner, &salt, &0, &None); + + // Backend / indexer can correlate the commitment to a stable escrow ID. + let escrow_id = crate::escrow_id::derive_partial_escrow_id( + &env, + &token, + amount_due, + initial, + &owner, + &Bytes::from_slice(&env, b"id_map_salt"), + 0, + &None, + ) + .unwrap(); + + let mapped = client.get_escrow_id_commitment(&escrow_id); + assert_eq!(mapped, Some(commitment)); +}