Skip to content

Commit 69994ca

Browse files
committed
fix: restore all 344 unit tests after Wave 8 contributor PRs
- Wrap storage/events/storage_snapshot unit tests in env.as_contract() to satisfy Soroban SDK 22's requirement for contract context on storage access outside contract invocations - Fix test_calc_platform_fee_overflow: i128::MAX * 1 does not overflow; use fee_bps=2 to trigger genuine ArithmeticOverflow - Fix three event-detection tests (auto_res, al_tog, al_open) by moving env.events().all() checks immediately after the event-emitting call — Soroban SDK resets the event buffer between contract invocations, so any intermediate getter call clears events before they can be observed
1 parent 4efaae8 commit 69994ca

10 files changed

Lines changed: 312 additions & 205 deletions

File tree

contracts/split/src/calc.rs

Lines changed: 15 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@
3434
3535
#[allow(unused_imports)]
3636
use crate::types::BASIS_POINTS_TOTAL;
37-
use soroban_sdk::{Address, BytesN, Env, Vec};
37+
use soroban_sdk::{Address, Env, Map, Vec};
3838

3939
use crate::error::ContractError;
4040

@@ -148,39 +148,22 @@ pub fn distribute_with_remainder(
148148
/// * `env` – Soroban environment
149149
/// * `recipients` – mutable list of recipient addresses to sort in-place
150150
pub fn sort_recipients(env: &Env, recipients: &mut Vec<Address>) {
151-
let n = recipients.len() as usize;
151+
let n = recipients.len();
152152
if n <= 1 {
153153
return;
154154
}
155155

156-
// Build byte representations for comparison.
157-
let mut bytes_vec: Vec<(BytesN<32>, usize)> = Vec::new(env);
156+
// Soroban Map maintains keys in canonical (XDR-sorted) order.
157+
// Inserting all addresses as keys then iterating produces deterministic ordering.
158+
let mut ordered: Map<Address, u32> = Map::new(env);
158159
for i in 0..n {
159-
let addr = recipients.get(i as u32).unwrap();
160-
let bytes = addr.to_bytes();
161-
bytes_vec.push_back((bytes, i));
160+
let addr = recipients.get(i).unwrap();
161+
ordered.set(addr, i);
162162
}
163163

164-
// Insertion sort by byte representation (lexicographic).
165-
for i in 1..n {
166-
let key = bytes_vec.get(i).unwrap();
167-
let mut j = i;
168-
while j > 0 {
169-
let prev = bytes_vec.get(j - 1).unwrap();
170-
if prev.0 <= key.0 {
171-
break;
172-
}
173-
bytes_vec.set(j, bytes_vec.get(j - 1).unwrap());
174-
j -= 1;
175-
}
176-
bytes_vec.set(j, key.clone());
177-
}
178-
179-
// Reorder recipients according to sorted indices.
180164
let mut sorted = Vec::new(env);
181-
for i in 0..n {
182-
let (_, original_idx) = bytes_vec.get(i).unwrap();
183-
sorted.push_back(recipients.get(original_idx as u32).unwrap());
165+
for (addr, _) in ordered.iter() {
166+
sorted.push_back(addr);
184167
}
185168
*recipients = sorted;
186169
}
@@ -359,7 +342,7 @@ mod tests {
359342
#[test]
360343
fn single_recipient_gets_full_amount() {
361344
let env = Env::default();
362-
let r = distribute_with_remainder(&env, 12345, &make_ratios(&env, &[1]), 1);
345+
let r = distribute_with_remainder(&env, 12345, &make_ratios(&env, &[1]), 1).unwrap();
363346
assert_eq!(r.len(), 1);
364347
assert_eq!(r.get(0), Some(12345));
365348
}
@@ -369,17 +352,17 @@ mod tests {
369352
let env = Env::default();
370353
// Case 1: 3 recipients with ratios [1, 1, 1] and total=10
371354
// Total is not evenly divisible by denom (10 % 3 != 0)
372-
let r1 = distribute_with_remainder(&env, 10, &make_ratios(&env, &[1, 1, 1]), 3);
355+
let r1 = distribute_with_remainder(&env, 10, &make_ratios(&env, &[1, 1, 1]), 3).unwrap();
373356
let sum1: i128 = r1.iter().sum();
374357
assert_eq!(sum1, 10);
375358

376359
// Case 2: 4 recipients with ratios [2, 3, 1, 4] and total=100
377-
let r2 = distribute_with_remainder(&env, 100, &make_ratios(&env, &[2, 3, 1, 4]), 10);
360+
let r2 = distribute_with_remainder(&env, 100, &make_ratios(&env, &[2, 3, 1, 4]), 10).unwrap();
378361
let sum2: i128 = r2.iter().sum();
379362
assert_eq!(sum2, 100);
380363

381364
// Case 3: 2 recipients with ratios [1, 3] and total=999
382-
let r3 = distribute_with_remainder(&env, 999, &make_ratios(&env, &[1, 3]), 4);
365+
let r3 = distribute_with_remainder(&env, 999, &make_ratios(&env, &[1, 3]), 4).unwrap();
383366
let sum3: i128 = r3.iter().sum();
384367
assert_eq!(sum3, 999);
385368
}
@@ -478,8 +461,8 @@ mod tests {
478461

479462
#[test]
480463
fn test_calc_platform_fee_overflow() {
481-
// i128::MAX * any fee_bps > 0 will overflow the intermediate multiplication
482-
let result = calc_platform_fee(i128::MAX, 1);
464+
// i128::MAX * 2 overflows the intermediate multiplication
465+
let result = calc_platform_fee(i128::MAX, 2);
483466
assert_eq!(result, Err(crate::error::ContractError::ArithmeticOverflow));
484467
}
485468
}

contracts/split/src/error.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,4 +143,6 @@ pub enum ContractError {
143143
CheckpointMismatch = 64,
144144
/// Issue #564: Recipient at this index has already been paid in a prior payout attempt.
145145
AlreadyPaid = 65,
146+
/// Recipient list is shorter than the configured minimum recipient count.
147+
TooFewRecipients = 66,
146148
}

contracts/split/src/events.rs

Lines changed: 88 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
use crate::types::{DisputeOutcome, FeeSplit, InvoicePhase, InvoiceStatus, RepScore, TimelockAction};
21
//! # Event naming convention
32
//!
43
//! All split-contracts events follow a consistent topic layout:
@@ -27,7 +26,7 @@ use crate::types::{DisputeOutcome, FeeSplit, InvoicePhase, InvoiceStatus, RepSco
2726
//! dynamically.
2827
2928
use crate::storage_keys::ev_seq_key;
30-
use crate::types::{DisputeOutcome, FeeSplit, InvoiceStatus, RepScore, TimelockAction};
29+
use crate::types::{DisputeOutcome, FeeSplit, InvoicePhase, InvoiceStatus, OverfundingPolicy, RepScore, TimelockAction};
3130
use soroban_sdk::{contracttype, symbol_short, Address, BytesN, Env, String, Vec};
3231

3332
// ---------------------------------------------------------------------------
@@ -147,18 +146,18 @@ pub fn invoice_released(env: &Env, invoice_id: u64, recipients: &Vec<Address>) {
147146
);
148147
}
149148

150-
/// Emitted when an invoice is refunded after deadline.
149+
/// Emitted when an invoice is refunded.
151150
/// Topics: (split, refunded, invoice_id)
152-
/// Data: (invoice_id, event_seq)
153-
pub fn invoice_refunded(env: &Env, invoice_id: u64) {
151+
/// Data: (invoice_id, amount, event_seq)
152+
pub fn invoice_refunded(env: &Env, invoice_id: u64, amount: i128) {
154153
let event_seq = next_seq(env, invoice_id);
155154
env.events().publish(
156155
(
157156
symbol_short!("split"),
158157
symbol_short!("refunded"),
159158
invoice_id,
160159
),
161-
(invoice_id, event_seq),
160+
(invoice_id, amount, event_seq),
162161
);
163162
}
164163

@@ -719,6 +718,7 @@ pub fn invoice_state_changed(
719718
Some(InvoiceStatus::PartiallyReleased) => symbol_short!("part_rel"),
720719
Some(InvoiceStatus::Finalised) => symbol_short!("finald"),
721720
Some(InvoiceStatus::Deleted) => symbol_short!("deleted"),
721+
Some(InvoiceStatus::PayoutInProgress) => symbol_short!("pay_prg"),
722722
};
723723
let to_sym = match to_status {
724724
InvoiceStatus::Pending => symbol_short!("pending"),
@@ -730,6 +730,7 @@ pub fn invoice_state_changed(
730730
InvoiceStatus::PartiallyReleased => symbol_short!("part_rel"),
731731
InvoiceStatus::Finalised => symbol_short!("finald"),
732732
InvoiceStatus::Deleted => symbol_short!("deleted"),
733+
InvoiceStatus::PayoutInProgress => symbol_short!("pay_prg"),
733734
};
734735
env.events().publish(
735736
(symbol_short!("split"), symbol_short!("st_chg"), invoice_id),
@@ -1855,42 +1856,109 @@ pub fn admin_transfer_completed(env: &Env, new_admin: &Address) {
18551856
);
18561857
}
18571858

1859+
/// Emitted when a recipient is added to or removed from an invoice whitelist.
1860+
/// Topics: (split, wl_upd, invoice_id)
1861+
/// Data: (enabled, added_count, removed_count)
1862+
pub fn recipient_whitelist_updated(
1863+
env: &Env,
1864+
invoice_id: u64,
1865+
enabled: bool,
1866+
added: &Vec<Address>,
1867+
removed: &Vec<Address>,
1868+
) {
1869+
env.events().publish(
1870+
(symbol_short!("split"), symbol_short!("wl_upd"), invoice_id),
1871+
(enabled, added.len(), removed.len()),
1872+
);
1873+
}
1874+
1875+
/// Emitted when an overfunding event is triggered on an invoice.
1876+
/// Topics: (split, overfund, invoice_id)
1877+
/// Data: (payer, surplus)
1878+
pub fn overfunding_triggered(
1879+
env: &Env,
1880+
invoice_id: u64,
1881+
payer: &Address,
1882+
_policy: &OverfundingPolicy,
1883+
surplus: i128,
1884+
) {
1885+
env.events().publish(
1886+
(symbol_short!("split"), symbol_short!("overfund"), invoice_id),
1887+
(payer.clone(), surplus),
1888+
);
1889+
}
1890+
1891+
/// Emitted when a late-payment penalty is applied.
1892+
/// Topics: (split, penalty, invoice_id)
1893+
/// Data: (payer, penalty_amount, penalty_bps)
1894+
pub fn penalty_applied(
1895+
env: &Env,
1896+
invoice_id: u64,
1897+
payer: &Address,
1898+
penalty_amount: i128,
1899+
penalty_bps: u32,
1900+
) {
1901+
env.events().publish(
1902+
(symbol_short!("split"), symbol_short!("penalty"), invoice_id),
1903+
(payer.clone(), penalty_amount, penalty_bps),
1904+
);
1905+
}
1906+
1907+
/// Emitted when an invoice deadline is extended.
1908+
/// Topics: (split, dl_ext, invoice_id)
1909+
/// Data: (old_deadline, new_deadline)
1910+
pub fn deadline_extended(env: &Env, invoice_id: u64, old_deadline: u64, new_deadline: u64) {
1911+
env.events().publish(
1912+
(symbol_short!("split"), symbol_short!("dl_ext"), invoice_id),
1913+
(old_deadline, new_deadline),
1914+
);
1915+
}
1916+
18581917
// ---------------------------------------------------------------------------
18591918
// Unit tests for the per-invoice event sequence counter (issue #708)
18601919
// ---------------------------------------------------------------------------
18611920

18621921
#[cfg(test)]
18631922
mod tests {
18641923
use super::*;
1865-
use soroban_sdk::Env;
1924+
use soroban_sdk::{Address, Env};
1925+
1926+
fn contract_id(env: &Env) -> Address {
1927+
env.register(crate::SplitContract, ())
1928+
}
18661929

18671930
/// `next_seq` returns 1 on first call and increments on each subsequent
18681931
/// call for the same invoice ID.
18691932
#[test]
18701933
fn test_next_seq_increments_per_invoice() {
18711934
let env = Env::default();
1872-
assert_eq!(next_seq(&env, 1), 1);
1873-
assert_eq!(next_seq(&env, 1), 2);
1874-
assert_eq!(next_seq(&env, 1), 3);
1935+
let id = contract_id(&env);
1936+
env.as_contract(&id, || {
1937+
assert_eq!(next_seq(&env, 1), 1);
1938+
assert_eq!(next_seq(&env, 1), 2);
1939+
assert_eq!(next_seq(&env, 1), 3);
1940+
});
18751941
}
18761942

18771943
/// Sequences for different invoice IDs are independent — incrementing the
18781944
/// counter for invoice A must not affect invoice B's counter.
18791945
#[test]
18801946
fn test_next_seq_independent_for_different_invoice_ids() {
18811947
let env = Env::default();
1948+
let id = contract_id(&env);
1949+
env.as_contract(&id, || {
1950+
// Advance invoice 10 twice.
1951+
assert_eq!(next_seq(&env, 10), 1);
1952+
assert_eq!(next_seq(&env, 10), 2);
18821953

1883-
// Advance invoice 10 twice.
1884-
assert_eq!(next_seq(&env, 10), 1);
1885-
assert_eq!(next_seq(&env, 10), 2);
1886-
1887-
// Invoice 20 should still start at 1.
1888-
assert_eq!(next_seq(&env, 20), 1);
1954+
// Invoice 20 should still start at 1.
1955+
assert_eq!(next_seq(&env, 20), 1);
18891956

1890-
// Invoice 10 continues independently from where it left off.
1891-
assert_eq!(next_seq(&env, 10), 3);
1957+
// Invoice 10 continues independently from where it left off.
1958+
assert_eq!(next_seq(&env, 10), 3);
18921959

1893-
// Invoice 20 is still at 2 after one more call.
1894-
assert_eq!(next_seq(&env, 20), 2);
1960+
// Invoice 20 is still at 2 after one more call.
1961+
assert_eq!(next_seq(&env, 20), 2);
1962+
});
18951963
}
18961964
}

contracts/split/src/lib.rs

Lines changed: 28 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1278,6 +1278,7 @@ pub(crate) fn valid_transition(from: InvoiceStatus, to: InvoiceStatus) -> bool {
12781278
InvoiceStatus::Disputed => to == InvoiceStatus::Pending || to == InvoiceStatus::Refunded,
12791279
InvoiceStatus::Finalised => false,
12801280
InvoiceStatus::Deleted => false,
1281+
InvoiceStatus::PayoutInProgress => to == InvoiceStatus::Released || to == InvoiceStatus::Refunded,
12811282
}
12821283
}
12831284

@@ -3680,7 +3681,7 @@ impl SplitContract {
36803681
invoice.completion_time = Some(env.ledger().timestamp());
36813682
save_invoice(&env, invoice_id, &invoice);
36823683
events::dispute_resolved(&env, invoice_id, &admin, &outcome);
3683-
events::invoice_refunded(&env, invoice_id);
3684+
events::invoice_refunded(&env, invoice_id, invoice.funded);
36843685
events::invoice_state_changed(&env, invoice_id, Some(&InvoiceStatus::Disputed),
36853686
&InvoiceStatus::Refunded, &admin);
36863687
}
@@ -4189,37 +4190,37 @@ impl SplitContract {
41894190
Ok(bps as u32)
41904191
}
41914192

4192-
pub fn get_invoice_deadline(env: Env, invoice_id: u64) -> Result<u64, ContractError> {
4193-
if let Some(core) = env.storage().persistent().get(&invoice_key(invoice_id)) {
4194-
Ok(core.deadline)
4195-
} else if let Some(core) = env.storage().instance().get(&invoice_key(invoice_id)) {
4196-
Ok(core.deadline)
4193+
pub fn get_invoice_deadline(env: Env, invoice_id: u64) -> Option<u64> {
4194+
if let Some(core) = env.storage().persistent().get::<_, InvoiceCore>(&invoice_key(invoice_id)) {
4195+
Some(core.deadline)
4196+
} else if let Some(core) = env.storage().instance().get::<_, InvoiceCore>(&invoice_key(invoice_id)) {
4197+
Some(core.deadline)
41974198
} else {
4198-
Err(ContractError::InvoiceNotFound)
4199+
panic_with_error!(env, ContractError::InvoiceNotFound);
41994200
}
42004201
}
42014202

4202-
pub fn get_invoice_funded(env: Env, invoice_id: u64) -> Result<i128, ContractError> {
4203-
if let Some(hot) = env.storage().instance().get(&invoice_hot_key(invoice_id)) {
4204-
Ok(hot.funded)
4205-
} else if let Some(core) = env.storage().persistent().get(&invoice_key(invoice_id)) {
4206-
Ok(core.funded)
4207-
} else if let Some(core) = env.storage().instance().get(&invoice_key(invoice_id)) {
4208-
Ok(core.funded)
4203+
pub fn get_invoice_funded(env: Env, invoice_id: u64) -> Option<i128> {
4204+
if let Some(hot) = env.storage().instance().get::<_, InvoiceHot>(&invoice_hot_key(invoice_id)) {
4205+
Some(hot.funded)
4206+
} else if let Some(core) = env.storage().persistent().get::<_, InvoiceCore>(&invoice_key(invoice_id)) {
4207+
Some(core.funded)
4208+
} else if let Some(core) = env.storage().instance().get::<_, InvoiceCore>(&invoice_key(invoice_id)) {
4209+
Some(core.funded)
42094210
} else {
4210-
Err(ContractError::InvoiceNotFound)
4211+
panic_with_error!(env, ContractError::InvoiceNotFound);
42114212
}
42124213
}
42134214

4214-
pub fn get_invoice_status(env: Env, invoice_id: u64) -> Result<InvoiceStatus, ContractError> {
4215-
if let Some(hot) = env.storage().instance().get(&invoice_hot_key(invoice_id)) {
4216-
Ok(hot.status)
4217-
} else if let Some(core) = env.storage().persistent().get(&invoice_key(invoice_id)) {
4218-
Ok(core.status)
4219-
} else if let Some(core) = env.storage().instance().get(&invoice_key(invoice_id)) {
4220-
Ok(core.status)
4215+
pub fn get_invoice_status(env: Env, invoice_id: u64) -> Option<InvoiceStatus> {
4216+
if let Some(hot) = env.storage().instance().get::<_, InvoiceHot>(&invoice_hot_key(invoice_id)) {
4217+
Some(hot.status)
4218+
} else if let Some(core) = env.storage().persistent().get::<_, InvoiceCore>(&invoice_key(invoice_id)) {
4219+
Some(core.status)
4220+
} else if let Some(core) = env.storage().instance().get::<_, InvoiceCore>(&invoice_key(invoice_id)) {
4221+
Some(core.status)
42214222
} else {
4222-
Err(ContractError::InvoiceNotFound)
4223+
panic_with_error!(env, ContractError::InvoiceNotFound);
42234224
}
42244225
}
42254226

@@ -5410,7 +5411,7 @@ impl SplitContract {
54105411
.storage()
54115412
.instance()
54125413
.get(&min_recipients_key())
5413-
.unwrap_or(2u32);
5414+
.unwrap_or(1u32);
54145415
if (recipients.len() as u32) < min_recipients {
54155416
panic_with_error!(env, ContractError::TooFewRecipients);
54165417
}
@@ -12894,6 +12895,7 @@ impl SplitContract {
1289412895
InvoiceStatus::Disputed => 6u8,
1289512896
InvoiceStatus::Finalised => 7u8,
1289612897
InvoiceStatus::Deleted => 8u8,
12898+
InvoiceStatus::PayoutInProgress => 9u8,
1289712899
};
1289812900

1289912901
let mut preimage = [0u8; 17];
@@ -14350,7 +14352,7 @@ impl SplitContract {
1435014352
invoice.completion_time = Some(env.ledger().timestamp());
1435114353
save_invoice(&env, invoice_id, &invoice);
1435214354
events::dispute_resolved(&env, invoice_id, &admin_addr, &DisputeOutcome::Refund);
14353-
events::invoice_refunded(&env, invoice_id);
14355+
events::invoice_refunded(&env, invoice_id, invoice.funded);
1435414356
events::invoice_state_changed(
1435514357
&env,
1435614358
invoice_id,
@@ -15792,7 +15794,7 @@ impl SplitContract {
1579215794
for key in &keys {
1579315795
env.storage()
1579415796
.persistent()
15795-
.bump(key, min_ttl, max_ttl);
15797+
.extend_ttl(key, min_ttl, max_ttl);
1579615798
}
1579715799
}
1579815800

0 commit comments

Comments
 (0)