-
Notifications
You must be signed in to change notification settings - Fork 25
Add multisig signer #424
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Add multisig signer #424
Changes from all commits
0a089c0
8180d9e
4c48ac8
10d0b65
c3125de
b8cc403
2ba0026
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,257 @@ | ||
| // SPDX-License-Identifier: MIT | ||
| // OpenZeppelin Compact Contracts v0.0.1-alpha.1 (multisig/Signer.compact) | ||
|
|
||
| pragma language_version >= 0.21.0; | ||
|
|
||
| /** | ||
| * @module Signer<T> | ||
| * @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. | ||
| * | ||
| * 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<T> { | ||
| import CompactStandardLibrary; | ||
| import "../security/Initializable" prefix Initializable_; | ||
|
|
||
| // ─── State ────────────────────────────────────────────────────────────────── | ||
|
|
||
| export ledger _signers: Set<T>; | ||
| 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<n, T>} signers - The initial signer set. | ||
| * @param {Uint<8>} thresh - The minimum number of approvals required. | ||
| * @returns {[]} Empty tuple. | ||
| */ | ||
| export circuit initialize<#n>( | ||
| signers: Vector<n, T>, | ||
| thresh: Uint<8> | ||
| ): [] { | ||
| Initializable_initialize(); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. non-blocking:
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. hmm idk if I agree. Consider that the threshold cannot be zero. So if we have a threshold of two, for example, and only one signer, then I assume we can agree that this is a "threshold exceeds signer count", right? If we provide zero signers, isn't that the exact same issue...just with zero signers instead of one? |
||
|
|
||
| for (const signer of signers) { | ||
| _addSigner(signer); | ||
| } | ||
|
|
||
| _changeThreshold(thresh); | ||
| } | ||
coderabbitai[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| // ─── 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. | ||
|
Comment on lines
+234
to
+237
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. non-blocking: I think it worth adding this in a
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good idea! |
||
| * | ||
| * @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); | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
non-blocking: What do you think of this suggestion by Claude?