tests: add settlement panic coverage and fix settlement event assertions#625
tests: add settlement panic coverage and fix settlement event assertions#625GoSTEAN wants to merge 1 commit intoSynapse-bridgez:mainfrom
Conversation
|
@GoSTEAN 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.
Pull request overview
This PR updates tests/contract_test.rs to compile against the current register_deposit signature and adds/adjusts settlement-related tests (event assertions and panic-path coverage).
Changes:
- Added the missing
callback_typeargument (&None) to someregister_depositcalls in tests. - Reworked the settlement event assertion test to index into the event stream and validate
Settled/SettlementFinalizedpayloads. - Added new panic tests for
finalize_settlementtotal mismatch and double-settle scenarios.
Comments suppressed due to low confidence (1)
tests/contract_test.rs:1256
- This event-order assertion assumes the last 3 events are
Settled,Settled,SettlementFinalized, butfinalize_settlementalso emitsSettlementCreatedbeforeSettlementFinalized(seesrc/lib.rsfinalize_settlement). As written,event_count - 3will likely point atSettled(tx_id_2, ...)(orSettlementCreated), making the test fail. Update the test to account for theSettlementCreatedevent (e.g., assert 4 events at the end, or search the event stream for the expected sequence).
let (event_contract_1, event_topics_1, event_data_1) = all_events.get(event_count - 3).unwrap();
let (event_contract_2, event_topics_2, event_data_2) = all_events.get(event_count - 2).unwrap();
let (event_contract_3, event_topics_3, event_data_3) = all_events.get(event_count - 1).unwrap();
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| let tx_id = client.register_deposit( | ||
| &relayer, | ||
| &SorobanString::from_str(&env, "lifecycle-guard-1"), | ||
| &Address::generate(&env), | ||
| &50_000_000, | ||
| &usd(&env), | ||
| &None, | ||
| &None, | ||
| &None, | ||
| ); |
There was a problem hiding this comment.
This call site was updated to pass the new register_deposit arguments (memo, memo_type, callback_type). However, other tests in this file still call client.register_deposit with fewer arguments (e.g. mark_processing_panics_when_completed / mark_processing_panics_when_failed above), which will not compile against the current contract signature. Update those remaining call sites (or route them through a helper) so the test suite compiles.
| #[test] | ||
| #[should_panic(expected = "total_amount mismatch")] | ||
| fn test_finalize_settlement_panics_on_total_mismatch() { | ||
| let env = Env::default(); | ||
| let (admin, _, client) = setup(&env); | ||
| let relayer = Address::generate(&env); | ||
| client.grant_relayer(&admin, &relayer); | ||
| client.add_asset(&admin, &usd(&env)); | ||
|
|
||
| let tx_id = client.register_deposit( | ||
| &relayer, | ||
| &SorobanString::from_str(&env, "mismatch-total-1"), | ||
| &Address::generate(&env), | ||
| &100_000_000, | ||
| &usd(&env), | ||
| &None, | ||
| &None, | ||
| &None, | ||
| ); | ||
| client.mark_processing(&relayer, &tx_id); | ||
| client.mark_completed(&relayer, &tx_id); | ||
|
|
||
| client.finalize_settlement( | ||
| &relayer, | ||
| &usd(&env), | ||
| &vec![&env, tx_id], | ||
| &999_999_999, | ||
| &0u64, |
There was a problem hiding this comment.
#[should_panic(expected = "total_amount mismatch")] will fail because finalize_settlement currently doesn’t validate total_amount against the sum of the referenced transactions (no such panic is emitted in src/lib.rs). Either add the total-sum validation to the contract implementation, or change/remove this test to match the current behavior.
| #[test] | ||
| #[should_panic(expected = "total_amount mismatch")] | ||
| fn test_finalize_settlement_panics_on_total_mismatch() { | ||
| let env = Env::default(); | ||
| let (admin, _, client) = setup(&env); | ||
| let relayer = Address::generate(&env); | ||
| client.grant_relayer(&admin, &relayer); | ||
| client.add_asset(&admin, &usd(&env)); | ||
|
|
||
| let tx_id = client.register_deposit( | ||
| &relayer, | ||
| &SorobanString::from_str(&env, "mismatch-total-1"), | ||
| &Address::generate(&env), | ||
| &100_000_000, | ||
| &usd(&env), | ||
| &None, | ||
| &None, | ||
| &None, | ||
| ); | ||
| client.mark_processing(&relayer, &tx_id); | ||
| client.mark_completed(&relayer, &tx_id); | ||
|
|
||
| client.finalize_settlement( | ||
| &relayer, | ||
| &usd(&env), | ||
| &vec![&env, tx_id], | ||
| &999_999_999, | ||
| &0u64, | ||
| &1u64, | ||
| ); | ||
| } | ||
|
|
||
| #[test] | ||
| #[should_panic(expected = "transaction already settled")] | ||
| fn test_finalize_settlement_panics_on_double_settle() { | ||
| let env = Env::default(); | ||
| let (admin, _, client) = setup(&env); | ||
| let relayer = Address::generate(&env); | ||
| client.grant_relayer(&admin, &relayer); | ||
| client.add_asset(&admin, &usd(&env)); | ||
|
|
||
| let tx_id = client.register_deposit( | ||
| &relayer, | ||
| &SorobanString::from_str(&env, "double-settle-1"), | ||
| &Address::generate(&env), | ||
| &50_000_000, | ||
| &usd(&env), | ||
| &None, | ||
| &None, | ||
| &None, | ||
| ); | ||
| client.mark_processing(&relayer, &tx_id); | ||
| client.mark_completed(&relayer, &tx_id); | ||
|
|
||
| client.finalize_settlement( | ||
| &relayer, | ||
| &usd(&env), | ||
| &vec![&env, tx_id.clone()], | ||
| &50_000_000, | ||
| &0u64, | ||
| &1u64, | ||
| ); | ||
|
|
||
| client.finalize_settlement( | ||
| &relayer, | ||
| &usd(&env), | ||
| &vec![&env, tx_id], | ||
| &50_000_000, | ||
| &0u64, | ||
| &1u64, | ||
| ); | ||
| } | ||
|
|
||
| #[test] |
There was a problem hiding this comment.
These newly added settlement panic tests duplicate existing coverage later in this same file (e.g. finalize_settlement_panics_when_total_amount_mismatch and settle_already_settled_tx_panics). Keeping both copies increases maintenance burden and can introduce contradictory expectations; consider removing the duplicates or consolidating into a single set of tests.
| #[test] | |
| #[should_panic(expected = "total_amount mismatch")] | |
| fn test_finalize_settlement_panics_on_total_mismatch() { | |
| let env = Env::default(); | |
| let (admin, _, client) = setup(&env); | |
| let relayer = Address::generate(&env); | |
| client.grant_relayer(&admin, &relayer); | |
| client.add_asset(&admin, &usd(&env)); | |
| let tx_id = client.register_deposit( | |
| &relayer, | |
| &SorobanString::from_str(&env, "mismatch-total-1"), | |
| &Address::generate(&env), | |
| &100_000_000, | |
| &usd(&env), | |
| &None, | |
| &None, | |
| &None, | |
| ); | |
| client.mark_processing(&relayer, &tx_id); | |
| client.mark_completed(&relayer, &tx_id); | |
| client.finalize_settlement( | |
| &relayer, | |
| &usd(&env), | |
| &vec![&env, tx_id], | |
| &999_999_999, | |
| &0u64, | |
| &1u64, | |
| ); | |
| } | |
| #[test] | |
| #[should_panic(expected = "transaction already settled")] | |
| fn test_finalize_settlement_panics_on_double_settle() { | |
| let env = Env::default(); | |
| let (admin, _, client) = setup(&env); | |
| let relayer = Address::generate(&env); | |
| client.grant_relayer(&admin, &relayer); | |
| client.add_asset(&admin, &usd(&env)); | |
| let tx_id = client.register_deposit( | |
| &relayer, | |
| &SorobanString::from_str(&env, "double-settle-1"), | |
| &Address::generate(&env), | |
| &50_000_000, | |
| &usd(&env), | |
| &None, | |
| &None, | |
| &None, | |
| ); | |
| client.mark_processing(&relayer, &tx_id); | |
| client.mark_completed(&relayer, &tx_id); | |
| client.finalize_settlement( | |
| &relayer, | |
| &usd(&env), | |
| &vec![&env, tx_id.clone()], | |
| &50_000_000, | |
| &0u64, | |
| &1u64, | |
| ); | |
| client.finalize_settlement( | |
| &relayer, | |
| &usd(&env), | |
| &vec![&env, tx_id], | |
| &50_000_000, | |
| &0u64, | |
| &1u64, | |
| ); | |
| } | |
| #[test] | |
| #[test] |
| #[test] | ||
| #[should_panic(expected = "total_amount mismatch")] | ||
| fn test_finalize_settlement_panics_on_total_mismatch() { | ||
| let env = Env::default(); | ||
| let (admin, _, client) = setup(&env); | ||
| let relayer = Address::generate(&env); | ||
| client.grant_relayer(&admin, &relayer); | ||
| client.add_asset(&admin, &usd(&env)); | ||
|
|
||
| let tx_id = client.register_deposit( | ||
| &relayer, | ||
| &SorobanString::from_str(&env, "mismatch-total-1"), | ||
| &Address::generate(&env), | ||
| &100_000_000, | ||
| &usd(&env), | ||
| &None, | ||
| &None, | ||
| &None, | ||
| ); | ||
| client.mark_processing(&relayer, &tx_id); | ||
| client.mark_completed(&relayer, &tx_id); | ||
|
|
||
| client.finalize_settlement( | ||
| &relayer, | ||
| &usd(&env), | ||
| &vec![&env, tx_id], | ||
| &999_999_999, | ||
| &0u64, | ||
| &1u64, | ||
| ); | ||
| } | ||
|
|
||
| #[test] | ||
| #[should_panic(expected = "transaction already settled")] | ||
| fn test_finalize_settlement_panics_on_double_settle() { | ||
| let env = Env::default(); | ||
| let (admin, _, client) = setup(&env); |
There was a problem hiding this comment.
New tests here are the only ones in this file using the test_ prefix (most others are named descriptively without it). Rename them to match the existing naming pattern for consistency (e.g., finalize_settlement_panics_on_total_mismatch / finalize_settlement_panics_on_double_settle).
Description:
This PR adds and fixes test coverage in contract_test.rs for settlement and lifecycle guard scenarios.
Closes #602
Closes #604
Closes #605
Closes #606
What changed: