Multisig implementation#480
Draft
axic wants to merge 64 commits into
Draft
Conversation
axic
force-pushed
the
multisig
branch
4 times, most recently
from
cd68f43 to
2abe47b
Compare
Mirror std.ABIGeneric for storage: teach StorageSize / StorageType / CanStore about the primitive SOP types that Generic maps data types onto (sum / pair / unit), then bridge any type with a Generic(rep) instance to those layouts. - StorageSize sum(f,g) = 1 (tag) + max(size f, size g); () and (a,b) are already in std. - StorageType for (), (a,b) and sum(f,g): slot layouts (tag word at the base, payload at base+1 for sums; size(a)-offset second component for products). - default instance a:StorageType bridges any Generic(rep) type via its rep. - default instance a:CanStore(b) lets a storage(t) reference load/store an ADT through StorageType (the path contract field access uses); gated on Generic so the hand-written std CanStore instances keep precedence. - storeGeneric / loadGeneric helpers mirror ABIGeneric's encode / decode. Generic instances are auto-derived for local data types, so ADTs get storage support with no per-type boilerplate. Add a compile test (cases/storage-generic.solc) exercising sum, product and nested ADTs through StorageSize, StorageType, CanStore and the helpers. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DF2E75dBr7inENk4fHLbCu
Exercises the contract field-access path for an algebraic data type: reading (`return someValue` / `match someValue`) and writing (`someValue = v` / `someValue = Some(v)`) resolve `storage(Option):CanStore(Option)` through the Generic bridge added in std.StorageGeneric, with the Generic instance auto-derived for Option. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DF2E75dBr7inENk4fHLbCu
…ze asserts
- Move the ADT-storage-field test from cases/ to a runnable dispatch test
(test/examples/dispatch/storage_adt_field.{solc,json}); register it in the
dispatches group and run_contests.sh.
- Add a product ADT (Triple) alongside the sum ADT (Option) as a second
storage field, exercising both sum and product layouts and verifying the two
fields don't overlap (writing one leaves the other intact).
- Add per-type StorageSize instances (Option -> 2 slots, Triple -> 3 slots)
that forward to the representation's structural StorageSize; these give the
field-layout machinery correct slot sizes. A blanket StorageSize bridge is
impossible (std already owns the single catch-all default returning 1).
- Assert StorageSize.size for each ADT in the constructor (Option == 2,
Triple == 3).
The contract exposes uint256/()-typed accessors so the ABI boundary stays
standard; the ADTs live entirely in storage. Selectors and expected
return/revert data computed for the testrunner.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DF2E75dBr7inENk4fHLbCu
The CanStore default gated on `b:Generic(rep)`, but `rep` appeared only in the context — never in the instance head or the method bodies — so the solver could not ground it, producing "Ambiguous type variable" when resolving a field assignment (e.g. someValue = Some(v)). Gate on `b:StorageType` alone, which is exactly what store/load need; ADTs still resolve via the StorageType Generic bridge, and std's concrete CanStore instances keep precedence. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DF2E75dBr7inENk4fHLbCu
The contract field-access RVA/LVA instances resolve storage(storageType):CanStore(loadType) and must *infer* loadType from the slot type. That needs a functional (per-type) CanStore instance; the generic default instance a:CanStore(b) in StorageGeneric is non-functional (accepts any storable b), so loadType stayed an unsolved phantom -> "Ambiguous type variable" on someValue = Some(v). The default still serves direct CanStore calls where b is already known. Add concrete storage(Option):CanStore(Option) and storage(Triple):CanStore(Triple) to the dispatch test, each delegating to the StorageType bridge. These are exactly the instances a DeriveGeneric extension would emit per ADT. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DF2E75dBr7inENk4fHLbCu
Make ADTs usable in contract storage with no per-type boilerplate, the
"generate StorageType/StorageSize/CanStore for them" goal.
DeriveGeneric, when std.StorageGeneric is in scope (detected via the new
exported `StorageDeriving` marker class), now emits per data type alongside its
Generic instance:
- instance T : StorageSize (forwards to the representation's structural size,
overriding std's catch-all default of 1 so field layout gets the right slot
count)
- instance storage(T) : CanStore(T) (functional, so contract field access can
infer the stored type from the slot type; delegates to the StorageType
Generic bridge)
StorageType stays fully generic via the bridge, so it is not emitted per type.
Recursive types are skipped (unbounded storage size). Emission is gated to the
same set as Generic derivation, and only when StorageGeneric is imported, so
existing tests (generic_sum, generic_product, derive-generic-sum) are
unaffected.
std.StorageGeneric: add the StorageDeriving marker class and drop the
non-functional CanStore default (replaced by the derived per-type instances).
Tests: the dispatch test now relies entirely on derivation (manual StorageSize
and CanStore stubs removed); remove the redundant storage-generic cases test.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DF2E75dBr7inENk4fHLbCu
Fixes a specialization panic specCall: no resolution found for Generic.to : (uint256,(uint256,uint256)) -> uint256 when reading an ADT storage field. Root cause: std.StorageGeneric provided `default instance a:StorageType` whose `load` returns the head variable `a` via Generic.to. For a default instance the specializer cannot monomorphize a result-position type variable (it is not pinned by the arguments), so it mis-resolved the codomain of Generic.to. This is why ABIGeneric only uses a default for encode (Generic.from — `a` is in the argument) and makes decode (Generic.to — `a` in the result) a plain function. Fix: DeriveGeneric now emits a concrete per-type `instance T:StorageType` (alongside StorageSize and CanStore), where the data type is fixed in the instance head so Generic.to resolves. Drop the default StorageType bridge from std.StorageGeneric; the structural sum/pair/unit instances and the type's Generic representation still do the work. Concrete per-type StorageType also makes nested ADTs storable. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DF2E75dBr7inENk4fHLbCu
Make Option parametric (Option(a)) and add a third storage field
someTriple : Option(Triple) — a sum-of-product nested ADT. This exercises:
- parametric ADT derivation (StorageSize/StorageType/CanStore with a param
context),
- the nested case the concrete per-type StorageType enables: loading
Option(Triple) reuses Triple:StorageType inside sum((), Triple).
Constructor asserts the slot counts (Option(uint256)=2, Triple=3,
Option(Triple)=4); the test sequence checks set/get/clear of all three fields
and that writing one field never corrupts the others (disjoint slot layout).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DF2E75dBr7inENk4fHLbCu
When std.StorageGeneric is in scope, DeriveGeneric derived StorageSize /
StorageType / CanStore instances for every local data type. Types that carry
a dynamically-sized pointer field (memory/calldata) have no fixed storage slot
layout, so the synthesised StorageType.load body demanded an unsatisfiable
constraint, e.g. for multisig.sol's Signature type:
Cannot entail: memory(bytes) : StorageType
even though Signature is only ever passed as an argument and never stored.
Skip storage-instance derivation for such types, mirroring the existing
recursive-type skip. Non-storability is computed as a fixpoint so it
propagates through nested ADTs. These types still get a Generic instance;
they simply cannot live in storage.
Adds a dispatch regression test (storage_skip_memory.solc).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UUHqBJRMKbw9AQPxqWp28M
…denylist Replaces the earlier syntactic memory/calldata skip with a trait-driven approach: each derived storage instance now carries a context requiring its representation (StorageType/StorageSize) or the type itself (CanStore) to be storable. The instance body discharges that obligation by assumption, so a type with a non-storable field still type-checks; the unsatisfiable constraint is deferred to a real storage use (a contract field) instead of failing eagerly inside an instance nobody uses. Net effect on multisig.sol is the same as before for the reported error (Signature / BatchOperation are never stored, so their instances are never demanded and compile), but storability is now decided by the class machinery rather than a hardcoded list of pointer constructors. An Operation actually placed in storage now fails at the field where the error belongs. Because these contexts mention composite types, they exceed the Patterson measure of the small StorageType/StorageSize instance heads, and pragmas are file-scoped (an imported no-patterson-condition does not propagate). So DeriveGeneric emits the relaxation itself when it derives storage instances, keeping StorageGeneric usable without per-file pragma boilerplate. Renames the regression test to storage_unstored_memory.solc and drops its manual pragmas so it also exercises the auto-emitted relaxation (mirroring multisig.sol, which declares none). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UUHqBJRMKbw9AQPxqWp28M
The derived per-type StorageType instance forced an ADT's whole representation to be StorageType, which has no instance for memory(bytes) — so any data type carrying a dynamically-sized field (multisig's Signature/Operation) failed to derive, with "Cannot entail: memory(bytes) : StorageType". But memory(bytes) IS storable: std has storage(bytes):CanStore(memory(bytes)) (Solidity-style dynamic bytes, one slot), and contract field access already loads/stores every field through CanStore + StorageSize, never through StorageType on the value type. StorageType is only the fixed-slot leaf encoding. So route ADT storage through CanStore instead: - std.StorageGeneric: add structural CanStore instances for the SOP primitives unit / pair / sum, decomposing a value and storing each leaf via that leaf's own CanStore (fixed leaves -> storage(word)/..., dynamic -> storage(bytes)). Plus storage(memory(bytes))/storage(memory(string)) at the uniform storage(t) handle the decomposition asks for. - DeriveGeneric: drop the per-type StorageType derivation; derive storage(T):CanStore(T) that goes Generic.from/to <-> structural CanStore over the representation. Keep the per-type StorageSize (slot footprint for offsets); it already resolves memory(bytes):StorageSize = 1. No pragmas needed: the structural CanStore instances satisfy Patterson/coverage on their own, and the derived CanStore/StorageSize carry only tyvar-headed contexts. A data type with a dynamic field is now genuinely storable (multisig's Operation included), and a field whose type has no CanStore instance fails at the storage site rather than while deriving an unused instance. Regression test storage_dynamic_field.solc stores an ADT with a memory(bytes) variant in a contract field (no pragmas), mirroring Operation's Call. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UUHqBJRMKbw9AQPxqWp28M
Mapping writes already go through CanStore (Assign -> CanStore.store), but
reads went through StorageType (ridx / readStorage / RValueIdxAccess). After
dropping the per-type StorageType derivation, every mapping read of an ADT
value broke ("Cannot entail: OperationStatus : StorageType") even though the
write side was fine.
Make reads use CanStore too, so a mapping can hold any value with a CanStore
instance — including ADTs whose fields are dynamic (memory(bytes)) and which
therefore have no StorageType. ridx/readStorage/RValueIdxAccess now require
storage(a):CanStore(a) and call CanStore.load.
Add storage(bool):CanStore(bool): bool is a builtin with no StorageType /
Typedef(word) instance, but it round-trips through word via frombool/tobool,
so it can be stored. This unblocks mapping(_, bool) (e.g. multisig's
approved_signed_hashes).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UUHqBJRMKbw9AQPxqWp28M
The contract dispatch decodes method parameters from calldata via the ABIDecode
class. ABIAttribs and ABIEncode are bridged generically (default instances over
Generic), but ABIDecode cannot be a default instance — its decode returns the
head type variable via Generic.to, a result-position variable the specializer
can't monomorphize (same reason storage uses a concrete per-type instance). So
ABIGeneric only offered a free `decode` function, leaving an ADT-typed parameter
(multisig's Operation / Signature) with no ABIDecode instance:
Cannot entail: ABIDecoder(Signature, CalldataWordReader) : ABIDecode(Signature)
Add an ABIDeriving marker class to std.ABIGeneric (mirroring StorageDeriving) and
have DeriveGeneric emit, for each local ADT, a concrete
ABIDecoder(T, reader):ABIDecode(T) instance that delegates to the rep's
structural ABIDecode (the body of the free `decode`, with T fixed in the head).
The context is just reader:WordReader plus, for parametric types, each
parameter's decodability — the rep is decoded through the structural instances,
so no rep-sized context (which would break Patterson) is needed.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UUHqBJRMKbw9AQPxqWp28M
The dispatch builds each method's selector from the SigString of its parameter
types. SigString had instances for the primitives, pair, and unit, but none for
sums or for ADTs, so an ADT-typed parameter (multisig's Operation / Signature)
failed:
Cannot entail: Signature : SigString
SigString.sigStr takes Proxy(t) (the type is in argument position), so unlike
ABIDecode it can be a default Generic bridge — the same way ABIAttribs / ABIEncode
bridge in std.ABIGeneric. Add:
- sum(f,g):SigString — a tagged union has no standard ABI type, so this is the
structural comma-joined branch signatures (deterministic selector; refine if a
specific on-the-wire sum convention is wanted).
- a default `a:Generic(rep), rep:SigString => a:SigString` bridge, so any data
type inherits its signature from its representation.
dispatch.solc now imports std.Generic for the bridge constraint.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UUHqBJRMKbw9AQPxqWp28M
Restore the multisig contract source as test/examples/dispatch/multisig.solc. The earlier "Rename to .solc" commit deleted multisig.sol (467 lines) without adding the .solc, leaving the dispatch example with no source file; this brings the source back under the .solc name the contest harness expects. Add a basic multisig.json contest suite focused on add_signer/remove_signer, without changing required_signatures. To make the otherwise-internal add_signer/remove_signer observable from calldata (the real governance path queues an Operation ADT, whose dynamic Call variant is not yet ABI-decodable through the dispatcher), expose thin public test wrappers (signersCount/signersRequired/isSignerOf/addSigner/removeSigner) mirroring the test helpers already used by miniERC20.solc. The suite covers: constructor seeding the deployer as the sole signer, adding two signers, the swap-with-last removal path, idempotency reverts (SignerAlreadyExists, NotASigner, CannotRemoveOnlySigner), and asserts signers_required stays at 1 throughout. Registered in run_contests.sh. Selectors were derived with keccak-256 and validated against the existing ownable/generic_sum fixtures. NOTE: the Haskell/solc/testrunner toolchain is not available in this environment, so the suite was authored but not executed here; it should be validated via `bash run_contests.sh` / `nix flake check`. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BR2MPCQRaiQUw7uoBXMeM9
The generic `storage(sum(f, g)) : CanStore` load bound each decoded payload to a `let x : f` / `let y : g` intermediate before wrapping it with inl/inr. For `inr(y)` where `y : g`, the compiler inferred the inl/inr application's type from the intermediate's branch type (`g`) instead of the full `sum(f, g)`, so EmitHull produced `inr<g>(y)` instead of `inr<sum(f, g)>(y)`. Yul codegen then rejected the result with a sum-nesting type mismatch (expected the right-branch sum, got the parent sum — i.e. off by one inl/inr level). This surfaced on the multisig contract, whose 9-constructor `Operation` (and `OperationStatus` / `Vote`) live in storage and exercise the recursive sum load. It affects any sum type with >= 3 constructors stored on-chain. Fix: inline the loaded payload directly into inl(...) / inr(...), matching the working `ABIGeneric.decode` pattern, so the constructor application is typed against the function's `sum(f, g)` return type. Behaviour is unchanged; only the intermediate binding is removed. NOTE: the underlying compiler bug — `inr(<let-bound var of type g>)` being typed `g` rather than `sum(f, g)`, while `inr(<inline call>)` types correctly — still wants a proper fix in inference/specialisation. This is the std-side workaround that unblocks codegen. Verified by static analysis of the emitted Hull (no Haskell toolchain available in this environment); please confirm with `bash contest.sh test/examples/dispatch/multisig.json`. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BR2MPCQRaiQUw7uoBXMeM9
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)