Skip to content

release_milestone` (single-asset variant) silently under-delivers for multi-asset campaigns #7

Description

@P3az3

Problem Statement

CampaignContract::release_milestone(env, index, recipient) (campaign/src/release_milestone.rs) only transfers from campaign.accepted_assets.first(). If a campaign is initialized with three accepted assets (USDC, XLM, NGNT), and the creator calls the single-asset release path on milestone 0, every donor whose contribution sat in USDC or NGNT is effectively diluted. The milestone's released_amount is fully credited to the released milestone, but only a single asset's worth of tokens leaves the contract.

Why it matters

Two precedents in the codebase imply this is unintentional: (a) the dedicated release_milestone_multi_asset function exists precisely to handle multi-asset distribution, and (b) the comment in release_milestone says "calls release_milestone_multi_asset instead" — yet that comment is in the doc-comment, not enforced. A creator can call release_milestone blindly and silently syphon funds out of multi-asset contributors.

Technical Context

The two release functions diverge in actual behavior, and the divergence is invisible at the call site. There is no typed error when an unsuitable single-asset release is attempted against a multi-asset campaign. Tests cover the single-asset happy path; nothing captures the divergence.

Expected Outcome

If campaign.accepted_assets.len() > 1, release_milestone should either:

  • Refuse with a new typed error Error::UseMultiAssetRelease = 82, or
  • Forward to release_milestone_multi_asset after a creator-side deposit confirmation.

The author's preferred solution (per issue text) is refuse-and-route — the off-chain UI should be educated to choose the right function.

Acceptance Criteria

  • New typed error returned by release_milestone when accepted_assets.len() > 1.
  • New tests cover:
    • Single-asset campaign, single-asset release succeeds.
    • Multi-asset campaign, single-asset release panics with new error.
    • Multi-asset campaign, multi-asset release succeeds and proportionally transfers (existing tests must still pass).
  • Doc comments on both release functions are updated to call out the choice rule.
  • docs/events.md clarifies that the same milestone_released event is emitted in both paths.

Implementation Notes

  • The check should be at the top of release_milestone::release_milestone, ideally right after the reentrancy lock and freeze gate but before any campaign-load reading.
  • Add a doc-comment mat on both functions: "Use \n\n## Single-asset vs multi-asset\n\n- 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.\n- Multi-asset release: when the campaign accepts more than one asset. This proportionally distributes across all assets.\n\nCalling the wrong one is unidiomatic and will be rejected."

Affected Files / Modules

  • campaign/src/release_milestone.rs
  • campaign/src/types.rs (new error variant)
  • campaign/src/test/release_milestone_tests.rs (new tests)
  • campaign/src/test/negative_path_tests.rs

Dependencies — None.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

GrantFox OSSIssue tracked in GrantFox OSSMaybe RewardedIssue may be eligible for a GrantFox rewardOfficial Campaign | FWC26Campaign: Official Campaign | FWC26bugSomething isn't workingfund-routingToken-routing or transfer-flow risk.priority/p0Highest priority. Drop everything else.securitySecurity-impact change.smart-contractSoroban/Stellar contract logic.

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions