From 7b82604bea710a8cbf5c33cff6761f5a0439f090 Mon Sep 17 00:00:00 2001 From: weare Date: Thu, 27 Aug 2026 11:40:41 -0800 Subject: [PATCH] issues/631-634: expand docs, add event naming convention, replace magic numbers, add Errors sections - #631 expand OverflowBehavior and OverfundingPolicy docs with # Relationship section - #632 import BASIS_POINTS_TOTAL in calc.rs and document typical denom values - #633 add //! module-level doc comment to events.rs explaining naming convention, topic layout, next_seq usage, and symbol_short! vs Symbol::new guidance - #634 add /// # Errors sections to assert_unique_recipients and assert_recipients_have_trustlines in validation.rs - clean .gitignore for test snapshots and generated artifacts --- .gitignore | 26 ++++++++------------------ contracts/split/src/calc.rs | 4 +++- contracts/split/src/events.rs | 27 +++++++++++++++++++++++++++ contracts/split/src/types.rs | 17 ++++++++++++++++- contracts/split/src/validation.rs | 6 ++++-- 5 files changed, 58 insertions(+), 22 deletions(-) diff --git a/.gitignore b/.gitignore index 852dc9f..06fe511 100644 --- a/.gitignore +++ b/.gitignore @@ -9,38 +9,28 @@ wasm32-unknown-unknown/ .env.local .env.*.local -# OS files +# OS / Editor files .DS_Store Thumbs.db desktop.ini .vscode/ .idea/ -*.log -*.tmp -*.bak -test_snapshots/ -snapshots/ -**/snapshots/ -*.swp -*.swo -*~ - -# Editor/IDE -.vscode/ -.idea/ *.swp *.swo *~ *.sublime-project *.sublime-workspace -# Test snapshots +# Test snapshots and generated artifacts **/__snapshots__/ *.snap -**/test_snapshots/ +*.snap.orig *.snapshot -**/snapshots/ *.snap.bak +test_snapshots/ +**/test_snapshots/ +snapshots/ +**/snapshots/ # Stellar CLI .stellar/ @@ -70,7 +60,7 @@ profdata/ .*.swp .*.swo -# Node / JS tooling (if any frontend tooling is added) +# Node / JS tooling node_modules/ dist/ .pnp diff --git a/contracts/split/src/calc.rs b/contracts/split/src/calc.rs index cd8a120..789d182 100644 --- a/contracts/split/src/calc.rs +++ b/contracts/split/src/calc.rs @@ -4,6 +4,8 @@ //! across recipients proportionally, ensuring every stroop is accounted for //! (i.e. `sum(result) == total` always holds). +#[allow(unused_imports)] +use crate::types::BASIS_POINTS_TOTAL; use soroban_sdk::{Env, Vec}; /// Distribute `total` among recipients according to their `ratios` out of @@ -13,7 +15,7 @@ use soroban_sdk::{Env, Vec}; /// * `env` – Soroban environment (needed to allocate the result `Vec`) /// * `total` – total amount to distribute (stroops); must be ≥ 0 /// * `ratios` – relative weight of each recipient (must be non-empty, all ≥ 0) -/// * `denom` – sum of all ratios (must be > 0) +/// * `denom` – sum of all ratios (must be > 0); typically [`BASIS_POINTS_TOTAL`] /// /// # Guarantees /// * `result.iter().sum::() == total` always diff --git a/contracts/split/src/events.rs b/contracts/split/src/events.rs index 46db8b6..f386148 100644 --- a/contracts/split/src/events.rs +++ b/contracts/split/src/events.rs @@ -1,3 +1,30 @@ +//! # Event naming convention +//! +//! All split-contracts events follow a consistent topic layout: +//! `(symbol_short!("split"), , invoice_id?)`. +//! +//! - `` is an 8-char symbol identifying the lifecycle stage +//! (`created`, `paid`, `released`, `refunded`, `st_chg`, …). +//! - `invoice_id` is included as the third topic for per-invoice events +//! so indexers can filter by invoice without inspecting event data. +//! +//! ## When to call `next_seq` +//! +//! Events that represent discrete, countable occurrences on a single invoice +//! should include an auto-incrementing `event_seq` (fetched via `next_seq`) +//! as the last field in the event data. This gives indexers a stable, +//! per-invoice ordering key. Do **not** call `next_seq` for: +//! - global/contract-level events with no `invoice_id` +//! - events that already contain a unique identifier (e.g. `action_id`, +//! `milestone_number`, `new_id`) +//! +//! ## `symbol_short!` vs `Symbol::new` +//! +//! Prefer `symbol_short!("abbr")` for event action topics because they are +//! short, fixed strings. Use `Symbol::new(env, "LongName")` only when the +//! symbol exceeds the short-macro length limit or must be constructed +//! dynamically. + use crate::types::{DisputeOutcome, FeeSplit, InvoiceStatus, RepScore, TimelockAction}; use soroban_sdk::{contracttype, symbol_short, Address, BytesN, Env, String, Vec}; diff --git a/contracts/split/src/types.rs b/contracts/split/src/types.rs index 7d491be..dcf9de0 100644 --- a/contracts/split/src/types.rs +++ b/contracts/split/src/types.rs @@ -14,8 +14,14 @@ pub struct AssetPair { #[contracttype] #[derive(Clone, Debug, PartialEq)] pub enum OverflowBehavior { + /// Reject the payment outright. The payer receives an error and the + /// transaction does not credit the invoice. Reject, + /// Accept the full payment and mark the surplus for refund to the payer + /// at release time. Refund, + /// Accept the full payment and treat the surplus as a protocol donation; + /// no refund is issued. Donate, } @@ -26,10 +32,19 @@ pub enum OverflowBehavior { /// default, and the value legacy invoices are migrated to — preserves the /// historical behaviour by delegating to the per-invoice [`OverflowBehavior`] /// setting, so invoices created before this field existed are unaffected. +/// +/// # Relationship +/// +/// `OverfundingPolicy` is the *outer* policy selector stored on the invoice. +/// When it is `Cap`, the contract falls back to the per-invoice +/// [`OverflowBehavior`] value to decide the exact outcome. The other two +/// variants (`AcceptAll`, `ReturnSurplus`) bypass `OverflowBehavior` entirely +/// and implement their own semantics directly in `_pay`. #[contracttype] #[derive(Clone, Debug, PartialEq)] pub enum OverfundingPolicy { - /// Reject any payment that would take `funded` past the invoice total. + /// Preserve legacy behaviour by delegating to the invoice's + /// [`OverflowBehavior`] field. Cap, /// Accept the payment in full; `funded` is allowed to exceed the total and /// the surplus is distributed pro-rata to recipients at release time. diff --git a/contracts/split/src/validation.rs b/contracts/split/src/validation.rs index e0c16da..bdaef29 100644 --- a/contracts/split/src/validation.rs +++ b/contracts/split/src/validation.rs @@ -12,8 +12,8 @@ use soroban_sdk::{symbol_short, Address, Env, Map, Vec}; /// Uses a `soroban_sdk::Map` for O(n log n) membership tracking — no /// unbounded heap allocation and a single pass over the slice. /// -/// Returns `Ok(())` when every address is distinct, or -/// `Err(ContractError::DuplicateRecipient)` on the first duplicate found. +/// # Errors +/// Returns [ContractError::DuplicateRecipient] when a duplicate is found. pub fn assert_unique_recipients(env: &Env, recipients: &[Address]) -> Result<(), ContractError> { let mut seen: Map = Map::new(env); for r in recipients.iter() { @@ -40,6 +40,8 @@ pub fn assert_unique_recipients(env: &Env, recipients: &[Address]) -> Result<(), /// Returns `Ok(())` when every recipient returns a valid balance, or /// `Err(ContractError::RecipientMissingTrustline)` with the offending address /// surfaced in the panic message. +/// # Errors +/// Returns [ContractError::RecipientMissingTrustline] when a balance call fails. pub fn assert_recipients_have_trustlines( env: &Env, token: &Address,