diff --git a/contracts/events/src/errors.rs b/contracts/events/src/errors.rs index fd8959c..40eb478 100644 --- a/contracts/events/src/errors.rs +++ b/contracts/events/src/errors.rs @@ -15,8 +15,8 @@ pub enum Error { NotAdmin = 11, // Shared by both two-step rotations (admin and event manager): no pending // proposal / target mismatch (12) and pending proposal expired (13). The - // enum is at the 50-case XDR cap, so the manager flow reuses these rather - // than adding variants. + // manager flow reuses these because the two flows are structurally + // identical, not to duplicate a variant per flow. PendingRotationMismatch = 12, PendingRotationExpired = 13, @@ -54,9 +54,8 @@ pub enum Error { OpAlreadySeen = 60, - // Also returned by append_submission's cap check — the enum is at - // the 50-case XDR cap, so the hackathon submission cap reuses this - // rather than adding a variant. + // Also returned by append_submission's cap check: the hackathon submission + // cap reuses this rather than adding a near-duplicate "TooManySubmissions". TooManyContributors = 61, CancellationNotStarted = 62, @@ -70,9 +69,11 @@ pub enum Error { MigrationAlreadyApplied = 69, Paused = 70, + EventIdOverflow = 71, ProfileCallFailed = 80, - // Enum is at the 50-case XDR cap; consolidate before adding another. + // contracterror caps at 50 cases (48 used). Discriminants are not dense — + // 91 is a numeric label, not the case count. PrizeAlreadyClaimed = 91, } diff --git a/contracts/events/src/event_ops.rs b/contracts/events/src/event_ops.rs index 814cf08..8e002cb 100644 --- a/contracts/events/src/event_ops.rs +++ b/contracts/events/src/event_ops.rs @@ -128,7 +128,7 @@ pub fn create_event(env: &Env, params: CreateEventParams, op_id: BytesN<32>) -> ); } - let id = idempotency::next_event_id(env); + let id = idempotency::next_event_id(env)?; let record = EventRecord { id, ..provisional }; storage::set_event(env, id, &record); storage::set_non_owner_contribution_total(env, id, 0); diff --git a/contracts/events/src/idempotency.rs b/contracts/events/src/idempotency.rs index 65187b7..c052cc5 100644 --- a/contracts/events/src/idempotency.rs +++ b/contracts/events/src/idempotency.rs @@ -25,11 +25,13 @@ pub fn id_base(env: &Env) -> u64 { (seq as u64) << 32 } -pub fn next_event_id(env: &Env) -> u64 { +pub fn next_event_id(env: &Env) -> Result { let base = id_base(env); - let id = storage::get_next_event_id(env, base.saturating_add(1)); - storage::set_next_event_id(env, id.saturating_add(1)); - id + let fallback = base.checked_add(1).ok_or(Error::EventIdOverflow)?; + let id = storage::get_next_event_id(env, fallback); + let next = id.checked_add(1).ok_or(Error::EventIdOverflow)?; + storage::set_next_event_id(env, next); + Ok(id) } pub mod tag { diff --git a/contracts/events/src/tests/op_id_security.rs b/contracts/events/src/tests/op_id_security.rs index 8c7931b..87fe2be 100644 --- a/contracts/events/src/tests/op_id_security.rs +++ b/contracts/events/src/tests/op_id_security.rs @@ -10,6 +10,7 @@ use soroban_sdk::{ }; use crate::idempotency::{self, tag}; +use crate::storage; use crate::types::{CreateEventParams, Pillar, ReleaseKind, WinnerSpec}; use crate::{EventsContract, EventsContractClient}; @@ -24,6 +25,7 @@ struct Ctx<'a> { events_id: Address, profile: ProfileContractClient<'a>, owner: Address, + fee_account: Address, applicant: Address, token_addr: Address, } @@ -68,6 +70,7 @@ fn setup<'a>() -> Ctx<'a> { events_id, profile, owner, + fee_account, applicant, token_addr, } @@ -209,6 +212,64 @@ fn events_domain_child_op_id_replay_still_rejected() { ); } +/// Calling create_event when the stored next_event_id is at u64::MAX must revert +/// with EventIdOverflow rather than silently returning the same id forever. +#[test] +fn event_id_overflow_reverts() { + let ctx = setup(); + let env = &ctx.env; + + let token = token::Client::new(env, &ctx.token_addr); + let owner_balance_before = token.balance(&ctx.owner); + let fee_balance_before = token.balance(&ctx.fee_account); + + env.as_contract(&ctx.events_id, || { + storage::set_next_event_id(env, u64::MAX); + }); + + let params = CreateEventParams { + pillar: Pillar::Bounty, + owner: ctx.owner.clone(), + token: ctx.token_addr.clone(), + total_budget: TOTAL_BUDGET, + release_kind: ReleaseKind::Single, + content_uri: String::from_str(env, "https://api.boundless.fi/events/overflow"), + title: String::from_str(env, "Overflow"), + deadline: Some(env.ledger().timestamp() + 86_400), + winner_distribution: dist_100(env), + fee_bps_override: None, + manager: None, + }; + + let err = ctx + .events + .try_create_event(¶ms, &BytesN::random(env)) + .err() + .expect("event creation should fail when next_event_id overflows") + .unwrap(); + assert_eq!(err, crate::errors::Error::EventIdOverflow); + + // Verify transaction rollback: no funds moved, no event persisted. + assert_eq!( + token.balance(&ctx.owner), + owner_balance_before, + "owner balance unchanged after failed create_event" + ); + assert_eq!( + token.balance(&ctx.fee_account), + fee_balance_before, + "fee account balance unchanged after failed create_event" + ); + env.as_contract(&ctx.events_id, || { + let next_id = storage::get_next_event_id(env, 0); + assert_eq!( + next_id, + u64::MAX, + "next_event_id unchanged after failed create_event" + ); + }); +} + /// Events-side OpSeen is namespaced by the authorizing caller: a permissionless /// entrypoint (apply) cannot pre-mark an op_id and block a privileged one /// (select_winners) that reuses it. Before namespacing, the shared global diff --git a/contracts/profile/src/earnings.rs b/contracts/profile/src/earnings.rs index 04d59f3..c05c605 100644 --- a/contracts/profile/src/earnings.rs +++ b/contracts/profile/src/earnings.rs @@ -23,7 +23,7 @@ pub fn register( } let current = storage::get_earnings(env, &user, &token); - let new = current.saturating_add(amount); + let new = current.checked_add(amount).ok_or(Error::EarningsOverflow)?; storage::set_earnings(env, &user, &token, new); evt::EarningsRegistered { diff --git a/contracts/profile/src/errors.rs b/contracts/profile/src/errors.rs index 12884d2..662b378 100644 --- a/contracts/profile/src/errors.rs +++ b/contracts/profile/src/errors.rs @@ -22,6 +22,7 @@ pub enum Error { ReasonRequired = 13, OpAlreadySeen = 20, + EarningsOverflow = 21, Paused = 30, UpgradeNotProposed = 40, diff --git a/contracts/profile/src/tests/earnings.rs b/contracts/profile/src/tests/earnings.rs index b413729..4ee4c25 100644 --- a/contracts/profile/src/tests/earnings.rs +++ b/contracts/profile/src/tests/earnings.rs @@ -179,7 +179,7 @@ fn register_earnings_rejects_duplicate_op_id() { } #[test] -fn register_earnings_saturating_add() { +fn register_earnings_overflow_reverts() { let ctx = setup(); ctx.client.set_events_contract(&events_addr(&ctx.env)); @@ -187,11 +187,16 @@ fn register_earnings_saturating_add() { let t = token(&ctx.env); ctx.client - .register_earnings(&u, &t, &(i128::MAX - 1), &BytesN::random(&ctx.env)); - assert_eq!(ctx.client.get_earnings(&u, &t), i128::MAX - 1); + .register_earnings(&u, &t, &i128::MAX, &BytesN::random(&ctx.env)); + assert_eq!(ctx.client.get_earnings(&u, &t), i128::MAX); - ctx.client - .register_earnings(&u, &t, &100_i128, &BytesN::random(&ctx.env)); + let err = ctx + .client + .try_register_earnings(&u, &t, &1_i128, &BytesN::random(&ctx.env)) + .err() + .expect("overflow should revert") + .unwrap(); + assert_eq!(err, Error::EarningsOverflow); assert_eq!(ctx.client.get_earnings(&u, &t), i128::MAX); }