Skip to content

Commit 246d307

Browse files
fix(events): require acceptance for manager delegation (#70) (#88)
Manager delegation previously granted privileged authority (select_winners, cancel, manager rotation) to an address that never authorized. A typo, wrong-network address, or a swapped params.manager at signing could lock the owner out permanently, or hand escrow-draining select_winners authority to an attacker with no further owner sign-off. Replace the immediate assignment with a two-step propose/accept flow mirroring the existing admin rotation: - propose_manager: current authority (manager, else owner) records a pending proposal with a short expiry; no authority transfers. - accept_manager: the proposed address must require_auth to accept before the role transfers. - cancel_pending_manager: current authority vetoes a pending proposal. - create_event now only proposes params.manager; the owner stays in control until acceptance. Emit ManagerProposed and ManagerChanged (the latter also closes the missing set_manager storage-change event). Remove set_manager. Storage layout extended append-only (DataKey::PendingManager, PendingManager). Co-authored-by: Collins Ikechukwu <collinschristroa@gmail.com>
1 parent 46b3e1c commit 246d307

10 files changed

Lines changed: 277 additions & 45 deletions

File tree

contracts/events/src/admin.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -79,12 +79,12 @@ pub fn set_admin(env: &Env, new_admin: Address) -> Result<(), Error> {
7979
}
8080

8181
pub fn accept_admin(env: &Env) -> Result<(), Error> {
82-
let pending = storage::get_pending_admin(env).ok_or(Error::PendingAdminMismatch)?;
82+
let pending = storage::get_pending_admin(env).ok_or(Error::PendingRotationMismatch)?;
8383

8484
if env.ledger().sequence() > pending.expires_at_ledger {
8585
storage::clear_pending_admin(env);
8686
storage::touch_instance(env);
87-
return Err(Error::PendingAdminExpired);
87+
return Err(Error::PendingRotationExpired);
8888
}
8989

9090
pending.target.require_auth();

contracts/events/src/errors.rs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,12 @@ pub enum Error {
1313

1414
Unauthorized = 10,
1515
NotAdmin = 11,
16-
PendingAdminMismatch = 12,
17-
PendingAdminExpired = 13,
16+
// Shared by both two-step rotations (admin and event manager): no pending
17+
// proposal / target mismatch (12) and pending proposal expired (13). The
18+
// enum is at the 50-case XDR cap, so the manager flow reuses these rather
19+
// than adding variants.
20+
PendingRotationMismatch = 12,
21+
PendingRotationExpired = 13,
1822

1923
TokenNotSupported = 20,
2024
FeeAccountMissingTrustline = 21,

contracts/events/src/event_ops.rs

Lines changed: 87 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -13,14 +13,16 @@ use crate::profile_client;
1313
use crate::storage;
1414
use crate::token_whitelist;
1515
use crate::types::{
16-
CancellationBranch, CancellationState, CreateEventParams, EventRecord, EventStatus, Pillar,
17-
PrizeAward, ReleaseKind, Submission, Winner, WinnerSpec,
16+
CancellationBranch, CancellationState, CreateEventParams, EventRecord, EventStatus,
17+
PendingManager, Pillar, PrizeAward, ReleaseKind, Submission, Winner, WinnerSpec,
1818
};
1919

2020
const MAX_TITLE_LEN: u32 = 120;
2121

2222
const MAX_WINNERS_PER_SELECT: u32 = 50;
2323

24+
const PENDING_MANAGER_TTL_LEDGERS: u32 = 17_280;
25+
2426
// Anchored at selection time, not the event deadline (which usually passes
2527
// before winners are selected). A per-event override needs a migration.
2628
pub const PRIZE_CLAIM_WINDOW_SECS: u64 = 90 * 24 * 60 * 60;
@@ -135,10 +137,6 @@ pub fn create_event(env: &Env, params: CreateEventParams, op_id: BytesN<32>) ->
135137
storage::set_event(env, id, &record);
136138
storage::set_non_owner_contribution_total(env, id, 0);
137139

138-
if let Some(manager) = &params.manager {
139-
storage::set_event_manager(env, id, manager);
140-
}
141-
142140
if is_crowdfunding {
143141
storage::append_winner(
144142
env,
@@ -164,15 +162,91 @@ pub fn create_event(env: &Env, params: CreateEventParams, op_id: BytesN<32>) ->
164162
}
165163
.publish(env);
166164

165+
if let Some(manager) = &params.manager {
166+
let expires_at = env
167+
.ledger()
168+
.sequence()
169+
.saturating_add(PENDING_MANAGER_TTL_LEDGERS);
170+
let pending = PendingManager {
171+
target: manager.clone(),
172+
expires_at_ledger: expires_at,
173+
};
174+
storage::set_pending_manager(env, id, &pending);
175+
evt::ManagerProposed {
176+
event_id: id,
177+
target: manager.clone(),
178+
expires_at_ledger: expires_at,
179+
}
180+
.publish(env);
181+
}
182+
167183
idempotency::mark_seen(env, &op_id);
168184
Ok(id)
169185
}
170186

171-
pub fn set_manager(env: &Env, event_id: u64, new_manager: Address) -> Result<(), Error> {
187+
// ============================================================
188+
// MANAGER ROTATION (two-step propose / accept)
189+
// ============================================================
190+
pub fn propose_manager(env: &Env, event_id: u64, new_manager: Address) -> Result<(), Error> {
172191
admin::require_not_paused(env)?;
173192
let event = storage::get_event(env, event_id).ok_or(Error::EventNotFound)?;
174193
resolve_manager(env, event_id, &event.owner).require_auth();
175-
storage::set_event_manager(env, event_id, &new_manager);
194+
195+
let expires_at = env
196+
.ledger()
197+
.sequence()
198+
.saturating_add(PENDING_MANAGER_TTL_LEDGERS);
199+
let pending = PendingManager {
200+
target: new_manager.clone(),
201+
expires_at_ledger: expires_at,
202+
};
203+
storage::set_pending_manager(env, event_id, &pending);
204+
205+
evt::ManagerProposed {
206+
event_id,
207+
target: new_manager,
208+
expires_at_ledger: expires_at,
209+
}
210+
.publish(env);
211+
Ok(())
212+
}
213+
214+
pub fn accept_manager(env: &Env, event_id: u64) -> Result<(), Error> {
215+
admin::require_not_paused(env)?;
216+
storage::get_event(env, event_id).ok_or(Error::EventNotFound)?;
217+
218+
let pending =
219+
storage::get_pending_manager(env, event_id).ok_or(Error::PendingRotationMismatch)?;
220+
221+
if env.ledger().sequence() > pending.expires_at_ledger {
222+
storage::clear_pending_manager(env, event_id);
223+
return Err(Error::PendingRotationExpired);
224+
}
225+
226+
pending.target.require_auth();
227+
228+
storage::set_event_manager(env, event_id, &pending.target);
229+
storage::clear_pending_manager(env, event_id);
230+
231+
evt::ManagerChanged {
232+
event_id,
233+
new_manager: pending.target,
234+
}
235+
.publish(env);
236+
Ok(())
237+
}
238+
239+
pub fn cancel_pending_manager(env: &Env, event_id: u64) -> Result<(), Error> {
240+
admin::require_not_paused(env)?;
241+
let event = storage::get_event(env, event_id).ok_or(Error::EventNotFound)?;
242+
resolve_manager(env, event_id, &event.owner).require_auth();
243+
244+
if storage::get_pending_manager(env, event_id).is_none() {
245+
return Err(Error::PendingRotationMismatch);
246+
}
247+
storage::clear_pending_manager(env, event_id);
248+
249+
evt::PendingManagerCancelled { event_id }.publish(env);
176250
Ok(())
177251
}
178252

@@ -181,6 +255,11 @@ pub fn get_manager(env: &Env, event_id: u64) -> Result<Address, Error> {
181255
Ok(resolve_manager(env, event_id, &event.owner))
182256
}
183257

258+
pub fn get_pending_manager(env: &Env, event_id: u64) -> Result<Option<PendingManager>, Error> {
259+
storage::get_event(env, event_id).ok_or(Error::EventNotFound)?;
260+
Ok(storage::get_pending_manager(env, event_id))
261+
}
262+
184263
// ============================================================
185264
// ADD FUNDS (partner / community contribution)
186265
// ============================================================

contracts/events/src/events.rs

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,24 @@ pub struct EventCancelled {
2020
pub id: u64,
2121
}
2222

23+
#[contractevent]
24+
pub struct ManagerProposed {
25+
pub event_id: u64,
26+
pub target: Address,
27+
pub expires_at_ledger: u32,
28+
}
29+
30+
#[contractevent]
31+
pub struct ManagerChanged {
32+
pub event_id: u64,
33+
pub new_manager: Address,
34+
}
35+
36+
#[contractevent]
37+
pub struct PendingManagerCancelled {
38+
pub event_id: u64,
39+
}
40+
2341
#[contractevent]
2442
pub struct FundsAdded {
2543
pub event_id: u64,

contracts/events/src/lib.rs

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -228,14 +228,26 @@ impl EventsContract {
228228
// ============================================================
229229
// MANAGEMENT AUTHORITY (manager != funder/owner)
230230
// ============================================================
231-
pub fn set_manager(env: Env, event_id: u64, new_manager: Address) -> Result<(), Error> {
232-
event_ops::set_manager(&env, event_id, new_manager)
231+
pub fn propose_manager(env: Env, event_id: u64, new_manager: Address) -> Result<(), Error> {
232+
event_ops::propose_manager(&env, event_id, new_manager)
233+
}
234+
235+
pub fn accept_manager(env: Env, event_id: u64) -> Result<(), Error> {
236+
event_ops::accept_manager(&env, event_id)
237+
}
238+
239+
pub fn cancel_pending_manager(env: Env, event_id: u64) -> Result<(), Error> {
240+
event_ops::cancel_pending_manager(&env, event_id)
233241
}
234242

235243
pub fn get_manager(env: Env, event_id: u64) -> Result<Address, Error> {
236244
event_ops::get_manager(&env, event_id)
237245
}
238246

247+
pub fn get_pending_manager(env: Env, event_id: u64) -> Result<Option<PendingManager>, Error> {
248+
event_ops::get_pending_manager(&env, event_id)
249+
}
250+
239251
pub fn claim_milestone(
240252
env: Env,
241253
event_id: u64,

contracts/events/src/storage.rs

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,8 @@ use soroban_sdk::String;
66

77
use crate::errors::Error;
88
use crate::types::{
9-
CancellationState, DataKey, EventRecord, PendingAdmin, PendingUpgrade, PrizeAward, Submission,
10-
Winner,
9+
CancellationState, DataKey, EventRecord, PendingAdmin, PendingManager, PendingUpgrade,
10+
PrizeAward, Submission, Winner,
1111
};
1212

1313
// ============================================================
@@ -287,6 +287,27 @@ pub fn set_event_manager(env: &Env, id: u64, manager: &Address) {
287287
touch_event_persistent(env, &key);
288288
}
289289

290+
pub fn get_pending_manager(env: &Env, id: u64) -> Option<PendingManager> {
291+
let key = DataKey::PendingManager(id);
292+
let p: Option<PendingManager> = env.storage().persistent().get(&key);
293+
if p.is_some() {
294+
touch_event_persistent(env, &key);
295+
}
296+
p
297+
}
298+
299+
pub fn set_pending_manager(env: &Env, id: u64, pending: &PendingManager) {
300+
let key = DataKey::PendingManager(id);
301+
env.storage().persistent().set(&key, pending);
302+
touch_event_persistent(env, &key);
303+
}
304+
305+
pub fn clear_pending_manager(env: &Env, id: u64) {
306+
env.storage()
307+
.persistent()
308+
.remove(&DataKey::PendingManager(id));
309+
}
310+
290311
// ============================================================
291312
// APPLICANTS (paged, persistent)
292313
// ============================================================

contracts/events/src/tests/cancel_refund.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -310,7 +310,8 @@ fn non_manager_cranks_and_finalizes_with_exact_payout_deltas() {
310310
let ctx = setup();
311311
let id = create_hackathon(&ctx);
312312
let manager = Address::generate(&ctx.env);
313-
ctx.events.set_manager(&id, &manager);
313+
ctx.events.propose_manager(&id, &manager);
314+
ctx.events.accept_manager(&id);
314315
let p1 = Address::generate(&ctx.env);
315316
let p2 = Address::generate(&ctx.env);
316317
let p1_amount = 200_0000000_i128;

0 commit comments

Comments
 (0)