Skip to content

fix(#442): batch verify_campaigns now returns (verified_ids, failed_ids) - #720

Closed
dedukpe wants to merge 4 commits into
Iris-IV:mainfrom
dedukpe:fix/442-batch-verify-return-ids
Closed

fix(#442): batch verify_campaigns now returns (verified_ids, failed_ids)#720
dedukpe wants to merge 4 commits into
Iris-IV:mainfrom
dedukpe:fix/442-batch-verify-return-ids

Conversation

@dedukpe

@dedukpe dedukpe commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes #442 — the batch admin verify_campaigns function previously captured only first_error and continued processing, then returned Err(first_error) if any campaign failed. This made it impossible for callers to distinguish partial success from total failure.

Changes

Contract (src/lib.rs)

  • Changed return type from Result<u32, Error> to Result<(Vec<u32>, Vec<u32>), Error>
  • Returns (verified_ids, failed_ids) so callers can inspect exactly which campaigns succeeded and which failed
  • Auth/paused checks still return Err and abort the entire batch
  • Updated event campaigns_bulk_verified now emits (verified_count, failed_count, total) instead of (verified_count, total)
  • Fixed event total to report batch_size (capped at 50) instead of the raw campaign_ids.len() for accuracy

Tests

  • test_verify_campaigns_extends_voting_state_ttl — destructures tuple instead of raw count
  • test_verify_campaigns_partial_failure_returns_err → renamed to test_verify_campaigns_partial_failure_returns_ids — now asserts the specific verified and failed IDs
  • test_verify_campaigns_extends_ttl_on_failure — destructures tuple for both first and second calls, asserts failed_ids contains the already-verified campaign

Documentation

  • EVENT_PAYLOADS.md — updated campaigns_bulk_verified payload to (verified_count, failed_count, total)

Breaking Change

This is a contract API breaking change — the return type of verify_campaigns changes from Result<u32, Error> to Result<(Vec<u32>, Vec<u32>), Error>. All clients and indexers that call this function will need to be updated.

Linked Issue

dedukpe added 4 commits July 29, 2026 22:00
… prevent overflow

Replace all unchecked i128 additions in contributions.rs and revenue.rs
with checked_add(...).ok_or(Error::Overflow)? to prevent silent
wrapping/overflow on high-balance tokens.

Fixes:
- contributions.rs: check_contribution_caps cap comparison
- contributions.rs: update_contribution_accounting (5 locations):
  amount_raised, effective_amount_raised, contribution, lifetime, total_raised
- contributions.rs: contribute() personal cap check
- contributions.rs: batch_contribute() personal cap check
- revenue.rs: deposit_revenue pool accumulation
- revenue.rs: claim_revenue claimed + distributed accumulation
- revenue.rs: claim_creator_revenue claimed accumulation

Also changes update_contribution_accounting return type from () to
Result<(), Error> so overflow errors propagate to callers.

Note: verify_with_votes in voting.rs already uses checked_add for
total_votes and total_weight (fixed in prior commit 0876e60).
fix(Iris-IV#445): replace unchecked i128 addition with checked_add to prevent overflow
@davidmaronio

Copy link
Copy Markdown
Contributor

thanks for the work here, the direction is good: returning (verified_ids, failed_ids) from verify_campaigns (src/lib.rs:230) instead of a count plus first-error is a genuinely better API for #442, and updating EVENT_PAYLOADS.md alongside is appreciated.

that said, #606 implements the same failed-ids return for #442, is already mergeable (clean state, passing checks) and was opened earlier, so this one is redundant. a few notes in case anything gets salvaged:

  1. src/lib.rs:246 - ids beyond MAX_BATCH_SIZE are silently dropped from both verified_ids and failed_ids, so a caller passing 60 ids gets 50 accounted for and 10 that vanish. truncated ids should land in failed_ids or the call should error on oversized batches.

  2. src/lib.rs:262 - the event now publishes batch_size as the total instead of campaign_ids.len(), quietly changing event semantics for indexers when a batch is truncated.

  3. the checked_add work in src/contributions.rs and src/revenue.rs overlaps fix(#445): replace unchecked i128/u32 additions with checked_add to prevent overflow #721 and does not belong in a [Bug] verify_campaigns (batch) silently skips failed entries — callers cannot distinguish partial success from total failure #442 PR; CI is red on fmt/clippy/test, likely from the stale base plus formatting of the new event tuple.

since #606 covers #442 and is ready to merge, i would close this one as a duplicate. happy to see the batch-truncation observation filed as a follow-up issue against #606's version.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] verify_campaigns (batch) silently skips failed entries — callers cannot distinguish partial success from total failure

2 participants