Clear2 0 order ratio bug #1049
Merged
Clear2 0 order ratio bug #1049
Conversation
This file contains 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
… balances due to zero ratio
…e/rain.orderbook into 783-clear2-0-order-ratio-bug
…e/rain.orderbook into 783-clear2-0-order-ratio-bug
thedavidmeister
requested review from
hardyjosh
and removed request for
thedavidmeister
| uint256 orderIORatio = calculateOrderStack[0]; | ||
| uint256 orderIORatio; | ||
| Output18Amount orderOutputMax18; | ||
| assembly ("memory-safe") { |
There was a problem hiding this comment.
this was done to make the contract size smaller so that it can be deployed, we're right at the max codesize limit
rouzwelt
approved these changes
Dec 27, 2024
hardyjosh
reviewed
Dec 28, 2024
src/concrete/ob/OrderBook.sol
Outdated
| @@ -786,14 +786,18 @@ contract OrderBook is IOrderBookV4, IMetaV1_2, ReentrancyGuard, Multicall, Order | |||
| new uint256[](0) | |||
| ); | |||
|
|
|||
| // This is redundant with the array index checks implied by solidity | |||
| // but it's a much clearer error message. | |||
| // This is a much error message and overall more efficient than | |||
…e/rain.orderbook into 783-clear2-0-order-ratio-bug
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.
Motivation
There seems to be a bug in an edge with clear2() where if Alice or Bob order ratio is 0, then the tx goes through silently without any reverts or issues, but nothing actually clears, no vault balance changes and no bounties.
This should not happen, in fact if the order ratio is 0, that measn all the output vault should go to the tx sender as bounty.
Solution
Add test showing the issue
Checks
By submitting this for review, I'm confirming I've done the following: