From 6cc1c963c908e9b44d583d35e4b28cf73ee71b3f Mon Sep 17 00:00:00 2001 From: mfw78 Date: Mon, 3 Aug 2026 07:46:46 +0000 Subject: [PATCH] fix(docs): the merkle leaf is hashed twice, not once `_auth` computes `keccak256(bytes.concat(hash(params)))`, so a leaf is the params hash hashed again. The payload standard documented a single `keccak256(abi.encode(params))`, which `_auth` rejects outright: an integrator following the published `leafEncoding: "v1"` spec would have built a tree whose every proof fails. The second hash is the second-preimage defence. It keeps a leaf preimage from ever being 64 bytes, which is what would let an internal node be presented as a leaf. The note added in #62 disqualified OpenZeppelin's `StandardMerkleTree` partly because it double-hashes leaves. That was the wrong ground: this does too. Tree shape remains the real incompatibility, for `StandardMerkleTree` and for Solady's `MerkleTreeLib`. `test_payloadTree_NormativeConstructionVerifiesLikeAuth` could not catch any of this. 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. It now goes through `getTradeableOrderWithSignature`, and a companion test pins that the old single-hash encoding is rejected. Also replaces `keccak256(bytes.concat(...))` at that site with `EfficientHashLib.hash`, which is the same value without allocating a 32-byte `bytes`: 696 gas off each merkle-root end to end path, 1,833 across the suite. AI Assistance: Claude Code used for the investigation, the fix and the tests. --- .gas-snapshot | 139 +++++++++++++++++---------------- docs/discovery.md | 30 ++++--- src/ComposableCow.sol | 3 +- test/ComposableCow.proof.t.sol | 60 +++++++++++--- 4 files changed, 139 insertions(+), 93 deletions(-) diff --git a/.gas-snapshot b/.gas-snapshot index b633ec6..737a38e 100644 --- a/.gas-snapshot +++ b/.gas-snapshot @@ -18,27 +18,27 @@ ComposableCowDiscoveryTest:test_module_RevertsContentAddressedWithURI() (gas: 39 ComposableCowDiscoveryTest:test_module_RevertsSha256WithoutURI() (gas: 38899) ComposableCowDiscoveryTest:test_module_RevertsUncommittedURI() (gas: 40273) ComposableCowDiscoveryTest:test_module_UncommittedDoesNotAdvertise() (gas: 1395475) -ComposableCowForwarderTest:test_ERC1271Forwarder_isValidSignature_RevertsOnBadHash() (gas: 630773) +ComposableCowForwarderTest:test_ERC1271Forwarder_isValidSignature_RevertsOnBadHash() (gas: 630770) ComposableCowForwarderTest:test_SetUpState_ComposableCowDomainSeparator_is_set() (gas: 11443) ComposableCowForwarderTest:test_SetUpState_ComposableCowDomainVerifier_is_set() (gas: 18481) ComposableCowGatTest:test_SetUpState_ComposableCowDomainSeparator_is_set() (gas: 11487) ComposableCowGatTest:test_SetUpState_ComposableCowDomainVerifier_is_set() (gas: 18657) -ComposableCowGatTest:test_generateOrder_FuzzContext(address,address,address,uint256,uint256,uint256,uint256,bool) (runs: 256, μ: 121421, ~: 121422) +ComposableCowGatTest:test_generateOrder_FuzzContext(address,address,address,uint256,uint256,uint256,uint256,bool) (runs: 256, μ: 121422, ~: 121420) ComposableCowGatTest:test_generateOrder_FuzzRevertBeforeStartTime(uint256,uint256) (runs: 256, μ: 21804, ~: 21804) -ComposableCowGatTest:test_generateOrder_FuzzRevertBelowMinBalance(uint256,uint256) (runs: 256, μ: 114443, ~: 114444) -ComposableCowGatTest:test_generateOrder_FuzzRevertTooLowOutput(uint256,uint256,uint256) (runs: 256, μ: 124247, ~: 124317) +ComposableCowGatTest:test_generateOrder_FuzzRevertBelowMinBalance(uint256,uint256) (runs: 256, μ: 114446, ~: 114454) +ComposableCowGatTest:test_generateOrder_FuzzRevertTooLowOutput(uint256,uint256,uint256) (runs: 256, μ: 124256, ~: 124320) ComposableCowGatTest:test_generateOrder_RevertZeroAmount() (gas: 113369) -ComposableCowGatTest:test_generateOrder_e2e_Fuzz(uint256,uint256,uint256,uint256,uint256,uint256) (runs: 256, μ: 299380, ~: 299347) -ComposableCowGatTest:test_generateOrder_e2e_FuzzWithPriceChecker(uint256,uint256,uint256,uint256,uint256) (runs: 256, μ: 308734, ~: 308599) +ComposableCowGatTest:test_generateOrder_e2e_Fuzz(uint256,uint256,uint256,uint256,uint256,uint256) (runs: 256, μ: 299418, ~: 299344) +ComposableCowGatTest:test_generateOrder_e2e_FuzzWithPriceChecker(uint256,uint256,uint256,uint256,uint256) (runs: 256, μ: 308759, ~: 308596) ComposableCowGatTest:test_pollHints_SingleShot() (gas: 16098) -ComposableCowGatTest:test_settle_e2e() (gas: 491422) -ComposableCowGatTest:test_verify_e2e_fuzz(uint256,uint256,uint256,uint256,uint256) (runs: 256, μ: 137170, ~: 137035) +ComposableCowGatTest:test_settle_e2e() (gas: 491417) +ComposableCowGatTest:test_verify_e2e_fuzz(uint256,uint256,uint256,uint256,uint256) (runs: 256, μ: 137200, ~: 137037) ComposableCowGuardsTest:test_BaseSwapGuard_supportsInterface() (gas: 138710) -ComposableCowGuardsTest:test_ReceiverLock_verify_FuzzRevertsWhenReceiverNotSelf(address) (runs: 256, μ: 299508, ~: 299508) +ComposableCowGuardsTest:test_ReceiverLock_verify_FuzzRevertsWhenReceiverNotSelf(address) (runs: 256, μ: 299499, ~: 299499) ComposableCowGuardsTest:test_SetUpState_ComposableCowDomainSeparator_is_set() (gas: 11487) ComposableCowGuardsTest:test_SetUpState_ComposableCowDomainVerifier_is_set() (gas: 18546) -ComposableCowGuardsTest:test_setSwapGuard_FuzzSetAndEmit(address,address) (runs: 256, μ: 31252, ~: 31236) -ComposableCowGuardsTest:test_setSwapGuard_e2e() (gas: 1030524) +ComposableCowGuardsTest:test_setSwapGuard_FuzzSetAndEmit(address,address) (runs: 256, μ: 31254, ~: 31236) +ComposableCowGuardsTest:test_setSwapGuard_e2e() (gas: 1030509) ComposableCowManifestTest:test_PSS_ManifestPage_NotFundedCarriesStatus() (gas: 37596) ComposableCowManifestTest:test_PSS_ManifestPage_WithBalance() (gas: 125366) ComposableCowManifestTest:test_PSS_ManifestReturnsUnbounded() (gas: 13902) @@ -62,27 +62,27 @@ ComposableCowManifestTest:test_manifest_DoesNotPerturbGeneratorInterfaceId() (ga ComposableCowManifestTest:test_manifest_SupportsInterface() (gas: 1538778) ComposableCowPollTest:test_SetUpState_ComposableCowDomainSeparator_is_set() (gas: 11531) ComposableCowPollTest:test_SetUpState_ComposableCowDomainVerifier_is_set() (gas: 19119) -ComposableCowPollTest:test_checkOrder_ComposesFillOverlay() (gas: 1833894) -ComposableCowPollTest:test_checkOrder_RevertInterfaceNotSupported() (gas: 51466) -ComposableCowPollTest:test_fillOverlay_FilledWithholdsSignature() (gas: 1835512) -ComposableCowPollTest:test_fillOverlay_ForeignOwnerOrderUidNotObserved() (gas: 1860267) -ComposableCowPollTest:test_fillOverlay_InvalidatedIsDistinctFromFilled() (gas: 1834924) -ComposableCowPollTest:test_fillOverlay_KindBuyTotalIsBuyAmount() (gas: 1835470) -ComposableCowPollTest:test_fillOverlay_KindSellSameAmountIsPartial() (gas: 1850563) -ComposableCowPollTest:test_fillOverlay_NoneReturnsSignature() (gas: 1851348) -ComposableCowPollTest:test_fillOverlay_OrderUidMatchesGPv2Construction() (gas: 1841715) -ComposableCowPollTest:test_fillOverlay_PartialFillKeepsPosting() (gas: 1850375) -ComposableCowPollTest:test_fillOverlay_PartialFillOnFillOrKillWithholdsSignature() (gas: 1815449) -ComposableCowPollTest:test_getTradeableOrderWithSignature_UsesPollInternally() (gas: 1375792) +ComposableCowPollTest:test_checkOrder_ComposesFillOverlay() (gas: 1833891) +ComposableCowPollTest:test_checkOrder_RevertInterfaceNotSupported() (gas: 51463) +ComposableCowPollTest:test_fillOverlay_FilledWithholdsSignature() (gas: 1835509) +ComposableCowPollTest:test_fillOverlay_ForeignOwnerOrderUidNotObserved() (gas: 1860264) +ComposableCowPollTest:test_fillOverlay_InvalidatedIsDistinctFromFilled() (gas: 1834921) +ComposableCowPollTest:test_fillOverlay_KindBuyTotalIsBuyAmount() (gas: 1835467) +ComposableCowPollTest:test_fillOverlay_KindSellSameAmountIsPartial() (gas: 1850560) +ComposableCowPollTest:test_fillOverlay_NoneReturnsSignature() (gas: 1851345) +ComposableCowPollTest:test_fillOverlay_OrderUidMatchesGPv2Construction() (gas: 1841712) +ComposableCowPollTest:test_fillOverlay_PartialFillKeepsPosting() (gas: 1850372) +ComposableCowPollTest:test_fillOverlay_PartialFillOnFillOrKillWithholdsSignature() (gas: 1815446) +ComposableCowPollTest:test_getTradeableOrderWithSignature_UsesPollInternally() (gas: 1375789) ComposableCowPollTest:test_poll_BareRequireMapsToTryNextBlock() (gas: 1250645) ComposableCowPollTest:test_poll_DecodesOrderNotValid() (gas: 1291406) ComposableCowPollTest:test_poll_DecodesPollNeedsOffchainInput() (gas: 1402236) ComposableCowPollTest:test_poll_DecodesPollTryAtBlock() (gas: 1330995) ComposableCowPollTest:test_poll_DecodesPollTryAtTimestamp() (gas: 1330907) ComposableCowPollTest:test_poll_DecodesPollTryNextBlock() (gas: 1290285) -ComposableCowPollTest:test_poll_FuzzOrderNotValid(bytes4) (runs: 256, μ: 1283321, ~: 1291095) -ComposableCowPollTest:test_poll_FuzzPollTryAtBlock(uint256,bytes4) (runs: 256, μ: 1323038, ~: 1330890) -ComposableCowPollTest:test_poll_FuzzPollTryAtTimestamp(uint256,bytes4) (runs: 256, μ: 1323566, ~: 1331418) +ComposableCowPollTest:test_poll_FuzzOrderNotValid(bytes4) (runs: 256, μ: 1283632, ~: 1291095) +ComposableCowPollTest:test_poll_FuzzPollTryAtBlock(uint256,bytes4) (runs: 256, μ: 1323738, ~: 1330890) +ComposableCowPollTest:test_poll_FuzzPollTryAtTimestamp(uint256,bytes4) (runs: 256, μ: 1324266, ~: 1331418) ComposableCowPollTest:test_poll_NeedsInputHandlerPostsWithInput() (gas: 1404879) ComposableCowPollTest:test_poll_PanicMapsToTryNextBlock() (gas: 1294536) ComposableCowPollTest:test_poll_ReturnsPostOnValidOrder() (gas: 1763768) @@ -94,19 +94,20 @@ ComposableCowPollTest:test_tryGenerateOrder_ReturnsFullPanicData() (gas: 1296614 ComposableCowPollTest:test_tryGenerateOrder_SuccessReturnsOrder() (gas: 1677206) ComposableCowPollTest:test_verify_RevertsOnHashMismatch() (gas: 1764501) ComposableCowPollTest:test_verify_UsesGenerateOrder() (gas: 1761834) -ComposableCowProofTest:test_SetUpState_ComposableCowDomainSeparator_is_set() (gas: 11349) -ComposableCowProofTest:test_SetUpState_ComposableCowDomainVerifier_is_set() (gas: 18303) -ComposableCowProofTest:test_payloadTree_NormativeConstructionVerifiesLikeAuth() (gas: 573322) -ComposableCowProofTest:test_setRoot_BlobSubsetAttached() (gas: 41153) -ComposableCowProofTest:test_setRoot_BlobsAttached() (gas: 42837) -ComposableCowProofTest:test_setRoot_EmitsUriMirrors() (gas: 48180) -ComposableCowProofTest:test_setRoot_RevertsBlobNotAttached() (gas: 14819) -ComposableCowProofTest:test_setRoot_RevertsBlobNotAttachedNoBlobs() (gas: 13210) -ComposableCowProofTest:test_setRoot_RevertsZeroRootWithBlobs() (gas: 13352) -ComposableCowProofTest:test_setRoot_RevertsZeroRootWithUris() (gas: 12726) -ComposableCowProofTest:test_setRoot_ZeroRootClears() (gas: 32859) +ComposableCowProofTest:test_SetUpState_ComposableCowDomainSeparator_is_set() (gas: 11393) +ComposableCowProofTest:test_SetUpState_ComposableCowDomainVerifier_is_set() (gas: 18325) +ComposableCowProofTest:test_payloadTree_NormativeConstructionVerifiesLikeAuth() (gas: 1522463) +ComposableCowProofTest:test_payloadTree_SingleHashedLeafIsRejected() (gas: 144624) +ComposableCowProofTest:test_setRoot_BlobSubsetAttached() (gas: 41235) +ComposableCowProofTest:test_setRoot_BlobsAttached() (gas: 42919) +ComposableCowProofTest:test_setRoot_EmitsUriMirrors() (gas: 48304) +ComposableCowProofTest:test_setRoot_RevertsBlobNotAttached() (gas: 14901) +ComposableCowProofTest:test_setRoot_RevertsBlobNotAttachedNoBlobs() (gas: 13268) +ComposableCowProofTest:test_setRoot_RevertsZeroRootWithBlobs() (gas: 13434) +ComposableCowProofTest:test_setRoot_RevertsZeroRootWithUris() (gas: 12784) +ComposableCowProofTest:test_setRoot_ZeroRootClears() (gas: 32934) ComposableCowStopLossTest:test_OracleNormalisesPrice_concrete() (gas: 27435) -ComposableCowStopLossTest:test_OracleNormalisesPrice_fuzz(uint8,uint8,uint8,uint8) (runs: 256, μ: 29597, ~: 29613) +ComposableCowStopLossTest:test_OracleNormalisesPrice_fuzz(uint8,uint8,uint8,uint8) (runs: 256, μ: 29593, ~: 29613) ComposableCowStopLossTest:test_OracleRevertOnExpiredOrder_fuzz(uint32,uint32) (runs: 256, μ: 23185, ~: 23185) ComposableCowStopLossTest:test_OracleRevertOnInvalidPrice_fuzz(int256,int256) (runs: 256, μ: 36635, ~: 36635) ComposableCowStopLossTest:test_OracleRevertOnStalePrice_fuzz(uint256,uint256,uint256) (runs: 256, μ: 25348, ~: 25348) @@ -117,56 +118,56 @@ ComposableCowStopLossTest:test_generateOrder_RevertZeroAmount() (gas: 22286) ComposableCowStopLossTest:test_pollHints_SingleShot() (gas: 15780) ComposableCowStopLossTest:test_strikePriceMet_fuzz(int256,int256,int256,uint32) (runs: 256, μ: 29077, ~: 29077) ComposableCowStopLossTest:test_strikePriceNotMet_concrete() (gas: 26555) -ComposableCowTatTest:test_BalanceMet_fuzz(address,uint256,bytes32,uint256) (runs: 256, μ: 113409, ~: 113392) +ComposableCowTatTest:test_BalanceMet_fuzz(address,uint256,bytes32,uint256) (runs: 256, μ: 113402, ~: 113386) ComposableCowTatTest:test_SetUpState_ComposableCowDomainSeparator_is_set() (gas: 11283) ComposableCowTatTest:test_SetUpState_ComposableCowDomainVerifier_is_set() (gas: 18177) -ComposableCowTatTest:test_generateOrder_FuzzRevertBelowThreshold(uint256,uint256) (runs: 256, μ: 111614, ~: 111616) +ComposableCowTatTest:test_generateOrder_FuzzRevertBelowThreshold(uint256,uint256) (runs: 256, μ: 111617, ~: 111620) ComposableCowTest:test_SetUpState_ComposableCowDomainSeparator_is_set() (gas: 11575) ComposableCowTest:test_SetUpState_ComposableCowDomainVerifier_is_set() (gas: 18877) -ComposableCowTest:test_createAndRemove_FuzzSetAndEmit(address,address,bytes32,bytes) (runs: 256, μ: 40044, ~: 39905) -ComposableCowTest:test_createAndRemove_e2e() (gas: 468147) -ComposableCowTest:test_createWithContextAndRemove_FuzzSetAndEmit(address,address,bytes32,bytes,bytes32) (runs: 256, μ: 63585, ~: 63575) +ComposableCowTest:test_createAndRemove_FuzzSetAndEmit(address,address,bytes32,bytes) (runs: 256, μ: 39997, ~: 39883) +ComposableCowTest:test_createAndRemove_e2e() (gas: 468135) +ComposableCowTest:test_createWithContextAndRemove_FuzzSetAndEmit(address,address,bytes32,bytes,bytes32) (runs: 256, μ: 63661, ~: 63592) ComposableCowTest:test_create_RevertOnInvalidHandler() (gas: 10433) -ComposableCowTest:test_getTradeableOrderWithSignature_FuzzRevertInvalidProof(address,bytes32[],bytes32,address,bytes32,bytes) (runs: 256, μ: 97806, ~: 99033) -ComposableCowTest:test_getTradeableOrderWithSignature_FuzzRevertInvalidSingleOrder(address,address,bytes32,bytes) (runs: 256, μ: 18665, ~: 18645) -ComposableCowTest:test_getTradeableOrderWithSignature_ReturnsValidPayloadForNonSafe() (gas: 642433) -ComposableCowTest:test_getTradeableOrderWithSignature_ReturnsValidPayloadForSafe() (gas: 109925) -ComposableCowTest:test_getTradeableOrderWithSignature_RevertInterfaceNotSupported() (gas: 52460) -ComposableCowTest:test_isValidSafeSignature_BaseConditionalOrder_RevertOnInvalidHash() (gas: 60749) -ComposableCowTest:test_isValidSafeSignature_FuzzPassesContextToHandler(address,bytes32) (runs: 256, μ: 57674, ~: 57674) -ComposableCowTest:test_isValidSafeSignature_FuzzRevertInvalidProof(address,bytes32[],bytes32,address,bytes32,bytes) (runs: 256, μ: 99444, ~: 100634) -ComposableCowTest:test_isValidSafeSignature_FuzzRevertInvalidSingleOrder(address,address,bytes32,bytes) (runs: 256, μ: 18645, ~: 18618) +ComposableCowTest:test_getTradeableOrderWithSignature_FuzzRevertInvalidProof(address,bytes32[],bytes32,address,bytes32,bytes) (runs: 256, μ: 97007, ~: 96804) +ComposableCowTest:test_getTradeableOrderWithSignature_FuzzRevertInvalidSingleOrder(address,address,bytes32,bytes) (runs: 256, μ: 18659, ~: 18642) +ComposableCowTest:test_getTradeableOrderWithSignature_ReturnsValidPayloadForNonSafe() (gas: 642427) +ComposableCowTest:test_getTradeableOrderWithSignature_ReturnsValidPayloadForSafe() (gas: 109919) +ComposableCowTest:test_getTradeableOrderWithSignature_RevertInterfaceNotSupported() (gas: 52457) +ComposableCowTest:test_isValidSafeSignature_BaseConditionalOrder_RevertOnInvalidHash() (gas: 60746) +ComposableCowTest:test_isValidSafeSignature_FuzzPassesContextToHandler(address,bytes32) (runs: 256, μ: 57671, ~: 57671) +ComposableCowTest:test_isValidSafeSignature_FuzzRevertInvalidProof(address,bytes32[],bytes32,address,bytes32,bytes) (runs: 256, μ: 98629, ~: 98333) +ComposableCowTest:test_isValidSafeSignature_FuzzRevertInvalidSingleOrder(address,address,bytes32,bytes) (runs: 256, μ: 18637, ~: 18615) ComposableCowTest:test_remove_EmitsConditionalOrderRemoved() (gas: 36281) -ComposableCowTest:test_remove_FuzzEmitsEvent(address,bytes32) (runs: 256, μ: 31572, ~: 31546) +ComposableCowTest:test_remove_FuzzEmitsEvent(address,bytes32) (runs: 256, μ: 31569, ~: 31544) ComposableCowTest:test_safeSignaturePayload_SelectorMatchesMuxerMagicValue() (gas: 1145) -ComposableCowTest:test_setRootWithContext_FuzzSetAndEmit(address,bytes32,bytes32) (runs: 256, μ: 73774, ~: 73774) -ComposableCowTest:test_setRootWithContext_e2e() (gas: 13600334) -ComposableCowTest:test_setRoot_FuzzSetAndEmit(address,bytes32) (runs: 256, μ: 42105, ~: 42105) -ComposableCowTest:test_setRoot_e2e() (gas: 13568973) +ComposableCowTest:test_setRootWithContext_FuzzSetAndEmit(address,bytes32,bytes32) (runs: 256, μ: 73696, ~: 73774) +ComposableCowTest:test_setRootWithContext_e2e() (gas: 13599638) +ComposableCowTest:test_setRoot_FuzzSetAndEmit(address,bytes32) (runs: 256, μ: 41952, ~: 42105) +ComposableCowTest:test_setRoot_e2e() (gas: 13568277) ComposableCowTwapTest:test_SetUpState_ComposableCowDomainSeparator_is_set() (gas: 11488) ComposableCowTwapTest:test_SetUpState_ComposableCowDomainVerifier_is_set() (gas: 18987) -ComposableCowTwapTest:test_TWAPOrderMathLib_calculateValidTo(uint256,uint256,uint256,uint256,uint256) (runs: 256, μ: 12678, ~: 12784) +ComposableCowTwapTest:test_TWAPOrderMathLib_calculateValidTo(uint256,uint256,uint256,uint256,uint256) (runs: 256, μ: 12642, ~: 12389) ComposableCowTwapTest:test_describeOrder_RevertOnZeroFrequency() (gas: 21049) ComposableCowTwapTest:test_describeOrder_TwapParts() (gas: 29107) ComposableCowTwapTest:test_generateOrder_FuzzRevertIfBeforeStart(uint256,uint256) (runs: 256, μ: 29035, ~: 29035) -ComposableCowTwapTest:test_generateOrder_FuzzRevertIfExpired(uint256,uint256) (runs: 256, μ: 30430, ~: 30108) -ComposableCowTwapTest:test_generateOrder_FuzzRevertIfOrderAfterBlocktimestampValidity(uint256,uint256) (runs: 256, μ: 186369, ~: 187178) -ComposableCowTwapTest:test_generateOrder_FuzzRevertIfOrderBeforeBlockTimestamp(uint256,uint256) (runs: 256, μ: 187292, ~: 187292) -ComposableCowTwapTest:test_generateOrder_FuzzRevertIfOutsideSpan(uint256,uint256) (runs: 256, μ: 32310, ~: 32137) -ComposableCowTwapTest:test_generateOrder_FuzzRevertOnInvalidFrequency(uint256) (runs: 256, μ: 19746, ~: 19747) -ComposableCowTwapTest:test_generateOrder_FuzzRevertOnInvalidNumParts(uint256) (runs: 256, μ: 19603, ~: 19605) +ComposableCowTwapTest:test_generateOrder_FuzzRevertIfExpired(uint256,uint256) (runs: 256, μ: 30257, ~: 30108) +ComposableCowTwapTest:test_generateOrder_FuzzRevertIfOrderAfterBlocktimestampValidity(uint256,uint256) (runs: 256, μ: 186801, ~: 187176) +ComposableCowTwapTest:test_generateOrder_FuzzRevertIfOrderBeforeBlockTimestamp(uint256,uint256) (runs: 256, μ: 187290, ~: 187290) +ComposableCowTwapTest:test_generateOrder_FuzzRevertIfOutsideSpan(uint256,uint256) (runs: 256, μ: 32236, ~: 32137) +ComposableCowTwapTest:test_generateOrder_FuzzRevertOnInvalidFrequency(uint256) (runs: 256, μ: 19744, ~: 19747) +ComposableCowTwapTest:test_generateOrder_FuzzRevertOnInvalidNumParts(uint256) (runs: 256, μ: 19601, ~: 19605) ComposableCowTwapTest:test_generateOrder_FuzzRevertOnInvalidSpan(uint256,uint256) (runs: 256, μ: 20164, ~: 20164) ComposableCowTwapTest:test_generateOrder_FuzzRevertOnInvalidStartTime(uint256) (runs: 256, μ: 19006, ~: 19006) ComposableCowTwapTest:test_generateOrder_RevertOnSameTokens() (gas: 18983) ComposableCowTwapTest:test_generateOrder_RevertOnTokenZero() (gas: 24954) ComposableCowTwapTest:test_generateOrder_RevertOnZeroMinPartLimit() (gas: 19268) ComposableCowTwapTest:test_generateOrder_RevertOnZeroPartSellAmount() (gas: 19364) -ComposableCowTwapTest:test_generateOrder_e2e_fuzz(uint256,uint256) (runs: 256, μ: 203752, ~: 203484) -ComposableCowTwapTest:test_generateOrder_e2e_fuzz_WithContext(uint32,uint256) (runs: 256, μ: 233400, ~: 236324) +ComposableCowTwapTest:test_generateOrder_e2e_fuzz(uint256,uint256) (runs: 256, μ: 203703, ~: 203480) +ComposableCowTwapTest:test_generateOrder_e2e_fuzz_WithContext(uint32,uint256) (runs: 256, μ: 234204, ~: 236320) ComposableCowTwapTest:test_getNextPollTimestamp_FinalPartStopsPolling() (gas: 19664) ComposableCowTwapTest:test_getNextPollTimestamp_PointsAtNextPart() (gas: 20153) ComposableCowTwapTest:test_getNextPollTimestamp_RevertOnZeroFrequency() (gas: 19285) ComposableCowTwapTest:test_getNextPollTimestamp_RevertOnZeroNumParts() (gas: 19451) -ComposableCowTwapTest:test_settle_e2e() (gas: 13514775) -ComposableCowTwapTest:test_simulate_fuzz(uint32,uint32,uint32) (runs: 256, μ: 20411289, ~: 20821712) -ComposableCowTwapTest:test_verify_e2e_fuzz(uint256,uint256) (runs: 256, μ: 36266, ~: 35995) \ No newline at end of file +ComposableCowTwapTest:test_settle_e2e() (gas: 13514423) +ComposableCowTwapTest:test_simulate_fuzz(uint32,uint32,uint32) (runs: 256, μ: 20619419, ~: 21359116) +ComposableCowTwapTest:test_verify_e2e_fuzz(uint256,uint256) (runs: 256, μ: 36221, ~: 35995) \ No newline at end of file diff --git a/docs/discovery.md b/docs/discovery.md index b0804a1..042e028 100644 --- a/docs/discovery.md +++ b/docs/discovery.md @@ -637,19 +637,25 @@ recomputing the root. ``` - `leafEncoding: "v1"` pins the full tree construction, byte-exact against - `_auth`: `leaf = keccak256(abi.encode(ConditionalOrderParams))`; the tree is - built bottom-up over the ascending-sorted leaf array; each internal node is + `_auth`: the leaf is hashed **twice**, + `leaf = keccak256(keccak256(abi.encode(ConditionalOrderParams)))`, matching + `keccak256(bytes.concat(hash(params)))` in `_auth`. The second hash is what + keeps a leaf preimage from ever being 64 bytes, which is what would otherwise + let an internal node be presented as a leaf. The tree is then built bottom-up + over the ascending-sorted leaf array; each internal node is `keccak256(sorted-pair(a, b))`; an odd trailing node at any level is promoted - unchanged to the next level. Sorted-pair hashing alone does not determine - tree shape, so implementations MUST follow this construction. Neither - OpenZeppelin's `StandardMerkleTree` (it double-hashes leaves) nor Solady's - `MerkleTreeLib` (it builds a complete `2n-1` node tree, diverging wherever - the odd-promotion rule fires, such as at 5, 7 or 9 leaves) produces this - shape. The mismatch is silent under verification: `MerkleProofLib.verify` is - sorted-pair and therefore shape-agnostic, so a non-conforming tree still - verifies against its own root, and the divergence surfaces only when a - consumer recomputes `root` from `leaves`. Reference test vectors are - published alongside the contracts. + unchanged to the next level. +- Sorted-pair hashing alone does not determine tree shape, so implementations + MUST follow this construction. OpenZeppelin's `StandardMerkleTree` hashes + leaves twice as this does, but still does not produce this tree, and Solady's + `MerkleTreeLib` builds a complete `2n-1` node tree that diverges wherever the + odd-promotion rule fires, such as at 5, 7 or 9 leaves. The mismatch is silent + under verification: `MerkleProofLib.verify` is sorted-pair and therefore + shape-agnostic, so a non-conforming tree still verifies against its own root, + and the divergence surfaces only when a consumer recomputes `root` from + `leaves`. A wrong leaf encoding fails harder and sooner, with `_auth` + rejecting every proof outright. Reference test vectors are published + alongside the contracts. - `leaves` MUST be sorted ascending by leaf hash and deduplicated; consumers MUST reject on the first out-of-order or duplicate leaf. - Producers MUST serialize with RFC 8785; content addresses and digests commit diff --git a/src/ComposableCow.sol b/src/ComposableCow.sol index 991c846..b99f6ef 100644 --- a/src/ComposableCow.sol +++ b/src/ComposableCow.sol @@ -1,6 +1,7 @@ // SPDX-License-Identifier: GPL-3.0 pragma solidity >=0.8.0 <0.9.0; +import {EfficientHashLib} from "solady/utils/EfficientHashLib.sol"; import {MerkleProofLib} from "solady/utils/MerkleProofLib.sol"; import {IERC165, ERC1271, ISignatureVerifierMuxer, ISafeSignatureVerifier, Safe} from "./vendor/Safe.sol"; @@ -515,7 +516,7 @@ contract ComposableCow is ISafeSignatureVerifier { { if (proof.length != 0) { // The order is part of a merkle tree - bytes32 leaf = keccak256(bytes.concat(hash(params))); + bytes32 leaf = EfficientHashLib.hash(hash(params)); require(MerkleProofLib.verify(proof, roots[owner], leaf), ProofNotAuthed()); } else { // The order is a single order diff --git a/test/ComposableCow.proof.t.sol b/test/ComposableCow.proof.t.sol index 0842d6f..29a37db 100644 --- a/test/ComposableCow.proof.t.sol +++ b/test/ComposableCow.proof.t.sol @@ -243,19 +243,32 @@ contract ComposableCowProofTest is BaseComposableCowTest { } } + /// @dev The leaf `_auth` computes: the params hash, hashed again. Hashing + /// twice keeps a leaf preimage from ever being 64 bytes, which is what + /// would let an internal node be passed off as a leaf. + function _authLeaf(IConditionalOrder.ConditionalOrderParams memory params) internal pure returns (bytes32) { + return keccak256(bytes.concat(keccak256(abi.encode(params)))); + } + /** - * @dev The normative construction (`leafEncoding: "v1"`) is verifiable by - * exactly the check `_auth` performs (`MerkleProofLib.verify`), for - * every leaf across minimal, even, and odd tree sizes - including - * the odd-promotion levels + * @dev The normative construction (`leafEncoding: "v1"`) is accepted by + * `_auth` itself, for every leaf across minimal, even and odd tree + * sizes, including the odd-promotion levels. + * + * This goes through `getTradeableOrderWithSignature` rather than + * calling `MerkleProofLib.verify` directly. Verifying a tree against + * its own root only proves the tree is self-consistent, which a tree + * built with the wrong leaf encoding also is: the earlier version of + * this test passed while using a leaf `_auth` never computes. */ function test_payloadTree_NormativeConstructionVerifiesLikeAuth() public { uint256[5] memory sizes = [uint256(2), 3, 4, 5, 7]; for (uint256 s = 0; s < sizes.length; s++) { IConditionalOrder.ConditionalOrderParams[] memory bundle = getBundle(safe1, sizes[s]); + bytes32[] memory hashes = new bytes32[](bundle.length); for (uint256 i = 0; i < bundle.length; i++) { - hashes[i] = keccak256(abi.encode(bundle[i])); + hashes[i] = _authLeaf(bundle[i]); } bytes32[] memory forRoot = new bytes32[](hashes.length); @@ -264,18 +277,43 @@ contract ComposableCowProofTest is BaseComposableCowTest { } bytes32 root = _normativeRoot(forRoot); + _setRoot( + address(safe1), + root, + ComposableCow.Proof({uris: new string[](0), blobVersionedHashes: new bytes32[](0)}) + ); + for (uint256 i = 0; i < bundle.length; i++) { - bytes32 leaf = keccak256(abi.encode(bundle[i])); bytes32[] memory forProof = new bytes32[](hashes.length); for (uint256 j = 0; j < hashes.length; j++) { forProof[j] = hashes[j]; } - bytes32[] memory proof = _normativeProof(forProof, leaf); - // the exact check _auth performs - assertTrue( - MerkleProofLib.verify(proof, root, leaf), "normative proof rejected by MerkleProofLib.verify" - ); + bytes32[] memory proof = _normativeProof(forProof, _authLeaf(bundle[i])); + + // reverts `ProofNotAuthed` if the construction is not what `_auth` accepts + composableCow.getTradeableOrderWithSignature(address(safe1), bundle[i], bytes(""), proof); } } } + + /// @dev The encoding the payload standard used to document, a single hash, + /// is rejected. Pins that the two are not interchangeable, so the spec + /// cannot drift back without this failing. + function test_payloadTree_SingleHashedLeafIsRejected() public { + IConditionalOrder.ConditionalOrderParams[] memory bundle = getBundle(safe1, 2); + + bytes32 a = keccak256(abi.encode(bundle[0])); + bytes32 b = keccak256(abi.encode(bundle[1])); + bytes32[] memory proof = new bytes32[](1); + proof[0] = b; + + _setRoot( + address(safe1), + _hashSortedPair(a, b), + ComposableCow.Proof({uris: new string[](0), blobVersionedHashes: new bytes32[](0)}) + ); + + vm.expectRevert(ComposableCow.ProofNotAuthed.selector); + composableCow.getTradeableOrderWithSignature(address(safe1), bundle[0], bytes(""), proof); + } }