[LOW] CampaignTotals::increment accepts negative amount — should reject like storage_increment_total_raised does

[Alqku]

Overview

CampaignTotals::increment (crates/tools/src/campaign_totals.rs line 21) accepts any i128 amount without sign or range validation. Callers like the vault flow can therefore subtract from the running total by passing a negative amount (subtraction by signed add is well-defined in Rust). The accounting model in this struct is supposed to mirror monetary inflows only.

Evidence

// crates/tools/src/campaign_totals.rs
pub fn increment(&mut self, campaign_id: u64, asset: &str, amount: i128) -> i128 {
    let entry = self.asset_totals.entry((campaign_id, asset.to_string())).or_insert(0);
    *entry += amount;          // accepts any i128, including negative
    *entry
}

Compare with storage_increment_total_raised in the smart contract, which rejects negatives with a typed panic:

pub fn storage_increment_total_raised(env: &Env, delta: i128) -> i128 {
    if delta <= 0 {
        panic_with_error!(env, Error::InvalidAmount);
    }
    ...
}

Impact

• A bookkeeping bug in a custom integration script can drive (campaign_id, asset) totals negative without a loud error — downstream get() callers see negative numbers masquerading as totals.
• Dashboards plotting the cumulative sum interpret this as "~(...) raised", confusing for users.
• The struct is a side-channel model used by analytics pipelines; an inconsistent semantic between "amount added" (might be negative) and "amount raised" (always positive) defeats the purpose of having both an on-chain authoritative counter and an off-chain mirror.

Recommended Approach

Match the smart-contract pattern: panic / return Err on non-positive amounts:

pub fn increment(&mut self, campaign_id: u64, asset: &str, amount: i128) -> Result<i128> {
    if amount <= 0 {
        return Err(anyhow!("CampaignTotals::increment requires positive amount; got {}", amount));
    }
    let entry = self.asset_totals.entry((campaign_id, asset.to_string())).or_insert(0);
    *entry = entry.checked_add(amount).ok_or_else(|| anyhow!("overflow in CampaignTotals"))?;
    Ok(*entry)
}

If #[must_use] semantics and the existing call sites need to stay infallible, raise an error variant on the perimeter and propagate via a typed error, but the primary need is loud failure on non-positive amounts.

Acceptance Criteria

• CampaignTotals::increment rejects amount <= 0 with a typed error, not a silent accept
• Saturated additions (amount = i128::MAX - entry) produce a typed error rather than wrapping
• Existing tests adjusted to pass positive amounts and to assert the new error path
• A negative test passes an explicit negative amount and asserts the rejection

Affected Files

• crates/tools/src/campaign_totals.rs
• crates/tools/src/main.rs (call sites — none in current code that I'm aware of, so this is purely API hardening)
• crates/tools/tests/integration_test.rs or crates/tools/src/campaign_totals.rs::mod tests (new negative test)

[NoobFaris]

Hi, I would like to claim this issue. I have experience with Rust smart contracts and data validation. I'll review the CampaignTotals::increment function and implement the necessary validation checks to ensure it rejects negative amounts, matching the behavior of storage_increment_total_raised. I can submit a PR within 2 days

[gloskull]

Hello, I have 4 years of experience delivering solutions for projects like this, covering backend, frontend, and smart contract development. After carefully reviewing your requirements, I have a clear understanding of the task and am confident in my ability to deliver a clean, efficient, and thoroughly tested solution on time. I prioritize code quality, maintainability, and industry best practices throughout the development process.
My PR will be submitted in under 2days.

[cybermax4200]

I would like to work on this issue. I have experience with Rust and implementing robust accounting and validation logic, particularly in systems where off-chain models need to maintain semantic consistency with on-chain state.

For this task, I will update CampaignTotals::increment to reject non-positive amounts with a typed error instead of silently accepting them. I will also replace unchecked arithmetic with checked_add to ensure overflow conditions are reported explicitly rather than wrapping, preserving the integrity of cumulative campaign totals.

I will review and update any affected call sites to handle the new result type appropriately, adjust existing tests to use valid positive amounts, and add dedicated tests for the new error paths. In particular, I will add coverage for negative and zero amounts as well as overflow scenarios to verify that invalid inputs fail loudly and predictably.

My goal is to align the off-chain accounting model with the smart contract semantics so that analytics pipelines and dashboards cannot be silently corrupted by negative contributions or arithmetic overflow.

[grantfox-oss]

🦊 GrantFox — @gloskull has been assigned to this issue as part of the Official Campaign campaign!

Next steps:

1. Open a Pull Request referencing this issue (e.g., Closes #50)
2. Your PR will be reviewed by the OrbitChainLabs maintainers

Good luck! Track your progress on GrantFox.

[grantfox-oss]

🦊 GrantFox — @cybermax4200 has been assigned to this issue as part of the Official Campaign campaign!

Next steps:

1. Open a Pull Request referencing this issue (e.g., Closes #50)
2. Your PR will be reviewed by the OrbitChainLabs maintainers

Good luck! Track your progress on GrantFox.

[GBOYEE]

Hi! I'd like to work on this issue.

I have experience with this stack and can deliver a clean, tested solution. Please assign this to me and I'll submit a PR shortly.

My approach:

• Analyze the root cause
• Implement a fix following existing code patterns
• Add tests and documentation

Thanks!