diff --git a/CHANGELOG.md b/CHANGELOG.md index 42519074..57224fe5 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 (#424) + ### Changes - Add defensive Buffer copy to ZOwnablePKWitnesses (#397) diff --git a/contracts/src/multisig/Signer.compact b/contracts/src/multisig/Signer.compact new file mode 100644 index 00000000..93c638cf --- /dev/null +++ b/contracts/src/multisig/Signer.compact @@ -0,0 +1,263 @@ +// 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. + * + * 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 + * 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. + * + * 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 ────────────────────────────────────────────────────────────────── + + 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. + * 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. + * + * @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> + ): [] { + Initializable_initialize(); + + 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. + * + * @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 + * 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..77dee572 --- /dev/null +++ b/contracts/src/multisig/test/Signer.test.ts @@ -0,0 +1,341 @@ +import { beforeEach, describe, expect, it } from 'vitest'; +import { SignerSimulator } from './simulators/SignerSimulator.js'; + +const THRESHOLD = 2n; +const IS_INIT = true; + +// 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('Signer', () => { + 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 = [SIGNER, SIGNER, 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); + }).not.toThrow(); + + expect(contract.getThreshold()).toEqual(THRESHOLD); + expect(contract.getSignerCount()).toEqual(BigInt(SIGNERS.length)); + expect(() => { + for (let i = 0; i < SIGNERS.length; i++) { + contract.assertSigner(SIGNERS[i]); + } + }).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(() => { + contract = new SignerSimulator(SIGNERS, THRESHOLD, IS_INIT); + }); + + describe('assertSigner', () => { + it('should pass with good signer', () => { + expect(() => contract.assertSigner(SIGNER)).not.toThrow(); + }); + + it('should fail with bad signer', () => { + expect(() => { + contract.assertSigner(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(SIGNER)).toEqual(true); + }); + + it('should return false for a non-signer', () => { + expect(contract.isSigner(OTHER)).toEqual(false); + }); + }); + + describe('_addSigner', () => { + it('should add a new signer', () => { + contract._addSigner(OTHER); + + expect(contract.isSigner(OTHER)).toEqual(true); + expect(contract.getSignerCount()).toEqual(BigInt(SIGNERS.length) + 1n); + }); + + it('should fail when adding an existing signer', () => { + contract._addSigner(OTHER); + + expect(() => { + contract._addSigner(OTHER); + }).toThrow('Signer: signer already active'); + }); + + it('should add multiple new signers', () => { + contract._addSigner(OTHER); + contract._addSigner(OTHER2); + + 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(SIGNER)).toEqual(true); + + contract._removeSigner(SIGNER); + expect(contract.isSigner(SIGNER)).toEqual(false); + + contract._addSigner(SIGNER); + expect(contract.isSigner(SIGNER)).toEqual(true); + }); + }); + + describe('_removeSigner', () => { + it('should remove an existing signer', () => { + contract._removeSigner(SIGNER3); + + 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(OTHER); + }).toThrow('Signer: not a signer'); + }); + + it('should fail when removal would breach threshold', () => { + contract._removeSigner(SIGNER3); + + expect(() => { + contract._removeSigner(SIGNER2); + }).toThrow('Signer: removal would breach threshold'); + }); + + it('should allow removal after threshold is lowered', () => { + contract._changeThreshold(1n); + contract._removeSigner(SIGNER3); + contract._removeSigner(SIGNER2); + + expect(contract.getSignerCount()).toEqual(1n); + 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(OTHER); + contract._addSigner(OTHER2); + contract._removeSigner(SIGNER3); + contract._removeSigner(OTHER); + + expect(contract.getSignerCount()).toEqual(3n); + 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); + }); + }); + + 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(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(SIGNER); + contract._addSigner(SIGNER2); + contract._addSigner(SIGNER3); + contract._changeThreshold(2n); + + expect(contract.getSignerCount()).toEqual(3n); + expect(contract.getThreshold()).toEqual(2n); + expect(contract.isSigner(SIGNER)).toEqual(true); + }); + + it('should allow setting threshold then adding signers to meet it', () => { + contract._setThreshold(2n); + contract._addSigner(SIGNER); + contract._addSigner(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(SIGNER); + contract._addSigner(SIGNER2); + + expect(() => contract.assertThresholdMet(2n)).not.toThrow(); + }); + + it('should fail assertThresholdMet before threshold is set', () => { + 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 new file mode 100644 index 00000000..fac093d1 --- /dev/null +++ b/contracts/src/multisig/test/mocks/MockSigner.compact @@ -0,0 +1,65 @@ +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, Bytes<32>>, thresh: Uint<8>, isInit: Boolean) { + if (disclose(isInit)) { + Signer_initialize<3>(signers, thresh); + } +} + +// 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); +} + +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: Bytes<32>): Boolean { + return Signer_isSigner(account); +} + +export circuit _addSigner(signer: Bytes<32>): [] { + return Signer__addSigner(signer); +} + +export circuit _removeSigner(signer: Bytes<32>): [] { + 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..3ade6168 --- /dev/null +++ b/contracts/src/multisig/test/simulators/SignerSimulator.ts @@ -0,0 +1,92 @@ +import { + type BaseSimulatorOptions, + createSimulator, +} from '@openzeppelin-compact/contracts-simulator'; +import { + ledger, + Contract as MockSigner, +} from '../../../../artifacts/MockSigner/contract/index.js'; +import { + SignerPrivateState, + SignerWitnesses, +} from '../../witnesses/SignerWitnesses.js'; + +/** + * Type constructor args + */ +type SignerArgs = readonly [ + signers: Uint8Array[], + 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: Uint8Array[], + thresh: bigint, + isInit: boolean, + options: BaseSimulatorOptions< + SignerPrivateState, + ReturnType + > = {}, + ) { + super([signers, thresh, isInit], options); + } + + public initialize(signers: Uint8Array[], thresh: bigint) { + return this.circuits.impure.initialize(signers, thresh); + } + + public assertSigner(caller: Uint8Array) { + 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: Uint8Array): boolean { + return this.circuits.impure.isSigner(account); + } + + public _addSigner(signer: Uint8Array) { + return this.circuits.impure._addSigner(signer); + } + + public _removeSigner(signer: Uint8Array) { + 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 = () => ({});