build!: upstream Safe v1.5.0, forge-std v1.11.0, and via_ir by default by mfw78 · Pull Request #62 · nullislabs/composable-cow

mfw78 · 2026-08-02T23:13:33Z

Based on develop. #59 is stacked on this, so this merges first.

Drops the safe-contracts fork, moves to upstream Safe v1.5.0, bumps forge-std, and makes via_ir the default. Four commits, readable in order.

Why

via_ir produces smaller bytecode. That matters most for OwnedTWAP: at 15,478 bytes it is the only contract with a real EIP-170 ceiling to worry about.
It could not be enabled, and every blocker sat in a dependency rather than in this repository.

What

1. Vendor the Safe interfaces

src imported the fork for six symbols. All are small, and Safe is only ever a typed address: isValidSafeSignature takes one as a parameter, and _auth reads this registry's own roots and singleOrders.
src/vendor/Safe.sol declares them, identically to upstream. rg 'safe/' src/ now returns nothing.
ExtensibleFallbackHandler is not vendored at all. It was only ever asked supportsInterface, so IERC165 says the same thing.

2. Upstream Safe v1.5.0, forge-std v1.11.0, via_ir

lib/safe was cowdao-grants/extensible-fallback-handler, pinned by commit on main. Its Safe.sol uses inline assembly that is not annotated memory-safe, so the IR pipeline cannot allocate its stack:
```
Cannot swap Variable var_data_offset with Variable _1: too deep in the stack by 5 slots
No memoryguard was present.
```
Upstream absorbed the ExtensibleFallbackHandler work, and in v1.5.0 every assembly block in Safe.sol is assembly ("memory-safe"). lib/safe is now safe-global/safe-smart-account at tag v1.5.0, so the repository depends on a tagged release rather than a commit on a fork's branch.
forge-std v1.5.1 then hit the same limit in StdStorage.find, which reaches every test through Test.sol. Bumped to v1.11.0, which also declares parseJsonKeys natively, so the locally declared interface in the descriptor test is gone.
With all three cleared, via_ir is the default. There is one bytecode again: tests, the gas snapshot, the descriptors and whatever gets deployed are all built the same way.

The Safe bump is not source compatible:

Change	Files
`safe/common/Enum.sol` to `safe/libraries/Enum.sol`	4
`MarshalLib` no longer re-exported by `ExtensibleFallbackHandler.sol`	1
`Safe.sol` no longer re-exports `IERC165` or `Enum`	1
a relative `../lib/safe/contracts/...` import in `deploy_ProdStack`	1

That last one is worth knowing about: it bypassed the remapping entirely, so the fork kept being compiled no matter what safe/ pointed at.

One behavioural difference, accepted knowingly:

The fork replaced the manual calldata offsets in SignatureVerifierMuxer with a single abi.decode. Upstream v1.5.0 still computes offsets by hand, so this reverts that change. It sits on the Safe-owner settlement path.
The other two fork commits do not matter: the ExtensibleBase rename is present upstream, and the remaining one only corrects a comment.

3. Regenerate the gas snapshot

Every one of the 122 shared entries moved, so it is a separate commit.
The delta should not be read as a via_ir result. It conflates two changes: tests dominated by contract deployment fall sharply, up to 195k, because the bytecode is smaller, while the Safe-based end to end tests rise by around 110k, which is upstream v1.5.0 being different code from the fork.
The median entry moves +2.72%, so runtime gas is not uniformly better.

Contract	before	after
TWAP	12,629	11,483	-9.1%
GoodAfterTime	9,870	9,024	-8.6%
StopLoss	10,183	9,360	-8.1%
OwnedTWAP	15,478	14,316	-7.5%
ComposableCow	10,659	10,047	-5.7%

4. Merkle tree shape note (docs only, touches no code)

An unrelated rider, kept here rather than in its own PR. Say the word and it moves.
Solady's MerkleTreeLib does not produce the leafEncoding: "v1" shape that docs/discovery.md pins: it builds a complete 2n-1 node tree, which diverges from the odd-promotion rule at 5, 7 and 9 leaves (2, 3, 4, 6 and 8 agree, measured).
Nothing is broken by this. _auth verifies through MerkleProofLib.verify, which is sorted-pair and therefore shape-agnostic, so any self-consistent tree passes.
The trap is the payload rule requiring consumers to recompute root from leaves: an integrator reaching for MerkleTreeLib computes a non-conforming root at those sizes and rejects valid payloads.
Now named alongside OpenZeppelin's StandardMerkleTree, which was already called out.

Testing

182 tests pass at the tip of feat: rotatable handler variants and CREATE2 deployment scripts #59, 172 here.
forge fmt --check clean, descriptors current.
CI does a clean checkout with submodules: recursive, so a green run is also the proof that the new submodule pins resolve.
Gas and size numbers above are measured on this branch, not estimated.
The tree shape divergence was measured over leaf counts 2 to 9 rather than reasoned about.

Note for anyone touching this branch

jj does not record submodule gitlink changes. Both bumps had to be written with git plumbing, and jj git push on this branch will silently revert lib/safe and lib/forge-std to the old pins. Use git push.

AI Assistance

AI Assistance: Claude Code used for the dependency migration, the vendored interface file, the gas and size measurements, the merkle shape measurement, and this PR description.

Based on `develop`. #62 will be restacked on top of this. `src` used OpenZeppelin for exactly two things: `MerkleProof.verify` at a single call site, and `SafeCast.toUint32` in three files. Both have direct solady equivalents, and `MerkleProofLib.verify` has a memory variant matching the existing call, so the swap is mechanical. **murky goes too.** Its `Merkle` was only used by the test helper, and solady's `MerkleTreeLib` covers the same surface. That removes the transitive `openzeppelin-contracts` murky vendored, so this is a real removal rather than a partial one. It also aligns construction with verification: `ComposableCow` verifies with `MerkleProofLib`, and `MerkleTreeLib` is designed to pair with it. Previously trees were built with murky and verified with solady, which happened to work. ### The two are not interchangeable, which was worth checking For leaf counts that are not powers of two they build **different trees**: | leaves | 2 | 3 | 4 | 5 | 6 | 7 | 8 | 9 | |--------|---|---|---|---|---|---|---|---| | same root | yes | **no** | yes | **no** | **no** | **no** | yes | **no** | What holds either way is that a proof verifies against a root from the **same** library, for every count tested. That is the only property anything here relies on, since each tree is built and verified by one library and nothing mixes them. Worth stating because the naive reading of "they diverge" would be that the previous setup was broken; it was not. ## Gas Measured against the same Safe, forge-std and optimizer settings, so this isolates the dependency change. | Test | delta | |------|------:| | `setRootWithContext_e2e` | **-41,025** | | `setRoot_e2e` | -41,003 | | `settle_e2e` | -39,552 | | `setSwapGuard_e2e` | -11,330 | | `payloadTree_NormativeConstruction` | -7,210 | | `createAndRemove_e2e` | -5,786 | **95 of 128 entries move, 35 rise but none by more than 45, total -152,161 gas.** Median unchanged at 0.000%. The merkle-heavy paths dominate, which is what changing the tree library should look like. `ComposableCow` drops 10,659 to 10,595 bytes. `getRootAndProof` also loses its two function pointers, which existed only to inject murky's `getRoot` and `getProof`. ## The ERC20 question `IERC20` in `GPv2Order.Data` was never OpenZeppelin's. It comes from cowprotocol through `IConditionalOrder` and `BaseConditionalOrder`, so handler `Data` structs were already consistent with the settlement types. Nothing about that changes here, and there is no mismatch to introduce. The one place OpenZeppelin's `IERC20` did leak in was `test/vendored/GPv2TradeEncoder.sol`, which now takes cowprotocol's like everything else. ## Test mocks Moved too, so this is a removal rather than a partial one. `MockERC20` is rebuilt on solady's `ERC20`, which declares `name` and `symbol` abstract rather than storing them in the base, so the mock holds them now. ## Dependencies after this Four submodules: `forge-std`, `cowprotocol`, `safe`, `solady`. OpenZeppelin is gone directly and transitively. ## Verification 172 tests pass, `forge fmt --check` clean, and `rg 'openzeppelin|murky' src/ test/ script/` returns nothing. ## Note for anyone touching this branch `jj` does not record submodule gitlink changes, and this commit adds one (`lib/solady`) and removes two (`lib/@openzeppelin`, `lib/murky`). The tree was assembled with git plumbing, so **`jj git push` here will undo both.** Use `git push`.

… via_ir `src` no longer depends on `safe-contracts`. The six symbols it used are vendored into `src/vendor/Safe.sol`, declared identically so a contract written against either compiles against this. The dependency was a compilation liability rather than a design one. `Safe.sol` uses inline assembly that is not annotated memory-safe, so the IR pipeline cannot allocate its stack and `via_ir` fails on the whole project. Nothing in `src` needed the implementation: `Safe` is only ever a typed address, since `isValidSafeSignature` takes one as a parameter and `_auth` reads this registry's own `roots` and `singleOrders`. `ExtensibleFallbackHandler` is not vendored at all; it was only ever asked `supportsInterface`, so `IERC165` says the same thing. With `src` clean, `via_ir` compiles, and the deployed bytecode is smaller: TWAP 12,629 -> 11,483 -9.1% GoodAfterTime 9,870 -> 9,024 -8.6% TradeAboveThreshold 7,844 -> 7,194 -8.3% StopLoss 10,183 -> 9,360 -8.1% OwnedTWAP 15,478 -> 14,316 -7.5% ComposableCow 10,659 -> 10,047 -5.7% It cannot be the default profile. The tests and the four Safe-side deploy scripts still link `safe-contracts`, deliberately: they are the only thing proving the `ExtensibleFallbackHandler` and `domainVerifier` path works, which is the integration this registry primarily exists to serve. Mocking that would test the mock. So `[profile.prod]` carries `via_ir` and skips them, and CI builds it alongside the normal one so the two cannot diverge unnoticed. Whatever is deployed must come from `FOUNDRY_PROFILE=prod`. The gas snapshot, the descriptors and the test suite all run against the legacy build. Tests keep the real `Safe` for fixtures and use the vendored type only where they call into the registry, which is the one place the two meet.

Replaces the two-profile split from the previous commit. `via_ir` is now the default, so the tests, the gas snapshot, the descriptors and whatever gets deployed all run against the same bytecode. Maintaining two was a footgun. Three things blocked it, and each turned out to be a dependency rather than our code: - `src` imported the Safe fork. Fixed in the previous commit by vendoring the six interfaces it used. - `lib/safe` was `cowdao-grants/extensible-fallback-handler`, pinned by commit on `main`, whose `Safe.sol` uses inline assembly that is not annotated memory-safe, so the IR pipeline cannot allocate its stack. It is now `safe-global/safe-smart-account` at tag `v1.5.0`, where every assembly block in `Safe.sol` is `assembly ("memory-safe")`. Upstream absorbed the `ExtensibleFallbackHandler` work, so nothing is lost by leaving the fork. - `forge-std` v1.5.1's `StdStorage.find` hit the same stack limit, and `forge-std` reaches every test through `Test.sol`. Bumped to v1.11.0. The Safe bump is not source compatible. `Enum` moved to `libraries/`, `MarshalLib` is no longer re-exported from `ExtensibleFallbackHandler.sol`, and `Safe.sol` no longer re-exports `IERC165` or `Enum`. One import in `deploy_ProdStack` also reached into `../lib/safe/contracts/...` directly, bypassing the remapping, which is why the fork kept being compiled no matter what `safe/` pointed at. One behavioural difference is accepted knowingly. The fork replaced the manual calldata offsets in `SignatureVerifierMuxer` with a single `abi.decode`; upstream v1.5.0 still computes offsets by hand. Moving upstream reverts that. The other two fork commits do not matter: the `ExtensibleBase` rename is present upstream, and the remaining one only corrects a comment. `forge-std` v1.11.0 declares `parseJsonKeys`, so the local interface in the descriptor test is gone. `getRootAndProof` merges its two loops and scopes its intermediates. That was an attempt to fix the stack error before the real cause was found, and is kept only because it reads better. 182 tests pass, formatting is clean, descriptors are current, and the whole project compiles under `via_ir`.

Kept separate from the build change so that diff stays readable, because every one of the 122 shared entries moved. The delta conflates two things and should not be read as a via_ir result. Tests dominated by contract deployment fall sharply, up to 195k, since the bytecode is smaller. The Safe-based end to end tests rise by around 110k, which is upstream v1.5.0 being different code from the fork rather than anything the IR pipeline did. The median entry moves +2.72%, so runtime gas is not uniformly better. Six new entries: the suite gained tests since this file was last generated. Generated with the seed CI pins, so fuzz entries are reproducible.

`leafEncoding: "v1"` fixes the tree shape, but sorted-pair verification is shape-agnostic, so a tree built by the wrong library still verifies against its own root. The divergence only surfaces when a consumer recomputes `root` from `leaves`, which the payload rules require. Solady's `MerkleTreeLib` builds a complete 2n-1 node tree and diverges from the odd-promotion construction at 5, 7 and 9 leaves (measured; 2, 3, 4, 6 and 8 agree). It now ships as a dependency, so it is the likeliest wrong turn for an integrator, alongside OpenZeppelin's `StandardMerkleTree`.

Based on `develop`. The payload standard documents a merkle leaf encoding that `_auth` rejects. An integrator following the published `leafEncoding: "v1"` spec would build a tree whose every proof fails. ## Why - `_auth` computes `keccak256(bytes.concat(hash(params)))`, and `hash(params)` is itself `keccak256(abi.encode(params))`. The leaf is therefore hashed **twice**. - `docs/discovery.md` documented `leaf = keccak256(abi.encode(ConditionalOrderParams))`, a single hash. Not a wording slip: a tree built that way is rejected outright with `ProofNotAuthed`. - The contract is correct and unchanged in behaviour. The spec describing it, a note about that spec, and the test named after it were all wrong. ## What **1. The spec now matches the contract** - `leaf = keccak256(keccak256(abi.encode(ConditionalOrderParams)))`, stated against the `bytes.concat` form in `_auth` so the two can be compared by eye. - Says why the second hash is there: it keeps a leaf preimage from ever being 64 bytes, which is what would otherwise let an internal node be presented as a leaf. **2. The `StandardMerkleTree` note was wrong on its stated ground** - The note added in #62 disqualified OpenZeppelin's `StandardMerkleTree` partly because it double-hashes leaves. This double-hashes too, so that was never the difference. - Tree shape remains the real incompatibility, for `StandardMerkleTree` and for Solady's `MerkleTreeLib`, which is unchanged from #62 and still measured at 5, 7 and 9 leaves. - Adds the distinction that matters operationally: a wrong tree *shape* is silent under verification and only surfaces when a consumer recomputes `root` from `leaves`, whereas a wrong *leaf encoding* fails immediately on every proof. **3. `test_payloadTree_NormativeConstructionVerifiesLikeAuth` could not catch it** - Despite the name it never called `_auth`. It built single-hashed leaves and verified them against their own root with `MerkleProofLib`, which any self-consistent tree passes, so it was wrong in exactly the way the spec was and still green. - It now goes through `getTradeableOrderWithSignature`, so `_auth` is what accepts or rejects the construction. - `test_payloadTree_SingleHashedLeafIsRejected` pins the old encoding as rejected, so the spec cannot drift back without a failure. **4. `EfficientHashLib` at the leaf site** - `keccak256(bytes.concat(hash(params)))` becomes `EfficientHashLib.hash(hash(params))`: identical value, without allocating a 32-byte `bytes` to hash once and discard. - **696 gas** off each merkle-root end to end path, **352** off TWAP settlement, **1,833** across the suite. 23 of 138 snapshot entries move. - It is the only site in `src/` where it applies. `hash(params)` at line 373 encodes a struct with a dynamic `bytes staticInput`, so the encoding has to be materialised anyway, and that value is the order id. ## Testing - 173 tests pass; `forge fmt --check` clean; gas snapshot regenerated. - Mutation-checked, both directions: | Mutation | Killed by | |----------|-----------| | test leaf reverted to the single hash the spec described | `test_payloadTree_NormativeConstructionVerifiesLikeAuth` | | contract leaf single-hashed | `test_payloadTree_SingleHashedLeafIsRejected`, and 2 others | - The first mutation is the one that matters: it is the exact state the test was in before this PR, and it now fails. ## AI Assistance AI Assistance: Claude Code used for the investigation, the fix, the tests and this PR description.

mfw78 changed the base branch from feat/admin-commitments to develop August 2, 2026 23:26

mfw78 force-pushed the build/vendor-safe-via-ir branch from a1acc1f to 0c2475c Compare August 2, 2026 23:26

mfw78 changed the title ~~build!: vendor the Safe interfaces and build deployment bytecode with via_ir~~ build!: upstream Safe v1.5.0, forge-std v1.11.0, and via_ir by default Aug 3, 2026

mfw78 mentioned this pull request Aug 3, 2026

build!: replace OpenZeppelin and murky with solady #63

Merged

mfw78 force-pushed the build/vendor-safe-via-ir branch from 8ebe1be to 47f68b9 Compare August 3, 2026 01:29

mfw78 changed the base branch from develop to build/solady August 3, 2026 01:29

mfw78 force-pushed the build/solady branch from 2366d8c to 7cfc472 Compare August 3, 2026 02:48

mfw78 force-pushed the build/vendor-safe-via-ir branch from 47f68b9 to 398dc6e Compare August 3, 2026 02:48

Base automatically changed from build/solady to develop August 3, 2026 02:50

mfw78 added 3 commits August 3, 2026 02:53

mfw78 force-pushed the build/vendor-safe-via-ir branch from 398dc6e to 5218889 Compare August 3, 2026 02:54

mfw78 merged commit 17a3f24 into develop Aug 3, 2026
1 check passed

mfw78 deleted the build/vendor-safe-via-ir branch August 3, 2026 04:20

This was referenced Aug 3, 2026

feat: rotatable handler variants and CREATE2 deployment scripts #59

Open

fix(docs): the merkle leaf is hashed twice, not once #64

Merged

Navigation Menu

Uh oh!

build!: upstream Safe v1.5.0, forge-std v1.11.0, and via_ir by default - #62

build!: upstream Safe v1.5.0, forge-std v1.11.0, and via_ir by default#62
mfw78 merged 4 commits into
developfrom
build/vendor-safe-via-ir

mfw78 commented Aug 2, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

mfw78 commented Aug 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why

What

Testing

Note for anyone touching this branch

AI Assistance

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

mfw78 commented Aug 2, 2026 •

edited

Loading