Repository navigation
feat: treat maxBidPrice == 0 as no effective cap - #366
Merged
zhongeric merged 1 commit intoAug 24, 2026
Conversation
`MaxBidPriceValidationHook(0)` is a brick today. CCA requires every bid to price strictly above the clearing price, which starts at a floor of at least `ConstantsLib.MIN_FLOOR_PRICE` (2**32 + 1), so every bid that could otherwise be placed has `maxPrice > 0` and reverts with `MaxBidPriceExceeded`. A bid at `maxPrice == 0` clears the hook and then fails on `BidMustBeAboveClearingPrice`. No bid can ever be placed. Since nobody would deploy that deliberately, spend the degenerate value on the more useful meaning: document 0 as "no effective cap" in `IMaxBidPriceValidationHook`, and guard the reference implementation with `maxBidPrice != 0`. This changes nothing for any cap anyone would actually set, and gives integrators that build the hook from config a natural "unset" value instead of an auction that silently accepts no bids. The interface note is the contract; the guard is one implementation of it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
zhongeric
approved these changes
Aug 24, 2026
Contributor
|
thank you! |
zhongeric
merged commit Aug 24, 2026
58a6314
into
Uniswap:feat/add-max-bid-price-validation-hook
5 checks passed
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.
Stacked on #365 — targets
feat/add-max-bid-price-validation-hook, so the diff here is just this change.The degenerate case
MaxBidPriceValidationHook(0)is a brick as written. CCA requires every bid to price strictly above the clearing price:and the clearing price starts at a floor of at least
ConstantsLib.MIN_FLOOR_PRICE(2**32 + 1), rising from there. So every bid that could otherwise be placed hasmaxPrice > 0and reverts withMaxBidPriceExceeded. A bid atmaxPrice == 0passes the hook and then fails onBidMustBeAboveClearingPrice. The result is an auction that can never accept a bid.The proposal
Since nobody would deploy that deliberately, spend the degenerate value on the more useful meaning:
maxBidPrice == 0as "no effective cap" inIMaxBidPriceValidationHook, so it's part of the interface contract rather than one implementation's quirk.if (maxBidPrice != 0 && maxPrice > maxBidPrice).For this hook it changes nothing anyone would deploy: every reachable cap is
>= MIN_FLOOR_PRICE, and behavior at those values is identical. What it buys is a natural "unset" for integrators building the hook from config — today a missing/zero value yields an auction that silently accepts no bids, which is a bad failure mode for a value that's easy to leave at its default.Tests
The BTT tree gains a
when maxBidPrice is 0branch, and the existing revert case is scoped underwhen maxBidPrice is not 0:Note
test_WhenMaxPriceGTMaxBidPricepreviously fuzzed_maxBidPricefrom0, so it would have caught this change — its lower bound moves to1, and the zero case is asserted explicitly instead.7/7 pass at 2500 fuzz runs. Nothing outside these three files references the hook.
Happy to drop this if you'd rather keep
0meaning a literal cap — the interface note is the part I think is worth having either way, since the current behavior at0is surprising enough to deserve a line of natspec.