Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
257 changes: 257 additions & 0 deletions contracts/src/multisig/Signer.compact
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>;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

`Uint<8>` for both count and threshold is implicit. If the intended maximum is < 255 (e.g., 100), the module gives no way to enforce this. Consider either a named constant `MAX_SIGNERS` or a `Uint<7>` type if the design intends a lower limit.

non-blocking: What do you think of this suggestion by Claude?

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();
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

non-blocking: initialize with an zero signers vector gives a misleading error ("threshold exceeds signer count") — should assert non-empty explicitly

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The 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);
}

// ─── 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
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

non-blocking: I think it worth adding this in a @warning or @notice separate block as it is important note.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The 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);
}
}
Loading
Loading