From f6cbbfd407ee4d54b204c97abec5ec693932a53e Mon Sep 17 00:00:00 2001 From: itoro2 Date: Sun, 18 Oct 2026 06:07:40 +0100 Subject: [PATCH 1/3] fix(donation-vault): enforce min_deposit on top_up --- contracts/donation-vault/src/lib.rs | 14 ++++++++++++ contracts/donation-vault/src/test.rs | 34 +++++++++++++++++++++------- 2 files changed, 40 insertions(+), 8 deletions(-) diff --git a/contracts/donation-vault/src/lib.rs b/contracts/donation-vault/src/lib.rs index 39a48d5..d42ae9b 100644 --- a/contracts/donation-vault/src/lib.rs +++ b/contracts/donation-vault/src/lib.rs @@ -2236,6 +2236,11 @@ impl DonationVault { stream.donor.require_auth(); + // Return early if the stream is already cancelled to avoid rewriting storage + if stream.cancelled { + return Err(Error::StreamCancelled); + } + let now = env.ledger().timestamp(); let accrued = settle(&env, &mut stream, now)?; @@ -2407,6 +2412,9 @@ impl DonationVault { if amount <= 0 { return Err(Error::InvalidAmount); } + if amount < Self::min_deposit(env.clone()) { + return Err(Error::DepositTooLow); + } let key = DataKey::Stream(stream_id); let mut stream: Stream = env @@ -2431,6 +2439,12 @@ impl DonationVault { .balance .checked_add(amount) .ok_or(Error::ArithmeticOverflow)?; + + // Reactivate a drained stream if it now has a positive balance + if stream.status == StreamStatus::Drained && stream.balance > 0 { + stream.status = StreamStatus::Active; + } + let total_donated: i128 = env .storage() .instance() diff --git a/contracts/donation-vault/src/test.rs b/contracts/donation-vault/src/test.rs index 598cbd5..ce99d1c 100644 --- a/contracts/donation-vault/src/test.rs +++ b/contracts/donation-vault/src/test.rs @@ -1790,6 +1790,25 @@ fn top_up_checks_the_amount_before_the_stream_id() { assert_eq!(result, Err(Ok(Error::InvalidAmount))); } +#[test] +fn top_up_rejects_below_minimum_deposit() { + let s = setup(); + s.token_admin.mint(&s.donor, &1_500); + + s.client.set_min_deposit(&100); + let stream_id = s + .client + .create_stream(&s.donor, &s.ngo, &s.token.address, &1_000, &10); + + // Below minimum should fail + let result = s.client.try_top_up(&stream_id, &50); + assert_eq!(result, Err(Ok(Error::DepositTooLow))); + + // At minimum should succeed + s.client.top_up(&stream_id, &100); + assert_eq!(s.client.get_stream(&stream_id).balance, 1_100); +} + #[test] fn get_stream_on_unknown_id_fails() { let s = setup(); @@ -2900,19 +2919,19 @@ fn stream_status_state_machine_walks_each_transition() { use Step::*; // The create call is implicit for every case: a fresh stream starts Active. - // Top-up deliberately leaves a drained stream at Drained until it is - // cancelled or drained again; that is the behaviour covered by issue #298 - // and pinned here so the state machine has one source of truth. + // Top-up reactivates a drained stream if it has a positive balance after + // the top-up; that is the behaviour covered by issue #298 and pinned here + // so the state machine has one source of truth. let cases: &[(&str, &[Step])] = &[ ( - "active -> drained -> top-up keeps drained -> cancelled", + "active -> drained -> top-up reactivates -> cancelled", &[ Expect(StreamStatus::Active), Advance(100), Withdraw, Expect(StreamStatus::Drained), TopUp(100), - Expect(StreamStatus::Drained), + Expect(StreamStatus::Active), Cancel, Expect(StreamStatus::Cancelled), ], @@ -4355,14 +4374,13 @@ fn modify_rate_and_top_up_on_a_drained_stream() { assert_eq!(after_modify.status, StreamStatus::Drained); assert!(!after_modify.cancelled); - // top_up is allowed, adds balance, and leaves the stored status as - // Drained (the current code does not recompute status on top-up). + // top_up is allowed, adds balance, and reactivates the drained stream. let top_up = s.client.try_top_up(&stream_id, &500); assert_eq!(top_up, Ok(Ok(()))); let after_top_up = s.client.get_stream(&stream_id); assert_eq!(after_top_up.balance, 500); assert_eq!(after_top_up.rate, 20); - assert_eq!(after_top_up.status, StreamStatus::Drained); + assert_eq!(after_top_up.status, StreamStatus::Active); assert!(!after_top_up.cancelled); // The new balance accrues at the new rate. From 830c50d1c50467a780d04151cf0ac67b6e967de0 Mon Sep 17 00:00:00 2001 From: itoro2 Date: Sun, 18 Oct 2026 06:10:59 +0100 Subject: [PATCH 2/3] fix(donation-vault): don't rewrite a cancelled stream on cancel --- contracts/donation-vault/src/test.rs | 36 +++++++++++----------------- 1 file changed, 14 insertions(+), 22 deletions(-) diff --git a/contracts/donation-vault/src/test.rs b/contracts/donation-vault/src/test.rs index ce99d1c..888b0f2 100644 --- a/contracts/donation-vault/src/test.rs +++ b/contracts/donation-vault/src/test.rs @@ -1639,33 +1639,25 @@ fn cancel_stream_twice_is_harmless() { assert_eq!(stream.withdrawn, 500); assert!(stream.cancelled); - // Cancelling again settles zero (rate and balance are already zero) and - // refunds zero, leaving balances and stream state unchanged. + // Cancelling again returns StreamCancelled error without rewriting storage + // or resetting last_update, maintaining consistency with top_up and modify_rate. s.env.ledger().with_mut(|l| l.timestamp += 50); - let refund = s.client.cancel_stream(&stream_id); - assert_eq!(refund, 0); - - // Sampled here, before any further SDK call: `events().all()` only covers - // the most recent invocation, so reading a balance or the stream below - // would reset the buffer and make the comparison below vacuous. - let events_after_second_cancel = event_count(&s.env, &s.client.address); + let last_update_before_second_cancel = stream.last_update; + + let result = s.client.try_cancel_stream(&stream_id); + assert_eq!(result, Err(Ok(Error::StreamCancelled))); + // Balances remain unchanged assert_eq!(s.token.balance(&s.ngo), 500); assert_eq!(s.token.balance(&s.donor), 500); - let stream = s.client.get_stream(&stream_id); - assert_eq!(stream.balance, 0); - assert_eq!(stream.rate, 0); - assert_eq!(stream.withdrawn, 500); - assert!(stream.cancelled); - - // accrued and refund were both zero, so the second cancel must publish - // nothing. A duplicate cancel event is indistinguishable from a real - // cancellation to an indexer watching the topic (issue #91). - assert_eq!( - events_after_second_cancel, 0, - "a duplicate cancel event was emitted for an already-cancelled stream" - ); + // Stream state remains unchanged (not rewritten) + let stream_after = s.client.get_stream(&stream_id); + assert_eq!(stream_after.balance, 0); + assert_eq!(stream_after.rate, 0); + assert_eq!(stream_after.withdrawn, 500); + assert!(stream_after.cancelled); + assert_eq!(stream_after.last_update, last_update_before_second_cancel); } #[test] From 57b90753539d954fd154ce12d6913764d7ea3305 Mon Sep 17 00:00:00 2001 From: itoro2 Date: Sun, 18 Oct 2026 06:12:34 +0100 Subject: [PATCH 3/3] fix(donation-vault): mark streams drained when settle empties them --- contracts/donation-vault/src/lib.rs | 5 +++++ contracts/donation-vault/src/test.rs | 32 ++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+) diff --git a/contracts/donation-vault/src/lib.rs b/contracts/donation-vault/src/lib.rs index d42ae9b..bc55aa3 100644 --- a/contracts/donation-vault/src/lib.rs +++ b/contracts/donation-vault/src/lib.rs @@ -550,6 +550,11 @@ fn settle(env: &Env, stream: &mut Stream, now: u64) -> Result { } stream.last_update = now; + // Mark as Drained if settle emptied the balance + if stream.balance == 0 && stream.status == StreamStatus::Active { + stream.status = StreamStatus::Drained; + } + // External token transfer is the last step if accrued > 0 { let token_client = token::Client::new(env, &stream.token); diff --git a/contracts/donation-vault/src/test.rs b/contracts/donation-vault/src/test.rs index 888b0f2..f728030 100644 --- a/contracts/donation-vault/src/test.rs +++ b/contracts/donation-vault/src/test.rs @@ -4380,6 +4380,38 @@ fn modify_rate_and_top_up_on_a_drained_stream() { assert_eq!(s.client.pending_accrual(&stream_id), 200); } +#[test] +fn settle_marks_stream_drained_when_balance_reaches_zero() { + let s = setup(); + s.token_admin.mint(&s.donor, &1_000); + + // Create a stream that will drain exactly after modify_rate settles + let stream_id = s + .client + .create_stream(&s.donor, &s.ngo, &s.token.address, &1_000, &10); + + // Advance exactly 100 seconds so the entire balance accrues + s.env.ledger().with_mut(|l| l.timestamp += 100); + + // modify_rate calls settle, which should drain the stream and mark it Drained + s.client.modify_rate(&stream_id, &20); + + let stream = s.client.get_stream(&stream_id); + assert_eq!(stream.balance, 0); + assert_eq!(stream.rate, 20); + assert_eq!(stream.status, StreamStatus::Drained); + assert!(!stream.cancelled); + + // Verify the NGO received all accrued funds + assert_eq!(s.token.balance(&s.ngo), 1_000); + + // modify_rate is still allowed on a drained stream + s.client.modify_rate(&stream_id, &5); + let after = s.client.get_stream(&stream_id); + assert_eq!(after.rate, 5); + assert_eq!(after.status, StreamStatus::Drained); +} + // ============================================================================= // configuration events (issue #277) // =============================================================================