escrow-proxy/examples/escrow-swap-demo.rs (1)

235-236: Clarify NEP-141 vs NEP-245 token ID intent.
The constants are labeled NEP-245 but parsed as Nep141TokenId. If these are NEP-245 tokens, consider using Nep245TokenId; otherwise rename the constants to avoid confusion.

tests/src/tests/escrow/partial_fills.rs (1)

275-280: Consider moving imports to the top of the file.

Placing use statements inside the function body is valid Rust, but unconventional. Moving these imports to the file-level (alongside lines 1-27) improves readability and consistency with the existing test pattern in this file.

tests/src/tests/escrow_with_proxy/swap_with_fees.rs (1)

192-209: Assertions are weak for a comprehensive fee test.

The assertions only verify balances are > 0, but for a test exercising protocol fees (1%), surplus fees (50%), and integrator fees (2%), verifying exact expected amounts after fee deductions would increase confidence in the fee calculation logic. Consider computing and asserting the expected amounts:

• Proxy should receive: swap_amount - (protocol_fee + surplus_fee + integrator_fee)
• Maker should receive: solver_amount (or amount after any taker-side fees)

💡 Example of stronger assertions

// Calculate expected amounts after fees
let protocol_fee = swap_amount * 1 / 100;  // 1%
let integrator_fee = swap_amount * 2 / 100;  // 2%
// surplus calculation depends on price difference logic
let expected_proxy_src = swap_amount - protocol_fee - integrator_fee - surplus_portion;

assert_eq!(
    env.defuse.mt_balance_of(proxy.id(), &token_a_defuse_id.to_string()).await.unwrap(),
    expected_proxy_src,
    "Proxy should receive expected src after fees"
);

escrow-proxy/src/state.rs (1)

3-10: Consider adding documentation for the public config struct.

ProxyConfig is publicly exported and used across multiple crates (tests, examples). Adding doc comments would clarify the purpose of each field, especially notifier vs auth_contract which may not be immediately obvious to users.

📝 Suggested documentation

+/// Configuration for the escrow proxy contract.
 #[near(serializers = [borsh, json])]
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub struct ProxyConfig {
+    /// Account authorized to perform administrative operations.
     pub owner_id: AccountId,
+    /// Global contract ID for oneshot-condvar instances.
     pub oneshot_condvar_global_id: GlobalContractId,
+    /// Contract that verifies authorization (e.g., defuse).
     pub auth_contract: AccountId,
+    /// Account that triggers condvar notifications.
     pub notifier: AccountId,
 }

sandbox/src/lib.rs (1)

122-136: Consider documenting the unwrap invariant on line 136.

The .unwrap() on line 136 assumes the Option<Sandbox> inside the mutex is always Some. This invariant holds because:

1. The sandbox is only initialized once (via get_or_init)
2. The only place that calls take() is cleanup_sandbox, which runs at program exit

While this is safe in practice, a brief comment would clarify the invariant for future maintainers.

📝 Suggested documentation

     let (sandbox_arc, root_account) = mutex
         .lock()
         .unwrap()
         .as_ref()
+        // SAFETY: The Option is always Some until atexit cleanup runs
         .map(|shared| (shared.sandbox.clone(), shared.root.clone()))
         .unwrap();

tests/src/tests/escrow/mod.rs (1)

1-1: Consider narrowing the allow scope to specific modules or items.

#![allow(async_fn_in_trait, dead_code)] at the module root hides warnings across all escrow tests; scoping it to the specific modules or items that need it keeps signal high.

tests/Cargo.toml (1)

26-35: New dependencies align with PR objectives.

The additions for defuse-escrow-proxy, defuse-oneshot-condvar, and supporting crates (defuse-core, defuse-nep245, defuse-token-id) are appropriate for testing the new contracts introduced in this PR. The near-sdk feature additions for unit-testing and deterministic-account-ids are suitable for sandbox testing.

One observation: defuse-escrow-swap is gated behind the escrow feature (line 17), but defuse-escrow-proxy and defuse-oneshot-condvar are unconditionally included. If these crates are only used when testing escrow functionality, consider making them optional as well for consistency and to reduce compile times when the escrow feature is disabled.

♻️ Optional: Gate escrow-related dependencies behind the `escrow` feature

-escrow = ["dep:defuse-escrow-swap"]
+escrow = ["dep:defuse-escrow-swap", "dep:defuse-escrow-proxy", "dep:defuse-oneshot-condvar"]

-defuse-escrow-proxy = { workspace = true, features = ["escrow-swap"] }
+defuse-escrow-proxy = { workspace = true, features = ["escrow-swap"], optional = true }

-defuse-oneshot-condvar.workspace = true
+defuse-oneshot-condvar = { workspace = true, optional = true }

tests/src/tests/escrow_with_proxy/swap_with_fees.rs (3)

128-142: Consider naming the salt constant for clarity.

The magic number [2u8; 32] on line 130 is used as a salt. For test readability and maintainability, consider extracting this to a named constant with a brief comment explaining its purpose.

♻️ Suggested refactor

+    // Arbitrary salt for proxy message - distinct from other test salts
+    const PROXY_SALT: [u8; 32] = [2u8; 32];
+
     let proxy_msg = ProxyTransferMessage {
         receiver_id: escrow_instance_id.clone(),
-        salt: [2u8; 32],
+        salt: PROXY_SALT,
         msg: serde_json::to_string(&fill_escrow_msg).unwrap(),
     };

---

192-210: Strengthen assertions to verify fee distribution and expected amounts.

The current assertions only check > 0 balances for proxy and maker, but don't verify:

• Fee collector received protocol and surplus fees
• Integrator received their 2% fee
• Expected amounts after fee deductions are correct

For a worst-case fee scenario test, verifying the fee distribution would confirm the full flow executed as intended and catch potential fee calculation bugs.

♻️ Suggested additional assertions

     // Verify maker received dst tokens (minus fees)
     assert!(
         env.defuse
             .mt_balance_of(maker.id(), &token_b_defuse_id.to_string())
             .await
             .unwrap()
             > 0,
         "Maker should have received token-b"
     );
+
+    // Verify fee collector received fees (protocol + surplus)
+    assert!(
+        env.defuse
+            .mt_balance_of(fee_collector.id(), &token_b_defuse_id.to_string())
+            .await
+            .unwrap()
+            > 0,
+        "Fee collector should have received protocol and surplus fees"
+    );
+
+    // Verify integrator received their fee
+    assert!(
+        env.defuse
+            .mt_balance_of(integrator.id(), &token_b_defuse_id.to_string())
+            .await
+            .unwrap()
+            > 0,
+        "Integrator should have received integrator fee"
+    );

---

188-190: Consider adding a gas upper bound assertion to detect regressions.

Currently, the test only prints the gas consumption without asserting any bounds. For a benchmark test, adding an upper bound assertion would help catch gas regressions in CI.

♻️ Suggested addition

     let total_gas = Gas::from_gas(result.total_gas_burnt.as_gas());

     eprintln!("Proxy fill total gas consumed (worst case with fees): {total_gas:?}");
+
+    // Assert gas is within expected bounds to detect regressions
+    // Adjust this threshold based on baseline measurements
+    const MAX_EXPECTED_GAS: Gas = Gas::from_tgas(300);
+    assert!(
+        total_gas <= MAX_EXPECTED_GAS,
+        "Gas consumption {total_gas:?} exceeds expected maximum {MAX_EXPECTED_GAS:?}"
+    );

tests/src/env/mod.rs (1)

157-182: Different storage deposit strategy between FT and MT methods is intentional but worth documenting.

The FT method does storage deposits for all user accounts, while this MT method only deposits for defuse and root. This is correct since MT tokens flow through the defuse contract, but a brief doc comment explaining the difference would help future maintainers.

+    /// Creates an MT token and deposits specified balances to each user via defuse.
+    ///
+    /// Unlike `create_ft_token_with_initial_balances`, this only requires storage deposits
+    /// for defuse and root since MT tokens are deposited through the defuse contract.
     pub async fn create_mt_token_with_initial_balances(