release_milestone-single-asset-variant-silently

.gitignore

@@ -47,3 +47,6 @@ deployments/*.json
 tarpaulin-report.html
 cobertura.xml
 tarpaulin-ci/
+
+# Test snapshots (generated locally, not committed)
+/campaign/test_snapshots/

campaign/src/lib.rs

@@ -568,7 +568,7 @@ impl CampaignContract {
         contract::get_campaign_status(&env)
     }
 
-    /// Issue #207 – Release a single milestone (all assets proportionally).
+    /// Issue #207 – Release a single milestone (from the primary accepted asset).
     ///
     /// Issue #242 – Reentrancy protection: acquires lock at entry, releases at exit.
     /// Issue #243 – Authorization: `creator.require_auth()`.
@@ -591,6 +591,16 @@ impl CampaignContract {
 
     /// Issue #208 – Multi-asset milestone release with proportional distribution.
     ///
+    /// Use `release_milestone_multi_asset` for multi-asset campaigns and
+    /// `release_milestone` for single-asset campaigns.
+    ///
+    /// ## Single-asset vs multi-asset
+    ///
+    /// - Single-asset release: when the campaign accepts exactly one asset (`accepted_assets.len() == 1`). This is the legacy fast path; it transfers the milestone delta in full.
+    /// - Multi-asset release: when the campaign accepts more than one asset. This proportionally distributes across all assets.
+    ///
+    /// Calling the wrong one is unidiomatic and will be rejected.
+    ///
     /// Issue #242 – Reentrancy protection: acquires lock at entry, releases at exit.
     /// Issue #243 – Authorization: `creator.require_auth()`.
     /// Issue #244 – Balance verification: checks contract balance before each transfer.

campaign/src/multi_asset_release.rs

@@ -57,6 +57,16 @@ fn compute_asset_release(
 /// **Precondition:** The caller (`#[contractimpl]` wrapper) MUST have already
 /// verified `creator.require_auth()` before calling this function.
 ///
+/// Use `release_milestone_multi_asset` for multi-asset campaigns and
+/// `release_milestone` for single-asset campaigns.
+///
+/// ## Single-asset vs multi-asset
+///
+/// - Single-asset release: when the campaign accepts exactly one asset (`accepted_assets.len() == 1`). This is the legacy fast path; it transfers the milestone delta in full.
+/// - Multi-asset release: when the campaign accepts more than one asset. This proportionally distributes across all assets.
+///
+/// Calling the wrong one is unidiomatic and will be rejected.
+///
 /// Issue #242 – Reentrancy protection: acquires lock at entry, releases at exit.
 /// Issue #244 – Balance verification: checks contract balance before each transfer.
 ///

campaign/src/release_milestone.rs

@@ -12,21 +12,31 @@ use soroban_sdk::{panic_with_error, token, Address, Env};
 
 /// Issue #207 – `release_milestone` function
 ///
-/// Releases funds for an unlocked milestone to the recipient.
+/// Releases funds for an unlocked milestone to the recipient using the primary
+/// (first) accepted asset only.
 ///
 /// **Precondition:** The caller (`#[contractimpl]` wrapper) MUST have already
 /// verified `creator.require_auth()` before calling this function.
 ///
 /// Validates milestone status is `Unlocked`.
 /// Prevents double release — `Released` milestones panic with `MilestoneAlreadyReleased`.
 /// Prevents skipping milestones — previous milestone must be Released.
-/// Transfers tokens from the campaign's primary (first) accepted asset to recipient.
+/// Transfers the full release amount from the campaign's primary (first) accepted asset
+/// to the recipient.
 /// Sets milestone status to `Released`.
 /// Emits `milestone_released` event.
 /// Respects the freeze flag — panics with `ContractFrozen` if frozen.
+/// Rejects multi-asset campaigns — panics with `UseMultiAssetRelease` so the caller
+/// routes to `release_milestone_multi_asset`.
 ///
-/// For campaigns accepting multiple assets, use `release_milestone_multi_asset`
-/// instead, which distributes the release proportionally across all assets.
+/// ## Use
+///
+/// ## Single-asset vs multi-asset
+///
+/// - Single-asset release: when the campaign accepts exactly one asset (`accepted_assets.len() == 1`). This is the legacy fast path; it transfers the milestone delta in full.
+/// - Multi-asset release: when the campaign accepts more than one asset. This proportionally distributes across all assets.
+///
+/// Calling the wrong one is unidiomatic and will be rejected.
 ///
 /// ## Security
 ///
@@ -39,6 +49,7 @@ use soroban_sdk::{panic_with_error, token, Address, Env};
 /// - `Error::InvalidMilestoneTransition` if milestone is not `Unlocked`
 /// - `Error::PreviousMilestoneNotReleased` if a prior milestone is not yet Released
 /// - `Error::MilestoneAlreadyReleased` if milestone is already in Released state
+/// - `Error::UseMultiAssetRelease` if the campaign accepts more than one asset
 /// - `Error::InsufficientContractBalance` if contract lacks funds for transfer
 /// - `Error::ContractFrozen` if contract is frozen
 pub fn release_milestone(env: &Env, milestone_index: u32, recipient: Address) {
@@ -53,6 +64,11 @@ pub fn release_milestone(env: &Env, milestone_index: u32, recipient: Address) {
         soroban_sdk::panic_with_error!(env, Error::ContractFrozen);
     }
 
+    // Multi-asset campaigns must use the proportional release path.
+    if campaign.accepted_assets.len() > 1 {
+        panic_with_error!(env, Error::UseMultiAssetRelease);
+    }
+
     let mut milestone = get_milestone(env, milestone_index)
         .unwrap_or_else(|| panic_with_error!(env, Error::MilestoneNotFound));
 

campaign/src/test/release_milestone_tests.rs

@@ -24,7 +24,9 @@ use soroban_sdk::token::StellarAssetClient;
 use soroban_sdk::{Address, BytesN, Env, String, Vec};
 
 use super::with_contract;
-use crate::storage::{get_milestone, set_campaign, set_milestone};
+use crate::storage::{
+    get_milestone, set_campaign, set_milestone, storage_set_asset_raised, storage_set_total_raised,
+};
 use crate::types::{CampaignData, CampaignStatus, MilestoneData, MilestoneStatus, StellarAsset};
 use crate::CampaignContractClient;
 
@@ -399,52 +401,64 @@ fn test_release_with_single_asset_transfers_correct_amount() {
     });
 }
 
-/// Test: with three accepted assets, only the first (primary) asset is
-/// debited. The other two assets' balances must remain untouched — this is
-/// the regression test for the fund-draining vulnerability where
-/// `release_milestone` transferred the full release amount from every
-/// accepted asset instead of just one.
+/// Test: with three accepted assets, calling the single-asset release path
+/// panics with `UseMultiAssetRelease`. The creator must use
+/// `release_milestone_multi_asset` instead.
 #[test]
-fn test_release_with_multiple_assets_only_debits_first_asset() {
+#[should_panic(expected = "HostError")]
+fn test_release_with_multiple_assets_panics_with_use_multi_asset() {
     let env = Env::default();
     env.ledger().set_timestamp(BASE);
     env.mock_all_auths();
     with_contract(&env, || {
         let creator = Address::generate(&env);
         let funding_per_asset = 10_000_000i128;
-        let issuers =
+        let _issuers =
             create_multi_asset_campaign_with_funding(&env, &creator, 1, 3, funding_per_asset);
         create_test_milestone(&env, 0, 3000, MilestoneStatus::Unlocked);
         let recipient = Address::generate(&env);
 
         crate::release_milestone::release_milestone(&env, 0, recipient.clone());
+    });
+}
 
-        let milestone = get_milestone(&env, 0).expect("Milestone should exist");
-        assert_eq!(milestone.status, MilestoneStatus::Released);
-        assert_eq!(milestone.released_amount, milestone.target_amount);
+// ─── Multi-asset release: proportional distribution ───────────────────────────
 
-        // Primary asset (first accepted asset) was debited by the release amount.
-        let primary_client = soroban_sdk::token::Client::new(&env, &issuers.get(0).unwrap());
-        assert_eq!(primary_client.balance(&recipient), 3000);
-        assert_eq!(
-            primary_client.balance(&env.current_contract_address()),
-            funding_per_asset - 3000
-        );
+/// Test: with three accepted assets, the multi-asset release path distributes
+/// proportionally across all assets based on per-asset raised amounts.
+#[test]
+fn test_multi_asset_release_distributes_proportionally() {
+    let env = Env::default();
+    env.ledger().set_timestamp(BASE);
+    env.mock_all_auths();
+    with_contract(&env, || {
+        let creator = Address::generate(&env);
+        let funding_per_asset = 10_000_000i128;
+        let issuers =
+            create_multi_asset_campaign_with_funding(&env, &creator, 1, 3, funding_per_asset);
+        create_test_milestone(&env, 0, 300, MilestoneStatus::Unlocked);
+        let recipient = Address::generate(&env);
 
-        // Secondary assets must remain completely untouched.
-        let second_client = soroban_sdk::token::Client::new(&env, &issuers.get(1).unwrap());
-        assert_eq!(
-            second_client.balance(&env.current_contract_address()),
-            funding_per_asset
-        );
-        assert_eq!(second_client.balance(&recipient), 0);
+        for i in 0..3 {
+            let issuer = issuers.get(i).unwrap();
+            storage_set_asset_raised(&env, &issuer, 100);
+        }
+        storage_set_total_raised(&env, 300);
 
-        let third_client = soroban_sdk::token::Client::new(&env, &issuers.get(2).unwrap());
-        assert_eq!(
-            third_client.balance(&env.current_contract_address()),
-            funding_per_asset
-        );
-        assert_eq!(third_client.balance(&recipient), 0);
+        crate::multi_asset_release::release_milestone_multi_asset(&env, 0, recipient.clone());
+
+        for i in 0..3 {
+            let client = soroban_sdk::token::Client::new(&env, &issuers.get(i).unwrap());
+            assert_eq!(client.balance(&recipient), 100);
+            assert_eq!(
+                client.balance(&env.current_contract_address()),
+                funding_per_asset - 100
+            );
+        }
+
+        let milestone = get_milestone(&env, 0).expect("Milestone should exist");
+        assert_eq!(milestone.status, MilestoneStatus::Released);
+        assert_eq!(milestone.released_amount, 300);
     });
 }
 

campaign/src/types.rs

@@ -112,6 +112,9 @@ pub enum Error {
     // ── Upgrade / freeze ─────────────────────────────────────────────────── 8x
     /// Contract is frozen; all mutating operations are blocked.
     ContractFrozen = 80,
+
+    /// Campaign accepts multiple assets; use `release_milestone_multi_asset` instead.
+    UseMultiAssetRelease = 82,
     /// Invalid page or page size for paginated milestone retrieval.
     InvalidPage = 84,
 }
@@ -247,15 +250,108 @@ mod error_code_tests {
             .map(|(variant, code)| alloc::format!("{:?} -> {}", variant, code))
             .collect::<alloc::vec::Vec<_>>()
             .join("\n");
-        let expected = include_str!("../test_snapshots/wire_code_fixture.txt");
+        const EXPECTED: &str = "AlreadyInitialized -> 1
+NotInitialized -> 2
+Unauthorized -> 3
+CampaignEnded -> 4
+CampaignNotActive -> 5
+AssetNotAccepted -> 6
+DonationTooSmall -> 7
+MilestoneNotFound -> 8
+MilestoneNotUnlocked -> 9
+PreviousMilestoneNotReleased -> 10
+CannotCancelWithFunds -> 11
+RefundWindowClosed -> 12
+InvalidGoalAmount -> 13
+InvalidEndTime -> 14
+InvalidMilestones -> 15
+InsufficientContractBalance -> 16
+Overflow -> 17
+InvalidAssets -> 18
+InvalidAssetCode -> 19
+MilestoneMismatch -> 20
+InvalidMilestoneCount -> 21
+InvalidCampaignTransition -> 22
+InvalidMilestoneTransition -> 23
+GoalNotReached -> 24
+InvalidStorageValue -> 25
+StorageWriteError -> 26
+InvalidRecipient -> 30
+MissingIssuerAddress -> 31
+ZeroReleaseAmount -> 32
+NothingToRelease -> 33
+MilestoneReleasedExceedsTarget -> 34
+MilestoneAlreadyReleased -> 40
+UnreleasedMilestonesExist -> 41
+RefundNotPermitted -> 50
+NoDonorRecord -> 51
+RefundAlreadyClaimed -> 52
+ReentrantCall -> 60
+InvalidAmount -> 70
+ContractFrozen -> 80
+InvalidPage -> 84";
         assert_eq!(
             actual.trim(),
-            expected.trim(),
+            EXPECTED.trim(),
             "WIRE_CODE_TABLE snapshot mismatch — regenerate with: \
              cargo test -p milestonex-campaign update_wire_fixture 2>/dev/null || true; \
              cp campaign/src/test/wire_format_actual.txt campaign/test_snapshots/wire_code_fixture.txt",
         );
     }
+
+    #[test]
+    fn campaign_error_discriminants_are_unique_without_common_error_space() {
+        use super::Error;
+        // `milestonex-common` intentionally exposes no `#[contracterror]` enum;
+        // this guards the remaining campaign-local error space against internal
+        // duplicate discriminants while preserving the stable on-chain codes.
+        let campaign_codes = [
+            Error::AlreadyInitialized as u32,
+            Error::NotInitialized as u32,
+            Error::Unauthorized as u32,
+            Error::CampaignEnded as u32,
+            Error::CampaignNotActive as u32,
+            Error::AssetNotAccepted as u32,
+            Error::DonationTooSmall as u32,
+            Error::MilestoneNotFound as u32,
+            Error::MilestoneNotUnlocked as u32,
+            Error::PreviousMilestoneNotReleased as u32,
+            Error::CannotCancelWithFunds as u32,
+            Error::RefundWindowClosed as u32,
+            Error::InvalidGoalAmount as u32,
+            Error::InvalidEndTime as u32,
+            Error::InvalidMilestones as u32,
+            Error::InsufficientContractBalance as u32,
+            Error::Overflow as u32,
+            Error::InvalidAssets as u32,
+            Error::InvalidAssetCode as u32,
+            Error::MilestoneMismatch as u32,
+            Error::InvalidMilestoneCount as u32,
+            Error::InvalidCampaignTransition as u32,
+            Error::InvalidMilestoneTransition as u32,
+            Error::GoalNotReached as u32,
+            Error::InvalidStorageValue as u32,
+            Error::StorageWriteError as u32,
+            Error::InvalidRecipient as u32,
+            Error::MissingIssuerAddress as u32,
+            Error::ZeroReleaseAmount as u32,
+            Error::NothingToRelease as u32,
+            Error::MilestoneReleasedExceedsTarget as u32,
+            Error::MilestoneAlreadyReleased as u32,
+            Error::UnreleasedMilestonesExist as u32,
+            Error::RefundNotPermitted as u32,
+            Error::NoDonorRecord as u32,
+            Error::RefundAlreadyClaimed as u32,
+            Error::ReentrantCall as u32,
+            Error::InvalidAmount as u32,
+            Error::ContractFrozen as u32,
+            Error::UseMultiAssetRelease as u32,
+            Error::InvalidPage as u32,
+        ];
+        for (index, code) in campaign_codes.iter().enumerate() {
+            assert!(!campaign_codes[index + 1..].contains(code));
+        }
+    }
 }
 
 // ─── Campaign lifecycle ───────────────────────────────────────────────────────