From 0a089c056b682851791ceef03cfabd61aa87910b Mon Sep 17 00:00:00 2001 From: andrew Date: Wed, 8 Apr 2026 13:12:09 -0300 Subject: [PATCH 1/9] add generic signer module --- contracts/src/multisig/Signer.compact | 247 +++++++++++++ contracts/src/multisig/test/Signer.test.ts | 341 ++++++++++++++++++ .../multisig/test/mocks/MockSigner.compact | 59 +++ .../test/simulators/SignerSimulator.ts | 93 +++++ .../src/multisig/witnesses/SignerWitnesses.ts | 6 + 5 files changed, 746 insertions(+) create mode 100644 contracts/src/multisig/Signer.compact create mode 100644 contracts/src/multisig/test/Signer.test.ts create mode 100644 contracts/src/multisig/test/mocks/MockSigner.compact create mode 100644 contracts/src/multisig/test/simulators/SignerSimulator.ts create mode 100644 contracts/src/multisig/witnesses/SignerWitnesses.ts diff --git a/contracts/src/multisig/Signer.compact b/contracts/src/multisig/Signer.compact new file mode 100644 index 00000000..cdf8b9c8 --- /dev/null +++ b/contracts/src/multisig/Signer.compact @@ -0,0 +1,247 @@ +// SPDX-License-Identifier: MIT +// OpenZeppelin Compact Contracts v0.0.1-alpha.1 (multisig/Signer.compact) + +pragma language_version >= 0.21.0; + +/** + * @module Signer + * @description Manages signer registry, threshold enforcement, and signer + * validation for multisig governance contracts. + * + * Parameterized over the signer identity type `T`, allowing the consuming + * contract to choose the identity mechanism at import time. Common + * instantiations include: + * + * - `Bytes<32>` for commitment-based identity (e.g., hash of ECDSA public key) + * - `JubjubPoint` for Schnorr/MuSig aggregated key + * + * The Signer module does not resolve caller identity. It receives a validated + * caller from the contract layer and checks it against the registry. + * This separation allows the identity mechanism to change without + * modifying the module. + * + * Multi-step signer reconfigurations (e.g., removing a signer and + * lowering the threshold) may produce intermediate states where the + * module's invariants temporarily hold but the contract's intended + * configuration is incomplete. This is a contract-layer concern. + * Contracts should either perform reconfigurations atomically in a + * single circuit or use a configuration nonce to invalidate proposals + * created under a stale signer set. + * + * Underscore-prefixed circuits (_addSigner, _removeSigner, + * _changeThreshold) have no access control enforcement. The consuming + * contract must gate these behind its own authorization policy. + */ +module Signer { + import CompactStandardLibrary; + + // ─── State ────────────────────────────────────────────────────────────────── + + export ledger _signers: Set; + export ledger _signerCount: Uint<8>; + export ledger _threshold: Uint<8>; + + // ─── Initialization ───────────────────────────────────────────────────────── + + /** + * @description Initializes the signer module with the given threshold + * and an initial set of signers. + * Must be called in the contract's constructor. + * + * @circuitInfo k=11, rows=1815 + * + * Requirements: + * + * - `thresh` must not be zero. + * - `thresh` must not exceed the number of `signers`. + * - `signers` must not contain duplicates. + * + * @param {Vector} signers - The initial signer set. + * @param {Uint<8>} thresh - The minimum number of approvals required. + * @returns {[]} Empty tuple. + */ + export circuit initialize<#n>( + signers: Vector, + thresh: Uint<8> + ): [] { + for (const signer of signers) { + _addSigner(signer); + } + + _changeThreshold(thresh); + } + + // ─── Guards ───────────────────────────────────────────────────────────── + + /** + * @description Asserts that the given caller is an active signer. + * + * @circuitInfo k=10, rows=585 + * + * Requirements: + * + * - `caller` must be a member of the signers registry. + * + * @param {T} caller - The identity to validate. + * @returns {[]} Empty tuple. + */ + export circuit assertSigner(caller: T): [] { + assert(isSigner(caller), "Signer: not a signer"); + } + + /** + * @description Asserts that the given approval count meets the threshold. + * + * @circuitInfo k=9, rows=54 + * + * Requirements: + * + * - Ledger threshold must be set (not be zero). + * - `approvalCount` must be >= threshold. + * + * @param {Uint<8>} approvalCount - The current number of approvals. + * @returns {[]} Empty tuple. + */ + export circuit assertThresholdMet(approvalCount: Uint<8>): [] { + assert(_threshold != 0, "Signer: threshold not set"); + assert(approvalCount >= _threshold, "Signer: threshold not met"); + } + + // ─── View ────────────────────────────────────────────────────────── + + /** + * @description Returns the current signer count. + * + * @circuitInfo k=6, rows=26 + * + * @returns {Uint<8>} The number of active signers. + */ + export circuit getSignerCount(): Uint<8> { + return _signerCount; + } + + /** + * @description Returns the approval threshold. + * + * @circuitInfo k=6, rows=26 + * + * @returns {Uint<8>} The threshold. + */ + export circuit getThreshold(): Uint<8> { + return _threshold; + } + + /** + * @description Returns whether the given account is an active signer. + * + * @circuitInfo k=10, rows=605 + * + * @param {T} account - The account to check. + * @returns {Boolean} True if the account is an active signer. + */ + export circuit isSigner(account: T): Boolean { + return _signers.member(disclose(account)); + } + + // ─── Signer Management ───────────────────────────────────────────────────── + + /** + * @description Adds a new signer to the registry. + * + * @notice Access control is NOT enforced here. + * The consuming contract must gate this behind its own + * authorization policy. + * + * @circuitInfo k=10, rows=598 + * + * Requirements: + * + * - `signer` must not already be an active signer. + * + * @param {T} signer - The signer to add. + * @returns {[]} Empty tuple. + */ + export circuit _addSigner(signer: T): [] { + assert( + !isSigner(signer), + "Signer: signer already active" + ); + + _signers.insert(disclose(signer)); + _signerCount = _signerCount + 1 as Uint<8>; + } + + /** + * @description Removes a signer from the registry. + * + * @notice Access control is NOT enforced here. + * The consuming contract must gate this behind its own + * authorization policy. + * + * @circuitInfo k=10, rows=612 + * + * Requirements: + * + * - `signer` must be an active signer. + * - Removal must not drop signer count below threshold. + * + * @param {T} signer - The signer to remove. + * @returns {[]} Empty tuple. + */ + export circuit _removeSigner(signer: T): [] { + assert(isSigner(signer), "Signer: not a signer"); + + const newCount = _signerCount - 1 as Uint<8>; + assert(newCount >= _threshold, "Signer: removal would breach threshold"); + + _signers.remove(disclose(signer)); + _signerCount = newCount; + } + + /** + * @description Updates the approval threshold. + * + * @notice Access control is NOT enforced here. + * The consuming contract must gate this behind its own + * authorization policy. + * + * @circuitInfo k=9, rows=53 + * + * Requirements: + * + * - `newThreshold` must not be zero. + * - `newThreshold` must not exceed the current signer count. + * + * @param {Uint<8>} newThreshold - The new minimum number of approvals required. + * @returns {[]} Empty tuple. + */ + export circuit _changeThreshold(newThreshold: Uint<8>): [] { + assert(newThreshold <= _signerCount, "Signer: threshold exceeds signer count"); + _setThreshold(newThreshold); + } + + /** + * @description Sets the approval threshold without checking + * against the current signer count. Intended for use during + * contract construction or custom setup flows where signers + * may not yet be registered. + * + * @notice Access control is NOT enforced here. + * The consuming contract must gate this behind its own + * authorization policy. Use `_changeThreshold` for + * operational threshold changes with signer count validation. + * + * @circuitInfo k=6, rows=40 + * + * Requirements: + * + * - `newThreshold` must not be zero. + * + * @param {Uint<8>} newThreshold - The minimum number of approvals required. + * @returns {[]} Empty tuple. + */ + export circuit _setThreshold(newThreshold: Uint<8>): [] { + assert(newThreshold != 0, "Signer: threshold must not be zero"); + _threshold = disclose(newThreshold); + } +} diff --git a/contracts/src/multisig/test/Signer.test.ts b/contracts/src/multisig/test/Signer.test.ts new file mode 100644 index 00000000..ccd675d9 --- /dev/null +++ b/contracts/src/multisig/test/Signer.test.ts @@ -0,0 +1,341 @@ +import { beforeEach, describe, expect, it } from 'vitest'; +import * as utils from '#test-utils/address.js'; +import { SignerSimulator } from './simulators/SignerSimulator.js'; + +const THRESHOLD = 2n; +const IS_INIT = true; + +const [_SIGNER, Z_SIGNER] = utils.generateEitherPubKeyPair('SIGNER'); +const [_SIGNER2, Z_SIGNER2] = utils.generateEitherPubKeyPair('SIGNER2'); +const [_SIGNER3, Z_SIGNER3] = utils.generateEitherPubKeyPair('SIGNER3'); +const SIGNERS = [Z_SIGNER, Z_SIGNER2, Z_SIGNER3]; +const [_OTHER, Z_OTHER] = utils.generateEitherPubKeyPair('OTHER'); +const [_OTHER2, Z_OTHER2] = utils.generateEitherPubKeyPair('OTHER2'); + +let contract: SignerSimulator; + +describe('SigningManager', () => { + describe('initialization', () => { + it('should fail with a threshold of zero', () => { + expect(() => { + new SignerSimulator(SIGNERS, 0n, IS_INIT); + }).toThrow('Signer: threshold must not be zero'); + }); + + it('should fail when threshold exceeds signer count', () => { + expect(() => { + new SignerSimulator(SIGNERS, BigInt(SIGNERS.length) + 1n, IS_INIT); + }).toThrow('Signer: threshold exceeds signer count'); + }); + + it('should fail with duplicate signers', () => { + const duplicateSigners = [Z_SIGNER, Z_SIGNER, Z_SIGNER2]; + expect(() => { + new SignerSimulator(duplicateSigners, THRESHOLD, IS_INIT); + }).toThrow('Signer: signer already active'); + }); + + it('should initialize with threshold equal to signer count', () => { + const contract = new SignerSimulator( + SIGNERS, + BigInt(SIGNERS.length), + IS_INIT, + ); + expect(contract.getThreshold()).toEqual(BigInt(SIGNERS.length)); + }); + + it('should initialize', () => { + expect(() => { + contract = new SignerSimulator(SIGNERS, THRESHOLD, IS_INIT); + }).to.be.ok; + + // Check thresh + expect(contract.getThreshold()).toEqual(THRESHOLD); + + // Check signers + expect(contract.getSignerCount()).toEqual(BigInt(SIGNERS.length)); + expect(() => { + for (let i = 0; i < SIGNERS.length; i++) { + contract.assertSigner(SIGNERS[i]); + } + }).to.be.ok; + }); + }); + + beforeEach(() => { + contract = new SignerSimulator(SIGNERS, THRESHOLD, IS_INIT); + }); + + describe('assertSigner', () => { + it('should pass with good signer', () => { + expect(() => contract.assertSigner(Z_SIGNER)).not.toThrow(); + }); + + it('should fail with bad signer', () => { + expect(() => { + contract.assertSigner(Z_OTHER); + }).toThrow('Signer: not a signer'); + }); + }); + + describe('assertThresholdMet', () => { + it('should pass when approvals equal threshold', () => { + expect(() => contract.assertThresholdMet(THRESHOLD)).not.toThrow(); + }); + + it('should pass when approvals exceed threshold', () => { + expect(() => contract.assertThresholdMet(THRESHOLD + 1n)).not.toThrow(); + }); + + it('should fail when approvals are below threshold', () => { + expect(() => { + contract.assertThresholdMet(THRESHOLD - 1n); + }).toThrow('Signer: threshold not met'); + }); + + it('should fail with zero approvals', () => { + expect(() => { + contract.assertThresholdMet(0n); + }).toThrow('Signer: threshold not met'); + }); + + it('should fail with any count when threshold not set', () => { + const isNotInit = false; + const uninit = new SignerSimulator(SIGNERS, 0n, isNotInit); + expect(() => uninit.assertThresholdMet(5n)).toThrow( + 'Signer: threshold not set', + ); + }); + }); + + describe('isSigner', () => { + it('should return true for an active signer', () => { + expect(contract.isSigner(Z_SIGNER)).toEqual(true); + }); + + it('should return false for a non-signer', () => { + expect(contract.isSigner(Z_OTHER)).toEqual(false); + }); + }); + + describe('_addSigner', () => { + it('should add a new signer', () => { + contract._addSigner(Z_OTHER); + + expect(contract.isSigner(Z_OTHER)).toEqual(true); + expect(contract.getSignerCount()).toEqual(BigInt(SIGNERS.length) + 1n); + }); + + it('should fail when adding an existing signer', () => { + contract._addSigner(Z_OTHER); + + expect(() => { + contract._addSigner(Z_OTHER); + }).toThrow('Signer: signer already active'); + }); + + it('should add multiple new signers', () => { + contract._addSigner(Z_OTHER); + contract._addSigner(Z_OTHER2); + + expect(contract.isSigner(Z_OTHER)).toEqual(true); + expect(contract.isSigner(Z_OTHER2)).toEqual(true); + expect(contract.getSignerCount()).toEqual(BigInt(SIGNERS.length) + 2n); + }); + + it('should allow re-adding a previously removed signer', () => { + expect(contract.isSigner(Z_SIGNER)).toEqual(true); + + // Remove signer + contract._removeSigner(Z_SIGNER); + expect(contract.isSigner(Z_SIGNER)).toEqual(false); + + // Re-add signer + contract._addSigner(Z_SIGNER); + expect(contract.isSigner(Z_SIGNER)).toEqual(true); + }); + }); + + describe('_removeSigner', () => { + it('should remove an existing signer', () => { + contract._removeSigner(Z_SIGNER3); + + expect(contract.isSigner(Z_SIGNER3)).toEqual(false); + expect(contract.getSignerCount()).toEqual(BigInt(SIGNERS.length) - 1n); + }); + + it('should fail when removing a non-signer', () => { + expect(() => { + contract._removeSigner(Z_OTHER); + }).toThrow('Signer: not a signer'); + }); + + it('should fail when removal would breach threshold', () => { + // Remove one signer: count goes from 3 to 2, threshold is 2 — ok + contract._removeSigner(Z_SIGNER3); + + // Remove another: count would go from 2 to 1, threshold is 2 — breach + expect(() => { + contract._removeSigner(Z_SIGNER2); + }).toThrow('Signer: removal would breach threshold'); + }); + + it('should allow removal after threshold is lowered', () => { + contract._changeThreshold(1n); + contract._removeSigner(Z_SIGNER3); + contract._removeSigner(Z_SIGNER2); + + expect(contract.getSignerCount()).toEqual(1n); + expect(contract.isSigner(Z_SIGNER)).toEqual(true); + expect(contract.isSigner(Z_SIGNER2)).toEqual(false); + expect(contract.isSigner(Z_SIGNER3)).toEqual(false); + }); + + it('should keep signer count in sync after multiple add/remove operations', () => { + contract._addSigner(Z_OTHER); + contract._addSigner(Z_OTHER2); + contract._removeSigner(Z_SIGNER3); + contract._removeSigner(Z_OTHER); + + expect(contract.getSignerCount()).toEqual(3n); + expect(contract.isSigner(Z_SIGNER)).toEqual(true); + expect(contract.isSigner(Z_SIGNER2)).toEqual(true); + expect(contract.isSigner(Z_SIGNER3)).toEqual(false); + expect(contract.isSigner(Z_OTHER)).toEqual(false); + expect(contract.isSigner(Z_OTHER2)).toEqual(true); + }); + }); + + describe('_changeThreshold', () => { + it('should update the threshold', () => { + contract._changeThreshold(3n); + + expect(contract.getThreshold()).toEqual(3n); + }); + + it('should allow lowering the threshold', () => { + contract._changeThreshold(1n); + + expect(contract.getThreshold()).toEqual(1n); + }); + + it('should fail with a threshold of zero', () => { + expect(() => { + contract._changeThreshold(0n); + }).toThrow('Signer: threshold must not be zero'); + }); + + it('should fail when threshold exceeds signer count', () => { + expect(() => { + contract._changeThreshold(BigInt(SIGNERS.length) + 1n); + }).toThrow('Signer: threshold exceeds signer count'); + }); + + it('should allow threshold equal to signer count', () => { + contract._changeThreshold(BigInt(SIGNERS.length)); + + expect(contract.getThreshold()).toEqual(BigInt(SIGNERS.length)); + }); + + it('should reflect new threshold in assertThresholdMet', () => { + contract._changeThreshold(3n); + + expect(() => { + contract.assertThresholdMet(2n); + }).toThrow('Signer: threshold not met'); + + expect(() => contract.assertThresholdMet(3n)).not.toThrow(); + }); + }); + + describe('_setThreshold', () => { + beforeEach(() => { + const isNotInit = false; + contract = new SignerSimulator(SIGNERS, 0n, isNotInit); + }); + + it('should have an empty state', () => { + expect(contract.getThreshold()).toEqual(0n); + expect(contract.getSignerCount()).toEqual(0n); + expect(contract.getPublicState().Signer__signers.isEmpty()).toEqual(true); + }); + + it('should set threshold without signers', () => { + expect(contract.getThreshold()).toEqual(0n); + + contract._setThreshold(2n); + expect(contract.getThreshold()).toEqual(2n); + }); + + it('should set threshold multiple times', () => { + contract._setThreshold(2n); + contract._setThreshold(3n); + expect(contract.getThreshold()).toEqual(3n); + }); + + it('should fail with zero threshold', () => { + expect(() => { + contract._setThreshold(0n); + }).toThrow('Signer: threshold must not be zero'); + }); + }); + + describe('custom setup flow when not initialized', () => { + beforeEach(() => { + const isNotInit = false; + contract = new SignerSimulator(SIGNERS, 0n, isNotInit); + }); + + it('should have no signers by default', () => { + expect(contract.getSignerCount()).toEqual(0n); + expect(contract.isSigner(Z_SIGNER)).toEqual(false); + }); + + it('should have zero threshold by default', () => { + expect(contract.getThreshold()).toEqual(0n); + }); + + it('should allow adding signers then setting threshold', () => { + contract._addSigner(Z_SIGNER); + contract._addSigner(Z_SIGNER2); + contract._addSigner(Z_SIGNER3); + contract._changeThreshold(2n); + + expect(contract.getSignerCount()).toEqual(3n); + expect(contract.getThreshold()).toEqual(2n); + expect(contract.isSigner(Z_SIGNER)).toEqual(true); + }); + + it('should allow setting threshold then adding signers to meet it', () => { + contract._setThreshold(2n); + contract._addSigner(Z_SIGNER); + contract._addSigner(Z_SIGNER2); + + expect(contract.getSignerCount()).toEqual(2n); + expect(contract.getThreshold()).toEqual(2n); + }); + + it('should fail _changeThreshold before signers are added', () => { + expect(() => { + contract._changeThreshold(2n); + }).toThrow('Signer: threshold exceeds signer count'); + }); + + it('should allow assertThresholdMet after custom setup', () => { + contract._setThreshold(2n); + contract._addSigner(Z_SIGNER); + contract._addSigner(Z_SIGNER2); + + expect(() => contract.assertThresholdMet(2n)).not.toThrow(); + }); + + it('should fail assertThresholdMet before threshold is set', () => { + contract._addSigner(Z_SIGNER); + contract._addSigner(Z_SIGNER2); + + expect(() => contract.assertThresholdMet(0n)).toThrow( + 'Signer: threshold not set', + ); + }); + }); +}); diff --git a/contracts/src/multisig/test/mocks/MockSigner.compact b/contracts/src/multisig/test/mocks/MockSigner.compact new file mode 100644 index 00000000..a3cea9fc --- /dev/null +++ b/contracts/src/multisig/test/mocks/MockSigner.compact @@ -0,0 +1,59 @@ +pragma language_version >= 0.21.0; + +import CompactStandardLibrary; + +import "../../Signer"> prefix Signer_; + +export { ZswapCoinPublicKey, ContractAddress, Either, Maybe }; +export { Signer__signers }; + +/** + * @description `isInit` is a param for testing. + * + * If `isInit` is false, the constructor will not initialize the contract. + * This behavior is to test that _setThreshold will work for custom deployments + * where contracts don't `initialize` the signer module and instead have some sort + * of gated access control prior to setting up the signers +*/ +constructor(signers: Vector<3, Either>, thresh: Uint<8>, isInit: Boolean) { + if (disclose(isInit)) { + Signer_initialize<3>(signers, thresh); + } +} + +export circuit assertSigner(caller: Either): [] { + return Signer_assertSigner(caller); +} + +export circuit assertThresholdMet(approvalCount: Uint<8>): [] { + return Signer_assertThresholdMet(approvalCount); +} + +export circuit getSignerCount(): Uint<8> { + return Signer_getSignerCount(); +} + +export circuit getThreshold(): Uint<8> { + return Signer_getThreshold(); +} + +export circuit isSigner(account: Either): Boolean { + return Signer_isSigner(account); +} + +export circuit _addSigner(signer: Either): [] { + return Signer__addSigner(signer); +} + +export circuit _removeSigner(signer: Either): [] { + return Signer__removeSigner(signer); +} + +export circuit _changeThreshold(newThreshold: Uint<8>): [] { + return Signer__changeThreshold(newThreshold); +} + +export circuit _setThreshold(newThreshold: Uint<8>): [] { + return Signer__setThreshold(newThreshold); +} + diff --git a/contracts/src/multisig/test/simulators/SignerSimulator.ts b/contracts/src/multisig/test/simulators/SignerSimulator.ts new file mode 100644 index 00000000..ca963e5d --- /dev/null +++ b/contracts/src/multisig/test/simulators/SignerSimulator.ts @@ -0,0 +1,93 @@ +import { + type BaseSimulatorOptions, + createSimulator, +} from '@openzeppelin-compact/contracts-simulator'; +import { + type ContractAddress, + type Either, + ledger, + Contract as MockSigner, + type ZswapCoinPublicKey, +} from '../../../../artifacts/MockSigner/contract/index.js'; +import { + SignerPrivateState, + SignerWitnesses, +} from '../../witnesses/SignerWitnesses.js'; + +/** + * Type constructor args + */ +type SignerArgs = readonly [ + signers: Either[], + thresh: bigint, + isInit: boolean, +]; + +const SignerSimulatorBase = createSimulator< + SignerPrivateState, + ReturnType, + ReturnType, + MockSigner, + SignerArgs +>({ + contractFactory: (witnesses) => new MockSigner(witnesses), + defaultPrivateState: () => SignerPrivateState, + contractArgs: (signers, thresh, isInit) => [signers, thresh, isInit], + ledgerExtractor: (state) => ledger(state), + witnessesFactory: () => SignerWitnesses(), +}); + +/** + * Signer Simulator + */ +export class SignerSimulator extends SignerSimulatorBase { + constructor( + signers: Either[], + thresh: bigint, + isInit: boolean, + options: BaseSimulatorOptions< + SignerPrivateState, + ReturnType + > = {}, + ) { + super([signers, thresh, isInit], options); + } + + public assertSigner(caller: Either) { + return this.circuits.impure.assertSigner(caller); + } + + public assertThresholdMet(approvalCount: bigint) { + return this.circuits.impure.assertThresholdMet(approvalCount); + } + + public getSignerCount(): bigint { + return this.circuits.impure.getSignerCount(); + } + + public getThreshold(): bigint { + return this.circuits.impure.getThreshold(); + } + + public isSigner( + account: Either, + ): boolean { + return this.circuits.impure.isSigner(account); + } + + public _addSigner(signer: Either) { + return this.circuits.impure._addSigner(signer); + } + + public _removeSigner(signer: Either) { + return this.circuits.impure._removeSigner(signer); + } + + public _changeThreshold(newThreshold: bigint) { + return this.circuits.impure._changeThreshold(newThreshold); + } + + public _setThreshold(newThreshold: bigint) { + return this.circuits.impure._setThreshold(newThreshold); + } +} diff --git a/contracts/src/multisig/witnesses/SignerWitnesses.ts b/contracts/src/multisig/witnesses/SignerWitnesses.ts new file mode 100644 index 00000000..b6e10ef5 --- /dev/null +++ b/contracts/src/multisig/witnesses/SignerWitnesses.ts @@ -0,0 +1,6 @@ +// SPDX-License-Identifier: MIT +// OpenZeppelin Compact Contracts v0.0.1-alpha.1 (multisig/witnesses/SignerWitnesses.ts) + +export type SignerPrivateState = Record; +export const SignerPrivateState: SignerPrivateState = {}; +export const SignerWitnesses = () => ({}); From 8180d9ebff22caed1ecdc74217b3c81eb5aa2662 Mon Sep 17 00:00:00 2001 From: andrew Date: Wed, 8 Apr 2026 13:12:23 -0300 Subject: [PATCH 2/9] add changelog entry --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 42519074..6fe36928 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## Unreleased +### Added + +- Signer module in multisig directory (#) + ### Changes - Add defensive Buffer copy to ZOwnablePKWitnesses (#397) From 4c48ac8d01383bca5be744725985902e7bd16e6a Mon Sep 17 00:00:00 2001 From: andrew Date: Wed, 8 Apr 2026 13:15:37 -0300 Subject: [PATCH 3/9] update changelog entry --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6fe36928..57224fe5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added -- Signer module in multisig directory (#) +- Signer module in multisig directory (#424) ### Changes From 10d0b65057065d7621fe37e427aa7dfa65185274 Mon Sep 17 00:00:00 2001 From: andrew Date: Wed, 8 Apr 2026 15:35:21 -0300 Subject: [PATCH 4/9] fix tests --- contracts/src/multisig/test/Signer.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/contracts/src/multisig/test/Signer.test.ts b/contracts/src/multisig/test/Signer.test.ts index ccd675d9..439b794f 100644 --- a/contracts/src/multisig/test/Signer.test.ts +++ b/contracts/src/multisig/test/Signer.test.ts @@ -47,7 +47,7 @@ describe('SigningManager', () => { it('should initialize', () => { expect(() => { contract = new SignerSimulator(SIGNERS, THRESHOLD, IS_INIT); - }).to.be.ok; + }).not.toThrow(); // Check thresh expect(contract.getThreshold()).toEqual(THRESHOLD); @@ -58,7 +58,7 @@ describe('SigningManager', () => { for (let i = 0; i < SIGNERS.length; i++) { contract.assertSigner(SIGNERS[i]); } - }).to.be.ok; + }).not.toThrow(); }); }); From c3125debf49852a7510ea14d0a5c8509baf67fe2 Mon Sep 17 00:00:00 2001 From: andrew Date: Wed, 8 Apr 2026 18:12:49 -0300 Subject: [PATCH 5/9] refactor tests and mock to use Bytes<32> --- contracts/src/multisig/Signer.compact | 12 +- contracts/src/multisig/test/Signer.test.ts | 126 +++++++++--------- .../multisig/test/mocks/MockSigner.compact | 18 ++- .../test/simulators/SignerSimulator.ts | 19 +-- 4 files changed, 96 insertions(+), 79 deletions(-) diff --git a/contracts/src/multisig/Signer.compact b/contracts/src/multisig/Signer.compact index cdf8b9c8..6835facc 100644 --- a/contracts/src/multisig/Signer.compact +++ b/contracts/src/multisig/Signer.compact @@ -31,9 +31,16 @@ pragma language_version >= 0.21.0; * Underscore-prefixed circuits (_addSigner, _removeSigner, * _changeThreshold) have no access control enforcement. The consuming * contract must gate these behind its own authorization policy. + * + * Contracts may handle their own initialization and this module + * supports custom flows. Thus, contracts may choose to not + * call `initialize` in the contract's constructor. Contracts MUST NOT + * call `initialize` outside of the constructor context because + * this could corrupt the signer set and threshold configuration. */ module Signer { import CompactStandardLibrary; + import "../security/Initializable" prefix Initializable_; // ─── State ────────────────────────────────────────────────────────────────── @@ -46,12 +53,13 @@ module Signer { /** * @description Initializes the signer module with the given threshold * and an initial set of signers. - * Must be called in the contract's constructor. + * If used, it should only be called in the contract's constructor. * * @circuitInfo k=11, rows=1815 * * Requirements: * + * - If used, can only be called once (in the constructor). * - `thresh` must not be zero. * - `thresh` must not exceed the number of `signers`. * - `signers` must not contain duplicates. @@ -64,6 +72,8 @@ module Signer { signers: Vector, thresh: Uint<8> ): [] { + Initializable_initialize(); + for (const signer of signers) { _addSigner(signer); } diff --git a/contracts/src/multisig/test/Signer.test.ts b/contracts/src/multisig/test/Signer.test.ts index 439b794f..77dee572 100644 --- a/contracts/src/multisig/test/Signer.test.ts +++ b/contracts/src/multisig/test/Signer.test.ts @@ -1,20 +1,20 @@ import { beforeEach, describe, expect, it } from 'vitest'; -import * as utils from '#test-utils/address.js'; import { SignerSimulator } from './simulators/SignerSimulator.js'; const THRESHOLD = 2n; const IS_INIT = true; -const [_SIGNER, Z_SIGNER] = utils.generateEitherPubKeyPair('SIGNER'); -const [_SIGNER2, Z_SIGNER2] = utils.generateEitherPubKeyPair('SIGNER2'); -const [_SIGNER3, Z_SIGNER3] = utils.generateEitherPubKeyPair('SIGNER3'); -const SIGNERS = [Z_SIGNER, Z_SIGNER2, Z_SIGNER3]; -const [_OTHER, Z_OTHER] = utils.generateEitherPubKeyPair('OTHER'); -const [_OTHER2, Z_OTHER2] = utils.generateEitherPubKeyPair('OTHER2'); +// Simple `Bytes<32>` ids +const SIGNER = new Uint8Array(32).fill(1); +const SIGNER2 = new Uint8Array(32).fill(2); +const SIGNER3 = new Uint8Array(32).fill(3); +const SIGNERS = [SIGNER, SIGNER2, SIGNER3]; +const OTHER = new Uint8Array(32).fill(4); +const OTHER2 = new Uint8Array(32).fill(5); let contract: SignerSimulator; -describe('SigningManager', () => { +describe('Signer', () => { describe('initialization', () => { it('should fail with a threshold of zero', () => { expect(() => { @@ -29,7 +29,7 @@ describe('SigningManager', () => { }); it('should fail with duplicate signers', () => { - const duplicateSigners = [Z_SIGNER, Z_SIGNER, Z_SIGNER2]; + const duplicateSigners = [SIGNER, SIGNER, SIGNER2]; expect(() => { new SignerSimulator(duplicateSigners, THRESHOLD, IS_INIT); }).toThrow('Signer: signer already active'); @@ -49,10 +49,7 @@ describe('SigningManager', () => { contract = new SignerSimulator(SIGNERS, THRESHOLD, IS_INIT); }).not.toThrow(); - // Check thresh expect(contract.getThreshold()).toEqual(THRESHOLD); - - // Check signers expect(contract.getSignerCount()).toEqual(BigInt(SIGNERS.length)); expect(() => { for (let i = 0; i < SIGNERS.length; i++) { @@ -60,6 +57,13 @@ describe('SigningManager', () => { } }).not.toThrow(); }); + + it('should fail when initialized twice', () => { + contract = new SignerSimulator(SIGNERS, THRESHOLD, IS_INIT); + expect(() => { + contract.initialize(SIGNERS, THRESHOLD); + }).toThrow('Initializable: contract already initialized'); + }); }); beforeEach(() => { @@ -68,12 +72,12 @@ describe('SigningManager', () => { describe('assertSigner', () => { it('should pass with good signer', () => { - expect(() => contract.assertSigner(Z_SIGNER)).not.toThrow(); + expect(() => contract.assertSigner(SIGNER)).not.toThrow(); }); it('should fail with bad signer', () => { expect(() => { - contract.assertSigner(Z_OTHER); + contract.assertSigner(OTHER); }).toThrow('Signer: not a signer'); }); }); @@ -110,99 +114,95 @@ describe('SigningManager', () => { describe('isSigner', () => { it('should return true for an active signer', () => { - expect(contract.isSigner(Z_SIGNER)).toEqual(true); + expect(contract.isSigner(SIGNER)).toEqual(true); }); it('should return false for a non-signer', () => { - expect(contract.isSigner(Z_OTHER)).toEqual(false); + expect(contract.isSigner(OTHER)).toEqual(false); }); }); describe('_addSigner', () => { it('should add a new signer', () => { - contract._addSigner(Z_OTHER); + contract._addSigner(OTHER); - expect(contract.isSigner(Z_OTHER)).toEqual(true); + expect(contract.isSigner(OTHER)).toEqual(true); expect(contract.getSignerCount()).toEqual(BigInt(SIGNERS.length) + 1n); }); it('should fail when adding an existing signer', () => { - contract._addSigner(Z_OTHER); + contract._addSigner(OTHER); expect(() => { - contract._addSigner(Z_OTHER); + contract._addSigner(OTHER); }).toThrow('Signer: signer already active'); }); it('should add multiple new signers', () => { - contract._addSigner(Z_OTHER); - contract._addSigner(Z_OTHER2); + contract._addSigner(OTHER); + contract._addSigner(OTHER2); - expect(contract.isSigner(Z_OTHER)).toEqual(true); - expect(contract.isSigner(Z_OTHER2)).toEqual(true); + expect(contract.isSigner(OTHER)).toEqual(true); + expect(contract.isSigner(OTHER2)).toEqual(true); expect(contract.getSignerCount()).toEqual(BigInt(SIGNERS.length) + 2n); }); it('should allow re-adding a previously removed signer', () => { - expect(contract.isSigner(Z_SIGNER)).toEqual(true); + expect(contract.isSigner(SIGNER)).toEqual(true); - // Remove signer - contract._removeSigner(Z_SIGNER); - expect(contract.isSigner(Z_SIGNER)).toEqual(false); + contract._removeSigner(SIGNER); + expect(contract.isSigner(SIGNER)).toEqual(false); - // Re-add signer - contract._addSigner(Z_SIGNER); - expect(contract.isSigner(Z_SIGNER)).toEqual(true); + contract._addSigner(SIGNER); + expect(contract.isSigner(SIGNER)).toEqual(true); }); }); describe('_removeSigner', () => { it('should remove an existing signer', () => { - contract._removeSigner(Z_SIGNER3); + contract._removeSigner(SIGNER3); - expect(contract.isSigner(Z_SIGNER3)).toEqual(false); + expect(contract.isSigner(SIGNER3)).toEqual(false); expect(contract.getSignerCount()).toEqual(BigInt(SIGNERS.length) - 1n); }); it('should fail when removing a non-signer', () => { expect(() => { - contract._removeSigner(Z_OTHER); + contract._removeSigner(OTHER); }).toThrow('Signer: not a signer'); }); it('should fail when removal would breach threshold', () => { - // Remove one signer: count goes from 3 to 2, threshold is 2 — ok - contract._removeSigner(Z_SIGNER3); + contract._removeSigner(SIGNER3); - // Remove another: count would go from 2 to 1, threshold is 2 — breach expect(() => { - contract._removeSigner(Z_SIGNER2); + contract._removeSigner(SIGNER2); }).toThrow('Signer: removal would breach threshold'); }); it('should allow removal after threshold is lowered', () => { contract._changeThreshold(1n); - contract._removeSigner(Z_SIGNER3); - contract._removeSigner(Z_SIGNER2); + contract._removeSigner(SIGNER3); + contract._removeSigner(SIGNER2); expect(contract.getSignerCount()).toEqual(1n); - expect(contract.isSigner(Z_SIGNER)).toEqual(true); - expect(contract.isSigner(Z_SIGNER2)).toEqual(false); - expect(contract.isSigner(Z_SIGNER3)).toEqual(false); + expect(contract.isSigner(SIGNER)).toEqual(true); + expect(contract.isSigner(SIGNER2)).toEqual(false); + expect(contract.isSigner(SIGNER3)).toEqual(false); }); it('should keep signer count in sync after multiple add/remove operations', () => { - contract._addSigner(Z_OTHER); - contract._addSigner(Z_OTHER2); - contract._removeSigner(Z_SIGNER3); - contract._removeSigner(Z_OTHER); + contract._addSigner(OTHER); + contract._addSigner(OTHER2); + contract._removeSigner(SIGNER3); + contract._removeSigner(OTHER); expect(contract.getSignerCount()).toEqual(3n); - expect(contract.isSigner(Z_SIGNER)).toEqual(true); - expect(contract.isSigner(Z_SIGNER2)).toEqual(true); - expect(contract.isSigner(Z_SIGNER3)).toEqual(false); - expect(contract.isSigner(Z_OTHER)).toEqual(false); - expect(contract.isSigner(Z_OTHER2)).toEqual(true); + expect(contract.isSigner(SIGNER)).toEqual(true); + expect(contract.isSigner(SIGNER2)).toEqual(true); + expect(contract.isSigner(SIGNER3)).toEqual(false); + expect(contract.isSigner(OTHER)).toEqual(false); + expect(contract.isSigner(OTHER2)).toEqual(true); }); }); @@ -288,7 +288,7 @@ describe('SigningManager', () => { it('should have no signers by default', () => { expect(contract.getSignerCount()).toEqual(0n); - expect(contract.isSigner(Z_SIGNER)).toEqual(false); + expect(contract.isSigner(SIGNER)).toEqual(false); }); it('should have zero threshold by default', () => { @@ -296,20 +296,20 @@ describe('SigningManager', () => { }); it('should allow adding signers then setting threshold', () => { - contract._addSigner(Z_SIGNER); - contract._addSigner(Z_SIGNER2); - contract._addSigner(Z_SIGNER3); + contract._addSigner(SIGNER); + contract._addSigner(SIGNER2); + contract._addSigner(SIGNER3); contract._changeThreshold(2n); expect(contract.getSignerCount()).toEqual(3n); expect(contract.getThreshold()).toEqual(2n); - expect(contract.isSigner(Z_SIGNER)).toEqual(true); + expect(contract.isSigner(SIGNER)).toEqual(true); }); it('should allow setting threshold then adding signers to meet it', () => { contract._setThreshold(2n); - contract._addSigner(Z_SIGNER); - contract._addSigner(Z_SIGNER2); + contract._addSigner(SIGNER); + contract._addSigner(SIGNER2); expect(contract.getSignerCount()).toEqual(2n); expect(contract.getThreshold()).toEqual(2n); @@ -323,15 +323,15 @@ describe('SigningManager', () => { it('should allow assertThresholdMet after custom setup', () => { contract._setThreshold(2n); - contract._addSigner(Z_SIGNER); - contract._addSigner(Z_SIGNER2); + contract._addSigner(SIGNER); + contract._addSigner(SIGNER2); expect(() => contract.assertThresholdMet(2n)).not.toThrow(); }); it('should fail assertThresholdMet before threshold is set', () => { - contract._addSigner(Z_SIGNER); - contract._addSigner(Z_SIGNER2); + contract._addSigner(SIGNER); + contract._addSigner(SIGNER2); expect(() => contract.assertThresholdMet(0n)).toThrow( 'Signer: threshold not set', diff --git a/contracts/src/multisig/test/mocks/MockSigner.compact b/contracts/src/multisig/test/mocks/MockSigner.compact index a3cea9fc..fac093d1 100644 --- a/contracts/src/multisig/test/mocks/MockSigner.compact +++ b/contracts/src/multisig/test/mocks/MockSigner.compact @@ -2,7 +2,7 @@ pragma language_version >= 0.21.0; import CompactStandardLibrary; -import "../../Signer"> prefix Signer_; +import "../../Signer"> prefix Signer_; export { ZswapCoinPublicKey, ContractAddress, Either, Maybe }; export { Signer__signers }; @@ -15,13 +15,19 @@ export { Signer__signers }; * where contracts don't `initialize` the signer module and instead have some sort * of gated access control prior to setting up the signers */ -constructor(signers: Vector<3, Either>, thresh: Uint<8>, isInit: Boolean) { +constructor(signers: Vector<3, Bytes<32>>, thresh: Uint<8>, isInit: Boolean) { if (disclose(isInit)) { Signer_initialize<3>(signers, thresh); } } -export circuit assertSigner(caller: Either): [] { +// Exposed in order to test that contracts cannot be reinitialized. +// DO NOT EXPOSE in production +export circuit initialize(signers: Vector<3, Bytes<32>>, thresh: Uint<8>): [] { + return Signer_initialize<3>(signers, thresh); +} + +export circuit assertSigner(caller: Bytes<32>): [] { return Signer_assertSigner(caller); } @@ -37,15 +43,15 @@ export circuit getThreshold(): Uint<8> { return Signer_getThreshold(); } -export circuit isSigner(account: Either): Boolean { +export circuit isSigner(account: Bytes<32>): Boolean { return Signer_isSigner(account); } -export circuit _addSigner(signer: Either): [] { +export circuit _addSigner(signer: Bytes<32>): [] { return Signer__addSigner(signer); } -export circuit _removeSigner(signer: Either): [] { +export circuit _removeSigner(signer: Bytes<32>): [] { return Signer__removeSigner(signer); } diff --git a/contracts/src/multisig/test/simulators/SignerSimulator.ts b/contracts/src/multisig/test/simulators/SignerSimulator.ts index ca963e5d..dc0938eb 100644 --- a/contracts/src/multisig/test/simulators/SignerSimulator.ts +++ b/contracts/src/multisig/test/simulators/SignerSimulator.ts @@ -3,11 +3,8 @@ import { createSimulator, } from '@openzeppelin-compact/contracts-simulator'; import { - type ContractAddress, - type Either, ledger, Contract as MockSigner, - type ZswapCoinPublicKey, } from '../../../../artifacts/MockSigner/contract/index.js'; import { SignerPrivateState, @@ -18,7 +15,7 @@ import { * Type constructor args */ type SignerArgs = readonly [ - signers: Either[], + signers: Uint8Array[], thresh: bigint, isInit: boolean, ]; @@ -42,7 +39,7 @@ const SignerSimulatorBase = createSimulator< */ export class SignerSimulator extends SignerSimulatorBase { constructor( - signers: Either[], + signers: Uint8Array[], thresh: bigint, isInit: boolean, options: BaseSimulatorOptions< @@ -53,7 +50,11 @@ export class SignerSimulator extends SignerSimulatorBase { super([signers, thresh, isInit], options); } - public assertSigner(caller: Either) { + public initialize(signers: Uint8Array[], thresh: bigint) { + return this.circuits.impure.initialize(signers, thresh); + } + + public assertSigner(caller: Uint8Array) { return this.circuits.impure.assertSigner(caller); } @@ -70,16 +71,16 @@ export class SignerSimulator extends SignerSimulatorBase { } public isSigner( - account: Either, + account: Uint8Array, ): boolean { return this.circuits.impure.isSigner(account); } - public _addSigner(signer: Either) { + public _addSigner(signer: Uint8Array) { return this.circuits.impure._addSigner(signer); } - public _removeSigner(signer: Either) { + public _removeSigner(signer: Uint8Array) { return this.circuits.impure._removeSigner(signer); } From b8cc403ba83c931bd1a74f2feb14dc0e7bb09c21 Mon Sep 17 00:00:00 2001 From: andrew Date: Wed, 8 Apr 2026 18:13:31 -0300 Subject: [PATCH 6/9] fix fmt --- contracts/src/multisig/test/simulators/SignerSimulator.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/contracts/src/multisig/test/simulators/SignerSimulator.ts b/contracts/src/multisig/test/simulators/SignerSimulator.ts index dc0938eb..3ade6168 100644 --- a/contracts/src/multisig/test/simulators/SignerSimulator.ts +++ b/contracts/src/multisig/test/simulators/SignerSimulator.ts @@ -70,9 +70,7 @@ export class SignerSimulator extends SignerSimulatorBase { return this.circuits.impure.getThreshold(); } - public isSigner( - account: Uint8Array, - ): boolean { + public isSigner(account: Uint8Array): boolean { return this.circuits.impure.isSigner(account); } From 4d81be4f70cbf58ea3fb60c5f46e37229853e838 Mon Sep 17 00:00:00 2001 From: andrew Date: Fri, 10 Apr 2026 12:20:02 -0300 Subject: [PATCH 7/9] add warning to _setThreshold --- contracts/src/multisig/Signer.compact | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/contracts/src/multisig/Signer.compact b/contracts/src/multisig/Signer.compact index 6835facc..ae32b0a7 100644 --- a/contracts/src/multisig/Signer.compact +++ b/contracts/src/multisig/Signer.compact @@ -232,9 +232,10 @@ module Signer { /** * @description Sets the approval threshold without checking - * against the current signer count. Intended for use during - * contract construction or custom setup flows where signers - * may not yet be registered. + * against the current signer count. + * + * @warning This is intended for use during contract construction + * or custom setup flows where signers may not yet be registered. * * @notice Access control is NOT enforced here. * The consuming contract must gate this behind its own From e61ecc4672464592b4c101170b02fabbd0e91001 Mon Sep 17 00:00:00 2001 From: andrew Date: Sun, 12 Apr 2026 15:41:06 -0300 Subject: [PATCH 8/9] document signer count/threshold limit --- contracts/src/multisig/Signer.compact | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/contracts/src/multisig/Signer.compact b/contracts/src/multisig/Signer.compact index ae32b0a7..821a8b02 100644 --- a/contracts/src/multisig/Signer.compact +++ b/contracts/src/multisig/Signer.compact @@ -20,6 +20,11 @@ pragma language_version >= 0.21.0; * This separation allows the identity mechanism to change without * modifying the module. * +* The signer count and threshold are of type `Uint<8>`, limiting the + * maximum number of signers and threshold to 255. This is sufficient + * for any practical multisig use case. For large-scale governance + * requiring more signers, consider a Merkle tree-based variant. + * * Multi-step signer reconfigurations (e.g., removing a signer and * lowering the threshold) may produce intermediate states where the * module's invariants temporarily hold but the contract's intended From c2052d251960043984884a137140b82b732ac54f Mon Sep 17 00:00:00 2001 From: andrew Date: Sun, 12 Apr 2026 15:41:53 -0300 Subject: [PATCH 9/9] fix fmt --- contracts/src/multisig/Signer.compact | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/src/multisig/Signer.compact b/contracts/src/multisig/Signer.compact index 821a8b02..93c638cf 100644 --- a/contracts/src/multisig/Signer.compact +++ b/contracts/src/multisig/Signer.compact @@ -20,7 +20,7 @@ pragma language_version >= 0.21.0; * This separation allows the identity mechanism to change without * modifying the module. * -* The signer count and threshold are of type `Uint<8>`, limiting the + * The signer count and threshold are of type `Uint<8>`, limiting the * maximum number of signers and threshold to 255. This is sufficient * for any practical multisig use case. For large-scale governance * requiring more signers, consider a Merkle tree-based variant.