diff --git a/contracts/split/src/events.rs b/contracts/split/src/events.rs index a6b90e2..3c1f205 100644 --- a/contracts/split/src/events.rs +++ b/contracts/split/src/events.rs @@ -323,6 +323,14 @@ pub fn partial_refund_issued(env: &Env, invoice_id: u64, creator: &Address, bps: ); } + +/// Emitted when a recipient is substituted (Issue #230). +/// Topics: (split, sub_rec, invoice_id) +/// Data: (old_recipient, new_recipient) +pub fn recipient_updated(env: &Env, invoice_id: u64, old_recipient: &Address, new_recipient: &Address) { + env.events().publish( + (symbol_short!("split"), symbol_short!("sub_rec"), invoice_id), + (old_recipient.clone(), new_recipient.clone()), /// Emitted when a refund encounters insufficient balance and partial refunds are distributed. /// Topics: (split, ref_short, invoice_id) /// Data: shortfall_amount diff --git a/contracts/split/src/lib.rs b/contracts/split/src/lib.rs index 9e6a3b2..cbc2fd1 100644 --- a/contracts/split/src/lib.rs +++ b/contracts/split/src/lib.rs @@ -501,6 +501,7 @@ fn load_invoice(env: &Env, id: u64) -> Invoice { min_payment: 0, min_funding_amount: 0, priorities: Vec::new(env), + substitute_recipient_approvals: Vec::new(env), }); // Load compact representation if available @@ -1489,6 +1490,7 @@ impl SplitContract { min_payment: 0, min_funding_amount: 0, priorities: Vec::new(&env), + substitute_recipient_approvals: Vec::new(&env), }) }); let audit_log: Vec = get_audit_log(&env, invoice_id); @@ -5757,6 +5759,101 @@ impl SplitContract { env.storage().persistent().set(&key, &ids); } + /// Issue #230: Substitute a recipient address (e.g., if original address was compromised). + /// With co-signers configured, requires a fresh round of required_signatures approvals. + /// Without co-signers, creator auth alone suffices. + /// Recipient's corresponding amounts/claimed/tokens entries carry over to the new address. + pub fn substitute_recipient( + env: Env, + caller: Address, + invoice_id: u64, + old_recipient: Address, + new_recipient: Address, + ) { + require_not_paused(&env); + caller.require_auth(); + + let mut invoice = load_invoice(&env, invoice_id); + + assert!( + invoice.status == InvoiceStatus::Pending, + "invoice is not pending" + ); + assert!(!invoice.disputed, "invoice is disputed"); + assert!(invoice.creator == caller, "only creator can substitute recipient"); + + // Find the old recipient's index + let mut recipient_idx: Option = None; + for (idx, recipient) in invoice.recipients.iter().enumerate() { + if recipient == &old_recipient { + recipient_idx = Some(idx as u64); + break; + } + } + let idx = recipient_idx.expect("recipient not found") as usize; + + // If co-signers are configured, require a fresh round of approvals for this substitution + if !invoice.co_signers.is_empty() { + // Require fresh approvals from co-signers for this specific substitution + // Track approvals separately from release approvals + if invoice.substitute_recipient_approvals.len() < invoice.required_signatures as usize { + panic!("insufficient approvals for recipient substitution"); + } + // Clear the approval list after successful substitution + invoice.substitute_recipient_approvals.clear(); + } + + // Perform the substitution: update the recipient at idx + invoice.recipients.set(idx, new_recipient.clone()); + + save_invoice(&env, invoice_id, &invoice); + append_audit_entry(&env, invoice_id, symbol_short!("sub_rec"), &caller); + events::recipient_updated(&env, invoice_id, &old_recipient, &new_recipient); + + // Update recipient index: remove old_recipient, add new_recipient + let old_key = recipient_invoice_ids_key(&old_recipient); + if let Some(mut old_ids) = env.storage().persistent().get::<_, Vec>(&old_key) { + old_ids.retain(|id| id != &invoice_id); + if old_ids.is_empty() { + env.storage().persistent().remove(&old_key); + } else { + env.storage().persistent().set(&old_key, &old_ids); + } + } + + let new_key = recipient_invoice_ids_key(&new_recipient); + let mut new_ids: Vec = env + .storage() + .persistent() + .get(&new_key) + .unwrap_or_else(|| Vec::new(&env)); + new_ids.push_back(invoice_id); + env.storage().persistent().set(&new_key, &new_ids); + } + + /// Approve a pending recipient substitution. Only a configured co-signer may call this. + /// This approval is separate from release approvals. + pub fn approve_substitute_recipient(env: Env, invoice_id: u64, co_signer: Address) { + require_not_paused(&env); + co_signer.require_auth(); + + let mut invoice = load_invoice(&env, invoice_id); + + assert!( + invoice.co_signers.iter().any(|cs| cs == &co_signer), + "not a co-signer for this invoice" + ); + + // Check if this co-signer has already approved + if invoice.substitute_recipient_approvals.iter().any(|addr| addr == &co_signer) { + panic!("co-signer has already approved substitution"); + } + + invoice.substitute_recipient_approvals.push_back(co_signer.clone()); + save_invoice(&env, invoice_id, &invoice); + append_audit_entry(&env, invoice_id, symbol_short!("app_sub"), &co_signer); + } + // ----------------------------------------------------------------------- // Adjust split // ----------------------------------------------------------------------- @@ -6451,6 +6548,8 @@ impl SplitContract { cross_chain_ref: None, require_kyc: false, arbiter: None, disputed: false, admin_frozen: false, auction_on_expiry: false, auction_end: 0, bids: Vec::new(&env), + min_payment: 0, min_funding_amount: 0, priorities: Vec::new(&env), + substitute_recipient_approvals: Vec::new(&env), min_payment: 0, creation_timestamp: 0, min_payment_increment: 0, min_funding_amount: 0, priorities: Vec::new(&env), }); @@ -6506,6 +6605,7 @@ impl SplitContract { admin_frozen: false, auction_on_expiry: false, auction_end: 0, bids: Vec::new(&env), min_payment: 0, min_funding_amount: 0, priorities: Vec::new(&env), + substitute_recipient_approvals: Vec::new(&env), }); env.storage().instance().set(&invoice_key(id), &core); diff --git a/contracts/split/src/test.rs b/contracts/split/src/test.rs index bf1e60f..ed7abbe 100644 --- a/contracts/split/src/test.rs +++ b/contracts/split/src/test.rs @@ -5144,6 +5144,98 @@ fn test_auto_resolve_no_rules_match_no_fallback_is_noop() { let (env, contract_id, token_id) = setup(); let c = client(&env, &contract_id); let tk = token_client(&env, &token_id); + + let creator = Address::generate(&env); + let payer = Address::generate(&env); + let recipient = Address::generate(&env); + + StellarAssetClient::new(&env, &token_id).mint(&payer, &100); + env.ledger().set_timestamp(1_000); + + // Create invoice with 80% rule (Release) but only 50% funded + // and NO fallback_action + let mut rules = Vec::new(&env); + rules.push_back(types::ResolveRule { + min_funded_bps: 8000, // 80% + action: types::ResolveAction::Release, + }); + let mut opts = default_options(&env); + opts.auto_resolve_rules = rules; + opts.fallback_action = None; + + let mut recipients = Vec::new(&env); + recipients.push_back(recipient.clone()); + let mut amounts = Vec::new(&env); + amounts.push_back(100_i128); + let id = c.create_invoice(&creator, &recipients, &amounts, &token_id, &9_999_u64, &opts); + + // Pay 50 (50% of 100) + c.pay(&payer, &id, &50_i128, &0_u64, &false, &false); + + assert_eq!(c.get_invoice(&id).status, InvoiceStatus::Pending); + assert_eq!(tk.balance(&payer), 50); + assert_eq!(tk.balance(&recipient), 0); + + // Call auto_resolve; should be a no-op (no rule matches, no fallback) + c.auto_resolve(&id); + + // Invoice should still be Pending, payments unchanged + assert_eq!(c.get_invoice(&id).status, InvoiceStatus::Pending); + assert_eq!(tk.balance(&payer), 50); + assert_eq!(tk.balance(&recipient), 0); + + // Calling auto_resolve again should still be a no-op (idempotent) + c.auto_resolve(&id); + assert_eq!(c.get_invoice(&id).status, InvoiceStatus::Pending); +} + +#[test] +fn test_auto_resolve_rule_matches_ignores_fallback() { + let (env, contract_id, token_id) = setup(); + let c = client(&env, &contract_id); + let tk = token_client(&env, &token_id); + + let creator = Address::generate(&env); + let payer = Address::generate(&env); + let recipient = Address::generate(&env); + + StellarAssetClient::new(&env, &token_id).mint(&payer, &100); + env.ledger().set_timestamp(1_000); + + // Create invoice with 50% rule (Release) and 50% funded + // and fallback_action = Refund (but should be ignored) + let mut rules = Vec::new(&env); + rules.push_back(types::ResolveRule { + min_funded_bps: 5000, // 50% + action: types::ResolveAction::Release, + }); + let mut opts = default_options(&env); + opts.auto_resolve_rules = rules; + opts.fallback_action = Some(types::ResolveAction::Refund); + + let mut recipients = Vec::new(&env); + recipients.push_back(recipient.clone()); + let mut amounts = Vec::new(&env); + amounts.push_back(100_i128); + let id = c.create_invoice(&creator, &recipients, &amounts, &token_id, &9_999_u64, &opts); + + // Pay exactly 50 (50% of 100) + c.pay(&payer, &id, &50_i128, &0_u64, &false, &false); + + assert_eq!(c.get_invoice(&id).status, InvoiceStatus::Pending); + + // Call auto_resolve; should execute the rule (Release), not fallback + c.auto_resolve(&id); + + assert_eq!(c.get_invoice(&id).status, InvoiceStatus::Released); + assert_eq!(tk.balance(&recipient), 50); +} + +#[test] +fn test_auto_resolve_idempotency_second_call_noop() { + let (env, contract_id, token_id) = setup(); + let c = client(&env, &contract_id); + let tk = token_client(&env, &token_id); let admin = Address::generate(&env); let creator = Address::generate(&env); let payer = Address::generate(&env); @@ -5283,6 +5375,9 @@ fn test_creator_self_limit_enforced_in_create_invoice() { StellarAssetClient::new(&env, &token_id).mint(&payer, &100); env.ledger().set_timestamp(1_000); + // Create invoice with fallback_action = Release + let mut opts = default_options(&env); + opts.fallback_action = Some(types::ResolveAction::Release); // Create invoice with 80% rule (Release) but only 50% funded // and NO fallback_action let mut rules = Vec::new(&env); @@ -5300,6 +5395,97 @@ fn test_creator_self_limit_enforced_in_create_invoice() { amounts.push_back(100_i128); let id = c.create_invoice(&creator, &recipients, &amounts, &token_id, &9_999_u64, &opts); + // Pay 30 (30% of 100) + c.pay(&payer, &id, &30_i128, &0_u64, &false, &false); + + assert_eq!(c.get_invoice(&id).status, InvoiceStatus::Pending); + + // First auto_resolve call executes fallback (Release) + c.auto_resolve(&id); + assert_eq!(c.get_invoice(&id).status, InvoiceStatus::Released); + assert_eq!(tk.balance(&recipient), 30); + + // Second auto_resolve call should be a no-op (invoice not Pending) + // and not panic about "invoice is not pending" + c.auto_resolve(&id); + assert_eq!(c.get_invoice(&id).status, InvoiceStatus::Released); + assert_eq!(tk.balance(&recipient), 30); // unchanged +} + +#[test] +#[should_panic(expected = "invoice under dispute")] +fn test_refund_blocked_when_disputed() { + let (env, contract_id, token_id) = setup(); + let c = client(&env, &contract_id); + let tk = token_client(&env, &token_id); + + let creator = Address::generate(&env); + let arbiter = Address::generate(&env); + let payer = Address::generate(&env); + let recipient = Address::generate(&env); + + StellarAssetClient::new(&env, &token_id).mint(&payer, &100); + env.ledger().set_timestamp(1_000); + + let id = make_invoice(&env, &c, &creator, &recipient, 500, &token_id, 2_000); + + // Set arbiter so disputes can be raised + let admin = Address::generate(&env); + c.set_arbiter(&admin, &id, &arbiter); + + // Pay 100 + c.pay(&payer, &id, &100_i128, &0_u64, &false, &false); + + // Raise dispute + c.raise_dispute(&id, &arbiter); + assert!(c.get_invoice(&id).disputed); + + // Move time far past deadline + env.ledger().set_timestamp(10_000); + + // Attempt refund should panic with "invoice under dispute" + // even though deadline has long since passed + c.refund(&id); +} + +#[test] +fn test_refund_allowed_after_dispute_resolved() { + let (env, contract_id, token_id) = setup(); + let c = client(&env, &contract_id); + let tk = token_client(&env, &token_id); + + let creator = Address::generate(&env); + let arbiter = Address::generate(&env); + let payer = Address::generate(&env); + let recipient = Address::generate(&env); + + StellarAssetClient::new(&env, &token_id).mint(&payer, &100); + env.ledger().set_timestamp(1_000); + + let id = make_invoice(&env, &c, &creator, &recipient, 500, &token_id, 2_000); + + // Set arbiter + let admin = Address::generate(&env); + c.set_arbiter(&admin, &id, &arbiter); + + // Pay 100 + c.pay(&payer, &id, &100_i128, &0_u64, &false, &false); + + // Raise dispute + c.raise_dispute(&id, &arbiter); + assert!(c.get_invoice(&id).disputed); + + // Resolve the dispute with Refund + c.resolve_dispute(&id, &arbiter, types::ResolveAction::Refund); + + // Verify invoice is now Refunded and balance restored + assert_eq!(c.get_invoice(&id).status, InvoiceStatus::Refunded); + assert!(!c.get_invoice(&id).disputed); + assert_eq!(tk.balance(&payer), 100); +} + +#[test] +fn test_non_disputed_invoice_unaffected_by_dispute_logic() { // Pay 50 (50% of 100) c.pay(&payer, &id, &50_i128, &0_u64, &false, &false); @@ -5333,6 +5519,164 @@ fn test_auto_resolve_rule_matches_ignores_fallback() { StellarAssetClient::new(&env, &token_id).mint(&payer, &100); env.ledger().set_timestamp(1_000); + // Create invoice without setting an arbiter + let id = make_invoice(&env, &c, &creator, &recipient, 500, &token_id, 2_000); + + // Pay 100 + c.pay(&payer, &id, &100_i128, &0_u64, &false, &false); + + // Verify invoice is not disputed + assert!(!c.get_invoice(&id).disputed); + + // Move time past deadline + env.ledger().set_timestamp(3_000); + + // Refund should work normally (no dispute check impacts it) + c.refund(&id); + + assert_eq!(c.get_invoice(&id).status, InvoiceStatus::Refunded); + assert_eq!(tk.balance(&payer), 100); +} + + +#[test] +fn test_substitute_recipient_no_cosigners() { + let (env, contract_id, token_id) = setup(); + let c = client(&env, &contract_id); + let tk = token_client(&env, &token_id); + + let creator = Address::generate(&env); + let payer = Address::generate(&env); + let old_recipient = Address::generate(&env); + let new_recipient = Address::generate(&env); + + StellarAssetClient::new(&env, &token_id).mint(&payer, &100); + env.ledger().set_timestamp(1_000); + + let id = make_invoice(&env, &c, &creator, &old_recipient, 100, &token_id, 2_000); + + // Pay partial amount + c.pay(&payer, &id, &50_i128, &0_u64, &false, &false); + + // Substitute recipient (no co-signers, creator auth alone) + c.substitute_recipient(&creator, &id, &old_recipient, &new_recipient); + + let invoice = c.get_invoice(&id); + assert_eq!(invoice.recipients.get(0), Some(new_recipient.clone())); + + // Release to new recipient + c.release(&creator, &id); + assert_eq!(tk.balance(&new_recipient), 50); + assert_eq!(tk.balance(&old_recipient), 0); +} + +#[test] +#[should_panic(expected = "recipient not found")] +fn test_substitute_recipient_not_found() { + let (env, contract_id, token_id) = setup(); + let c = client(&env, &contract_id); + + let creator = Address::generate(&env); + let old_recipient = Address::generate(&env); + let new_recipient = Address::generate(&env); + let not_a_recipient = Address::generate(&env); + + env.ledger().set_timestamp(1_000); + + let id = make_invoice(&env, &c, &creator, &old_recipient, 100, &token_id, 2_000); + + // Try to substitute a non-existent recipient + c.substitute_recipient(&creator, &id, ¬_a_recipient, &new_recipient); +} + +#[test] +#[should_panic(expected = "insufficient approvals for recipient substitution")] +fn test_substitute_recipient_with_cosigners_requires_approvals() { + let (env, contract_id, token_id) = setup(); + let c = client(&env, &contract_id); + + let creator = Address::generate(&env); + let cosigner1 = Address::generate(&env); + let cosigner2 = Address::generate(&env); + let payer = Address::generate(&env); + let old_recipient = Address::generate(&env); + let new_recipient = Address::generate(&env); + + StellarAssetClient::new(&env, &token_id).mint(&payer, &100); + env.ledger().set_timestamp(1_000); + + let mut opts = default_options(&env); + let mut signers = Vec::new(&env); + signers.push_back(cosigner1.clone()); + signers.push_back(cosigner2.clone()); + opts.co_signers = signers; + opts.required_signatures = 2; + + let mut recipients = Vec::new(&env); + recipients.push_back(old_recipient.clone()); + let mut amounts = Vec::new(&env); + amounts.push_back(100_i128); + let id = c.create_invoice(&creator, &recipients, &amounts, &token_id, &2_000_u64, &opts); + + // Pay partial amount + c.pay(&payer, &id, &50_i128, &0_u64, &false, &false); + + // Try to substitute without approvals (should panic) + c.substitute_recipient(&creator, &id, &old_recipient, &new_recipient); +} + +#[test] +fn test_substitute_recipient_with_cosigners_after_approvals() { + let (env, contract_id, token_id) = setup(); + let c = client(&env, &contract_id); + let tk = token_client(&env, &token_id); + + let creator = Address::generate(&env); + let cosigner1 = Address::generate(&env); + let cosigner2 = Address::generate(&env); + let payer = Address::generate(&env); + let old_recipient = Address::generate(&env); + let new_recipient = Address::generate(&env); + + StellarAssetClient::new(&env, &token_id).mint(&payer, &100); + env.ledger().set_timestamp(1_000); + + let mut opts = default_options(&env); + let mut signers = Vec::new(&env); + signers.push_back(cosigner1.clone()); + signers.push_back(cosigner2.clone()); + opts.co_signers = signers; + opts.required_signatures = 2; + + let mut recipients = Vec::new(&env); + recipients.push_back(old_recipient.clone()); + let mut amounts = Vec::new(&env); + amounts.push_back(100_i128); + let id = c.create_invoice(&creator, &recipients, &amounts, &token_id, &2_000_u64, &opts); + + // Pay partial amount + c.pay(&payer, &id, &50_i128, &0_u64, &false, &false); + + // Get the required signatures (2) + // Get co-signers to approve the substitution + c.approve_substitute_recipient(&id, &cosigner1); + c.approve_substitute_recipient(&id, &cosigner2); + + // Now substitute should succeed + c.substitute_recipient(&creator, &id, &old_recipient, &new_recipient); + + let invoice = c.get_invoice(&id); + assert_eq!(invoice.recipients.get(0), Some(new_recipient.clone())); + + // Release to new recipient + c.release(&creator, &id); + assert_eq!(tk.balance(&new_recipient), 50); + assert_eq!(tk.balance(&old_recipient), 0); +} + +#[test] +#[should_panic(expected = "not a co-signer for this invoice")] +fn test_approve_substitute_recipient_not_cosigner() { // Create invoice with 50% rule (Release) and 50% funded // and fallback_action = Refund (but should be ignored) let mut rules = Vec::new(&env); @@ -5618,6 +5962,57 @@ fn test_local_only_invoice_unaffected_by_external_prerequisite() { let c = client(&env, &contract_id); let creator = Address::generate(&env); + let cosigner = Address::generate(&env); + let not_a_cosigner = Address::generate(&env); + let recipient = Address::generate(&env); + + env.ledger().set_timestamp(1_000); + + let mut opts = default_options(&env); + let mut signers = Vec::new(&env); + signers.push_back(cosigner.clone()); + opts.co_signers = signers; + opts.required_signatures = 1; + + let mut recipients = Vec::new(&env); + recipients.push_back(recipient.clone()); + let mut amounts = Vec::new(&env); + amounts.push_back(100_i128); + let id = c.create_invoice(&creator, &recipients, &amounts, &token_id, &2_000_u64, &opts); + + // Non-cosigner tries to approve + c.approve_substitute_recipient(&id, ¬_a_cosigner); +} + +#[test] +#[should_panic(expected = "co-signer has already approved substitution")] +fn test_approve_substitute_recipient_duplicate_approval() { + let (env, contract_id, token_id) = setup(); + let c = client(&env, &contract_id); + + let creator = Address::generate(&env); + let cosigner = Address::generate(&env); + let recipient = Address::generate(&env); + + env.ledger().set_timestamp(1_000); + + let mut opts = default_options(&env); + let mut signers = Vec::new(&env); + signers.push_back(cosigner.clone()); + opts.co_signers = signers; + opts.required_signatures = 1; + + let mut recipients = Vec::new(&env); + recipients.push_back(recipient.clone()); + let mut amounts = Vec::new(&env); + amounts.push_back(100_i128); + let id = c.create_invoice(&creator, &recipients, &amounts, &token_id, &2_000_u64, &opts); + + // Approve once + c.approve_substitute_recipient(&id, &cosigner); + + // Try to approve again (should panic) + c.approve_substitute_recipient(&id, &cosigner); let payer = Address::generate(&env); let recipient = Address::generate(&env); diff --git a/contracts/split/src/types.rs b/contracts/split/src/types.rs index 8024e66..552a876 100644 --- a/contracts/split/src/types.rs +++ b/contracts/split/src/types.rs @@ -369,6 +369,8 @@ pub struct InvoiceExt2 { pub min_payment: i128, pub min_funding_amount: i128, pub priorities: Vec, + /// Issue #230: co-signers who have approved the pending recipient substitution. + pub substitute_recipient_approvals: Vec
, } /// Issue #211: A single escalating penalty tier (seconds_after_deadline, bps). @@ -482,6 +484,8 @@ pub struct Invoice { pub priorities: Vec, pub clone_depth: u32, pub fallback_action: Option, + /// Issue #230: co-signers who have approved the pending recipient substitution. + pub substitute_recipient_approvals: Vec
, /// Issue #196: invoice creation timestamp for spam deposit age calculation. pub creation_timestamp: u64, /// Issue #201: minimum payment increment - reject payments below this threshold. @@ -578,6 +582,7 @@ impl Invoice { min_payment: self.min_payment, min_funding_amount: self.min_funding_amount, priorities: self.priorities, + substitute_recipient_approvals: Vec::new(self.notification_contract.env()), }, ) } @@ -879,14 +884,15 @@ impl Invoice { refund_grace_secs: None, penalty_tiers: Vec::::new(env), allowed_callers: None, - forward_to: None, - forward_invoice_id: None, notification_contract: None, overflow_behavior: OverflowBehavior::Reject, cross_chain_ref: None, - clone_depth: 0, - parent_invoice_id: None, priorities: Vec::new(env), + forward_to: None, + forward_invoice_id: None, + parent_invoice_id: None, + clone_depth: 0, + fallback_action: None, require_kyc: false, } }