build!: replace OpenZeppelin and murky with solady - #63
Merged
Conversation
`src` used OpenZeppelin for 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 exactly, 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 also removes the transitive
`openzeppelin-contracts` murky vendored, so this is a real removal rather than a
partial one, and it means trees are now built and verified by the same family:
`ComposableCow` verifies with `MerkleProofLib`, and `MerkleTreeLib` is designed
to pair with it.
The two libraries are not interchangeable, which was worth checking rather than
assuming. For leaf counts that are not powers of two they build different trees:
3, 5, 6, 7 and 9 all diverge, while 2, 4 and 8 agree. What holds either way is
that a proof verifies against a root from the same library, which is the only
property anything here relies on, since each tree is built and verified by one
library. Nothing mixes them.
Measured against the same Safe, forge-std and optimizer settings, so this
isolates the dependency change:
95 of 128 snapshot entries move
setRootWithContext_e2e -41,025
setRoot_e2e -41,003
settle_e2e -39,552
setSwapGuard_e2e -11,330
payloadTree -7,210
createAndRemove -5,786
35 entries rise, none by more than 45
total -152,161 gas
The merkle-heavy paths dominate, which is what you would expect when the tree
library changes. The median entry is unchanged.
`ComposableCow` also drops from 10,659 to 10,595 bytes. The rest are unchanged
in size, which is expected: only the merkle and cast paths moved.
Test mocks move too, so this is a removal rather than a partial one.
`MockERC20` is rebuilt on solady's `ERC20`, which declares `name` and `symbol`
abstract instead of storing them in the base, so the mock now holds them.
`GPv2TradeEncoder` took `IERC20` from OpenZeppelin where cowprotocol's is the
one every other type uses, so it now takes cowprotocol's.
The `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 and nothing
about that changes here.
`getRootAndProof` loses its two function pointers. They existed to inject
murky's `getRoot` and `getProof`, and with `MerkleTreeLib` called directly there
is nothing to inject.
Four submodules remain: forge-std, cowprotocol, safe and solady.
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
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.
Based on
develop. #62 will be restacked on top of this.srcused OpenZeppelin for exactly two things:MerkleProof.verifyat a single call site, andSafeCast.toUint32in three files. Both have direct solady equivalents, andMerkleProofLib.verifyhas a memory variant matching the existing call, so the swap is mechanical.murky goes too. Its
Merklewas only used by the test helper, and solady'sMerkleTreeLibcovers the same surface. That removes the transitiveopenzeppelin-contractsmurky vendored, so this is a real removal rather than a partial one.It also aligns construction with verification:
ComposableCowverifies withMerkleProofLib, andMerkleTreeLibis 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:
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.
setRootWithContext_e2esetRoot_e2esettle_e2esetSwapGuard_e2epayloadTree_NormativeConstructioncreateAndRemove_e2e95 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.
ComposableCowdrops 10,659 to 10,595 bytes.getRootAndProofalso loses its two function pointers, which existed only to inject murky'sgetRootandgetProof.The ERC20 question
IERC20inGPv2Order.Datawas never OpenZeppelin's. It comes from cowprotocol throughIConditionalOrderandBaseConditionalOrder, so handlerDatastructs were already consistent with the settlement types. Nothing about that changes here, and there is no mismatch to introduce.The one place OpenZeppelin's
IERC20did leak in wastest/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.
MockERC20is rebuilt on solady'sERC20, which declaresnameandsymbolabstract 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 --checkclean, andrg 'openzeppelin|murky' src/ test/ script/returns nothing.Note for anyone touching this branch
jjdoes 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, sojj git pushhere will undo both. Usegit push.