From 213d79c4c9081bed882ac3fee92edadb0540fefc Mon Sep 17 00:00:00 2001 From: Adedamolamiposi Date: Wed, 21 Oct 2026 23:18:05 +0100 Subject: [PATCH] fix: update jar event payloads and ownership controls --- contracts/tip-splitter/interface.json | 31 +++++ contracts/tip-splitter/src/lib.rs | 54 +++++++-- contracts/tip-splitter/src/test.rs | 167 ++++++++++++++++++++++++-- docs/CONTRACT.md | 34 ++++-- 4 files changed, 262 insertions(+), 24 deletions(-) diff --git a/contracts/tip-splitter/interface.json b/contracts/tip-splitter/interface.json index ebd920f..02f56c8 100644 --- a/contracts/tip-splitter/interface.json +++ b/contracts/tip-splitter/interface.json @@ -389,6 +389,21 @@ "name": "count", "type_": "u32" }, + { + "name": "splits", + "type_": { + "vec": { + "element_type": { + "tuple": { + "value_types": [ + "address", + "u32" + ] + } + } + } + } + }, { "name": "version", "type_": "u32" @@ -416,6 +431,22 @@ ] } }, + { + "udt_struct_v0": { + "lib": "", + "name": "JarDeletedEvent", + "fields": [ + { + "name": "jar_id", + "type_": "string" + }, + { + "name": "version", + "type_": "u32" + } + ] + } + }, { "udt_struct_v0": { "lib": "", diff --git a/contracts/tip-splitter/src/lib.rs b/contracts/tip-splitter/src/lib.rs index 884ec7f..125f9b7 100644 --- a/contracts/tip-splitter/src/lib.rs +++ b/contracts/tip-splitter/src/lib.rs @@ -148,7 +148,7 @@ const INTERFACE_VERSION: u32 = 4; /// One recipient and the share of every tip they receive, in basis points. #[contracttype] -#[derive(Clone)] +#[derive(Clone, Debug, Eq, PartialEq)] pub struct Split { /// Recipient that receives this share of every tip. pub to: Address, @@ -494,6 +494,7 @@ pub struct JarCreatedEvent { pub struct SplitsUpdatedEvent { pub version: u32, pub count: u32, + pub splits: Vec, } /// Data published by `jar_xfer`; fields are named for forward-compatible decoding. @@ -505,6 +506,14 @@ pub struct JarTransferredEvent { pub new_owner: Address, } +/// Data published by `jar_del`; fields are named for forward-compatible decoding. +#[contracttype] +#[derive(Clone, Debug, Eq, PartialEq)] +pub struct JarDeletedEvent { + pub version: u32, + pub jar_id: String, +} + /// Data published by `min_tip`; fields are named for forward-compatible decoding. #[contracttype] #[derive(Clone, Debug, Eq, PartialEq)] @@ -762,6 +771,7 @@ impl TipSplitter { } let split_count = splits.len(); + let new_splits = splits.clone(); let key = DataKey::Jar(jar_id.clone()); env.storage().persistent().set( &key, @@ -777,6 +787,7 @@ impl TipSplitter { SplitsUpdatedEvent { version: 1, count: split_count, + splits: new_splits, }, ); } @@ -954,10 +965,11 @@ impl TipSplitter { .publish((symbol_short!("splits"), jar_id), split_count); } - /// Transfer control of a jar to a new owner. Only the current owner may do - /// this; the new owner does not need to authorize. Splits are unchanged. - /// Emits a `jar_xfer` event carrying both the outgoing and the incoming - /// owner, so indexers can update who controls the jar. + /// Transfer control of a jar to a new owner. Both the current owner and the + /// incoming owner must authorize the exact `(jar_id, new_owner)` handoff. A + /// transfer to the current owner is rejected as a no-op. Splits are + /// unchanged. Emits a `jar_xfer` event carrying both owners, so indexers can + /// update who controls the jar. pub fn transfer_jar_ownership(env: Env, jar_id: String, new_owner: Address) { let key = DataKey::Jar(jar_id.clone()); let jar: Jar = env @@ -967,7 +979,6 @@ impl TipSplitter { .unwrap_or_else(|| panic_with_error!(&env, Error::JarNotFound)); // Issue #92: extend both jar and min-tip in lockstep. Self::extend_jar_and_min_ttl(&env, &jar_id); - jar.owner.require_auth(); // Issue #94: reject the contract itself or its token as new owner. let contract_addr = env.current_contract_address(); let token_addr: Address = env @@ -975,9 +986,11 @@ impl TipSplitter { .instance() .get(&DataKey::Token) .unwrap_or_else(|| panic_with_error!(&env, Error::NotInitialized)); - if new_owner == contract_addr || new_owner == token_addr { + if new_owner == contract_addr || new_owner == token_addr || new_owner == jar.owner { panic_with_error!(&env, Error::InvalidRecipient); } + jar.owner.require_auth(); + new_owner.require_auth_for_args((jar_id.clone(), new_owner.clone()).into_val(&env)); let prev_owner = jar.owner; env.storage().persistent().set( &key, @@ -1004,6 +1017,33 @@ impl TipSplitter { ); } + /// Delete a jar and release its slug. Only the current owner may do this, + /// and the slug must currently be registered. Removes the jar and its + /// per-jar metadata so the slug can be reused by `create_jar`. + pub fn delete_jar(env: Env, jar_id: String) { + let key = DataKey::Jar(jar_id.clone()); + let jar: Jar = env + .storage() + .persistent() + .get(&key) + .unwrap_or_else(|| panic_with_error!(&env, Error::JarNotFound)); + // Issue #92: extend both jar and min-tip in lockstep. + Self::extend_jar_and_min_ttl(&env, &jar_id); + jar.owner.require_auth(); + + env.storage().persistent().remove(&key); + env.storage().persistent().remove(&DataKey::Stats(jar_id.clone())); + env.storage().persistent().remove(&DataKey::MinTip(jar_id.clone())); + + env.events().publish( + (symbol_short!("jar_del"),), + JarDeletedEvent { + version: 1, + jar_id, + }, + ); + } + /// Transfer admin control to a new address. Only the current admin may do this. /// Rejects if the new admin is the same as the current one. Emits an event /// carrying both addresses so indexers can track the handover. diff --git a/contracts/tip-splitter/src/test.rs b/contracts/tip-splitter/src/test.rs index 57cfd10..22c5da2 100644 --- a/contracts/tip-splitter/src/test.rs +++ b/contracts/tip-splitter/src/test.rs @@ -2411,7 +2411,8 @@ fn update_splits_emits_splits_event() { ]; client.update_splits(&jar_id, &new_splits); - // The splits event must be published with the jar id and new split count. + // The splits event must carry the exact updated split vector, matching the + // jar immediately after the mutation. let expected_topics = vec![ env, symbol_short!("splits").into_val(env), @@ -2427,7 +2428,9 @@ fn update_splits_emits_splits_event() { let (_, _, data) = splits_events.first().unwrap(); let decoded: SplitsUpdatedEvent = TryFromVal::try_from_val(env, data).expect("splits event data shape"); - assert_eq!(decoded.count, new_splits.len()); + let jar = client.get_jar(&jar_id); + assert_eq!(decoded.count, jar.splits.len() as u32); + assert_eq!(decoded.splits, jar.splits); } #[test] @@ -2803,12 +2806,161 @@ fn transfer_jar_ownership_moves_control_to_new_owner() { ], ); + env.mock_auths(&[ + MockAuth { + address: &owner, + invoke: &MockAuthInvoke { + contract: &s.contract, + fn_name: "transfer_jar_ownership", + args: (jar_id.clone(), new_owner.clone()).into_val(env), + sub_invokes: &[], + }, + }, + MockAuth { + address: &new_owner, + invoke: &MockAuthInvoke { + contract: &s.contract, + fn_name: "transfer_jar_ownership", + args: (jar_id.clone(), new_owner.clone()).into_val(env), + sub_invokes: &[], + }, + }, + ]); + client.transfer_jar_ownership(&jar_id, &new_owner); assert_eq!(client.get_jar(&jar_id).owner, new_owner); // Splits must survive the transfer untouched. assert_eq!(client.get_jar(&jar_id).splits.get(0).unwrap().to, alice); } +#[test] +fn transfer_jar_ownership_requires_new_owner_auth() { + let s = setup(); + let env = &s.env; + let client = TipSplitterClient::new(env, &s.contract); + + let owner = Address::generate(env); + let new_owner = Address::generate(env); + let alice = Address::generate(env); + let jar_id = String::from_str(env, "@handoff_new_auth"); + + client.create_jar( + &owner, + &jar_id, + &vec![ + env, + Split { + to: alice.clone(), + bps: 10000, + }, + ], + ); + + env.mock_auths(&[MockAuth { + address: &owner, + invoke: &MockAuthInvoke { + contract: &s.contract, + fn_name: "transfer_jar_ownership", + args: (jar_id.clone(), new_owner.clone()).into_val(env), + sub_invokes: &[], + }, + }]); + + let res = client.try_transfer_jar_ownership(&jar_id, &new_owner); + assert!(res.is_err(), "the incoming owner must authorize the transfer"); + assert_eq!(client.get_jar(&jar_id).owner, owner); +} + +#[test] +fn transfer_jar_ownership_rejects_current_owner() { + let s = setup(); + let env = &s.env; + let client = TipSplitterClient::new(env, &s.contract); + let owner = Address::generate(env); + let alice = Address::generate(env); + let jar_id = String::from_str(env, "@self_xfer"); + + client.create_jar( + &owner, + &jar_id, + &vec![ + env, + Split { + to: alice.clone(), + bps: 10000, + }, + ], + ); + + env.mock_all_auths(); + let res = client.try_transfer_jar_ownership(&jar_id, &owner); + assert!(res.is_err(), "transferring to the current owner is invalid"); + assert_eq!(client.get_jar(&jar_id).owner, owner); +} + +#[test] +fn delete_jar_releases_slug_and_requires_owner_auth() { + let s = setup(); + let env = &s.env; + let client = TipSplitterClient::new(env, &s.contract); + + let owner = Address::generate(env); + let alice = Address::generate(env); + let jar_id = String::from_str(env, "@retire"); + client.create_jar( + &owner, + &jar_id, + &vec![ + env, + Split { + to: alice.clone(), + bps: 10000, + }, + ], + ); + + env.mock_auths(&[MockAuth { + address: &owner, + invoke: &MockAuthInvoke { + contract: &s.contract, + fn_name: "delete_jar", + args: (jar_id.clone(),).into_val(env), + sub_invokes: &[], + }, + }]); + client.delete_jar(&jar_id); + + assert!(!client.jar_exists(&jar_id)); + env.mock_auths(&[MockAuth { + address: &owner, + invoke: &MockAuthInvoke { + contract: &s.contract, + fn_name: "create_jar", + args: (owner.clone(), jar_id.clone(), vec![ + env, + Split { + to: alice.clone(), + bps: 10000, + }, + ]) + .into_val(env), + sub_invokes: &[], + }, + }]); + client.create_jar( + &owner, + &jar_id, + &vec![ + env, + Split { + to: alice.clone(), + bps: 10000, + }, + ], + ); + assert!(client.jar_exists(&jar_id)); +} + /// Creates a single-recipient jar owned by `owner` and transfers it to /// `new_owner` under `mock_all_auths`, so the authority tests below start /// from a completed handoff and only the call under test runs with strict @@ -4290,8 +4442,8 @@ fn leave_jar_reassigns_departing_share_to_last_entry() { client.leave_jar(&jar_id, &bob); - // a `splits` event was published with the new count — read it before any - // further invocation, since the test env only exposes the last call's events + // A `splits` event must publish the post-update split vector, which matches + // the jar contents read back immediately afterward. let expected_topics = vec![ env, symbol_short!("splits").into_val(env), @@ -4304,10 +4456,11 @@ fn leave_jar_reassigns_departing_share_to_last_entry() { .filter(|e| e.0 == s.contract && e.1 == expected_topics) .collect(); assert_eq!(evs.len(), 1); - let count: u32 = TryFromVal::try_from_val(env, &evs[0].2).unwrap(); - assert_eq!(count, 2); - + let decoded: SplitsUpdatedEvent = + TryFromVal::try_from_val(env, &evs[0].2).expect("splits event data shape"); let jar = client.get_jar(&jar_id); + assert_eq!(decoded.count, jar.splits.len() as u32); + assert_eq!(decoded.splits, jar.splits); assert_eq!(jar.splits.len(), 2); // middle entry removed; the last remaining entry (carol) absorbs bob's 3000. assert_eq!(jar.splits.get(0).unwrap().to, alice); diff --git a/docs/CONTRACT.md b/docs/CONTRACT.md index 97db253..dc54e9c 100644 --- a/docs/CONTRACT.md +++ b/docs/CONTRACT.md @@ -82,7 +82,8 @@ one belongs to a jar, the other to a `(jar, supporter)` pair. | `is_splits_locked(jar_id) -> bool` | — | Whether the jar's splits are locked. Panics with `JarNotFound` if the slug is free. | | `leave_jar(jar_id, recipient)` | `recipient` | Remove `recipient` from the jar's splits. Their share is added to the last remaining entry so the splits still sum to 10 000. Rejected with `SoleRecipient` (only entry) or `NotRecipient` (not in the jar). Emits a `splits` event. | | `set_min_tip_amount(jar_id, min_amount)` | jar `owner` | Set or clear an optional minimum tip amount for a jar. `min_amount` must be strictly positive (`> 0`) or `None` to clear. Emits a `min_tip` event. | -| `transfer_jar_ownership(jar_id, new_owner)` | current jar `owner` | Hand control of a jar to `new_owner`. Splits are unchanged; the new owner does not need to authorize. Emits a `jar_xfer` event carrying both owners. | +| `transfer_jar_ownership(jar_id, new_owner)` | current jar `owner` and `new_owner` | Hand control of a jar to `new_owner`. The exact `(jar_id, new_owner)` pair must be signed by both parties; a transfer to the current owner is rejected. Splits are unchanged. Emits a `jar_xfer` event carrying both owners. | +| `delete_jar(jar_id)` | current jar `owner` | Delete a registered jar and free its slug so it can be recreated. Removes the jar, its stats, and any minimum-tip value for that slug. Emits a `jar_del` event. | | `rename_jar(jar_id, new_jar_id)` | jar `owner` | Move a jar, its stats and its minimum to a new slug. The old slug stops resolving immediately; `new_jar_id` must be free and pass the same length checks as `create_jar`. Emits a `jar_rnm` event carrying both ids. | | `tip(from, jar_id, amount, message)` | `from` | Move `amount` USDC out of `from` and route it across the jar's recipients. A recipient who cannot receive the token has their share **escrowed in the contract** (see `get_held`/`withdraw_held`) instead of reverting the tip. Emits a `tip` event carrying the per-recipient breakdown. Rejects amounts below the jar's minimum with `BelowMinTipAmount`. | | `get_held(recipient) -> i128` | — | How much the contract is holding for `recipient` because a past tip could not deliver their share. `0` when nothing is held. | @@ -140,7 +141,8 @@ written as raised only when every earlier check has passed. | `create_jar(owner, jar_id, splits)` | `owner` (+ registrar co-auth when set) | `InvalidJarId` → `JarExists` → `SplitsEmpty` → `TooManyRecipients` → `SplitOutOfRange` / `DuplicateRecipient` (per entry) → `SplitSumNot100Pct` → Soroban auth error (missing registrar co-signature, #178) | Register a new jar. Fails if the slug is not a canonical `@`-prefixed `[a-z0-9_-]{3,32}` slug, already exists, or splits are invalid. The registrar's co-signature is demanded only after validation passes, so a malformed request never burns one. Emits a `jar_crtd` event. | | `update_splits(jar_id, splits)` | jar `owner` | `JarNotFound` → `SplitsEmpty` → `TooManyRecipients` → `SplitOutOfRange` / `DuplicateRecipient` (per entry) → `SplitSumNot100Pct` | Replace a jar's splits. Subject to the same validation as `create_jar`. Emits a `splits` event. | | `set_min_tip_amount(jar_id, min_amount)` | jar `owner` | `JarNotFound` → `InvalidMinimumAmount` | Set or clear an optional minimum tip amount for a jar. `min_amount` must be strictly positive (`> 0`) or `None` to clear. Emits a `min_tip` event. | -| `transfer_jar_ownership(jar_id, new_owner)` | current jar `owner` | `JarNotFound` → `InvalidRecipient` (owner is the contract or its token) | Hand control of a jar to `new_owner`. Splits are unchanged; the new owner does not need to authorize. Emits a `jar_xfer` event carrying both owners. | +| `transfer_jar_ownership(jar_id, new_owner)` | current jar `owner` and `new_owner` | `JarNotFound` → `InvalidRecipient` (new owner is the current owner, the contract itself, or the token) | Hand control of a jar to `new_owner`. The exact `(jar_id, new_owner)` pair is signed by both owners. Splits are unchanged. Emits a `jar_xfer` event carrying both owners. | +| `delete_jar(jar_id)` | current jar `owner` | `JarNotFound` | Delete a registered jar and free its slug for reuse. Removes the jar, its stats, and the associated minimum-tip value. Emits a `jar_del` event. | | `rename_jar(jar_id, new_jar_id)` | jar `owner` | `InvalidJarId` (new id not a canonical `@`-prefixed `[a-z0-9_-]{3,32}` slug) → `JarNotFound` → `JarIdInUse` | Move a jar, its stats and its minimum to `new_jar_id`. The old id stops resolving once the move is complete. Emits a `jar_rnm` event carrying both ids. | | `tip(from, jar_id, amount, message)` | `from` | `ContractPaused` → `NotInitialized` → `AmountZero` (amount = 0) / `AmountNegative` (amount < 0) → `MessageTooLong` → `JarNotFound` → `BelowMinTipAmount` → `InsufficientBalance` → `SplitAmountTooSmall` (a share truncates to zero) / `AmountOverflow` (`amount * bps` overflows `i128`) | Transfer `amount` USDC from `from`, split across the jar's recipients. Emits a `tip` event carrying the per-recipient breakdown. Rejects amounts below the jar's minimum with `BelowMinTipAmount` and amounts that cannot pay every recipient with `SplitAmountTooSmall`; see [the smallest amount a jar accepts](#the-smallest-amount-a-jar-accepts-86). A failing token `transfer` reverts the whole call with the **token contract's own error**, not one of this contract's codes (e.g. an insufficient balance). | | `get_jar(jar_id) -> Jar` | — | `JarNotFound` | Read a jar's configuration. Panics with `JarNotFound` if the slug is free. | @@ -722,7 +724,8 @@ Consequences a client has to design for: |----------|-----------|---------------| | `create_jar(owner, jar_id, splits)` | `owner` | The contract call only. No token moves. | | `update_splits(jar_id, splits)` | the jar's current `owner` | The contract call only. | -| `transfer_jar_ownership(jar_id, new_owner)` | the jar's current `owner` | The contract call only. `new_owner` signs nothing. | +| `transfer_jar_ownership(jar_id, new_owner)` | the jar's current `owner` and `new_owner` | The exact `(jar_id, new_owner)` transfer pair, signed by both parties. | +| `delete_jar(jar_id)` | the jar's current `owner` | The contract call only. | | `set_min_tip_amount(jar_id, min_amount)` | the jar's current `owner` | The contract call only. | Each of these is a single contract-level invocation with no nested token @@ -907,11 +910,12 @@ log or an indexer's jar list just to validate a name. ### `splits` — published on every successful `update_splits` - **Topics:** `(symbol "splits", jar_id: String)` -- **Data:** `SplitsUpdatedEvent { count: u32 }` +- **Data:** `SplitsUpdatedEvent { count: u32, splits: Vec }` Lets an indexer that has cached a jar's splits know they went stale, without -having to re-poll every jar on a schedule. The indexer refetches the jar via -`get_jar` when it sees this event. +having to re-poll every jar on a schedule. The event carries the updated +split vector itself, so a consumer can refresh immediately and avoid the stale +read window created by a second update racing with a follow-up `get_jar` call. ### `jar_xfer` — published on every successful `transfer_jar_ownership` @@ -930,14 +934,24 @@ means successive transfers chain: each event's `prev_owner` is the previous event's `new_owner`, so an ownership history can be reconstructed from the events alone. -`prev_owner` is the address that authorized the call — only the current owner -may transfer a jar — and is always different from `new_owner` in practice, -though the contract does not reject a transfer to the existing owner. Splits -are unchanged by a transfer, so no `splits` event accompanies this one. +`prev_owner` is the address that previously controlled the jar, and the +contract now requires both sides of the handoff to authorize the exact +`(jar_id, new_owner)` pair. A transfer to the current owner is rejected with +`InvalidRecipient`, so the event log never records a no-op handoff. Splits are +unchanged by a transfer, so no `splits` event accompanies this one. > **Consumer impact.** Event data is a named map. Decoders should read fields > by name and ignore unknown fields added in future versions. +### `jar_del` — published on every successful `delete_jar` + +- **Topics:** `(symbol "jar_del")` +- **Data:** `JarDeletedEvent { jar_id: String }` + +Lets an indexer drop a jar from its cached list as soon as the slug is retired, +without polling `jar_exists` or re-reading the jar. A deleted slug is free for +`create_jar` to reuse immediately after the write settles. + ### `min_tip` — published on every successful `set_min_tip_amount` - **Topics:** `(symbol "min_tip", jar_id: String)`