Multisig implementation - #480
Draft
axic wants to merge 57 commits into
Draft
Conversation
axic
force-pushed
the
multisig
branch
5 times, most recently
from
June 25, 2026 14:05
2abe47b to
7d6625e
Compare
axic
force-pushed
the
multisig
branch
2 times, most recently
from
July 26, 2026 03:39
9083a46 to
1623652
Compare
Adds a contest-style JSON test suite for the Multisig contract
(test/examples/dispatch/multisig.solc) and wires it into
run_contests.sh.
Under the testrunner's fixed sender (account 0 == the sole
constructor-set signer, required threshold 1), the suite covers:
- deployment (constructor)
- selector dispatch + ABI decode of uint256, (uint256,bytes), and
ADT-typed (Operation sum-of-products) method arguments
- operation lifecycle: queue(AddSigner) -> approve -> execute
- state-machine guards: OperationNotFound, SignerAlreadyApproved,
IncorrectSequence (strict ordering / no re-execute)
- queue sanity check revert (ChangeSigRequired(0) below minimum)
- reject flow: rejected operations are skipped as a no-op on execute
- payable fallback: accepts bare ETH transfers, rejects calldata
- per-method payable enforcement (NonPayableReceivedValue)
Selectors are keccak256(name + "(" + structural sigStr(args) + ")")
per std/dispatch.solc; sum-typed argument calldata follows the
tag-per-branch wire layout from std/ABIGeneric.solc.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01A4DS126xRP423EhyQQZ1pE
Extends the Multisig test suite with the ...WithSignature layer:
- registers a signer whose private key is known (AddSigner executed
by signer[0]), then exercises queueWithSignature with a real
secp256k1 EIP-2098 compact signature over the contract's (stubbed,
constant) signing hash keccak256(bytes32(1)):
* valid signer signature -> operation queued (caller not checked)
* non-signer signature -> NotASigner revert (0x12345678)
This covers the ECDSA branch of checkSignature and the ADT-typed
two-argument dispatch path queueWithSignature(Operation, Signature),
whose args decode as a pair with the Signature sum read inline at
headOffset + headSize(Operation).
The contract-signature (approved-hash) and EIP-1271 branches are
intentionally omitted: both staticcall an external signer contract,
and the integration testrunner deploys only the contract under test,
so there is no way to place signer-contract code at another address.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01A4DS126xRP423EhyQQZ1pE
The testrunner mocks precompiles with a static input->output map and aborts on any unseen ecrecover input. The multisig ...WithSignature tests recover over the contract's stubbed signing hash keccak256(bytes32(1)) = 0xb10e2d..., which no existing vector covers. Adds the two real recoveries used by multisig.json (v=28, low-s): - signer K -> 0xe05fcc23807536bee418f142d19fa0d21bb0cff7 - non-signer -> 0x0376aac07ad725e01357b1725b5cec61ae10473c Both are genuine secp256k1 recoveries, matching the r/s the JSON passes in the EIP-2098 Signature. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A4DS126xRP423EhyQQZ1pE
isValidSignature(bytes32,bytes) is the ERC-1271 entry point external
verifiers call, so it must be reachable through the contract dispatch.
Marking it `public` requires an ABIEncode instance for its bytes4
return, which std lacked (only bytes32 had one) -- add bytes4:ABIEncode,
mirroring bytes32 (the word rep is right-aligned in Solcore, so it is
written directly; headSize/isStatic come from the default ABIAttribs).
Its dispatch selector is keccak256("isValidSignature(bytes32,bytes)")
= 0x1626ba7e, i.e. exactly the EIP-1271 magic value.
New multisig.json coverage (also exercises the ApproveSignedHash
operation, previously untested):
- ApproveSignedHash(h) queued/approved/executed -> approved_signed_hashes[h]
- isValidSignature(h, "") -> returns the magic 0x1626ba7e
- isValidSignature(unknown,"") -> HashNotApproved
- isValidSignature(h, 0x01) -> EmptySignatureExpected
The bytes4 return is encoded right-aligned to match the contract's own
consumer convention (eip1271_verify checks `res == 0x1626ba7e` as a full
word); strict left-aligned EIP-1271 wire format would need both this
instance and eip1271_verify updated together.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01A4DS126xRP423EhyQQZ1pE
create_signature_hash returned a constant keccak256(bytes32(1)) stub, so the signed hash did not bind the operation. The intended keccak256(abi.encode(kind, operation)) cannot be spelled today: std's generic sum:ABIEncode is "static sums only", and Operation is a dynamic sum (its Call(address, uint256, memory(bytes)) branch), so abi_encode(operation) has no correct encoding until the symmetric dynamic-sum encode (the counterpart of the existing dynamic-sum ABIDecode) is added to std.ABIGeneric. Until then, derive the signing preimage explicitly with the already-working concat/keccak256_ helpers: keccak256([kind tag][constructor tag][fields...]) one 32-byte word per scalar field, dynamic bytes appended verbatim. It is only ever hashed, never decoded, so determinism + injectivity over (kind, operation) is all that is required. A TODO marks the abi.encode collapse for later. Because the signing hash is now operation-specific, the two stubbed ecrecover vectors for the queueWithSignature tests are recomputed to the new hashes (keccak256(bytes32(0) || bytes32(0) || bytes32(address)) for the AddSigner ops); r/s/v and the recovered signer/non-signer addresses are unchanged, so multisig.json needs no edit. Note: not verified against the build toolchain (unavailable in this environment) -- run `bash run_contests.sh` (multisig.json) to confirm. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HdTaDVFUfQwQzJRqxNfNW1
create_signature_hash previously hashed a bare, injective preimage keccak256([kind tag][constructor tag][fields...]) which bound a signature to neither the chain nor the deployment, so it could be replayed across contracts. Switch it to a proper EIP-712 typed digest using the std.eip712 helpers: keccak256(0x1901 || domainSeparator || hashStruct(message)) - domain: EIP712Domain(string name,string version,uint256 chainId, address verifyingContract), with name "Multisig", version "1", chainId from chainid() and verifyingContract = this contract. - message: MultisigOperation(uint256 kind,bytes operation), where `operation` is the same deterministic [tag][fields...] encoding as before (minus the kind word). Per EIP-712 the dynamic bytes member is hashed, so hashStruct binds the identical information the old preimage did, now domain-separated. The testrunner mocks the ecrecover precompile with a static (hash,v,r,s) -> address map, so the two queueWithSignature dispatch vectors are rekeyed on the recomputed EIP-712 digests (verifyingContract is the CREATE address of deployer 0x1212..0012 at nonce 1, chainId 1). The r/s and recovered signer/non-signer addresses are unchanged, so the dispatch paths exercised are identical. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MuwWq7e8RCzu5ouFjVzdcu
The multisig test calldata packed the Operation and Signature arguments inline ([tag][fields...], the static-sum layout), but Operation and Signature are DYNAMIC sums (their Call / EIP1271 branches carry `memory(bytes)`), so the compiler's ABIDecode (std.ABIGeneric) reads each via a 32-byte offset pointing at a [tag][branch] tail. The inline form only decoded correctly for AddSigner, where the leading tag word is 0 and happens to double as a valid self-offset; every other variant was read with its first tag word mistaken for an offset: - queue(ChangeSigRequired(0)) mis-decoded into a no-op branch instead of reverting (ThresholdBelowMinimum), and being stored shifted every later operation index -- cascading into the execute(2) failure. - queueWithSignature(...) mis-read the Signature (the second dynamic-sum arg) off the end of calldata as ECDSA(0,0), so eip2098_signer called ecrecover(hash, 27, 0, 0), an input the mocked precompile doesn't define -> abort. Re-encode all seven Operation/Signature-bearing calldatas with proper dynamic-sum offset encoding (nested dynamic sums rebased at each level; inner static sums -- the bytes32-only tail -- stay inline). The decoded operations are unchanged, so the EIP-712 signing digests and the ecrecover vectors registered in EVMHost.cpp already match: with the signature now decoding correctly, eip2098 yields (digest, 28, r, s) exactly as those vectors expect. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MuwWq7e8RCzu5ouFjVzdcu
…ne call) Exercises the batching layer end-to-end: a single batch() call carrying [Queue(AddSigner(cafe0006), ECDSA), Approve(5, ECDSA), Execute(5, "")] queues op argotorg#5 via signature, approves it, and executes AddSigner in one transaction. The ECDSA (r,s) is the existing signer-K vector, reused across queue and approve because create_signature_hash currently returns a constant hash. The calldata is the solcore generic-ABI encoding of calldata(array(BatchOperation)): a dynamic-element array (offset table) whose elements are the 4-constructor BatchOperation sum, with dynamic Operation and Signature sub-values carried via offset indirection. The batch() selector (121765fe) is the keccak of the structural signature that dispatch derives from BatchOperation's Generic representation. A follow-up approve(5) asserts op argotorg#5 is now Executed (UnexpectedStatus), confirming the batch ran all three sub-operations rather than just not reverting. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ni57Ez3ArtkjGtcVM1DJsZ
The batch() test runs [Queue(AddSigner(cafe0006)), Approve(op#5), Execute(op#5)] in one call. With create_signature_hash now an EIP-712 digest, the two ...WithSignature legs sign distinct digests (the kind word differs: Queue=0 vs Approve=1), so the mocked ecrecover needs a vector for each. Both recover to the registered signer e05f..cff7, reusing the signer-K v/r/s; only the leading EIP-712 digest differs per leg. Digests were derived from multisig's create_signature_hash / std.eip712 (domain bound to chainId 1 and verifyingContract c06a..e79e, the CREATE address of the deployer at nonce 1) and cross-checked by reproducing the existing cafe0003/cafe0004 queue vectors. Also refresh the batch test comment, which still claimed a constant signing hash from before the EIP-712 change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ni57Ez3ArtkjGtcVM1DJsZ
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Disclaimer: this is work in progress, for discussion only and not review.
There are three blockers currently:
keccak256(concat(abi_encode(kind), abi_encode(operation)))linearray(t)