feat(events): remove per-event participant caps, add paginated list getters - #104
Conversation
…submissions; implement pagination for applicant and contributor retrieval
Quota reachedYour plan allows 300 CI/CD file units per month. You've used 299 and this scan would add 7 more (total: 306). |
📝 WalkthroughWalkthroughPer-event applicant, contributor, and submission caps are removed in favor of u32 overflow checks. Applicant, winner, and contributor getters now support bounded pagination, with tests covering former-cap and pagination behavior. ChangesParticipant capacity and pagination
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant ContractGetters
participant EventOps
participant EventStorage
Caller->>ContractGetters: get_applicants_page(event_id, start, limit)
ContractGetters->>EventOps: get_applicants_page(event_id, start, limit)
EventOps->>EventStorage: applicants_snapshot(event_id, start, bounded_limit)
EventStorage-->>EventOps: applicant page
EventOps-->>ContractGetters: page result
ContractGetters-->>Caller: Vec<Address>
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
contracts/events/src/storage.rs (1)
678-688: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAdd contributor partition/overflow coverage.
contributors_snapshotandget_contributors_pagenow mirror the applicant/submission pagination behavior, but contributor-specific tests only cover normal top-up/de-dup/cancel paths. Add coverage for the contributor set: duplicate contribution rejoin, overflow aroundu32::MAXreturningTooManyContributors, and out-of-bounds pagination clamping.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@contracts/events/src/storage.rs` around lines 678 - 688, Extend the contributor-specific test coverage around contributors_snapshot and get_contributors_page to verify duplicate contribution rejoin behavior, contributor-count overflow near u32::MAX returns TooManyContributors, and pagination starting beyond the available range clamps safely without returning entries or panicking. Reuse the existing applicant/submission pagination and overflow test patterns, adapting them to the contributor set.
🧹 Nitpick comments (1)
contracts/events/src/errors.rs (1)
53-61: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winReused
TooManyContributorserror code conflates two unrelated overflow conditions. Bothappend_contributor's contributor-counter overflow andappend_submission's submission-counter overflow return the sameError::TooManyContributors, so callers/monitoring cannot distinguish which counter actually overflowed. The enum comment itself notes only 48 of 50contracterrorvariants are used, so a dedicated variant is feasible without hitting the XDR cap.
contracts/events/src/errors.rs#L53-L61: add a distinctTooManySubmissionsvariant (e.g. discriminant 92, since 91 is already the highest used) instead of documentingTooManyContributorsas dual-purpose.contracts/events/src/storage.rs#L463-L481: changeappend_submission'schecked_add(1).ok_or(Error::TooManyContributors)to return the new dedicated variant.♻️ Proposed fix
--- a/contracts/events/src/errors.rs @@ TooManyContributors = 61, + + // Distinct from TooManyContributors: signals append_submission's own + // u32 counter overflow rather than the contributor counter's. + TooManySubmissions = 92,--- a/contracts/events/src/storage.rs @@ - let next = cur.checked_add(1).ok_or(Error::TooManyContributors)?; + let next = cur.checked_add(1).ok_or(Error::TooManySubmissions)?;Note: adding a new variant also requires updating
hackathon_pillar.rs'ssubmit_at_counter_overflow_revertsassertion fromError::TooManyContributorsto the new variant.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@contracts/events/src/errors.rs` around lines 53 - 61, Add a dedicated TooManySubmissions error variant with discriminant 92 in the error enum, keeping TooManyContributors limited to contributor-counter overflow and updating its comments. In contracts/events/src/storage.rs lines 463-481, change append_submission’s checked_add overflow mapping to Error::TooManySubmissions; also update hackathon_pillar.rs’s submit_at_counter_overflow_reverts assertion to expect the new variant.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@contracts/events/src/storage.rs`:
- Around line 678-688: Extend the contributor-specific test coverage around
contributors_snapshot and get_contributors_page to verify duplicate contribution
rejoin behavior, contributor-count overflow near u32::MAX returns
TooManyContributors, and pagination starting beyond the available range clamps
safely without returning entries or panicking. Reuse the existing
applicant/submission pagination and overflow test patterns, adapting them to the
contributor set.
---
Nitpick comments:
In `@contracts/events/src/errors.rs`:
- Around line 53-61: Add a dedicated TooManySubmissions error variant with
discriminant 92 in the error enum, keeping TooManyContributors limited to
contributor-counter overflow and updating its comments. In
contracts/events/src/storage.rs lines 463-481, change append_submission’s
checked_add overflow mapping to Error::TooManySubmissions; also update
hackathon_pillar.rs’s submit_at_counter_overflow_reverts assertion to expect the
new variant.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: a535a1bd-f333-4de5-9736-8525051c9ca5
📒 Files selected for processing (7)
contracts/events/src/bounty.rscontracts/events/src/errors.rscontracts/events/src/event_ops.rscontracts/events/src/lib.rscontracts/events/src/storage.rscontracts/events/src/tests/bounty_pillar.rscontracts/events/src/tests/hackathon_pillar.rs
2 participants
Closes #103
Summary
Removes the 5,000-entry caps on applicants, contributors, and submissions. Participant storage is per-entry (one ledger entry per participant, paid by the participant's own transaction), and no state-changing path iterates the full set in one transaction, so the caps prevented no failure mode. They did, however, let an attacker fill all slots with throwaway wallets and lock real participants out. Events now support unlimited participants.
Changes
capparams fromappend_applicant,append_contributor, andappend_submission; the only remaining guard ischecked_addon the u32 counter (old error codes reused on overflow, none renumbered)MAX_APPLICANTS_PER_EVENT,MAX_CONTRIBUTORS_PER_EVENT,MAX_SUBMISSIONS_PER_EVENTMAX_WINNERS_PER_SELECT(50) andMAX_REFUNDS_PER_BATCH(25) unchanged; these batching limits are what make unbounded sets safeget_applicants/get_contributors/get_winnersto the first 100 entries (VIEW_PAGE_LIMIT) and addget_applicants_page/get_contributors_page/get_winners_page(event_id, start, limit)for reading beyond thatNotes
_pagevariantsSummary by CodeRabbit