From d17cea90325457014a6b9c6cd015c09c34e72e76 Mon Sep 17 00:00:00 2001 From: brightfootlimited-collab Date: Thu, 27 Aug 2026 03:12:45 -0700 Subject: [PATCH] fix(streaming): resolve panics, add pause guards, and add delegate events #467 - Convert cleanup_stream() panics to Result return type - Replace .unwrap() panics in get_archived_sent/received_streams paginators with safe if-let guards - Add StreamCleanedUpEvent struct and emit it from cleanup_stream() - Add missing Ok(()) so the function compiles and returns correctly #468 - Add require_not_paused guard to update_stream_metadata() - Metadata could previously be mutated while the contract was paused; now matches the guard present on every other state-changing function - Add test_pause_blocks_update_stream_metadata to confirm enforcement #469 - Add pause guard and events to set_delegate() / remove_delegate() - Both functions now call require_not_paused(&env) at entry - Add DelegateSetEvent and DelegateRemovedEvent structs - Both functions now publish their respective events, matching the pattern of every other state-changing function in the contract Closes #467 Closes #468 Closes #469 --- contracts/streaming/src/lib.rs | 57 +++++- contracts/streaming/src/test_features.rs | 237 +++++++++++++++++------ 2 files changed, 230 insertions(+), 64 deletions(-) diff --git a/contracts/streaming/src/lib.rs b/contracts/streaming/src/lib.rs index 4db80db..80f2c79 100644 --- a/contracts/streaming/src/lib.rs +++ b/contracts/streaming/src/lib.rs @@ -286,6 +286,28 @@ pub struct UnpauseEvent { pub timestamp: u64, } +#[soroban_sdk::contractevent] +pub struct DelegateSetEvent { + pub stream_id: u64, + pub recipient: Address, + pub delegate: Address, + pub timestamp: u64, +} + +#[soroban_sdk::contractevent] +pub struct DelegateRemovedEvent { + pub stream_id: u64, + pub recipient: Address, + pub timestamp: u64, +} + +#[soroban_sdk::contractevent] +pub struct StreamCleanedUpEvent { + pub stream_id: u64, + pub caller: Address, + pub timestamp: u64, +} + // ─── Contract ──────────────────────────────────────────────────────────────── #[contract] @@ -1083,7 +1105,9 @@ impl StreamingContract { let mut result = Vec::new(&env); let mut i = start; while i < end { - result.push_back(all.get(i).unwrap()); + if let Some(id) = all.get(i) { + result.push_back(id); + } i += 1; } result @@ -1111,7 +1135,9 @@ impl StreamingContract { let mut result = Vec::new(&env); let mut i = start; while i < end { - result.push_back(all.get(i).unwrap()); + if let Some(id) = all.get(i) { + result.push_back(id); + } i += 1; } result @@ -1170,6 +1196,15 @@ impl StreamingContract { env.storage() .persistent() .remove(&DataKey::Delegate(stream_id)); + + StreamCleanedUpEvent { + stream_id, + caller, + timestamp: env.ledger().timestamp(), + } + .publish(&env); + + Ok(()) } // ── Write: Bump TTL ────────────────────────────────────────────────────── @@ -1199,6 +1234,7 @@ impl StreamingContract { ) -> Result<(), StreamError> { let stream = Self::load_stream(&env, stream_id)?; stream.sender.require_auth(); + Self::require_not_paused(&env)?; env.storage() .persistent() @@ -1234,6 +1270,7 @@ impl StreamingContract { pub fn set_delegate(env: Env, stream_id: u64, delegate: Address) -> Result<(), StreamError> { let stream = Self::load_stream(&env, stream_id)?; stream.recipient.require_auth(); + Self::require_not_paused(&env)?; env.storage() .persistent() @@ -1244,6 +1281,14 @@ impl StreamingContract { PERSISTENT_TTL_LEDGERS, ); + DelegateSetEvent { + stream_id, + recipient: stream.recipient, + delegate, + timestamp: env.ledger().timestamp(), + } + .publish(&env); + Ok(()) } @@ -1251,11 +1296,19 @@ impl StreamingContract { pub fn remove_delegate(env: Env, stream_id: u64) -> Result<(), StreamError> { let stream = Self::load_stream(&env, stream_id)?; stream.recipient.require_auth(); + Self::require_not_paused(&env)?; env.storage() .persistent() .remove(&DataKey::Delegate(stream_id)); + DelegateRemovedEvent { + stream_id, + recipient: stream.recipient, + timestamp: env.ledger().timestamp(), + } + .publish(&env); + Ok(()) } diff --git a/contracts/streaming/src/test_features.rs b/contracts/streaming/src/test_features.rs index 16ae6b6..8556cb7 100644 --- a/contracts/streaming/src/test_features.rs +++ b/contracts/streaming/src/test_features.rs @@ -4,7 +4,9 @@ extern crate std; use super::*; use soroban_sdk::{ - Address, Env, testutils::{Address as _, Ledger}, token::{Client as TokenClient, StellarAssetClient}, vec + testutils::{Address as _, Ledger}, + token::{Client as TokenClient, StellarAssetClient}, + vec, Address, Env, }; // ─── Test helpers ───────────────────────────────────────────────────────────── @@ -27,12 +29,20 @@ impl TestEnv { let recipient = Address::generate(&env); let token_admin = Address::generate(&env); - let token_id = env.register_stellar_asset_contract_v2(token_admin.clone()).address(); + let token_id = env + .register_stellar_asset_contract_v2(token_admin.clone()) + .address(); let asset_client = StellarAssetClient::new(&env, &token_id); asset_client.mint(&sender, &1_000_000_0000000); - TestEnv { env, contract_id, token_id, sender, recipient } + TestEnv { + env, + contract_id, + token_id, + sender, + recipient, + } } fn client(&self) -> StreamingContractClient { @@ -72,11 +82,16 @@ fn test_stream_created_event_includes_all_fields() { let params = t.default_params(now); let total = params.total_amount; - t.token().approve(&t.sender, &t.contract_id, &total, &(t.env.ledger().sequence() + 500)); - + t.token().approve( + &t.sender, + &t.contract_id, + &total, + &(t.env.ledger().sequence() + 500), + ); + // Create stream and verify event is emitted with enriched fields let stream_id = client.create_stream(&t.sender, ¶ms); - + let stream = client.get_stream(&stream_id); assert_eq!(stream.sender, t.sender); assert_eq!(stream.recipient, params.recipient); @@ -96,17 +111,22 @@ fn test_withdraw_event_includes_remaining_withdrawable() { let params = t.default_params(now); let total = params.total_amount; - t.token().approve(&t.sender, &t.contract_id, &total, &(t.env.ledger().sequence() + 500)); + t.token().approve( + &t.sender, + &t.contract_id, + &total, + &(t.env.ledger().sequence() + 500), + ); let stream_id = client.create_stream(&t.sender, ¶ms); t.set_time(now + 500); let withdrawable = client.get_withdrawable(&stream_id); - + client.withdraw(&stream_id, &withdrawable); - + let stream = client.get_stream(&stream_id); assert_eq!(stream.withdrawn_amount, withdrawable); - + let remaining = client.get_withdrawable(&stream_id); assert!(remaining > 0); } @@ -121,13 +141,18 @@ fn test_cancel_event_includes_sender_recipient_timestamp() { let params = t.default_params(now); let total = params.total_amount; - t.token().approve(&t.sender, &t.contract_id, &total, &(t.env.ledger().sequence() + 500)); + t.token().approve( + &t.sender, + &t.contract_id, + &total, + &(t.env.ledger().sequence() + 500), + ); let stream_id = client.create_stream(&t.sender, ¶ms); t.set_time(now + 500); - + client.cancel(&stream_id); - + let stream = client.get_stream(&stream_id); assert!(stream.cancelled); } @@ -142,12 +167,17 @@ fn test_bump_stream_event_emitted() { let params = t.default_params(now); let total = params.total_amount; - t.token().approve(&t.sender, &t.contract_id, &total, &(t.env.ledger().sequence() + 500)); + t.token().approve( + &t.sender, + &t.contract_id, + &total, + &(t.env.ledger().sequence() + 500), + ); let stream_id = client.create_stream(&t.sender, ¶ms); // Bump TTL should succeed without panic client.bump_stream(&stream_id); - + let stream = client.get_stream(&stream_id); assert_eq!(stream.id, stream_id); } @@ -166,12 +196,17 @@ fn test_pause_blocks_create_stream() { // Initialize with admin client.initialize(&t.sender); - + // Pause contract client.pause(); - - t.token().approve(&t.sender, &t.contract_id, &total, &(t.env.ledger().sequence() + 500)); - + + t.token().approve( + &t.sender, + &t.contract_id, + &total, + &(t.env.ledger().sequence() + 500), + ); + // Attempt to create stream should panic client.create_stream(&t.sender, ¶ms); } @@ -187,14 +222,19 @@ fn test_pause_blocks_withdraw() { let total = params.total_amount; client.initialize(&t.sender); - - t.token().approve(&t.sender, &t.contract_id, &total, &(t.env.ledger().sequence() + 500)); + + t.token().approve( + &t.sender, + &t.contract_id, + &total, + &(t.env.ledger().sequence() + 500), + ); let stream_id = client.create_stream(&t.sender, ¶ms); t.set_time(now + 500); - + client.pause(); - + let withdrawable = client.get_withdrawable(&stream_id); client.withdraw(&stream_id, &withdrawable); } @@ -210,15 +250,54 @@ fn test_pause_blocks_cancel() { let total = params.total_amount; client.initialize(&t.sender); - - t.token().approve(&t.sender, &t.contract_id, &total, &(t.env.ledger().sequence() + 500)); + + t.token().approve( + &t.sender, + &t.contract_id, + &total, + &(t.env.ledger().sequence() + 500), + ); let stream_id = client.create_stream(&t.sender, ¶ms); client.pause(); - + client.cancel(&stream_id); } +#[test] +#[should_panic(expected = "Error(Contract, #16)")] +fn test_pause_blocks_update_stream_metadata() { + let t = TestEnv::setup(); + let now = 1_000_000u64; + t.set_time(now); + + let client = t.client(); + let params = t.default_params(now); + let total = params.total_amount; + + client.initialize(&t.sender); + + t.token().approve( + &t.sender, + &t.contract_id, + &total, + &(t.env.ledger().sequence() + 500), + ); + let stream_id = client.create_stream(&t.sender, ¶ms); + + client.pause(); + + // Attempting to mutate metadata while paused must be rejected with + // ContractPaused (#16) — the same guard that protects every other + // state-changing function. + let metadata = StreamMetadata { + name: soroban_sdk::String::from_str(&t.env, "Salary"), + category: soroban_sdk::String::from_str(&t.env, "payroll"), + memo: soroban_sdk::String::from_str(&t.env, "monthly"), + }; + client.update_stream_metadata(&stream_id, &metadata); +} + #[test] fn test_read_operations_work_while_paused() { let t = TestEnv::setup(); @@ -230,16 +309,21 @@ fn test_read_operations_work_while_paused() { let total = params.total_amount; client.initialize(&t.sender); - - t.token().approve(&t.sender, &t.contract_id, &total, &(t.env.ledger().sequence() + 500)); + + t.token().approve( + &t.sender, + &t.contract_id, + &total, + &(t.env.ledger().sequence() + 500), + ); let stream_id = client.create_stream(&t.sender, ¶ms); client.pause(); - + // Read operations should still work let stream = client.get_stream(&stream_id); assert_eq!(stream.id, stream_id); - + let withdrawable = client.get_withdrawable(&stream_id); assert!(withdrawable >= 0); } @@ -255,13 +339,18 @@ fn test_unpause_allows_operations() { let total = params.total_amount; client.initialize(&t.sender); - - t.token().approve(&t.sender, &t.contract_id, &total, &(t.env.ledger().sequence() + 500)); + + t.token().approve( + &t.sender, + &t.contract_id, + &total, + &(t.env.ledger().sequence() + 500), + ); let stream_id = client.create_stream(&t.sender, ¶ms); client.pause(); client.unpause(); - + // Operations should work again t.set_time(now + 500); let withdrawable = client.get_withdrawable(&stream_id); @@ -276,9 +365,9 @@ fn test_only_admin_can_pause() { let client = t.client(); client.initialize(&t.sender); - + let other = Address::generate(&t.env); - + // Non-admin should not be able to pause // This test assumes mock_all_auths is set; otherwise auth will fail client.pause(); @@ -293,10 +382,10 @@ fn test_only_admin_can_unpause() { let client = t.client(); client.initialize(&t.sender); client.pause(); - + // Only admin can unpause client.unpause(); - + let stream = client.get_stream(&1u64); } @@ -308,11 +397,11 @@ fn test_pause_events_emitted() { let client = t.client(); client.initialize(&t.sender); - + // Emit pause event client.pause(); client.unpause(); - + // Emit unpause event - verified by not panicking } @@ -325,16 +414,21 @@ fn test_index_operations_remain_functional_after_optimization() { t.set_time(now); let client = t.client(); - + // Create 3 streams for i in 0..3 { let mut params = t.default_params(now); let total = params.total_amount; - - t.token().approve(&t.sender, &t.contract_id, &total, &(t.env.ledger().sequence() + 500 + i as u32)); + + t.token().approve( + &t.sender, + &t.contract_id, + &total, + &(t.env.ledger().sequence() + 500 + i as u32), + ); client.create_stream(&t.sender, ¶ms); } - + // Verify sent streams are indexed correctly let sent = client.get_sent_streams(&t.sender, &0, &10); assert_eq!(sent.len(), 3); @@ -347,20 +441,25 @@ fn test_remove_from_index_o1_operation() { t.set_time(now); let client = t.client(); - + // Create 5 streams for i in 0..5 { let mut params = t.default_params(now); let total = params.total_amount; - - t.token().approve(&t.sender, &t.contract_id, &total, &(t.env.ledger().sequence() + 500 + i as u32)); + + t.token().approve( + &t.sender, + &t.contract_id, + &total, + &(t.env.ledger().sequence() + 500 + i as u32), + ); client.create_stream(&t.sender, ¶ms); } - + // Transfer stream 3 (remove from recipient index) let new_recipient = Address::generate(&t.env); client.transfer_stream(&3u64, &new_recipient); - + // Verify recipient index is correct let received = client.get_received_streams(&t.recipient, &0, &10); assert_eq!(received.len(), 4); @@ -373,20 +472,25 @@ fn test_pagination_works_with_optimized_index() { t.set_time(now); let client = t.client(); - + // Create 20 streams for i in 0..20 { let mut params = t.default_params(now); let total = params.total_amount; - - t.token().approve(&t.sender, &t.contract_id, &total, &(t.env.ledger().sequence() + 500 + i as u32)); + + t.token().approve( + &t.sender, + &t.contract_id, + &total, + &(t.env.ledger().sequence() + 500 + i as u32), + ); client.create_stream(&t.sender, ¶ms); } - + // Paginate through sent streams let page1 = client.get_sent_streams(&t.sender, &0, &10); let page2 = client.get_sent_streams(&t.sender, &10, &10); - + assert_eq!(page1.len(), 10); assert_eq!(page2.len(), 10); } @@ -423,15 +527,20 @@ fn test_get_sent_stream_count_accurate() { t.set_time(now); let client = t.client(); - + for i in 0..7 { let mut params = t.default_params(now); let total = params.total_amount; - - t.token().approve(&t.sender, &t.contract_id, &total, &(t.env.ledger().sequence() + 500 + i as u32)); + + t.token().approve( + &t.sender, + &t.contract_id, + &total, + &(t.env.ledger().sequence() + 500 + i as u32), + ); client.create_stream(&t.sender, ¶ms); } - + let count = client.get_sent_stream_count(&t.sender); assert_eq!(count, 7); } @@ -443,16 +552,20 @@ fn test_get_received_stream_count_accurate() { t.set_time(now); let client = t.client(); - + for i in 0..5 { let mut params = t.default_params(now); let total = params.total_amount; - - t.token().approve(&t.sender, &t.contract_id, &total, &(t.env.ledger().sequence() + 500 + i as u32)); + + t.token().approve( + &t.sender, + &t.contract_id, + &total, + &(t.env.ledger().sequence() + 500 + i as u32), + ); client.create_stream(&t.sender, ¶ms); } - + let count = client.get_received_stream_count(&t.recipient); assert_eq!(count, 5); } -