docs(primitives): correct hash_payload docstring to match impl (B3)

modules/primitives/Cargo.toml

@@ -10,3 +10,6 @@ doctest = false
 
 [dependencies]
 soroban-sdk = { workspace = true }
+
+[dev-dependencies]
+soroban-sdk = { workspace = true, features = ["testutils"] }

modules/primitives/src/lib.rs

@@ -102,57 +102,33 @@ pub struct AuthPayload {
 
 /// Constructs the payload for processing a bundle of UTXO operations.
 ///
-/// The payload is built by concatenating in order:
-///  - The contract address (32 bytes),
-///  - followed by all `create` conditions,
-///  - followed by all `deposit` conditions,
-///  - followed by all `withdraw` conditions,
-///  - followed by all `integration` conditions.
+/// The payload is built by concatenating, in this fixed order:
+///  1. the `contract` address bytes (the ~56-byte strkey string as passed in),
+///  2. the canonical XDR encoding of the ordered `Vec<Condition>` (`ToXdr`),
+///  3. the `live_until_ledger` (4-byte little-endian `u32`).
+///
+/// The condition list is serialized with `ToXdr` — the same self-delimiting
+/// on-wire representation that `equal_condition_sequence` compares against.
+/// Because XDR length-prefixes vectors and tags every enum variant, the encoding
+/// is **injective**: two distinct condition lists can never hash to the same
+/// bytes. In particular an `ExtDeposit(X, a)` and an `ExtWithdraw(X, a)` over the
+/// same address and amount now hash differently (the prior per-type bucket
+/// concatenation made them byte-identical), and reordering the conditions changes
+/// the hash — the digest binds the exact ordering the signer reviewed. The order
+/// is therefore preserved as-is; conditions are never sorted or canonicalized.
 ///
 /// The resulting byte stream is hashed using SHA-256 to produce a digest that is
 /// used for verifying the signatures of the bundle.
 ///
-/// For consistency, all integer amounts are encoded as little-endian 8-byte sequences.
-/// UTXO identifiers are represented as their raw byte arrays. Also it is suggested to sort
-/// the conditions in the same ordering as they are defined in the original bundle  to ensure
-/// deterministic payloads.
-///
 pub fn hash_payload(e: &Env, auth_payload: &AuthPayload, contract: &Bytes) -> Hash<32> {
     let mut b = Bytes::new(&e);
     b.append(&contract);
 
-    let mut b_create = Bytes::new(&e);
-    let mut b_deposit = Bytes::new(&e);
-    let mut b_withdraw = Bytes::new(&e);
-    let mut b_integrate = Bytes::new(&e);
-
-    for cond in auth_payload.conditions.iter() {
-        match cond {
-            Condition::Create(utxo, amount) => {
-                b_create.append(&Bytes::from_slice(&e, utxo.to_array().as_ref()));
-                b_create.append(&Bytes::from_slice(&e, &amount.to_le_bytes()));
-            }
-            Condition::ExtDeposit(addr, amount) => {
-                b_deposit.append(&addr.to_string().to_bytes());
-                b_deposit.append(&Bytes::from_slice(&e, &amount.to_le_bytes()));
-            }
-            Condition::ExtWithdraw(addr, amount) => {
-                b_withdraw.append(&addr.to_string().to_bytes());
-                b_withdraw.append(&Bytes::from_slice(&e, &amount.to_le_bytes()));
-            }
-            Condition::ExtIntegration(adapter, utxos, amount) => {
-                b_integrate.append(&adapter.to_string().to_bytes());
-                for utxo in utxos.iter() {
-                    b_integrate.append(&Bytes::from_slice(&e, utxo.to_array().as_ref()));
-                }
-                b_integrate.append(&Bytes::from_slice(&e, &amount.to_le_bytes()));
-            }
-        }
-    }
-    b.append(&b_create);
-    b.append(&b_deposit);
-    b.append(&b_withdraw);
-    b.append(&b_integrate);
+    // Canonical, order-sensitive XDR of the condition list. XDR is self-delimiting
+    // (length-prefixed vectors, variant-tagged enums), so distinct condition lists
+    // always produce distinct bytes — closing the deposit/withdraw and reordering
+    // collisions that the previous back-to-back bucket concatenation allowed.
+    b.append(&auth_payload.conditions.clone().to_xdr(e));
 
     b.append(&Bytes::from_slice(
         &e,
@@ -239,3 +215,92 @@ pub fn has_no_conflicting_conditions_in_sets(
     }
     true
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use soroban_sdk::{testutils::Address as _, vec, Address, Env};
+
+    fn contract_bytes(e: &Env) -> Bytes {
+        Address::generate(e).to_string().to_bytes()
+    }
+
+    /// A1: a deposit and a withdraw over the same (address, amount) previously
+    /// concatenated into separate buckets and hashed to byte-identical payloads,
+    /// letting a signed deposit be reinterpreted as a withdraw at tx-build time.
+    /// The XDR encoding tags the variant, so the two now hash differently.
+    #[test]
+    fn deposit_and_withdraw_over_same_address_amount_hash_differently() {
+        let e = Env::default();
+        let contract = contract_bytes(&e);
+        let addr = Address::generate(&e);
+        let amount: i128 = 1_000;
+
+        let deposit = AuthPayload {
+            conditions: vec![&e, Condition::ExtDeposit(addr.clone(), amount)],
+            live_until_ledger: 100,
+        };
+        let withdraw = AuthPayload {
+            conditions: vec![&e, Condition::ExtWithdraw(addr.clone(), amount)],
+            live_until_ledger: 100,
+        };
+
+        assert_ne!(
+            hash_payload(&e, &deposit, &contract).to_bytes(),
+            hash_payload(&e, &withdraw, &contract).to_bytes(),
+            "Dep(X,a) and Wd(X,a) must not share a signed digest"
+        );
+    }
+
+    /// A cross-type reordering the old bucket layout ignored (deposit and withdraw
+    /// land in fixed buckets regardless of input order). XDR preserves list order,
+    /// so the digest now binds exactly the ordering the signer reviewed.
+    #[test]
+    fn reordering_conditions_changes_the_hash() {
+        let e = Env::default();
+        let contract = contract_bytes(&e);
+        let a = Address::generate(&e);
+        let b = Address::generate(&e);
+
+        let dep = Condition::ExtDeposit(a, 10);
+        let wd = Condition::ExtWithdraw(b, 20);
+
+        let forward = AuthPayload {
+            conditions: vec![&e, dep.clone(), wd.clone()],
+            live_until_ledger: 100,
+        };
+        let reversed = AuthPayload {
+            conditions: vec![&e, wd, dep],
+            live_until_ledger: 100,
+        };
+
+        assert_ne!(
+            hash_payload(&e, &forward, &contract).to_bytes(),
+            hash_payload(&e, &reversed, &contract).to_bytes(),
+            "condition ordering must be bound by the digest"
+        );
+    }
+
+    /// The encoding stays deterministic: identical payloads hash identically, so
+    /// the existing signature-verification path is unaffected.
+    #[test]
+    fn identical_payloads_hash_identically() {
+        let e = Env::default();
+        let contract = contract_bytes(&e);
+        let addr = Address::generate(&e);
+
+        let build = || AuthPayload {
+            conditions: vec![
+                &e,
+                Condition::ExtDeposit(addr.clone(), 42),
+                Condition::Create(BytesN::from_array(&e, &[7u8; 65]), 99),
+            ],
+            live_until_ledger: 555,
+        };
+
+        assert_eq!(
+            hash_payload(&e, &build(), &contract).to_bytes(),
+            hash_payload(&e, &build(), &contract).to_bytes(),
+        );
+    }
+}