fix: refund basket tokens and cover messaging flows

ERROR_CODES.md

@@ -29,8 +29,6 @@ This document provides a comprehensive reference of all `ContractError` variants
 | **21** | `InvalidTrackingId` | The tracking ID supplied is empty or improperly formatted. | Provide a non-empty, valid string for the tracking ID. |
 | **22** | `DeliveryNotRecorded` | `auto_release` was attempted but the admin has not yet recorded the delivery. | Ensure that delivery is recorded via `record_delivery` if required by the flow. |
 | **23** | `ConflictingRoles` | Multiple roles (Seller, Buyer, Resolver) were assigned the same address. | Ensure that the Seller, Buyer, and Resolver are all distinct accounts. |
-| **38** | `DeliveryNotProposed` | `record_delivery` was called before a delivery proposal was initiated. | Admin must first call `propose_record_delivery` and wait for the timelock. |
-| **39** | `TimelockNotElapsed` | `record_delivery` was called before the required 24-hour timelock elapsed. | Wait until 24 hours have passed since `propose_record_delivery` was called. |
 | **24** | `DisputeWindowStillOpen` | A buyer attempted to confirm delivery while the dispute window is still open. | Wait for the dispute window to close before confirming delivery. |
 | **25** | `UnauthorizedResolver` | A resolver is not in the approved registry and strict mode is enabled. | Use an authorized resolver from the approved registry. |
 | **26** | `ContractNotPaused` | `emergency_drain` was called but the contract is not paused. | Ensure the contract is paused before calling emergency drain operations. |
@@ -45,3 +43,5 @@ This document provides a comprehensive reference of all `ContractError` variants
 | **35** | `NotInitialized` | A read accessor was called before the contract has been initialized. | Initialize the contract first by calling `initialize`. |
 | **36** | `IndexOutOfBounds` | An internal collection index is out of bounds, indicating a storage or argument invariant violation. | This is an internal error; contact the contract maintainer. |
 | **37** | `InvalidExpiration` | A supplied expiration timestamp is not strictly in the future. | Provide an expiration timestamp that is in the future. |
+| **38** | `DeliveryNotProposed` | `record_delivery` was called before a delivery proposal was initiated. | Admin must first call `propose_record_delivery` and wait for the timelock. |
+| **39** | `TimelockNotElapsed` | `record_delivery` was called before the required 24-hour timelock elapsed. | Wait until 24 hours have passed since `propose_record_delivery` was called. |

contracts/escrow/src/instructions.rs

@@ -364,8 +364,26 @@ impl Escrow {
         sender: Address,
         content: String,
     ) -> Result<(), ContractError> {
+        sender.require_auth();
         ensure_not_paused(&env)?;
-        let _ = load_escrow(&env, escrow_id)?;
+        let escrow = load_escrow(&env, escrow_id)?;
+
+        let mut is_payee = false;
+        for payee in escrow.payees.iter() {
+            if payee.address == sender {
+                is_payee = true;
+                break;
+            }
+        }
+        if escrow.buyer.as_ref() != Some(&sender) && !is_payee {
+            return Err(ContractError::NotAuthorized);
+        }
+        if content.is_empty() {
+            return Err(ContractError::InvalidAmount);
+        }
+        if content.len() > MAX_MESSAGE_LEN {
+            return Err(ContractError::InputTooLong);
+        }
 
         let message = Message {
             sender: sender.clone(),
@@ -1313,6 +1331,7 @@ impl Escrow {
 
         let token_client = token::Client::new(&env, &escrow.token);
         token_client.transfer(&env.current_contract_address(), &buyer, &escrow.amount);
+        payout_basket_tokens(&env, escrow_id, &buyer)?;
 
         let prev_state = escrow.state.clone();
         escrow.state = EscrowState::Refunded;

contracts/escrow/src/lib.rs

@@ -131,6 +131,8 @@ const MAX_STATE_HISTORY_ENTRIES: u32 = 50;
 pub const MAX_TRACKING_ID_LEN: u32 = 64;
 pub const MAX_DESCRIPTION_LEN: u32 = 256;
 pub const MAX_NOTES_LEN: u32 = 500;
+/// Maximum length for an on-chain buyer/seller message.
+pub const MAX_MESSAGE_LEN: u32 = 500;
 
 /// Minimum shipping window in seconds (1 second).
 /// A value of 0 would allow an immediate dispute with no shipping time, which is invalid.
@@ -186,6 +188,7 @@ mod test_helpers;
 mod test_initialize_twice;
 mod test_initialize_zero_admin;
 mod test_malicious_token;
+mod test_messaging;
 mod test_minimum_amount_guard;
 mod test_not_found;
 mod test_overflow;

contracts/escrow/src/test_basket_escrow.rs

@@ -276,3 +276,112 @@ fn zero_amount_token_is_skipped_on_fund_and_payout() {
     assert_eq!(zero.token.balance(&fx.seller), 0);
     assert_eq!(c.token.balance(&fx.seller), 200);
 }
+
+#[test]
+fn fund_basket_escrow_rejects_a_buyer_other_than_the_expected_buyer() {
+    let fx = setup();
+    let client = EscrowClient::new(&fx.env, &fx.contract_id);
+    let primary = make_token(&fx.env);
+    let expected_buyer = Address::generate(&fx.env);
+    let stranger = Address::generate(&fx.env);
+    let escrow_id = client.create_basket_escrow(
+        &fx.seller,
+        &Some(expected_buyer.clone()),
+        &fx.resolver,
+        &vec_addr(&fx.env, &[&primary.address]),
+        &vec_i128(&fx.env, &[100]),
+        &0_u32,
+        &SHIPPING_WINDOW,
+    );
+
+    primary.admin.mint(&stranger, &100);
+    assert_eq!(
+        client.try_fund_basket_escrow(&escrow_id, &stranger),
+        Err(Ok(ContractError::NotAuthorized))
+    );
+    assert_eq!(primary.token.balance(&stranger), 100);
+    assert_eq!(primary.token.balance(&fx.contract_id), 0);
+}
+
+#[test]
+fn fund_basket_escrow_is_atomic_when_the_buyer_only_has_part_of_the_basket() {
+    let fx = setup();
+    let client = EscrowClient::new(&fx.env, &fx.contract_id);
+    let primary = make_token(&fx.env);
+    let additional = make_token(&fx.env);
+    let escrow_id = client.create_basket_escrow(
+        &fx.seller,
+        &Some(fx.buyer.clone()),
+        &fx.resolver,
+        &vec_addr(&fx.env, &[&primary.address, &additional.address]),
+        &vec_i128(&fx.env, &[100, 50]),
+        &0_u32,
+        &SHIPPING_WINDOW,
+    );
+
+    primary.admin.mint(&fx.buyer, &100);
+    additional.admin.mint(&fx.buyer, &49);
+    assert!(client
+        .try_fund_basket_escrow(&escrow_id, &fx.buyer)
+        .is_err());
+
+    // The failed second transfer rolls back the first transfer and state update.
+    assert_eq!(primary.token.balance(&fx.buyer), 100);
+    assert_eq!(primary.token.balance(&fx.contract_id), 0);
+    assert_eq!(additional.token.balance(&fx.buyer), 49);
+    assert_eq!(additional.token.balance(&fx.contract_id), 0);
+    assert_eq!(client.get_escrow(&escrow_id).state, EscrowState::Pending);
+}
+
+#[test]
+fn fund_basket_escrow_rejects_a_basket_already_funded_by_fund_escrow() {
+    let fx = setup();
+    let client = EscrowClient::new(&fx.env, &fx.contract_id);
+    let primary = make_token(&fx.env);
+    let additional = make_token(&fx.env);
+    let escrow_id = client.create_basket_escrow(
+        &fx.seller,
+        &Some(fx.buyer.clone()),
+        &fx.resolver,
+        &vec_addr(&fx.env, &[&primary.address, &additional.address]),
+        &vec_i128(&fx.env, &[100, 50]),
+        &0_u32,
+        &SHIPPING_WINDOW,
+    );
+    primary.admin.mint(&fx.buyer, &100);
+    additional.admin.mint(&fx.buyer, &50);
+    client.fund_escrow(&escrow_id, &fx.buyer);
+
+    assert_eq!(
+        client.try_fund_basket_escrow(&escrow_id, &fx.buyer),
+        Err(Ok(ContractError::InvalidState))
+    );
+    assert_eq!(primary.token.balance(&fx.contract_id), 100);
+    assert_eq!(additional.token.balance(&fx.contract_id), 50);
+}
+
+#[test]
+fn fund_basket_escrow_transfers_only_the_configured_amounts() {
+    let fx = setup();
+    let client = EscrowClient::new(&fx.env, &fx.contract_id);
+    let primary = make_token(&fx.env);
+    let additional = make_token(&fx.env);
+    let escrow_id = client.create_basket_escrow(
+        &fx.seller,
+        &Some(fx.buyer.clone()),
+        &fx.resolver,
+        &vec_addr(&fx.env, &[&primary.address, &additional.address]),
+        &vec_i128(&fx.env, &[100, 50]),
+        &0_u32,
+        &SHIPPING_WINDOW,
+    );
+    primary.admin.mint(&fx.buyer, &150);
+    additional.admin.mint(&fx.buyer, &75);
+
+    client.fund_basket_escrow(&escrow_id, &fx.buyer);
+
+    assert_eq!(primary.token.balance(&fx.contract_id), 100);
+    assert_eq!(additional.token.balance(&fx.contract_id), 50);
+    assert_eq!(primary.token.balance(&fx.buyer), 50);
+    assert_eq!(additional.token.balance(&fx.buyer), 25);
+}

contracts/escrow/src/test_messaging.rs

@@ -0,0 +1,170 @@
+#![cfg(test)]
+
+//! Coverage for the on-chain buyer/seller messaging thread.
+
+use crate::test_helpers::{create_funded_escrow, setup_contract};
+use crate::{ContractError, EscrowClient};
+use soroban_sdk::{
+    testutils::{Address as _, Ledger as _},
+    Address, Env, String,
+};
+
+struct Fixture {
+    env: Env,
+    contract_id: Address,
+    admin: Address,
+    seller: Address,
+    buyer: Address,
+    escrow_id: u64,
+}
+
+impl Fixture {
+    fn client(&self) -> EscrowClient<'_> {
+        EscrowClient::new(&self.env, &self.contract_id)
+    }
+}
+
+fn fixture() -> Fixture {
+    let env = Env::default();
+    env.mock_all_auths();
+    let env2 = env.clone();
+    let (contract_id, client, admin, _fee_collector) = setup_contract(&env2);
+    let seller = Address::generate(&env);
+    let buyer = Address::generate(&env);
+    let resolver = Address::generate(&env);
+    let token = env
+        .register_stellar_asset_contract_v2(Address::generate(&env))
+        .address();
+    let escrow_id = create_funded_escrow(
+        &env, &client, &seller, &buyer, &resolver, &token, 100, 0, 3_600,
+    );
+    Fixture {
+        env,
+        contract_id,
+        admin,
+        seller,
+        buyer,
+        escrow_id,
+    }
+}
+
+#[test]
+fn buyer_can_post_a_message_and_it_is_stored_with_its_timestamp() {
+    let f = fixture();
+    f.env.ledger().set_timestamp(42);
+    let content = String::from_str(&f.env, "Where is my order?");
+
+    f.client().post_message(&f.escrow_id, &f.buyer, &content);
+
+    let messages = f.client().get_messages(&f.escrow_id, &0, &10);
+    assert_eq!(messages.len(), 1);
+    assert_eq!(messages.get(0).unwrap().sender, f.buyer);
+    assert_eq!(messages.get(0).unwrap().timestamp, 42);
+    assert_eq!(messages.get(0).unwrap().content, content);
+}
+
+#[test]
+fn seller_can_post_a_message() {
+    let f = fixture();
+    let content = String::from_str(&f.env, "It ships tomorrow.");
+
+    f.client().post_message(&f.escrow_id, &f.seller, &content);
+
+    assert_eq!(
+        f.client()
+            .get_messages(&f.escrow_id, &0, &1)
+            .get(0)
+            .unwrap()
+            .sender,
+        f.seller
+    );
+}
+
+#[test]
+fn messages_preserve_posting_order() {
+    let f = fixture();
+    let first = String::from_str(&f.env, "First");
+    let second = String::from_str(&f.env, "Second");
+    f.client().post_message(&f.escrow_id, &f.buyer, &first);
+    f.client().post_message(&f.escrow_id, &f.seller, &second);
+
+    let messages = f.client().get_messages(&f.escrow_id, &0, &10);
+    assert_eq!(messages.len(), 2);
+    assert_eq!(messages.get(0).unwrap().content, first);
+    assert_eq!(messages.get(1).unwrap().content, second);
+}
+
+#[test]
+fn non_participant_cannot_post_a_message() {
+    let f = fixture();
+    let stranger = Address::generate(&f.env);
+    let result = f.client().try_post_message(
+        &f.escrow_id,
+        &stranger,
+        &String::from_str(&f.env, "Let me in"),
+    );
+
+    assert_eq!(result, Err(Ok(ContractError::NotAuthorized)));
+    assert_eq!(f.client().get_messages(&f.escrow_id, &0, &10).len(), 0);
+}
+
+#[test]
+fn post_message_rejects_empty_content() {
+    let f = fixture();
+    let result = f
+        .client()
+        .try_post_message(&f.escrow_id, &f.buyer, &String::from_str(&f.env, ""));
+
+    assert_eq!(result, Err(Ok(ContractError::InvalidAmount)));
+}
+
+#[test]
+fn post_message_rejects_content_over_the_maximum_length() {
+    let f = fixture();
+    let content = String::from_str(&f.env, "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa");
+
+    assert_eq!(
+        f.client()
+            .try_post_message(&f.escrow_id, &f.buyer, &content),
+        Err(Ok(ContractError::InputTooLong))
+    );
+}
+
+#[test]
+fn get_messages_paginates_and_returns_empty_after_the_end() {
+    let f = fixture();
+    for text in ["one", "two", "three"] {
+        f.client()
+            .post_message(&f.escrow_id, &f.buyer, &String::from_str(&f.env, text));
+    }
+
+    let page = f.client().get_messages(&f.escrow_id, &1, &1);
+    assert_eq!(page.len(), 1);
+    assert_eq!(
+        page.get(0).unwrap().content,
+        String::from_str(&f.env, "two")
+    );
+    assert_eq!(f.client().get_messages(&f.escrow_id, &3, &1).len(), 0);
+}
+
+#[test]
+fn posting_to_an_unknown_escrow_is_rejected() {
+    let f = fixture();
+    assert_eq!(
+        f.client()
+            .try_post_message(&999_u64, &f.buyer, &String::from_str(&f.env, "Hello"),),
+        Err(Ok(ContractError::EscrowNotFound))
+    );
+}
+
+#[test]
+fn posting_is_blocked_while_the_contract_is_paused() {
+    let f = fixture();
+    f.client().pause_contract(&f.admin);
+
+    assert_eq!(
+        f.client()
+            .try_post_message(&f.escrow_id, &f.buyer, &String::from_str(&f.env, "Hello"),),
+        Err(Ok(ContractError::ContractPaused))
+    );
+}