Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 7 additions & 6 deletions contracts/events/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,

Expand Down Expand Up @@ -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,
Expand All @@ -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,
}
2 changes: 1 addition & 1 deletion contracts/events/src/event_ops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
10 changes: 6 additions & 4 deletions contracts/events/src/idempotency.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<u64, Error> {
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 {
Expand Down
61 changes: 61 additions & 0 deletions contracts/events/src/tests/op_id_security.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand All @@ -24,6 +25,7 @@ struct Ctx<'a> {
events_id: Address,
profile: ProfileContractClient<'a>,
owner: Address,
fee_account: Address,
applicant: Address,
token_addr: Address,
}
Expand Down Expand Up @@ -68,6 +70,7 @@ fn setup<'a>() -> Ctx<'a> {
events_id,
profile,
owner,
fee_account,
applicant,
token_addr,
}
Expand Down Expand Up @@ -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(&params, &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
Expand Down
2 changes: 1 addition & 1 deletion contracts/profile/src/earnings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
1 change: 1 addition & 0 deletions contracts/profile/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ pub enum Error {
ReasonRequired = 13,

OpAlreadySeen = 20,
EarningsOverflow = 21,
Paused = 30,

UpgradeNotProposed = 40,
Expand Down
15 changes: 10 additions & 5 deletions contracts/profile/src/tests/earnings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -179,19 +179,24 @@ 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));

let u = user(&ctx.env);
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);
}

Expand Down
Loading