Fix/issue 217 claim panic message tests#307
Conversation
- Add missing pause/resume event structs - Add missing Stream struct fields (paused, pause_started_at, paused_at, paused_duration, metadata) - Fix missing function signature for pause function - Fix vested_amount function syntax error - Fix create_stream to initialize token_client - Fix test syntax errors (missing function names, trailing commas) - Add #[should_panic] attribute to test_claim_fails_with_wrong_recipient
…ik4ever#217) - Update test_claim_fails_with_wrong_recipient to verify panic message 'recipient mismatch' - Add test_claim_after_stream_fully_completed to verify claiming after stream ends - Add test_claim_on_canceled_stream to verify correct error when claiming on canceled stream - Fix all compilation errors in test file (missing imports, mutable declarations, function arguments) - Fix duplicate test function names - All three new/updated tests pass successfully
|
@Phantomcall is attempting to deploy a commit to the ritik4ever's projects Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughThe contract's Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 0/1 reviews remaining, refill in 60 minutes.Comment |
|
@Phantomcall Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
contracts/src/snapshots/stellar_stream__test__create_split_stream_success.snap.new (1)
1-7:⚠️ Potential issue | 🟠 MajorAccept and commit the 4 unaccepted snapshot files.
The repository contains 4 unaccepted snapshots in
.snap.newform with no corresponding.snapfiles:
stellar_stream__test__create_split_stream_success.snap.newstellar_stream__test__pause_resume_snapshot_lifecycle.snap.newstellar_stream__test__stream_created_no_metadata.snap.newstellar_stream__test__stream_created_with_metadata.snap.newRun
cargo insta acceptor accept these snapshots through your snapshot testing workflow, then commit the resulting.snapfiles to prevent snapshot drift.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@contracts/src/snapshots/stellar_stream__test__create_split_stream_success.snap.new` around lines 1 - 7, The repository has four unaccepted snapshot files (stellar_stream__test__create_split_stream_success.snap.new, stellar_stream__test__pause_resume_snapshot_lifecycle.snap.new, stellar_stream__test__stream_created_no_metadata.snap.new, stellar_stream__test__stream_created_with_metadata.snap.new); run your snapshot acceptance workflow (e.g., execute `cargo insta accept` from the project root or use your IDE/test runner to accept the snapshots) to produce the corresponding `.snap` files, verify the changes cover the assertions around `src/test.rs` (e.g., the `children` assertion shown), and commit the generated `.snap` files so the `.snap.new` files are resolved and snapshot drift is prevented.contracts/src/snapshots/stellar_stream__test__stream_created_no_metadata.snap.new (1)
1-18:⚠️ Potential issue | 🟠 MajorResolve leftover snapshot artifact before merge (
.snap.new).Committing
*.snap.newusually means the snapshot was not accepted. Please accept/update the canonical.snapfile and remove this artifact to avoid CI/test drift.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@contracts/src/snapshots/stellar_stream__test__stream_created_no_metadata.snap.new` around lines 1 - 18, The .snap.new snapshot artifact was accidentally committed instead of updating the canonical snapshot; remove the leftover file (contracts/src/snapshots/stellar_stream__test__stream_created_no_metadata.snap.new), accept the new snapshot into the canonical .snap (updating the existing stellar_stream__test__stream_created_no_metadata.snap) so the recorded event StreamCreated { stream_id: 1, token_symbol: String(TEST), metadata: None, ... } is the canonical expected output, and ensure only the finalized .snap is committed.contracts/src/lib.rs (1)
413-445:⚠️ Potential issue | 🔴 Critical
pause/resumeare disconnected from the vesting state machine.These methods only drive
paused_at/paused_duration, butvested_amountfreezes onpausedandpause_started_at. That meanspausedoes not actually stop vesting, and a pause at ledger timestamp0can never be resumed because0is also your “not paused” sentinel. This should reuse the same state model aspause_stream/resume_streaminstead of maintaining a second one.💡 Suggested direction
- if stream.paused_at != 0 { + if stream.paused { panic!("stream is already paused"); } ... - stream.paused_at = now; + stream.paused = true; + stream.pause_started_at = Some(now); + stream.paused_at = now;- if stream.paused_at == 0 { + if !stream.paused { panic!("stream is not paused"); } let now = env.ledger().timestamp(); - let elapsed_pause = now - stream.paused_at; + let pause_started_at = stream + .pause_started_at + .unwrap_or_else(|| panic!("pause timestamp missing")); + let elapsed_pause = now.saturating_sub(pause_started_at); + stream.start_time = stream.start_time.saturating_add(elapsed_pause); stream.paused_duration += elapsed_pause; // Extend end_time so the recipient doesn't lose vesting time. stream.end_time += elapsed_pause; + stream.paused = false; + stream.pause_started_at = None; stream.paused_at = 0;Or simpler: make
pause/resumethin wrappers aroundpause_stream/resume_streamand keep only one pause representation.Also applies to: 447-481
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@contracts/src/lib.rs` around lines 413 - 445, The pause/resume functions are using their own paused_at/paused_duration fields and not the same state machine as pause_stream/resume_stream, causing vested_amount to freeze incorrectly and making ledger timestamp 0 ambiguous; fix this by refactoring pause and resume to be thin wrappers that call the existing pause_stream and resume_stream implementations (or otherwise consolidate to use the same pause_started_at/paused_duration state changes and vested_amount logic), updating references to paused_at/paused_duration in pause/resume to the shared fields (e.g., pause_stream, resume_stream, pause_started_at, paused_duration, paused_at, vested_amount, pause_started_at) so there is a single pause representation and correct vesting behavior.
🧹 Nitpick comments (2)
contracts/test_snapshots/test/test_event_emissions.1.json (1)
81-83: Narrow this snapshot to event-specific assertions.This test is about emitted events, but it also snapshots
create_streamwiring and full stream storage. The newcliff_secondsand pause fields already caused unrelated churn here; splitting storage checks into a separate test or snapshotting only event topics/data would make this much less brittle.Also applies to: 334-385, 1207-1209, 1426-1507
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@contracts/test_snapshots/test/test_event_emissions.1.json` around lines 81 - 83, The snapshot in test_event_emissions currently includes full stream storage (e.g., create_stream wiring) which makes the test brittle; narrow the snapshot to only the emitted event assertions (topics and data) or move the storage/state assertions into a separate snapshot/test. Concretely, update the assertions that reference create_stream and stream storage (the entries around the u64: 0 block and similar blocks) to only capture event topic names and payload fields, or extract those storage-related checks into a new test/snapshot so changes to fields like cliff_seconds or pause don't affect the event-emission snapshot.contracts/src/lib.rs (1)
252-307: Validate allocations before escrowing funds and creating child streams.The sum check runs only after the transfer and after every child stream has been built. Invalid input still reverts, but it burns unnecessary budget and can change the observed failure mode for large recipient lists. A read-only validation pass should happen first.
💡 Suggested refactor
- let contract_address = env.current_contract_address(); - token_client.transfer(&sender, &contract_address, &total_amount); - - let mut next_id: u64 = env - .storage() - .persistent() - .get(&DataKey::NextStreamId) - .unwrap_or(0); - let parent_stream_id = next_id + 1; - next_id = parent_stream_id; - let mut allocated_total = 0_i128; - let mut child_ids = Vec::<u64>::new(&env); for recipient_allocation in recipients.iter() { - let recipient = recipient_allocation.0.clone(); let allocation = recipient_allocation.1; if allocation <= 0 { panic!("allocation must be positive"); } allocated_total += allocation; - - next_id += 1; - let child_stream_id = next_id; - let child_stream = Stream { - sender: sender.clone(), - recipient: recipient.clone(), - token: token.clone(), - total_amount: allocation, - claimed_amount: 0, - start_time, - end_time, - cliff_seconds: 0, - canceled: false, - paused: false, - pause_started_at: None, - paused_at: 0, - paused_duration: 0, - metadata: None, - }; - env.storage() - .persistent() - .set(&DataKey::Stream(child_stream_id), &child_stream); - env.storage() - .persistent() - .set(&DataKey::ChildToParent(child_stream_id), &parent_stream_id); - child_ids.push_back(child_stream_id); - - env.events().publish( - (symbol_short!("Stream"), symbol_short!("Created")), - StreamCreated { - stream_id: child_stream_id, - sender: sender.clone(), - recipient, - token: token.clone(), - token_symbol: token_client.symbol(), - total_amount: allocation, - start_time, - end_time, - cliff_seconds: 0, - metadata: None, - }, - ); } if allocated_total != total_amount { panic!("allocations must equal total_amount"); } + + let contract_address = env.current_contract_address(); + token_client.transfer(&sender, &contract_address, &total_amount); + + let mut next_id: u64 = env + .storage() + .persistent() + .get(&DataKey::NextStreamId) + .unwrap_or(0); + let parent_stream_id = next_id + 1; + next_id = parent_stream_id; + let mut child_ids = Vec::<u64>::new(&env); + + for recipient_allocation in recipients.iter() { + let recipient = recipient_allocation.0.clone(); + let allocation = recipient_allocation.1; + next_id += 1; + // existing child creation path... + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@contracts/src/lib.rs` around lines 252 - 307, Perform a read-only validation pass over recipients before mutating state or escrowing funds: iterate through recipients (the same collection used in the loop that builds child_streams), sum allocations into allocated_total and ensure each allocation > 0 and that allocated_total == total_amount; only if validation passes proceed to increment next_id, write child Stream entries (DataKey::Stream / DataKey::ChildToParent), push child_ids, publish StreamCreated events, and perform any transfer/escrow. This moves the allocation check earlier to avoid creating child streams or consuming budget when inputs are invalid.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@contracts/src/lib.rs`:
- Around line 157-164: The creation paths (create_stream and
create_split_stream) build a TokenClient from the stored token and fail for
native streams because they use NATIVE_SENTINEL; before constructing TokenClient
(and before calling TokenClient::balance/transfer) resolve the sentinel to the
real native token address exactly as done in claim/cancel/clawback — add a
helper like resolve_token_address(env, &token) that returns the resolved Address
(checking token == NATIVE_SENTINEL and reading DataKey::NativeToken from storage
or panicking if missing) and use that resolved address to construct TokenClient
while still keeping the original token value stored on the Stream if you want
the sentinel persisted.
In `@contracts/test_snapshots/test/test_claimable_while_paused_clamped.1.json`:
- Around line 328-359: The pause handling currently sets pause_started_at but
leaves paused_at/paused_duration unchanged and only writes diagnostic
pause_stream entries; update the pause handler (the code path that writes
pause_stream) so that when pausing it also sets paused_at = pause_started_at (or
current block timestamp) and paused_duration = 0 in the same state update,
persist the full pause metadata (pause_started_at, paused_at, paused_duration,
paused=true), and emit the StreamPaused contract event (with the same stream
id/timestamp) atomically after the state write; mirror the same completeness for
the resume logic to update paused=false and paused_duration and emit the
StreamResumed event.
In
`@contracts/test_snapshots/test/test_create_split_stream_undersum_panics.1.json`:
- Around line 734-1335: The create_split_stream path is performing token
transfers and emitting Stream::Created events before validating that the sum of
allocations equals total_amount; compute the allocated_total first in the
create_split_stream implementation, compare it to total_amount and return/error
(panic) immediately if they differ, and only after that perform external calls
like transfer and child stream creation; update symbols referenced in
contracts/src/lib.rs (create_split_stream, allocated_total, total_amount,
transfer, Stream::Created) so the invariant check runs before any transfers or
child-stream creation.
In
`@contracts/test_snapshots/test/test_create_stream_fails_with_invalid_token_address.1.json`:
- Around line 153-176: The test shows a host "missing_value" when token.balance
is called through create_stream, so update create_stream to explicitly validate
the token contract and raise a stable panic (e.g., "invalid_token") before
invoking token methods; specifically, add an early check in create_stream that
verifies the token contract supports the expected interface (or required
storage/key) and call panic!("invalid_token") if it doesn't, rather than letting
token.balance trigger a host storage missing_value error — reference
create_stream and the token.balance invocation when locating where to add this
validation.
---
Outside diff comments:
In `@contracts/src/lib.rs`:
- Around line 413-445: The pause/resume functions are using their own
paused_at/paused_duration fields and not the same state machine as
pause_stream/resume_stream, causing vested_amount to freeze incorrectly and
making ledger timestamp 0 ambiguous; fix this by refactoring pause and resume to
be thin wrappers that call the existing pause_stream and resume_stream
implementations (or otherwise consolidate to use the same
pause_started_at/paused_duration state changes and vested_amount logic),
updating references to paused_at/paused_duration in pause/resume to the shared
fields (e.g., pause_stream, resume_stream, pause_started_at, paused_duration,
paused_at, vested_amount, pause_started_at) so there is a single pause
representation and correct vesting behavior.
In
`@contracts/src/snapshots/stellar_stream__test__create_split_stream_success.snap.new`:
- Around line 1-7: The repository has four unaccepted snapshot files
(stellar_stream__test__create_split_stream_success.snap.new,
stellar_stream__test__pause_resume_snapshot_lifecycle.snap.new,
stellar_stream__test__stream_created_no_metadata.snap.new,
stellar_stream__test__stream_created_with_metadata.snap.new); run your snapshot
acceptance workflow (e.g., execute `cargo insta accept` from the project root or
use your IDE/test runner to accept the snapshots) to produce the corresponding
`.snap` files, verify the changes cover the assertions around `src/test.rs`
(e.g., the `children` assertion shown), and commit the generated `.snap` files
so the `.snap.new` files are resolved and snapshot drift is prevented.
In
`@contracts/src/snapshots/stellar_stream__test__stream_created_no_metadata.snap.new`:
- Around line 1-18: The .snap.new snapshot artifact was accidentally committed
instead of updating the canonical snapshot; remove the leftover file
(contracts/src/snapshots/stellar_stream__test__stream_created_no_metadata.snap.new),
accept the new snapshot into the canonical .snap (updating the existing
stellar_stream__test__stream_created_no_metadata.snap) so the recorded event
StreamCreated { stream_id: 1, token_symbol: String(TEST), metadata: None, ... }
is the canonical expected output, and ensure only the finalized .snap is
committed.
---
Nitpick comments:
In `@contracts/src/lib.rs`:
- Around line 252-307: Perform a read-only validation pass over recipients
before mutating state or escrowing funds: iterate through recipients (the same
collection used in the loop that builds child_streams), sum allocations into
allocated_total and ensure each allocation > 0 and that allocated_total ==
total_amount; only if validation passes proceed to increment next_id, write
child Stream entries (DataKey::Stream / DataKey::ChildToParent), push child_ids,
publish StreamCreated events, and perform any transfer/escrow. This moves the
allocation check earlier to avoid creating child streams or consuming budget
when inputs are invalid.
In `@contracts/test_snapshots/test/test_event_emissions.1.json`:
- Around line 81-83: The snapshot in test_event_emissions currently includes
full stream storage (e.g., create_stream wiring) which makes the test brittle;
narrow the snapshot to only the emitted event assertions (topics and data) or
move the storage/state assertions into a separate snapshot/test. Concretely,
update the assertions that reference create_stream and stream storage (the
entries around the u64: 0 block and similar blocks) to only capture event topic
names and payload fields, or extract those storage-related checks into a new
test/snapshot so changes to fields like cliff_seconds or pause don't affect the
event-emission snapshot.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1f2172b2-7d95-4c64-999b-0f83696483e8
📒 Files selected for processing (76)
contracts/src/lib.rscontracts/src/snapshots/stellar_stream__test__create_split_stream_success.snap.newcontracts/src/snapshots/stellar_stream__test__pause_resume_snapshot_lifecycle.snap.newcontracts/src/snapshots/stellar_stream__test__stream_created_no_metadata.snap.newcontracts/src/snapshots/stellar_stream__test__stream_created_with_metadata.snap.newcontracts/src/test.rscontracts/test_snapshots/test/test_cancel_before_start_claim_attempt_panics.1.jsoncontracts/test_snapshots/test/test_cancel_before_start_recipient_claimable_is_zero.1.jsoncontracts/test_snapshots/test/test_cancel_before_start_refunds_full_amount_to_sender.1.jsoncontracts/test_snapshots/test/test_cancel_fails_with_wrong_sender.1.jsoncontracts/test_snapshots/test/test_cancel_idempotent_double_cancel_does_not_panic.1.jsoncontracts/test_snapshots/test/test_cancel_marks_stream_as_canceled.1.jsoncontracts/test_snapshots/test/test_cancel_recipient_cannot_claim_beyond_vested_at_cancel_time.1.jsoncontracts/test_snapshots/test/test_cancel_refunds_unclaimed_to_sender.1.jsoncontracts/test_snapshots/test/test_claim_after_stream_fully_completed.1.jsoncontracts/test_snapshots/test/test_claim_before_stream_start_panics.1.jsoncontracts/test_snapshots/test/test_claim_cannot_double_claim.1.jsoncontracts/test_snapshots/test/test_claim_cannot_exceed_vested_amount.1.jsoncontracts/test_snapshots/test/test_claim_fails_with_wrong_recipient.1.jsoncontracts/test_snapshots/test/test_claim_on_canceled_stream.1.jsoncontracts/test_snapshots/test/test_claim_partial_then_full.1.jsoncontracts/test_snapshots/test/test_claim_rapid_succession_prevents_double_pay.1.jsoncontracts/test_snapshots/test/test_claim_transfers_tokens_to_recipient.1.jsoncontracts/test_snapshots/test/test_claim_zero_amount_panics.1.jsoncontracts/test_snapshots/test/test_claimable_accounts_for_already_claimed.1.jsoncontracts/test_snapshots/test/test_claimable_after_end_time.1.jsoncontracts/test_snapshots/test/test_claimable_after_stream_end_caps_at_total.1.jsoncontracts/test_snapshots/test/test_claimable_at_end_time.1.jsoncontracts/test_snapshots/test/test_claimable_at_start_time.1.jsoncontracts/test_snapshots/test/test_claimable_before_stream_start_returns_zero.1.jsoncontracts/test_snapshots/test/test_claimable_during_stream_is_linear.1.jsoncontracts/test_snapshots/test/test_claimable_while_paused_clamped.1.jsoncontracts/test_snapshots/test/test_clawback_before_initialize_panics.1.jsoncontracts/test_snapshots/test/test_clawback_caps_at_unclaimed_vested.1.jsoncontracts/test_snapshots/test/test_clawback_emits_event.1.jsoncontracts/test_snapshots/test/test_clawback_non_admin_panics.1.jsoncontracts/test_snapshots/test/test_clawback_reduces_recipient_claimable.1.jsoncontracts/test_snapshots/test/test_clawback_token_conservation.1.jsoncontracts/test_snapshots/test/test_clawback_transfers_to_admin.1.jsoncontracts/test_snapshots/test/test_cliff_vesting_blocks_claim_before_cliff.1.jsoncontracts/test_snapshots/test/test_create_split_stream_creates_child_streams_and_links.1.jsoncontracts/test_snapshots/test/test_create_split_stream_empty_recipients_panics.1.jsoncontracts/test_snapshots/test/test_create_split_stream_oversum_panics.1.jsoncontracts/test_snapshots/test/test_create_split_stream_success.1.jsoncontracts/test_snapshots/test/test_create_split_stream_undersum_panics.1.jsoncontracts/test_snapshots/test/test_create_stream_fails_with_insufficient_sender_balance.1.jsoncontracts/test_snapshots/test/test_create_stream_fails_with_invalid_token_address.1.jsoncontracts/test_snapshots/test/test_create_stream_invalid_time_range_panics.1.jsoncontracts/test_snapshots/test/test_create_stream_zero_amount_panics.1.jsoncontracts/test_snapshots/test/test_event_emissions.1.jsoncontracts/test_snapshots/test/test_get_claimable_batch_empty.1.jsoncontracts/test_snapshots/test/test_get_claimable_batch_limit_exceeded.1.jsoncontracts/test_snapshots/test/test_get_claimable_batch_single_and_multi.1.jsoncontracts/test_snapshots/test/test_get_next_stream_id.1.jsoncontracts/test_snapshots/test/test_initialize_guard_clawback_rejected_before_init.1.jsoncontracts/test_snapshots/test/test_initialize_guard_different_admin_cannot_replace.1.jsoncontracts/test_snapshots/test/test_initialize_guard_double_init_panics.1.jsoncontracts/test_snapshots/test/test_initialize_guard_stores_admin_on_first_call.1.jsoncontracts/test_snapshots/test/test_metadata_multiple_labels_round_trip.1.jsoncontracts/test_snapshots/test/test_native_xlm_streaming.1.jsoncontracts/test_snapshots/test/test_no_id_collisions_across_mixed_stream_creations.1.jsoncontracts/test_snapshots/test/test_pause_already_paused_stream_panics.1.jsoncontracts/test_snapshots/test/test_pause_at_start_time_vested_is_zero.1.jsoncontracts/test_snapshots/test/test_pause_resume_freezes_vesting_and_extends_end_time.1.jsoncontracts/test_snapshots/test/test_pause_resume_normal_flow.1.jsoncontracts/test_snapshots/test/test_pause_resume_snapshot_lifecycle.1.jsoncontracts/test_snapshots/test/test_resume_stream_panic_on_missing_timestamp.1.jsoncontracts/test_snapshots/test/test_split_stream_child_ids_are_contiguous_and_match_mapping.1.jsoncontracts/test_snapshots/test/test_split_stream_claim_and_cancel_work_per_substream.1.jsoncontracts/test_snapshots/test/test_stream_created_large_metadata_no_budget_panic.1.jsoncontracts/test_snapshots/test/test_stream_created_with_metadata_snapshot.1.jsoncontracts/test_snapshots/test/test_stream_id_auto_increment_across_split_stream.1.jsoncontracts/test_snapshots/test/test_transfer_stream_claim_by_new_recipient.1.jsoncontracts/test_snapshots/test/test_transfer_stream_updates_recipient.1.jsoncontracts/test_snapshots/test/test_vested_constant_while_paused.1.jsoncontracts/test_snapshots/test/test_vesting_resumes_after_resume.1.json
| let token_client = TokenClient::new(&env, &token); | ||
| let sender_balance = token_client.balance(&sender); | ||
| if sender_balance < total_amount { | ||
| panic!("insufficient sender balance"); | ||
| } | ||
|
|
||
| // Escrow: transfer total_amount from sender into this contract. | ||
| let contract_address = env.current_contract_address(); | ||
| token_client.transfer(&sender, &contract_address, &total_amount); |
There was a problem hiding this comment.
Resolve the native sentinel before constructing TokenClient.
create_stream and create_split_stream build the client from token, but native streams pass NATIVE_SENTINEL. That address is not a token contract, so native-stream creation will fail on balance/transfer before escrow. The native-token resolution you already do in claim, cancel, and clawback needs to happen here too.
💡 Suggested fix
- let token_client = TokenClient::new(&env, &token);
+ let actual_token = resolve_token_address(&env, &token);
+ let token_client = TokenClient::new(&env, &actual_token);Use the same change in both creation paths, while still storing the original token on the Stream if you want the sentinel to remain the persisted identifier.
fn resolve_token_address(env: &Env, token: &Address) -> Address {
if token.to_string() == String::from_str(env, NATIVE_SENTINEL) {
env.storage()
.instance()
.get(&DataKey::NativeToken)
.unwrap_or_else(|| panic!("not initialized"))
} else {
token.clone()
}
}Also applies to: 236-242
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@contracts/src/lib.rs` around lines 157 - 164, The creation paths
(create_stream and create_split_stream) build a TokenClient from the stored
token and fail for native streams because they use NATIVE_SENTINEL; before
constructing TokenClient (and before calling TokenClient::balance/transfer)
resolve the sentinel to the real native token address exactly as done in
claim/cancel/clawback — add a helper like resolve_token_address(env, &token)
that returns the resolved Address (checking token == NATIVE_SENTINEL and reading
DataKey::NativeToken from storage or panicking if missing) and use that resolved
address to construct TokenClient while still keeping the original token value
stored on the Stream if you want the sentinel persisted.
| { | ||
| "key": { | ||
| "symbol": "pause_started_at" | ||
| }, | ||
| "val": { | ||
| "u64": 1500 | ||
| } | ||
| }, | ||
| { | ||
| "key": { | ||
| "symbol": "paused" | ||
| }, | ||
| "val": { | ||
| "bool": true | ||
| } | ||
| }, | ||
| { | ||
| "key": { | ||
| "symbol": "paused_at" | ||
| }, | ||
| "val": { | ||
| "u64": 0 | ||
| } | ||
| }, | ||
| { | ||
| "key": { | ||
| "symbol": "paused_duration" | ||
| }, | ||
| "val": { | ||
| "u64": 0 | ||
| } | ||
| }, |
There was a problem hiding this comment.
Complete the pause bookkeeping and emit a pause event.
At Line 333 the stream records pause_started_at = 1500, but Line 349 still leaves paused_at = 0, and Lines 1405-1458 show only diagnostic pause_stream entries with no contract event. That leaves the paused state self-contradictory and makes pause/resume history invisible to indexers. pause_stream should update the full pause metadata consistently and publish the StreamPaused event when the state change succeeds.
Also applies to: 1405-1458
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@contracts/test_snapshots/test/test_claimable_while_paused_clamped.1.json`
around lines 328 - 359, The pause handling currently sets pause_started_at but
leaves paused_at/paused_duration unchanged and only writes diagnostic
pause_stream entries; update the pause handler (the code path that writes
pause_stream) so that when pausing it also sets paused_at = pause_started_at (or
current block timestamp) and paused_duration = 0 in the same state update,
persist the full pause metadata (pause_started_at, paused_at, paused_duration,
paused=true), and emit the StreamPaused contract event (with the same stream
id/timestamp) atomically after the state write; mirror the same completeness for
the resume logic to update paused=false and paused_duration and emit the
StreamResumed event.
| { | ||
| "event": { | ||
| "ext": "v0", | ||
| "contract_id": "0000000000000000000000000000000000000000000000000000000000000001", | ||
| "type_": "diagnostic", | ||
| "body": { | ||
| "v0": { | ||
| "topics": [ | ||
| { | ||
| "symbol": "fn_call" | ||
| }, | ||
| { | ||
| "bytes": "8011bbf4cdf04e5bc6ac886935b99aa4b2c0cabde133f9d7fb3e656799f0a896" | ||
| }, | ||
| { | ||
| "symbol": "transfer" | ||
| } | ||
| ], | ||
| "data": { | ||
| "vec": [ | ||
| { | ||
| "address": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAHK3M" | ||
| }, | ||
| { | ||
| "address": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAD2KM" | ||
| }, | ||
| { | ||
| "i128": { | ||
| "hi": 0, | ||
| "lo": 1000 | ||
| } | ||
| } | ||
| ] | ||
| } | ||
| } | ||
| } | ||
| }, | ||
| "failed_call": true | ||
| }, | ||
| { | ||
| "event": { | ||
| "ext": "v0", | ||
| "contract_id": "8011bbf4cdf04e5bc6ac886935b99aa4b2c0cabde133f9d7fb3e656799f0a896", | ||
| "type_": "contract", | ||
| "body": { | ||
| "v0": { | ||
| "topics": [ | ||
| { | ||
| "symbol": "transfer" | ||
| }, | ||
| { | ||
| "address": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAHK3M" | ||
| }, | ||
| { | ||
| "address": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAD2KM" | ||
| }, | ||
| { | ||
| "string": "aaa:GAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAJXFF" | ||
| } | ||
| ], | ||
| "data": { | ||
| "i128": { | ||
| "hi": 0, | ||
| "lo": 1000 | ||
| } | ||
| } | ||
| } | ||
| } | ||
| }, | ||
| "failed_call": true | ||
| }, | ||
| { | ||
| "event": { | ||
| "ext": "v0", | ||
| "contract_id": "8011bbf4cdf04e5bc6ac886935b99aa4b2c0cabde133f9d7fb3e656799f0a896", | ||
| "type_": "diagnostic", | ||
| "body": { | ||
| "v0": { | ||
| "topics": [ | ||
| { | ||
| "symbol": "fn_return" | ||
| }, | ||
| { | ||
| "symbol": "transfer" | ||
| } | ||
| ], | ||
| "data": "void" | ||
| } | ||
| } | ||
| }, | ||
| "failed_call": true | ||
| }, | ||
| { | ||
| "event": { | ||
| "ext": "v0", | ||
| "contract_id": "0000000000000000000000000000000000000000000000000000000000000001", | ||
| "type_": "diagnostic", | ||
| "body": { | ||
| "v0": { | ||
| "topics": [ | ||
| { | ||
| "symbol": "fn_call" | ||
| }, | ||
| { | ||
| "bytes": "8011bbf4cdf04e5bc6ac886935b99aa4b2c0cabde133f9d7fb3e656799f0a896" | ||
| }, | ||
| { | ||
| "symbol": "symbol" | ||
| } | ||
| ], | ||
| "data": "void" | ||
| } | ||
| } | ||
| }, | ||
| "failed_call": true | ||
| }, | ||
| { | ||
| "event": { | ||
| "ext": "v0", | ||
| "contract_id": "8011bbf4cdf04e5bc6ac886935b99aa4b2c0cabde133f9d7fb3e656799f0a896", | ||
| "type_": "diagnostic", | ||
| "body": { | ||
| "v0": { | ||
| "topics": [ | ||
| { | ||
| "symbol": "fn_return" | ||
| }, | ||
| { | ||
| "symbol": "symbol" | ||
| } | ||
| ], | ||
| "data": { | ||
| "string": "aaa" | ||
| } | ||
| } | ||
| } | ||
| }, | ||
| "failed_call": true | ||
| }, | ||
| { | ||
| "event": { | ||
| "ext": "v0", | ||
| "contract_id": "0000000000000000000000000000000000000000000000000000000000000001", | ||
| "type_": "contract", | ||
| "body": { | ||
| "v0": { | ||
| "topics": [ | ||
| { | ||
| "symbol": "Stream" | ||
| }, | ||
| { | ||
| "symbol": "Created" | ||
| } | ||
| ], | ||
| "data": { | ||
| "map": [ | ||
| { | ||
| "key": { | ||
| "symbol": "cliff_seconds" | ||
| }, | ||
| "val": { | ||
| "u64": 0 | ||
| } | ||
| }, | ||
| { | ||
| "key": { | ||
| "symbol": "end_time" | ||
| }, | ||
| "val": { | ||
| "u64": 2000 | ||
| } | ||
| }, | ||
| { | ||
| "key": { | ||
| "symbol": "metadata" | ||
| }, | ||
| "val": "void" | ||
| }, | ||
| { | ||
| "key": { | ||
| "symbol": "recipient" | ||
| }, | ||
| "val": { | ||
| "address": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAK3IM" | ||
| } | ||
| }, | ||
| { | ||
| "key": { | ||
| "symbol": "sender" | ||
| }, | ||
| "val": { | ||
| "address": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAHK3M" | ||
| } | ||
| }, | ||
| { | ||
| "key": { | ||
| "symbol": "start_time" | ||
| }, | ||
| "val": { | ||
| "u64": 1000 | ||
| } | ||
| }, | ||
| { | ||
| "key": { | ||
| "symbol": "stream_id" | ||
| }, | ||
| "val": { | ||
| "u64": 2 | ||
| } | ||
| }, | ||
| { | ||
| "key": { | ||
| "symbol": "token" | ||
| }, | ||
| "val": { | ||
| "address": "CCABDO7UZXYE4W6GVSEGSNNZTKSLFQGKXXQTH6OX7M7GKZ4Z6CUJNGZN" | ||
| } | ||
| }, | ||
| { | ||
| "key": { | ||
| "symbol": "token_symbol" | ||
| }, | ||
| "val": { | ||
| "string": "aaa" | ||
| } | ||
| }, | ||
| { | ||
| "key": { | ||
| "symbol": "total_amount" | ||
| }, | ||
| "val": { | ||
| "i128": { | ||
| "hi": 0, | ||
| "lo": 400 | ||
| } | ||
| } | ||
| } | ||
| ] | ||
| } | ||
| } | ||
| } | ||
| }, | ||
| "failed_call": true | ||
| }, | ||
| { | ||
| "event": { | ||
| "ext": "v0", | ||
| "contract_id": "0000000000000000000000000000000000000000000000000000000000000001", | ||
| "type_": "diagnostic", | ||
| "body": { | ||
| "v0": { | ||
| "topics": [ | ||
| { | ||
| "symbol": "fn_call" | ||
| }, | ||
| { | ||
| "bytes": "8011bbf4cdf04e5bc6ac886935b99aa4b2c0cabde133f9d7fb3e656799f0a896" | ||
| }, | ||
| { | ||
| "symbol": "symbol" | ||
| } | ||
| ], | ||
| "data": "void" | ||
| } | ||
| } | ||
| }, | ||
| "failed_call": true | ||
| }, | ||
| { | ||
| "event": { | ||
| "ext": "v0", | ||
| "contract_id": "8011bbf4cdf04e5bc6ac886935b99aa4b2c0cabde133f9d7fb3e656799f0a896", | ||
| "type_": "diagnostic", | ||
| "body": { | ||
| "v0": { | ||
| "topics": [ | ||
| { | ||
| "symbol": "fn_return" | ||
| }, | ||
| { | ||
| "symbol": "symbol" | ||
| } | ||
| ], | ||
| "data": { | ||
| "string": "aaa" | ||
| } | ||
| } | ||
| } | ||
| }, | ||
| "failed_call": true | ||
| }, | ||
| { | ||
| "event": { | ||
| "ext": "v0", | ||
| "contract_id": "0000000000000000000000000000000000000000000000000000000000000001", | ||
| "type_": "contract", | ||
| "body": { | ||
| "v0": { | ||
| "topics": [ | ||
| { | ||
| "symbol": "Stream" | ||
| }, | ||
| { | ||
| "symbol": "Created" | ||
| } | ||
| ], | ||
| "data": { | ||
| "map": [ | ||
| { | ||
| "key": { | ||
| "symbol": "cliff_seconds" | ||
| }, | ||
| "val": { | ||
| "u64": 0 | ||
| } | ||
| }, | ||
| { | ||
| "key": { | ||
| "symbol": "end_time" | ||
| }, | ||
| "val": { | ||
| "u64": 2000 | ||
| } | ||
| }, | ||
| { | ||
| "key": { | ||
| "symbol": "metadata" | ||
| }, | ||
| "val": "void" | ||
| }, | ||
| { | ||
| "key": { | ||
| "symbol": "recipient" | ||
| }, | ||
| "val": { | ||
| "address": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAMDR4" | ||
| } | ||
| }, | ||
| { | ||
| "key": { | ||
| "symbol": "sender" | ||
| }, | ||
| "val": { | ||
| "address": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAHK3M" | ||
| } | ||
| }, | ||
| { | ||
| "key": { | ||
| "symbol": "start_time" | ||
| }, | ||
| "val": { | ||
| "u64": 1000 | ||
| } | ||
| }, | ||
| { | ||
| "key": { | ||
| "symbol": "stream_id" | ||
| }, | ||
| "val": { | ||
| "u64": 3 | ||
| } | ||
| }, | ||
| { | ||
| "key": { | ||
| "symbol": "token" | ||
| }, | ||
| "val": { | ||
| "address": "CCABDO7UZXYE4W6GVSEGSNNZTKSLFQGKXXQTH6OX7M7GKZ4Z6CUJNGZN" | ||
| } | ||
| }, | ||
| { | ||
| "key": { | ||
| "symbol": "token_symbol" | ||
| }, | ||
| "val": { | ||
| "string": "aaa" | ||
| } | ||
| }, | ||
| { | ||
| "key": { | ||
| "symbol": "total_amount" | ||
| }, | ||
| "val": { | ||
| "i128": { | ||
| "hi": 0, | ||
| "lo": 500 | ||
| } | ||
| } | ||
| } | ||
| ] | ||
| } | ||
| } | ||
| } | ||
| }, | ||
| "failed_call": true | ||
| }, | ||
| { | ||
| "event": { | ||
| "ext": "v0", | ||
| "contract_id": "0000000000000000000000000000000000000000000000000000000000000001", | ||
| "type_": "diagnostic", | ||
| "body": { | ||
| "v0": { | ||
| "topics": [ | ||
| { | ||
| "symbol": "log" | ||
| } | ||
| ], | ||
| "data": { | ||
| "vec": [ | ||
| { | ||
| "string": "caught panic 'allocations must equal total_amount' from contract function 'Symbol(obj#165)'" | ||
| }, | ||
| { | ||
| "address": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAHK3M" | ||
| }, | ||
| { | ||
| "address": "CCABDO7UZXYE4W6GVSEGSNNZTKSLFQGKXXQTH6OX7M7GKZ4Z6CUJNGZN" | ||
| }, | ||
| { | ||
| "i128": { | ||
| "hi": 0, | ||
| "lo": 1000 | ||
| } | ||
| }, | ||
| { | ||
| "u64": 1000 | ||
| }, | ||
| { | ||
| "u64": 2000 | ||
| }, | ||
| { | ||
| "vec": [ | ||
| { | ||
| "vec": [ | ||
| { | ||
| "address": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAK3IM" | ||
| }, | ||
| { | ||
| "i128": { | ||
| "hi": 0, | ||
| "lo": 400 | ||
| } | ||
| } | ||
| ] | ||
| }, | ||
| { | ||
| "vec": [ | ||
| { | ||
| "address": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAMDR4" | ||
| }, | ||
| { | ||
| "i128": { | ||
| "hi": 0, | ||
| "lo": 500 | ||
| } | ||
| } | ||
| ] | ||
| } | ||
| ] | ||
| } | ||
| ] | ||
| } | ||
| } | ||
| } | ||
| }, | ||
| "failed_call": true | ||
| }, | ||
| { | ||
| "event": { | ||
| "ext": "v0", | ||
| "contract_id": "0000000000000000000000000000000000000000000000000000000000000001", | ||
| "type_": "diagnostic", | ||
| "body": { | ||
| "v0": { | ||
| "topics": [ | ||
| { | ||
| "symbol": "error" | ||
| }, | ||
| { | ||
| "error": { | ||
| "wasm_vm": "invalid_action" | ||
| } | ||
| } | ||
| ], | ||
| "data": { | ||
| "string": "caught error from function" | ||
| } | ||
| } | ||
| } | ||
| }, | ||
| "failed_call": true | ||
| }, | ||
| { | ||
| "event": { | ||
| "ext": "v0", | ||
| "contract_id": null, | ||
| "type_": "diagnostic", | ||
| "body": { | ||
| "v0": { | ||
| "topics": [ | ||
| { | ||
| "symbol": "error" | ||
| }, | ||
| { | ||
| "error": { | ||
| "wasm_vm": "invalid_action" | ||
| } | ||
| } | ||
| ], | ||
| "data": { | ||
| "vec": [ | ||
| { | ||
| "string": "contract call failed" | ||
| }, | ||
| { | ||
| "symbol": "create_split_stream" | ||
| }, | ||
| { | ||
| "vec": [ | ||
| { | ||
| "address": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAHK3M" | ||
| }, | ||
| { | ||
| "address": "CCABDO7UZXYE4W6GVSEGSNNZTKSLFQGKXXQTH6OX7M7GKZ4Z6CUJNGZN" | ||
| }, | ||
| { | ||
| "i128": { | ||
| "hi": 0, | ||
| "lo": 1000 | ||
| } | ||
| }, | ||
| { | ||
| "u64": 1000 | ||
| }, | ||
| { | ||
| "u64": 2000 | ||
| }, | ||
| { | ||
| "vec": [ | ||
| { | ||
| "vec": [ | ||
| { | ||
| "address": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAK3IM" | ||
| }, | ||
| { | ||
| "i128": { | ||
| "hi": 0, | ||
| "lo": 400 | ||
| } | ||
| } | ||
| ] | ||
| }, | ||
| { | ||
| "vec": [ | ||
| { | ||
| "address": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAMDR4" | ||
| }, | ||
| { | ||
| "i128": { | ||
| "hi": 0, | ||
| "lo": 500 | ||
| } | ||
| } | ||
| ] | ||
| } | ||
| ] | ||
| } | ||
| ] | ||
| } | ||
| ] | ||
| } | ||
| } | ||
| } | ||
| }, | ||
| "failed_call": false | ||
| }, | ||
| { | ||
| "event": { | ||
| "ext": "v0", | ||
| "contract_id": null, | ||
| "type_": "diagnostic", | ||
| "body": { | ||
| "v0": { | ||
| "topics": [ | ||
| { | ||
| "symbol": "error" | ||
| }, | ||
| { | ||
| "error": { | ||
| "wasm_vm": "invalid_action" | ||
| } | ||
| } | ||
| ], | ||
| "data": { | ||
| "string": "escalating error to panic" | ||
| } | ||
| } | ||
| } | ||
| }, | ||
| "failed_call": false | ||
| } |
There was a problem hiding this comment.
Validate the allocation sum before any transfer or child-stream creation.
This trace reaches the token transfer at Line 749 and emits Stream.Created events through Line 1128 before the under-sum panic is logged at Line 1145. Even if everything is rolled back, doing external calls and building child streams before a cheap invariant check burns gas and creates reverted-event noise. Precompute allocated_total first, fail fast if it doesn't equal total_amount, and only then move funds / create children.
♻️ Suggested reshape in contracts/src/lib.rs
- let token_client = TokenClient::new(&env, &token);
- let sender_balance = token_client.balance(&sender);
- if sender_balance < total_amount {
- panic!("insufficient sender balance");
- }
- let contract_address = env.current_contract_address();
- token_client.transfer(&sender, &contract_address, &total_amount);
-
let mut next_id: u64 = env
.storage()
.persistent()
.get(&DataKey::NextStreamId)
.unwrap_or(0);
let parent_stream_id = next_id + 1;
next_id = parent_stream_id;
let mut allocated_total = 0_i128;
- let mut child_ids = Vec::<u64>::new(&env);
for recipient_allocation in recipients.iter() {
- let recipient = recipient_allocation.0.clone();
let allocation = recipient_allocation.1;
allocated_total += allocation;
- // create child stream...
}
if allocated_total != total_amount {
panic!("allocations must equal total_amount");
}
+
+ let token_client = TokenClient::new(&env, &token);
+ let sender_balance = token_client.balance(&sender);
+ if sender_balance < total_amount {
+ panic!("insufficient sender balance");
+ }
+ let contract_address = env.current_contract_address();
+ token_client.transfer(&sender, &contract_address, &total_amount);
+
+ let mut child_ids = Vec::<u64>::new(&env);
+ for recipient_allocation in recipients.iter() {
+ let recipient = recipient_allocation.0.clone();
+ let allocation = recipient_allocation.1;
+ // create child stream...
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@contracts/test_snapshots/test/test_create_split_stream_undersum_panics.1.json`
around lines 734 - 1335, The create_split_stream path is performing token
transfers and emitting Stream::Created events before validating that the sum of
allocations equals total_amount; compute the allocated_total first in the
create_split_stream implementation, compare it to total_amount and return/error
(panic) immediately if they differ, and only after that perform external calls
like transfer and child stream creation; update symbols referenced in
contracts/src/lib.rs (create_split_stream, allocated_total, total_amount,
transfer, Stream::Created) so the invariant check runs before any transfers or
child-stream creation.
| { | ||
| "event": { | ||
| "ext": "v0", | ||
| "contract_id": "0000000000000000000000000000000000000000000000000000000000000001", | ||
| "type_": "diagnostic", | ||
| "body": { | ||
| "v0": { | ||
| "topics": [ | ||
| { | ||
| "symbol": "error" | ||
| }, | ||
| { | ||
| "error": { | ||
| "storage": "missing_value" | ||
| } | ||
| } | ||
| ], | ||
| "data": { | ||
| "string": "trying to get non-existing value for contract instance" | ||
| } | ||
| } | ||
| } | ||
| }, | ||
| "failed_call": true |
There was a problem hiding this comment.
Don't lock in a host missing_value error for invalid token addresses.
Lines 166-171 show the failure coming from the token balance call as storage: missing_value, and Lines 285-346 bubble that back up as a generic contract-call failure. That means create_stream still doesn't reject bad token contracts with a stable, contract-defined error; callers just get a Soroban host/storage failure whose wording can change under you. Please normalize this into an explicit invalid-token panic before depending on token methods.
Also applies to: 267-352
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@contracts/test_snapshots/test/test_create_stream_fails_with_invalid_token_address.1.json`
around lines 153 - 176, The test shows a host "missing_value" when token.balance
is called through create_stream, so update create_stream to explicitly validate
the token contract and raise a stable panic (e.g., "invalid_token") before
invoking token methods; specifically, add an early check in create_stream that
verifies the token contract supports the expected interface (or required
storage/key) and call panic!("invalid_token") if it doesn't, rather than letting
token.balance trigger a host storage missing_value error — reference
create_stream and the token.balance invocation when locating where to add this
validation.
Add contract tests for claim panic messages and edge cases (issue #217)
Summary
This PR addresses issue #217 by adding comprehensive tests for claim function panic messages and edge cases.
Changes Made
Test Updates
test_claim_fails_with_wrong_recipient: Added
#[should_panic(expected = "recipient mismatch")]attribute to verify the exact panic message when a wrong recipient attempts to claimtest_claim_after_stream_fully_completed (NEW): Tests that a recipient can successfully claim the full total_amount after the stream has fully completed (time past end_time)
test_claim_on_canceled_stream (NEW): Tests that attempting to claim more than the vested amount on a canceled stream correctly panics with "amount exceeds claimable"
Bug Fixes
Fixed compilation errors in contracts/src/lib.rs:
Fixed compilation errors in contracts/src/test.rs:
Test Results
All three new/updated tests pass successfully:
Acceptance Criteria Met
Related Issue
Closes #217
Summary by CodeRabbit
Release Notes
New Features
Tests