From a5eaef1e53c3c325cd24b320955ce2fb99d71b89 Mon Sep 17 00:00:00 2001
From: thedavidmeister <thedavidmeister@gmail.com>
Date: Mon, 28 Aug 2023 22:53:39 +0400
Subject: [PATCH 1/9] test for precision on take order

---
 src/concrete/OrderBook.sol              | 10 ++--
 test/concrete/OrderBook.takeOrder.t.sol | 70 +++++++++++++++++++++++++
 2 files changed, 76 insertions(+), 4 deletions(-)
 create mode 100644 test/concrete/OrderBook.takeOrder.t.sol

diff --git a/src/concrete/OrderBook.sol b/src/concrete/OrderBook.sol
index 436721222..769bfbc82 100644
--- a/src/concrete/OrderBook.sol
+++ b/src/concrete/OrderBook.sol
@@ -400,10 +400,12 @@ contract OrderBook is IOrderBookV3, ReentrancyGuard, Multicall, OrderBookV3Flash
             );
         }
 
-        // We already updated vault balances before we took tokens from
-        // `msg.sender` which is usually NOT the correct order of operations for
-        // depositing to a vault. We rely on reentrancy guards to make this safe.
-        IERC20(config.output).safeTransferFrom(msg.sender, address(this), totalOutput);
+        if (totalOutput > 0) {
+            // We already updated vault balances before we took tokens from
+            // `msg.sender` which is usually NOT the correct order of operations for
+            // depositing to a vault. We rely on reentrancy guards to make this safe.
+            IERC20(config.output).safeTransferFrom(msg.sender, address(this), totalOutput);
+        }
     }
 
     /// @inheritdoc IOrderBookV3
diff --git a/test/concrete/OrderBook.takeOrder.t.sol b/test/concrete/OrderBook.takeOrder.t.sol
new file mode 100644
index 000000000..775ff9997
--- /dev/null
+++ b/test/concrete/OrderBook.takeOrder.t.sol
@@ -0,0 +1,70 @@
+// SPDX-License-Identifier: CAL
+pragma solidity =0.8.19;
+
+import "test/util/abstract/OrderBookExternalRealTest.sol";
+
+/// @title OrderBookTakeOrderTest
+/// @notice A test harness for testing the OrderBook takeOrder function.
+contract OrderBookTakeOrderTest is OrderBookExternalRealTest {
+    function testPrecision01() public {
+        uint256 vaultId = 0;
+        address inputToken = address(0x100);
+        address outputToken = address(0x101);
+        OrderConfigV2 memory config;
+        {
+            uint8 inputTokenDecimals = 6;
+            uint8 outputTokenDecimals = 18;
+            IO[] memory validInputs = new IO[](1);
+            validInputs[0] = IO(inputToken, inputTokenDecimals, vaultId);
+            IO[] memory validOutputs = new IO[](1);
+            validOutputs[0] = IO(outputToken, outputTokenDecimals, vaultId);
+            // These numbers are known to cause large rounding errors if the
+            // precision is not handled correctly.
+            (bytes memory bytecode, uint256[] memory constants) = IParserV1(address(iDeployer)).parse("output-max io-ratio:157116365680491867129910 318235466963885;:;");
+            EvaluableConfigV2 memory evaluableConfig = EvaluableConfigV2(
+                iDeployer,
+                bytecode,
+                constants
+            );
+            config = OrderConfigV2(
+                validInputs,
+                validOutputs,
+                evaluableConfig,
+                ""
+            );
+            // Etch with invalid.
+            vm.etch(outputToken, hex"fe");
+            vm.etch(inputToken, hex"fe");
+            // Mock every call to output as a success, so the orderbook thinks it
+            // is transferring tokens.
+            vm.mockCall(outputToken, "", abi.encode(true));
+            vm.mockCall(inputToken, "", abi.encode(true));
+        }
+        iOrderbook.deposit(outputToken, vaultId, 1000000e18);
+        vm.recordLogs();
+        iOrderbook.addOrder(config);
+        Vm.Log[] memory entries = vm.getRecordedLogs();
+        assertEq(entries.length, 3);
+        (,,Order memory order,) = abi.decode(entries[2].data, (address, address, Order, bytes32));
+
+        TakeOrderConfig[] memory orders = new TakeOrderConfig[](1);
+        orders[0] = TakeOrderConfig(
+            order,
+            0,
+            0,
+            new SignedContextV1[](0)
+        );
+        TakeOrdersConfigV2 memory takeOrdersConfig = TakeOrdersConfigV2(
+            inputToken,
+            outputToken,
+            0,
+            type(uint256).max,
+            type(uint256).max,
+            orders,
+            ""
+        );
+        (uint256 totalInput, uint256 totalOutput) = iOrderbook.takeOrders(takeOrdersConfig);
+        assertEq(totalInput, 157116365680491867129910);
+        assertEq(totalOutput, 50120121);
+    }
+}
\ No newline at end of file

From bd5c520665a88e7f0abefc75aa1bcf4e2391237b Mon Sep 17 00:00:00 2001
From: thedavidmeister <thedavidmeister@gmail.com>
Date: Mon, 28 Aug 2023 23:20:16 +0400
Subject: [PATCH 2/9] fix precision in take orders

---
 src/concrete/OrderBook.sol              | 77 ++++++++++++++-----------
 test/concrete/OrderBook.takeOrder.t.sol |  2 +-
 2 files changed, 44 insertions(+), 35 deletions(-)

diff --git a/src/concrete/OrderBook.sol b/src/concrete/OrderBook.sol
index 769bfbc82..901f63084 100644
--- a/src/concrete/OrderBook.sol
+++ b/src/concrete/OrderBook.sol
@@ -23,6 +23,8 @@ import "../interface/unstable/IOrderBookV3OrderTaker.sol";
 import "../lib/LibOrder.sol";
 import "../abstract/OrderBookV3FlashLender.sol";
 
+import "forge-std/console2.sol";
+
 /// This will exist in a future version of Open Zeppelin if their main branch is
 /// to be believed.
 error ReentrancyGuardReentrantCall();
@@ -359,16 +361,23 @@ contract OrderBook is IOrderBookV3, ReentrancyGuard, Multicall, OrderBookV3Flash
                     emit OrderZeroAmount(msg.sender, order.owner, orderHash);
                 } else {
                     // Don't exceed the maximum total input.
-                    uint256 input =
-                        remainingInput > orderIOCalculation.outputMax ? orderIOCalculation.outputMax : remainingInput;
-                    // Always round IO calculations up.
-                    uint256 output = input.fixedPointMul(orderIOCalculation.IORatio, Math.Rounding.Up);
-
-                    remainingInput -= input;
-                    totalOutput += output;
-
-                    recordVaultIO(order, output, input, orderIOCalculation);
-                    emit TakeOrder(msg.sender, takeOrder, input, output);
+                    // Round down taker inputs.
+                    uint256 takerInput =
+                        (remainingInput > orderIOCalculation.outputMax ? orderIOCalculation.outputMax : remainingInput).scaleN(
+                            order.validOutputs[takeOrder.outputIOIndex].decimals,
+                            FLAG_SATURATE
+                        );
+                    // Always round IO calculations up so the taker pays more.
+                    uint256 takerOutput = takerInput.fixedPointMul(orderIOCalculation.IORatio, Math.Rounding.Up).scaleN(
+                        order.validInputs[takeOrder.inputIOIndex].decimals,
+                        FLAG_ROUND_UP
+                    );
+
+                    remainingInput -= takerInput;
+                    totalOutput += takerOutput;
+
+                    recordVaultIO(order, takerOutput, takerInput, orderIOCalculation);
+                    emit TakeOrder(msg.sender, takeOrder, takerInput, takerOutput);
                 }
             }
 
@@ -553,30 +562,30 @@ contract OrderBook is IOrderBookV3, ReentrancyGuard, Multicall, OrderBookV3Flash
             uint256 orderOutputMax = calculateOrderStack[calculateOrderStack.length - 2];
             uint256 orderIORatio = calculateOrderStack[calculateOrderStack.length - 1];
 
-            // Rescale order output max from 18 FP to whatever decimals the
-            // output token is using.
-            // Always round order output down.
-            orderOutputMax = orderOutputMax.scaleN(
-                order.validOutputs[outputIOIndex].decimals,
-                // Saturate the order max output because if we were willing to
-                // give more than this on a scale up, we should be comfortable
-                // giving less.
-                // Round DOWN to be conservative and give away less if there's
-                // any loss of precision during scale down.
-                FLAG_SATURATE
-            );
-            // Rescale the ratio from 18 FP according to the difference in
-            // decimals between input and output.
-            // Always round IO ratio up.
-            orderIORatio = orderIORatio.scaleRatio(
-                order.validOutputs[outputIOIndex].decimals,
-                order.validInputs[inputIOIndex].decimals,
-                // DO NOT saturate ratios because this would reduce the effective
-                // IO ratio, which would mean that saturating would make the deal
-                // worse for the order. Instead we overflow, and round up to get
-                // the best possible deal.
-                FLAG_ROUND_UP
-            );
+            // // Rescale order output max from 18 FP to whatever decimals the
+            // // output token is using.
+            // // Always round order output down.
+            // orderOutputMax = orderOutputMax.scaleN(
+            //     order.validOutputs[outputIOIndex].decimals,
+            //     // Saturate the order max output because if we were willing to
+            //     // give more than this on a scale up, we should be comfortable
+            //     // giving less.
+            //     // Round DOWN to be conservative and give away less if there's
+            //     // any loss of precision during scale down.
+            //     FLAG_SATURATE
+            // );
+            // // Rescale the ratio from 18 FP according to the difference in
+            // // decimals between input and output.
+            // // Always round IO ratio up.
+            // orderIORatio = orderIORatio.scaleRatio(
+            //     order.validOutputs[outputIOIndex].decimals,
+            //     order.validInputs[inputIOIndex].decimals,
+            //     // DO NOT saturate ratios because this would reduce the effective
+            //     // IO ratio, which would mean that saturating would make the deal
+            //     // worse for the order. Instead we overflow, and round up to get
+            //     // the best possible deal.
+            //     FLAG_ROUND_UP
+            // );
 
             // The order owner can't send more than the smaller of their vault
             // balance or their per-order limit.
diff --git a/test/concrete/OrderBook.takeOrder.t.sol b/test/concrete/OrderBook.takeOrder.t.sol
index 775ff9997..d3c9fd4b8 100644
--- a/test/concrete/OrderBook.takeOrder.t.sol
+++ b/test/concrete/OrderBook.takeOrder.t.sol
@@ -65,6 +65,6 @@ contract OrderBookTakeOrderTest is OrderBookExternalRealTest {
         );
         (uint256 totalInput, uint256 totalOutput) = iOrderbook.takeOrders(takeOrdersConfig);
         assertEq(totalInput, 157116365680491867129910);
-        assertEq(totalOutput, 50120121);
+        assertEq(totalOutput, 50000000);
     }
 }
\ No newline at end of file

From f24014918645da0e22644d09fe8d3f4d98207d79 Mon Sep 17 00:00:00 2001
From: thedavidmeister <thedavidmeister@gmail.com>
Date: Tue, 29 Aug 2023 18:34:05 +0400
Subject: [PATCH 3/9] wip on precision

---
 src/concrete/OrderBook.sol              | 281 +++++++++++++++---------
 test/concrete/OrderBook.takeOrder.t.sol |  86 +++++---
 2 files changed, 232 insertions(+), 135 deletions(-)

diff --git a/src/concrete/OrderBook.sol b/src/concrete/OrderBook.sol
index 901f63084..704810f07 100644
--- a/src/concrete/OrderBook.sol
+++ b/src/concrete/OrderBook.sol
@@ -151,7 +151,9 @@ bytes32 constant CALLER_META_HASH = bytes32(0xf0c79e4006636a71899066ac45a478da4e
 /// @param kvs KVs returned from calculate order entrypoint to pass to the store
 /// before calling handle IO entrypoint.
 struct OrderIOCalculation {
-    uint256 outputMax;
+    Order order;
+    uint256 outputIOIndex;
+    Output18Amount outputMax;
     //solhint-disable-next-line var-name-mixedcase
     uint256 IORatio;
     uint256[][] context;
@@ -159,6 +161,10 @@ struct OrderIOCalculation {
     uint256[] kvs;
 }
 
+type Output18Amount is uint256;
+
+type Input18Amount is uint256;
+
 /// @title OrderBook
 /// See `IOrderBookV1` for more documentation.
 contract OrderBook is IOrderBookV3, ReentrancyGuard, Multicall, OrderBookV3FlashLender, DeployerDiscoverableMetaV2 {
@@ -327,28 +333,33 @@ contract OrderBook is IOrderBookV3, ReentrancyGuard, Multicall, OrderBookV3Flash
     function takeOrders(TakeOrdersConfigV2 calldata config)
         external
         nonReentrant
-        returns (uint256 totalInput, uint256 totalOutput)
+        returns (uint256 totalTakerInput, uint256 totalTakerOutput)
     {
         uint256 i = 0;
-        TakeOrderConfig memory takeOrder;
+        TakeOrderConfig memory takeOrderConfig;
         Order memory order;
-        uint256 remainingInput = config.maximumInput;
-        while (i < config.orders.length && remainingInput > 0) {
-            takeOrder = config.orders[i];
-            order = takeOrder.order;
+
+        uint256 remainingTakerInput = config.maximumInput;
+        while (i < config.orders.length && remainingTakerInput > 0) {
+            takeOrderConfig = config.orders[i];
+            order = takeOrderConfig.order;
             bytes32 orderHash = order.hash();
             if (sOrders[orderHash] == ORDER_DEAD) {
                 emit OrderNotFound(msg.sender, order.owner, orderHash);
             } else {
-                if (order.validInputs[takeOrder.inputIOIndex].token != config.output) {
-                    revert TokenMismatch(order.validInputs[takeOrder.inputIOIndex].token, config.output);
+                if (order.validInputs[takeOrderConfig.inputIOIndex].token != config.output) {
+                    revert TokenMismatch(order.validInputs[takeOrderConfig.inputIOIndex].token, config.output);
                 }
-                if (order.validOutputs[takeOrder.outputIOIndex].token != config.input) {
-                    revert TokenMismatch(order.validOutputs[takeOrder.outputIOIndex].token, config.input);
+                if (order.validOutputs[takeOrderConfig.outputIOIndex].token != config.input) {
+                    revert TokenMismatch(order.validOutputs[takeOrderConfig.outputIOIndex].token, config.input);
                 }
 
                 OrderIOCalculation memory orderIOCalculation = calculateOrderIO(
-                    order, takeOrder.inputIOIndex, takeOrder.outputIOIndex, msg.sender, takeOrder.signedContext
+                    order,
+                    takeOrderConfig.inputIOIndex,
+                    takeOrderConfig.outputIOIndex,
+                    msg.sender,
+                    takeOrderConfig.signedContext
                 );
 
                 // Skip orders that are too expensive rather than revert as we have
@@ -357,27 +368,45 @@ contract OrderBook is IOrderBookV3, ReentrancyGuard, Multicall, OrderBookV3Flash
                 // be valid so we want to take advantage of those if possible.
                 if (orderIOCalculation.IORatio > config.maximumIORatio) {
                     emit OrderExceedsMaxRatio(msg.sender, order.owner, orderHash);
-                } else if (orderIOCalculation.outputMax == 0) {
+                } else if (Output18Amount.unwrap(orderIOCalculation.outputMax) == 0) {
                     emit OrderZeroAmount(msg.sender, order.owner, orderHash);
                 } else {
-                    // Don't exceed the maximum total input.
-                    // Round down taker inputs.
-                    uint256 takerInput =
-                        (remainingInput > orderIOCalculation.outputMax ? orderIOCalculation.outputMax : remainingInput).scaleN(
-                            order.validOutputs[takeOrder.outputIOIndex].decimals,
-                            FLAG_SATURATE
+                    uint8 takerInputDecimals = order.validOutputs[takeOrderConfig.outputIOIndex].decimals;
+                    // Taker is just "market buying" the order output max.
+                    Input18Amount takerInput18 = Input18Amount.wrap(Output18Amount.unwrap(orderIOCalculation.outputMax));
+                    // Cap the taker input at the remaining input before
+                    // calculating the taker output. Keep everything in 18
+                    // decimals at this point, which requires rescaling the
+                    // remaining taker input to match.
+                    {
+                        // Round down and saturate when converting remaining taker input to 18 decimals.
+                        Input18Amount remainingTakerInput18 =
+                            Input18Amount.wrap(remainingTakerInput.scale18(takerInputDecimals, FLAG_SATURATE));
+                        if (Input18Amount.unwrap(takerInput18) > Input18Amount.unwrap(remainingTakerInput18)) {
+                            takerInput18 = remainingTakerInput18;
+                        }
+                    }
+
+                    uint256 takerOutput;
+                    {
+                        // Always round IO calculations up so the taker pays more.
+                        Output18Amount takerOutput18 = Output18Amount.wrap(
+                            Input18Amount.unwrap(takerInput18).fixedPointMul(
+                                orderIOCalculation.IORatio, Math.Rounding.Up
+                            )
                         );
-                    // Always round IO calculations up so the taker pays more.
-                    uint256 takerOutput = takerInput.fixedPointMul(orderIOCalculation.IORatio, Math.Rounding.Up).scaleN(
-                        order.validInputs[takeOrder.inputIOIndex].decimals,
-                        FLAG_ROUND_UP
-                    );
+                        takerOutput = Output18Amount.unwrap(takerOutput18).scaleN(
+                            order.validInputs[takeOrderConfig.inputIOIndex].decimals, FLAG_ROUND_UP
+                        );
+                    }
+
+                    uint256 takerInput = Input18Amount.unwrap(takerInput18).scaleN(takerInputDecimals, FLAG_SATURATE);
 
-                    remainingInput -= takerInput;
-                    totalOutput += takerOutput;
+                    remainingTakerInput -= takerInput;
+                    totalTakerOutput += takerOutput;
 
                     recordVaultIO(order, takerOutput, takerInput, orderIOCalculation);
-                    emit TakeOrder(msg.sender, takeOrder, takerInput, takerOutput);
+                    emit TakeOrder(msg.sender, takeOrderConfig, takerInput, takerOutput);
                 }
             }
 
@@ -385,10 +414,10 @@ contract OrderBook is IOrderBookV3, ReentrancyGuard, Multicall, OrderBookV3Flash
                 i++;
             }
         }
-        totalInput = config.maximumInput - remainingInput;
+        totalTakerInput = config.maximumInput - remainingTakerInput;
 
-        if (totalInput < config.minimumInput) {
-            revert MinimumInput(config.minimumInput, totalInput);
+        if (totalTakerInput < config.minimumInput) {
+            revert MinimumInput(config.minimumInput, totalTakerInput);
         }
 
         // Prioritise paying down any active flash loans before sending any
@@ -402,18 +431,18 @@ contract OrderBook is IOrderBookV3, ReentrancyGuard, Multicall, OrderBookV3Flash
         //   trades, which is important if the order logic itself is dependent on
         //   external data (e.g. prices) that could be modified by the caller's
         //   trades.
-        uint256 inputAmountSent = _decreaseFlashDebtThenSendToken(config.input, msg.sender, totalInput);
+        uint256 inputAmountSent = _decreaseFlashDebtThenSendToken(config.input, msg.sender, totalTakerInput);
         if (config.data.length > 0) {
             IOrderBookV3OrderTaker(msg.sender).onTakeOrders(
-                config.input, config.output, inputAmountSent, totalOutput, config.data
+                config.input, config.output, inputAmountSent, totalTakerOutput, config.data
             );
         }
 
-        if (totalOutput > 0) {
+        if (totalTakerOutput > 0) {
             // We already updated vault balances before we took tokens from
             // `msg.sender` which is usually NOT the correct order of operations for
             // depositing to a vault. We rely on reentrancy guards to make this safe.
-            IERC20(config.output).safeTransferFrom(msg.sender, address(this), totalOutput);
+            IERC20(config.output).safeTransferFrom(msg.sender, address(this), totalTakerOutput);
         }
     }
 
@@ -470,28 +499,28 @@ contract OrderBook is IOrderBookV3, ReentrancyGuard, Multicall, OrderBookV3Flash
         OrderIOCalculation memory bobOrderIOCalculation_ = calculateOrderIO(
             bob, clearConfig.bobInputIOIndex, clearConfig.bobOutputIOIndex, alice.owner, aliceSignedContext
         );
-        ClearStateChange memory clearStateChange_ =
+        ClearStateChange memory clearStateChange =
             calculateClearStateChange(aliceOrderIOCalculation_, bobOrderIOCalculation_);
 
-        recordVaultIO(alice, clearStateChange_.aliceInput, clearStateChange_.aliceOutput, aliceOrderIOCalculation_);
-        recordVaultIO(bob, clearStateChange_.bobInput, clearStateChange_.bobOutput, bobOrderIOCalculation_);
+        recordVaultIO(alice, clearStateChange.aliceInput, clearStateChange.aliceOutput, aliceOrderIOCalculation_);
+        recordVaultIO(bob, clearStateChange.bobInput, clearStateChange.bobOutput, bobOrderIOCalculation_);
 
         {
             // At least one of these will overflow due to negative bounties if
             // there is a spread between the orders.
-            uint256 aliceBounty_ = clearStateChange_.aliceOutput - clearStateChange_.bobInput;
-            uint256 bobBounty_ = clearStateChange_.bobOutput - clearStateChange_.aliceInput;
-            if (aliceBounty_ > 0) {
+            uint256 aliceBounty = clearStateChange.aliceOutput - clearStateChange.bobInput;
+            uint256 bobBounty = clearStateChange.bobOutput - clearStateChange.aliceInput;
+            if (aliceBounty > 0) {
                 sVaultBalances[msg.sender][alice.validOutputs[clearConfig.aliceOutputIOIndex].token][clearConfig
-                    .aliceBountyVaultId] += aliceBounty_;
+                    .aliceBountyVaultId] += aliceBounty;
             }
-            if (bobBounty_ > 0) {
+            if (bobBounty > 0) {
                 sVaultBalances[msg.sender][bob.validOutputs[clearConfig.bobOutputIOIndex].token][clearConfig
-                    .bobBountyVaultId] += bobBounty_;
+                    .bobBountyVaultId] += bobBounty;
             }
         }
 
-        emit AfterClear(msg.sender, clearStateChange_);
+        emit AfterClear(msg.sender, clearStateChange);
     }
 
     /// Main entrypoint into an order calculates the amount and IO ratio. Both
@@ -559,45 +588,36 @@ contract OrderBook is IOrderBookV3, ReentrancyGuard, Multicall, OrderBookV3Flash
                 .interpreter
                 .eval(order.evaluable.store, namespace, _calculateOrderDispatch(order.evaluable.expression), context);
 
-            uint256 orderOutputMax = calculateOrderStack[calculateOrderStack.length - 2];
+            Output18Amount orderOutputMax = Output18Amount.wrap(calculateOrderStack[calculateOrderStack.length - 2]);
             uint256 orderIORatio = calculateOrderStack[calculateOrderStack.length - 1];
 
-            // // Rescale order output max from 18 FP to whatever decimals the
-            // // output token is using.
-            // // Always round order output down.
-            // orderOutputMax = orderOutputMax.scaleN(
-            //     order.validOutputs[outputIOIndex].decimals,
-            //     // Saturate the order max output because if we were willing to
-            //     // give more than this on a scale up, we should be comfortable
-            //     // giving less.
-            //     // Round DOWN to be conservative and give away less if there's
-            //     // any loss of precision during scale down.
-            //     FLAG_SATURATE
-            // );
-            // // Rescale the ratio from 18 FP according to the difference in
-            // // decimals between input and output.
-            // // Always round IO ratio up.
-            // orderIORatio = orderIORatio.scaleRatio(
-            //     order.validOutputs[outputIOIndex].decimals,
-            //     order.validInputs[inputIOIndex].decimals,
-            //     // DO NOT saturate ratios because this would reduce the effective
-            //     // IO ratio, which would mean that saturating would make the deal
-            //     // worse for the order. Instead we overflow, and round up to get
-            //     // the best possible deal.
-            //     FLAG_ROUND_UP
-            // );
-
             // The order owner can't send more than the smaller of their vault
             // balance or their per-order limit.
-            uint256 ownerVaultBalance = sVaultBalances[order.owner][order.validOutputs[outputIOIndex].token][order
-                .validOutputs[outputIOIndex].vaultId];
-            orderOutputMax = orderOutputMax > ownerVaultBalance ? ownerVaultBalance : orderOutputMax;
-
-            // Populate the context with the output max rescaled and vault capped
-            // and the rescaled ratio.
-            context[CONTEXT_CALCULATIONS_COLUMN] = LibUint256Array.arrayFrom(orderOutputMax, orderIORatio);
-
-            return OrderIOCalculation(orderOutputMax, orderIORatio, context, namespace, calculateOrderKVs);
+            // uint256 ownerVaultBalance = sVaultBalances[order.owner][order.validOutputs[outputIOIndex].token][order
+            //     .validOutputs[outputIOIndex].vaultId];
+            // We round down vault balances and don't saturate because we're
+            // dealing with real token amounts here. If rescaling would somehow
+            // cause an overflow in a real token amount, that's basically an
+            // unsupported token, it implies a very small decimals value with
+            // very large token total supply. E.g. 0 decimals with a total supply
+            // around 10^60. That's beyond what even Uniswap handles, as they use
+            // uint112 values internally for tokens.
+            // It's possible that if a token has large decimals, e.g. much more
+            // than 18, that the owner vault balance could be rounded down enough
+            // to cause significant non-dust amounts to be untradeable. In this
+            // case the token is not really supported.
+            // In either case, the order owner can still withdraw their vault
+            // balances in full, they just can't trade that token effectively.
+            // uint256 ownerVaultBalanceD18 = ownerVaultBalance.scale18(order.validOutputs[outputIOIndex].decimals, 0);
+            // orderOutputMaxD18 = orderOutputMaxD18 > ownerVaultBalanceD18 ? ownerVaultBalanceD18 : orderOutputMaxD18;
+
+            // Populate the context with the output max rescaled and vault capped.
+            context[CONTEXT_CALCULATIONS_COLUMN] =
+                LibUint256Array.arrayFrom(Output18Amount.unwrap(orderOutputMax), orderIORatio);
+
+            return OrderIOCalculation(
+                order, outputIOIndex, orderOutputMax, orderIORatio, context, namespace, calculateOrderKVs
+            );
         }
     }
 
@@ -692,30 +712,93 @@ contract OrderBook is IOrderBookV3, ReentrancyGuard, Multicall, OrderBookV3Flash
         OrderIOCalculation memory aliceOrderIOCalculation,
         OrderIOCalculation memory bobOrderIOCalculation
     ) internal pure returns (ClearStateChange memory clearStateChange) {
-        // Alice's output is the smaller of their max output and Bob's input.
-        clearStateChange.aliceOutput = aliceOrderIOCalculation.outputMax.min(
-            // Bob's input is Alice's output.
-            // Alice cannot output more than their max.
-            // Bob wants input of their IO ratio * their output.
-            // Always round IO calculations up.
-            bobOrderIOCalculation.outputMax.fixedPointMul(bobOrderIOCalculation.IORatio, Math.Rounding.Up)
-        );
+        calculateClearStateAlice(clearStateChange, aliceOrderIOCalculation, bobOrderIOCalculation);
+        // Flip alice and bob to calculate bob's output.
+        calculateClearStateAlice(clearStateChange, bobOrderIOCalculation, aliceOrderIOCalculation);
+        // {
+        //     // This is the max input that bob can afford, given his own IO ratio
+        //     // and maximum spend/output.
+        //     Input18Amount bobInputMax18 = calculateInputFromIORatio(
+        //         bobOrderIOCalculation.outputMax,
+        //         bobOrderIOCalculation.IORatio
+        //     );
+        //     Output18Amount aliceOutputMax18 = aliceOrderIOCalculation.outputMax;
+        //     // Alice's doesn't need to provide more output than bob's max input.
+        //     if (Output18Amount.unwrap(aliceOutputMax18) > Input18Amount.unwrap(bobInputMax18)) {
+        //         aliceOutputMax18 = Output18Amount.wrap(Input18Amount.unwrap(bobInputMax18));
+        //     }
+        //     // Alice's final output is the scaled version of the 18 decimal output,
+        //     // rounded down to benefit Alice.
+        //     clearStateChange.aliceOutput = OutputTknAmount.wrap(Output18Amount.unwrap(aliceOutputMax18).scaleN(
+        //         aliceOrderIOCalculation.outputDecimals,
+        //         0
+        //     ));
+        // }
+
+        // {
+        //     // Bob's output
+
+        // }
+
+        // // Alice's output is the smaller of their max output and Bob's input.
+        // clearStateChange.aliceOutput = aliceOrderIOCalculation.outputMax.min(
+        //     // Bob's input is Alice's output.
+        //     // Alice cannot output more than their max.
+        //     // Bob wants input of their IO ratio * their output.
+        //     // Always round IO calculations up.
+        //     bobOrderIOCalculation.outputMax.fixedPointMul(bobOrderIOCalculation.IORatio, Math.Rounding.Up)
+        // );
         // Bob's output is the smaller of their max output and Alice's input.
-        clearStateChange.bobOutput = bobOrderIOCalculation.outputMax.min(
-            // Alice's input is Bob's output.
-            // Bob cannot output more than their max.
-            // Alice wants input of their IO ratio * their output.
-            // Always round IO calculations up.
-            aliceOrderIOCalculation.outputMax.fixedPointMul(aliceOrderIOCalculation.IORatio, Math.Rounding.Up)
-        );
+        // clearStateChange.bobOutput = bobOrderIOCalculation.outputMax.min(
+        //     // Alice's input is Bob's output.
+        //     // Bob cannot output more than their max.
+        //     // Alice wants input of their IO ratio * their output.
+        //     // Always round IO calculations up.
+        //     aliceOrderIOCalculation.outputMax.fixedPointMul(aliceOrderIOCalculation.IORatio, Math.Rounding.Up)
+        // );
         // Alice's input is Alice's output * their IO ratio.
         // Always round IO calculations up.
+        // clearStateChange.aliceInput =
+        //     clearStateChange.aliceOutput.fixedPointMul(aliceOrderIOCalculation.IORatio, Math.Rounding.Up);
+        // // Bob's input is Bob's output * their IO ratio.
+        // // Always round IO calculations up.
+        // clearStateChange.bobInput =
+        //     clearStateChange.bobOutput.fixedPointMul(bobOrderIOCalculation.IORatio, Math.Rounding.Up);
+    }
+
+    function calculateClearStateAlice(
+        ClearStateChange memory clearStateChange,
+        OrderIOCalculation memory aliceOrderIOCalculation,
+        OrderIOCalculation memory bobOrderIOCalculation
+    ) internal pure {
+        // Always round IO calculations up so that the counterparty pays more.
+        // This is the max input that bob can afford, given his own IO ratio
+        // and maximum spend/output.
+        Input18Amount bobInputMax18 = Input18Amount.wrap(
+            Output18Amount.unwrap(bobOrderIOCalculation.outputMax).fixedPointMul(
+                bobOrderIOCalculation.IORatio, Math.Rounding.Up
+            )
+        );
+        Output18Amount aliceOutputMax18 = aliceOrderIOCalculation.outputMax;
+        // Alice's doesn't need to provide more output than bob's max input.
+        if (Output18Amount.unwrap(aliceOutputMax18) > Input18Amount.unwrap(bobInputMax18)) {
+            aliceOutputMax18 = Output18Amount.wrap(Input18Amount.unwrap(bobInputMax18));
+        }
+        // Alice's final output is the scaled version of the 18 decimal output,
+        // rounded down to benefit Alice.
+        clearStateChange.aliceOutput = Output18Amount.unwrap(aliceOutputMax18).scaleN(
+            aliceOrderIOCalculation.order.validOutputs[aliceOrderIOCalculation.outputIOIndex].decimals, 0
+        );
+
+        // Alice's input is her output * her IO ratio, rounded up.
+        Input18Amount aliceInput18 = Input18Amount.wrap(
+            Output18Amount.unwrap(aliceOutputMax18).fixedPointMul(aliceOrderIOCalculation.IORatio, Math.Rounding.Up)
+        );
         clearStateChange.aliceInput =
-            clearStateChange.aliceOutput.fixedPointMul(aliceOrderIOCalculation.IORatio, Math.Rounding.Up);
-        // Bob's input is Bob's output * their IO ratio.
-        // Always round IO calculations up.
-        clearStateChange.bobInput =
-            clearStateChange.bobOutput.fixedPointMul(bobOrderIOCalculation.IORatio, Math.Rounding.Up);
+        // Use bob's output decimals as alice's input decimals.
+        Input18Amount.unwrap(aliceInput18).scaleN(
+            bobOrderIOCalculation.order.validOutputs[bobOrderIOCalculation.outputIOIndex].decimals, FLAG_ROUND_UP
+        );
     }
 
     function _calculateOrderDispatch(address expression_) internal pure returns (EncodedDispatch) {
diff --git a/test/concrete/OrderBook.takeOrder.t.sol b/test/concrete/OrderBook.takeOrder.t.sol
index d3c9fd4b8..157acd228 100644
--- a/test/concrete/OrderBook.takeOrder.t.sol
+++ b/test/concrete/OrderBook.takeOrder.t.sol
@@ -6,32 +6,27 @@ import "test/util/abstract/OrderBookExternalRealTest.sol";
 /// @title OrderBookTakeOrderTest
 /// @notice A test harness for testing the OrderBook takeOrder function.
 contract OrderBookTakeOrderTest is OrderBookExternalRealTest {
-    function testPrecision01() public {
+    function checkPrecision(
+        bytes memory rainString,
+        uint8 outputTokenDecimals,
+        uint8 inputTokenDecimals,
+        uint256 expectedTakerTotalInput,
+        uint256 expectedTakerTotalOutput
+    ) internal {
         uint256 vaultId = 0;
         address inputToken = address(0x100);
         address outputToken = address(0x101);
         OrderConfigV2 memory config;
         {
-            uint8 inputTokenDecimals = 6;
-            uint8 outputTokenDecimals = 18;
             IO[] memory validInputs = new IO[](1);
             validInputs[0] = IO(inputToken, inputTokenDecimals, vaultId);
             IO[] memory validOutputs = new IO[](1);
             validOutputs[0] = IO(outputToken, outputTokenDecimals, vaultId);
             // These numbers are known to cause large rounding errors if the
             // precision is not handled correctly.
-            (bytes memory bytecode, uint256[] memory constants) = IParserV1(address(iDeployer)).parse("output-max io-ratio:157116365680491867129910 318235466963885;:;");
-            EvaluableConfigV2 memory evaluableConfig = EvaluableConfigV2(
-                iDeployer,
-                bytecode,
-                constants
-            );
-            config = OrderConfigV2(
-                validInputs,
-                validOutputs,
-                evaluableConfig,
-                ""
-            );
+            (bytes memory bytecode, uint256[] memory constants) = IParserV1(address(iDeployer)).parse(rainString);
+            EvaluableConfigV2 memory evaluableConfig = EvaluableConfigV2(iDeployer, bytecode, constants);
+            config = OrderConfigV2(validInputs, validOutputs, evaluableConfig, "");
             // Etch with invalid.
             vm.etch(outputToken, hex"fe");
             vm.etch(inputToken, hex"fe");
@@ -40,31 +35,50 @@ contract OrderBookTakeOrderTest is OrderBookExternalRealTest {
             vm.mockCall(outputToken, "", abi.encode(true));
             vm.mockCall(inputToken, "", abi.encode(true));
         }
-        iOrderbook.deposit(outputToken, vaultId, 1000000e18);
+        if (expectedTakerTotalInput > 0) {
+            iOrderbook.deposit(outputToken, vaultId, expectedTakerTotalInput);
+        }
+        assertEq(iOrderbook.vaultBalance(address(this), outputToken, vaultId), expectedTakerTotalInput);
         vm.recordLogs();
         iOrderbook.addOrder(config);
         Vm.Log[] memory entries = vm.getRecordedLogs();
         assertEq(entries.length, 3);
-        (,,Order memory order,) = abi.decode(entries[2].data, (address, address, Order, bytes32));
+        (,, Order memory order,) = abi.decode(entries[2].data, (address, address, Order, bytes32));
 
         TakeOrderConfig[] memory orders = new TakeOrderConfig[](1);
-        orders[0] = TakeOrderConfig(
-            order,
-            0,
-            0,
-            new SignedContextV1[](0)
-        );
-        TakeOrdersConfigV2 memory takeOrdersConfig = TakeOrdersConfigV2(
-            inputToken,
-            outputToken,
-            0,
-            type(uint256).max,
-            type(uint256).max,
-            orders,
-            ""
-        );
-        (uint256 totalInput, uint256 totalOutput) = iOrderbook.takeOrders(takeOrdersConfig);
-        assertEq(totalInput, 157116365680491867129910);
-        assertEq(totalOutput, 50000000);
+        orders[0] = TakeOrderConfig(order, 0, 0, new SignedContextV1[](0));
+        TakeOrdersConfigV2 memory takeOrdersConfig =
+            TakeOrdersConfigV2(inputToken, outputToken, 0, type(uint256).max, type(uint256).max, orders, "");
+        (uint256 totalTakerInput, uint256 totalTakerOutput) = iOrderbook.takeOrders(takeOrdersConfig);
+        assertEq(totalTakerInput, expectedTakerTotalInput);
+        assertEq(totalTakerOutput, expectedTakerTotalOutput);
+        assertEq(iOrderbook.vaultBalance(address(this), outputToken, vaultId), 0);
+    }
+
+    function testTakeOrderPrecisionKnownBad01() public {
+        // Older versions of OB had precision issues with this IO setup.
+        bytes memory knownBad = "output-max io-ratio:157116365680491867129910 318235466963885;:;";
+        // Start with both tokens having 18 decimals.
+        // This gives the best precision for both.
+        checkPrecision(knownBad, 18, 18, 157116365680491867129910, 49999999999999844580);
+
+        // If the taker output token has low decimals then it will round up
+        // at that decimal precision, to force the taker to have to output the
+        // dust amount.
+        // Increasing the decimals of the taker input token will not impact the
+        // precision provided there is no overflow. It simply scales up the
+        // taker input amount.
+        checkPrecision(knownBad, 18, 6, 157116365680491867129910, 50e6);
+        checkPrecision(knownBad, 19, 6, 1571163656804918671299100, 50e6);
+        checkPrecision(knownBad, 20, 6, 15711636568049186712991000, 50e6);
+        checkPrecision(knownBad, 21, 6, 157116365680491867129910000, 50e6);
+        checkPrecision(knownBad, 50, 6, 15711636568049186712991000000000000000000000000000000000, 50e6);
+
+        // Flip the decimals for each token.
+        checkPrecision(knownBad, 6, 18, 157116365680, 49999999999999844580);
+        checkPrecision(knownBad, 6, 19, 157116365680, 499999999999998445800);
+        checkPrecision(knownBad, 6, 20, 157116365680, 4999999999999984458000);
+        checkPrecision(knownBad, 6, 21, 157116365680, 49999999999999844580000);
+        checkPrecision(knownBad, 6, 50, 157116365680, 4999999999999984458000000000000000000000000000000000);
     }
-}
\ No newline at end of file
+}

From 94469165c53c435f9665ac68009dd8d4a39b1882 Mon Sep 17 00:00:00 2001
From: thedavidmeister <thedavidmeister@gmail.com>
Date: Tue, 29 Aug 2023 19:32:34 +0400
Subject: [PATCH 4/9] lint

---
 src/concrete/OrderBook.sol | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/concrete/OrderBook.sol b/src/concrete/OrderBook.sol
index 704810f07..b6f2192ff 100644
--- a/src/concrete/OrderBook.sol
+++ b/src/concrete/OrderBook.sol
@@ -391,6 +391,8 @@ contract OrderBook is IOrderBookV3, ReentrancyGuard, Multicall, OrderBookV3Flash
                     {
                         // Always round IO calculations up so the taker pays more.
                         Output18Amount takerOutput18 = Output18Amount.wrap(
+                            // Use the capped taker input to calculate the taker
+                            // output.
                             Input18Amount.unwrap(takerInput18).fixedPointMul(
                                 orderIOCalculation.IORatio, Math.Rounding.Up
                             )

From 4758321924066a1272cf361669c67a7cf894c419 Mon Sep 17 00:00:00 2001
From: thedavidmeister <thedavidmeister@gmail.com>
Date: Tue, 29 Aug 2023 19:46:36 +0400
Subject: [PATCH 5/9] reinstate vault balance capping

---
 src/concrete/OrderBook.sol              | 49 ++++++++++++++-----------
 test/concrete/OrderBook.takeOrder.t.sol | 13 ++++---
 2 files changed, 35 insertions(+), 27 deletions(-)

diff --git a/src/concrete/OrderBook.sol b/src/concrete/OrderBook.sol
index b6f2192ff..a1cd03b55 100644
--- a/src/concrete/OrderBook.sol
+++ b/src/concrete/OrderBook.sol
@@ -590,35 +590,40 @@ contract OrderBook is IOrderBookV3, ReentrancyGuard, Multicall, OrderBookV3Flash
                 .interpreter
                 .eval(order.evaluable.store, namespace, _calculateOrderDispatch(order.evaluable.expression), context);
 
-            Output18Amount orderOutputMax = Output18Amount.wrap(calculateOrderStack[calculateOrderStack.length - 2]);
+            Output18Amount orderOutputMax18 = Output18Amount.wrap(calculateOrderStack[calculateOrderStack.length - 2]);
             uint256 orderIORatio = calculateOrderStack[calculateOrderStack.length - 1];
 
-            // The order owner can't send more than the smaller of their vault
-            // balance or their per-order limit.
-            // uint256 ownerVaultBalance = sVaultBalances[order.owner][order.validOutputs[outputIOIndex].token][order
-            //     .validOutputs[outputIOIndex].vaultId];
-            // We round down vault balances and don't saturate because we're
-            // dealing with real token amounts here. If rescaling would somehow
-            // cause an overflow in a real token amount, that's basically an
-            // unsupported token, it implies a very small decimals value with
-            // very large token total supply. E.g. 0 decimals with a total supply
-            // around 10^60. That's beyond what even Uniswap handles, as they use
-            // uint112 values internally for tokens.
-            // It's possible that if a token has large decimals, e.g. much more
-            // than 18, that the owner vault balance could be rounded down enough
-            // to cause significant non-dust amounts to be untradeable. In this
-            // case the token is not really supported.
-            // In either case, the order owner can still withdraw their vault
-            // balances in full, they just can't trade that token effectively.
-            // uint256 ownerVaultBalanceD18 = ownerVaultBalance.scale18(order.validOutputs[outputIOIndex].decimals, 0);
-            // orderOutputMaxD18 = orderOutputMaxD18 > ownerVaultBalanceD18 ? ownerVaultBalanceD18 : orderOutputMaxD18;
+            {
+                // The order owner can't send more than the smaller of their vault
+                // balance or their per-order limit.
+                uint256 ownerVaultBalance = sVaultBalances[order.owner][order.validOutputs[outputIOIndex].token][order
+                    .validOutputs[outputIOIndex].vaultId];
+                // We round down vault balances and don't saturate because we're
+                // dealing with real token amounts here. If rescaling would somehow
+                // cause an overflow in a real token amount, that's basically an
+                // unsupported token, it implies a very small decimals value with
+                // very large token total supply. E.g. 0 decimals with a total supply
+                // around 10^60. That's beyond what even Uniswap handles, as they use
+                // uint112 values internally for tokens.
+                // It's possible that if a token has large decimals, e.g. much more
+                // than 18, that the owner vault balance could be rounded down enough
+                // to cause significant non-dust amounts to be untradeable. In this
+                // case the token is not really supported.
+                // In either case, the order owner can still withdraw their vault
+                // balances in full, they just can't trade that token effectively.
+                Output18Amount ownerVaultBalance18 =
+                    Output18Amount.wrap(ownerVaultBalance.scale18(order.validOutputs[outputIOIndex].decimals, 0));
+                if (Output18Amount.unwrap(orderOutputMax18) > Output18Amount.unwrap(ownerVaultBalance18)) {
+                    orderOutputMax18 = ownerVaultBalance18;
+                }
+            }
 
             // Populate the context with the output max rescaled and vault capped.
             context[CONTEXT_CALCULATIONS_COLUMN] =
-                LibUint256Array.arrayFrom(Output18Amount.unwrap(orderOutputMax), orderIORatio);
+                LibUint256Array.arrayFrom(Output18Amount.unwrap(orderOutputMax18), orderIORatio);
 
             return OrderIOCalculation(
-                order, outputIOIndex, orderOutputMax, orderIORatio, context, namespace, calculateOrderKVs
+                order, outputIOIndex, orderOutputMax18, orderIORatio, context, namespace, calculateOrderKVs
             );
         }
     }
diff --git a/test/concrete/OrderBook.takeOrder.t.sol b/test/concrete/OrderBook.takeOrder.t.sol
index 157acd228..f73e1de8f 100644
--- a/test/concrete/OrderBook.takeOrder.t.sol
+++ b/test/concrete/OrderBook.takeOrder.t.sol
@@ -75,10 +75,13 @@ contract OrderBookTakeOrderTest is OrderBookExternalRealTest {
         checkPrecision(knownBad, 50, 6, 15711636568049186712991000000000000000000000000000000000, 50e6);
 
         // Flip the decimals for each token.
-        checkPrecision(knownBad, 6, 18, 157116365680, 49999999999999844580);
-        checkPrecision(knownBad, 6, 19, 157116365680, 499999999999998445800);
-        checkPrecision(knownBad, 6, 20, 157116365680, 4999999999999984458000);
-        checkPrecision(knownBad, 6, 21, 157116365680, 49999999999999844580000);
-        checkPrecision(knownBad, 6, 50, 157116365680, 4999999999999984458000000000000000000000000000000000);
+        // As the output has low decimals, it rounds down which then causes the
+        // input to be slightly smaller prorata.
+        checkPrecision(knownBad, 6, 18, 157116365680, 49999999999843315014);
+        checkPrecision(knownBad, 6, 19, 157116365680, 499999999998433150140);
+        checkPrecision(knownBad, 6, 20, 157116365680, 4999999999984331501400);
+        checkPrecision(knownBad, 6, 21, 157116365680, 49999999999843315014000);
+        checkPrecision(knownBad, 6, 50, 157116365680, 4999999999984331501400000000000000000000000000000000);
+
     }
 }

From f7eb4f2c17c00f59b3998641be6f116e65b8ae37 Mon Sep 17 00:00:00 2001
From: thedavidmeister <thedavidmeister@gmail.com>
Date: Tue, 29 Aug 2023 19:51:50 +0400
Subject: [PATCH 6/9] lint

---
 src/concrete/OrderBook.sol              | 49 -------------------------
 test/concrete/OrderBook.takeOrder.t.sol |  1 -
 2 files changed, 50 deletions(-)

diff --git a/src/concrete/OrderBook.sol b/src/concrete/OrderBook.sol
index a1cd03b55..0c4b7cb49 100644
--- a/src/concrete/OrderBook.sol
+++ b/src/concrete/OrderBook.sol
@@ -722,55 +722,6 @@ contract OrderBook is IOrderBookV3, ReentrancyGuard, Multicall, OrderBookV3Flash
         calculateClearStateAlice(clearStateChange, aliceOrderIOCalculation, bobOrderIOCalculation);
         // Flip alice and bob to calculate bob's output.
         calculateClearStateAlice(clearStateChange, bobOrderIOCalculation, aliceOrderIOCalculation);
-        // {
-        //     // This is the max input that bob can afford, given his own IO ratio
-        //     // and maximum spend/output.
-        //     Input18Amount bobInputMax18 = calculateInputFromIORatio(
-        //         bobOrderIOCalculation.outputMax,
-        //         bobOrderIOCalculation.IORatio
-        //     );
-        //     Output18Amount aliceOutputMax18 = aliceOrderIOCalculation.outputMax;
-        //     // Alice's doesn't need to provide more output than bob's max input.
-        //     if (Output18Amount.unwrap(aliceOutputMax18) > Input18Amount.unwrap(bobInputMax18)) {
-        //         aliceOutputMax18 = Output18Amount.wrap(Input18Amount.unwrap(bobInputMax18));
-        //     }
-        //     // Alice's final output is the scaled version of the 18 decimal output,
-        //     // rounded down to benefit Alice.
-        //     clearStateChange.aliceOutput = OutputTknAmount.wrap(Output18Amount.unwrap(aliceOutputMax18).scaleN(
-        //         aliceOrderIOCalculation.outputDecimals,
-        //         0
-        //     ));
-        // }
-
-        // {
-        //     // Bob's output
-
-        // }
-
-        // // Alice's output is the smaller of their max output and Bob's input.
-        // clearStateChange.aliceOutput = aliceOrderIOCalculation.outputMax.min(
-        //     // Bob's input is Alice's output.
-        //     // Alice cannot output more than their max.
-        //     // Bob wants input of their IO ratio * their output.
-        //     // Always round IO calculations up.
-        //     bobOrderIOCalculation.outputMax.fixedPointMul(bobOrderIOCalculation.IORatio, Math.Rounding.Up)
-        // );
-        // Bob's output is the smaller of their max output and Alice's input.
-        // clearStateChange.bobOutput = bobOrderIOCalculation.outputMax.min(
-        //     // Alice's input is Bob's output.
-        //     // Bob cannot output more than their max.
-        //     // Alice wants input of their IO ratio * their output.
-        //     // Always round IO calculations up.
-        //     aliceOrderIOCalculation.outputMax.fixedPointMul(aliceOrderIOCalculation.IORatio, Math.Rounding.Up)
-        // );
-        // Alice's input is Alice's output * their IO ratio.
-        // Always round IO calculations up.
-        // clearStateChange.aliceInput =
-        //     clearStateChange.aliceOutput.fixedPointMul(aliceOrderIOCalculation.IORatio, Math.Rounding.Up);
-        // // Bob's input is Bob's output * their IO ratio.
-        // // Always round IO calculations up.
-        // clearStateChange.bobInput =
-        //     clearStateChange.bobOutput.fixedPointMul(bobOrderIOCalculation.IORatio, Math.Rounding.Up);
     }
 
     function calculateClearStateAlice(
diff --git a/test/concrete/OrderBook.takeOrder.t.sol b/test/concrete/OrderBook.takeOrder.t.sol
index f73e1de8f..ff9213176 100644
--- a/test/concrete/OrderBook.takeOrder.t.sol
+++ b/test/concrete/OrderBook.takeOrder.t.sol
@@ -82,6 +82,5 @@ contract OrderBookTakeOrderTest is OrderBookExternalRealTest {
         checkPrecision(knownBad, 6, 20, 157116365680, 4999999999984331501400);
         checkPrecision(knownBad, 6, 21, 157116365680, 49999999999843315014000);
         checkPrecision(knownBad, 6, 50, 157116365680, 4999999999984331501400000000000000000000000000000000);
-
     }
 }

From 3f31ff64d0db014cf7ad28c04aa0d08b6e570d73 Mon Sep 17 00:00:00 2001
From: thedavidmeister <thedavidmeister@gmail.com>
Date: Tue, 29 Aug 2023 20:21:46 +0400
Subject: [PATCH 7/9] ensure token decimal matching in clear

---
 .gas-snapshot              | 17 +++++++++--------
 src/concrete/OrderBook.sol | 28 +++++++++++++++++++++++++++-
 2 files changed, 36 insertions(+), 9 deletions(-)

diff --git a/.gas-snapshot b/.gas-snapshot
index bcac5305d..6bf2b7c5d 100644
--- a/.gas-snapshot
+++ b/.gas-snapshot
@@ -3,7 +3,7 @@ GenericPoolOrderBookFlashBorrowerTest:testTakeOrdersSender() (gas: 2241348)
 LibOrderTest:testHashEqual((address,bool,(address,address,address),(address,uint8,uint256)[],(address,uint8,uint256)[])) (runs: 5096, μ: 194389, ~: 191140)
 LibOrderTest:testHashNotEqual((address,bool,(address,address,address),(address,uint8,uint256)[],(address,uint8,uint256)[]),(address,bool,(address,address,address),(address,uint8,uint256)[],(address,uint8,uint256)[])) (runs: 5096, μ: 298734, ~: 298554)
 OrderBookAddOrderMockTest:testAddOrderSameAccountWithDifferentConfig(address,((address,uint8,uint256)[],(address,uint8,uint256)[],(address,bytes,uint256[]),bytes),((address,uint8,uint256)[],(address,uint8,uint256)[],(address,bytes,uint256[]),bytes),address,address) (runs: 5096, μ: 2771808, ~: 2761475)
-OrderBookAddOrderMockTest:testAddOrderTwoAccountsWithDifferentConfig(address,address,((address,uint8,uint256)[],(address,uint8,uint256)[],(address,bytes,uint256[]),bytes),((address,uint8,uint256)[],(address,uint8,uint256)[],(address,bytes,uint256[]),bytes),address,address) (runs: 5096, μ: 2626613, ~: 2600883)
+OrderBookAddOrderMockTest:testAddOrderTwoAccountsWithDifferentConfig(address,address,((address,uint8,uint256)[],(address,uint8,uint256)[],(address,bytes,uint256[]),bytes),((address,uint8,uint256)[],(address,uint8,uint256)[],(address,bytes,uint256[]),bytes),address,address) (runs: 5096, μ: 2626417, ~: 2600441)
 OrderBookAddOrderMockTest:testAddOrderTwoAccountsWithSameConfig(address,address,((address,uint8,uint256)[],(address,uint8,uint256)[],(address,bytes,uint256[]),bytes),address) (runs: 5096, μ: 2493275, ~: 2492266)
 OrderBookAddOrderMockTest:testAddOrderWithCalculationsInputsAndOutputsSucceeds(address,((address,uint8,uint256)[],(address,uint8,uint256)[],(address,bytes,uint256[]),bytes),address) (runs: 5096, μ: 1295793, ~: 1281958)
 OrderBookAddOrderMockTest:testAddOrderWithNonEmptyMetaEmitsMetaV1(address,((address,uint8,uint256)[],(address,uint8,uint256)[],(address,bytes,uint256[]),bytes),address) (runs: 5096, μ: 1303882, ~: 1290859)
@@ -21,23 +21,24 @@ OrderBookDepositTest:testDepositEvent(address,uint256,uint256) (runs: 5096, μ:
 OrderBookDepositTest:testDepositFail(address,uint256,uint256) (runs: 5096, μ: 8937393460516740787, ~: 8937393460516740786)
 OrderBookDepositTest:testDepositGas00() (gas: 8176)
 OrderBookDepositTest:testDepositGas01() (gas: 34620)
-OrderBookDepositTest:testDepositMany((address,address,uint256,uint248)[]) (runs: 5096, μ: 5045831, ~: 4663301)
+OrderBookDepositTest:testDepositMany((address,address,uint256,uint248)[]) (runs: 5096, μ: 5110729, ~: 4755733)
 OrderBookDepositTest:testDepositOverflow(address,uint256,uint256,uint256) (runs: 5096, μ: 46645, ~: 46645)
-OrderBookDepositTest:testDepositReentrancy(address,uint256,uint256,address,uint256,uint256) (runs: 5096, μ: 495256, ~: 496632)
+OrderBookDepositTest:testDepositReentrancy(address,uint256,uint256,address,uint256,uint256) (runs: 5096, μ: 495105, ~: 496632)
 OrderBookDepositTest:testDepositSimple(address,uint256,uint256) (runs: 5096, μ: 37840, ~: 37840)
 OrderBookDepositTest:testDepositZero(address,uint256) (runs: 5096, μ: 12639, ~: 12639)
 OrderBookDepositTest:testVaultBalanceNoDeposits(address,uint256) (runs: 5096, μ: 8263, ~: 8263)
-OrderBookDepositTest:testVaultBalanceReentrant(address,uint256,uint256,address,address,uint256) (runs: 5096, μ: 494105, ~: 495964)
+OrderBookDepositTest:testVaultBalanceReentrant(address,uint256,uint256,address,address,uint256) (runs: 5096, μ: 494249, ~: 495964)
 OrderBookRemoveOrderMockTest:testRemoveOrderAddRemoveMulti(address,((address,uint8,uint256)[],(address,uint8,uint256)[],(address,bytes,uint256[]),bytes),address) (runs: 5096, μ: 7256892, ~: 7139695)
 OrderBookRemoveOrderMockTest:testRemoveOrderDifferent(address,((address,uint8,uint256)[],(address,uint8,uint256)[],(address,bytes,uint256[]),bytes),address,((address,uint8,uint256)[],(address,uint8,uint256)[],(address,bytes,uint256[]),bytes),address) (runs: 5096, μ: 5070545, ~: 5046552)
 OrderBookRemoveOrderMockTest:testRemoveOrderDifferentOwners(address,address,((address,uint8,uint256)[],(address,uint8,uint256)[],(address,bytes,uint256[]),bytes),address) (runs: 5096, μ: 4854493, ~: 4842171)
-OrderBookRemoveOrderMockTest:testRemoveOrderDifferentOwnersDifferent(address,address,((address,uint8,uint256)[],(address,uint8,uint256)[],(address,bytes,uint256[]),bytes),address,((address,uint8,uint256)[],(address,uint8,uint256)[],(address,bytes,uint256[]),bytes),address) (runs: 5096, μ: 10468846, ~: 10456300)
+OrderBookRemoveOrderMockTest:testRemoveOrderDifferentOwnersDifferent(address,address,((address,uint8,uint256)[],(address,uint8,uint256)[],(address,bytes,uint256[]),bytes),address,((address,uint8,uint256)[],(address,uint8,uint256)[],(address,bytes,uint256[]),bytes),address) (runs: 5096, μ: 10467503, ~: 10454009)
 OrderBookRemoveOrderMockTest:testRemoveOrderDoesNotExist(address,((address,uint8,uint256)[],(address,uint8,uint256)[],(address,bytes,uint256[]),bytes),address) (runs: 5096, μ: 386856, ~: 382108)
 OrderBookRemoveOrderMockTest:testRemoveOrderOnlyOwner(address,address,((address,uint8,uint256)[],(address,uint8,uint256)[],(address,bytes,uint256[]),bytes),address) (runs: 5096, μ: 2603327, ~: 2601305)
+OrderBookTakeOrderTest:testTakeOrderPrecisionKnownBad01() (gas: 2556867)
 OrderBookWithdrawTest:testWithdrawEmptyVault(address,address,uint256,uint256) (runs: 5096, μ: 15316, ~: 15316)
-OrderBookWithdrawTest:testWithdrawFailure(address,uint256,uint256,uint256) (runs: 5096, μ: 8937393460516719745, ~: 8937393460516738938)
+OrderBookWithdrawTest:testWithdrawFailure(address,uint256,uint256,uint256) (runs: 5096, μ: 8937393460516719607, ~: 8937393460516700429)
 OrderBookWithdrawTest:testWithdrawFullVault(address,uint256,uint256,uint256) (runs: 5096, μ: 41257, ~: 41256)
-OrderBookWithdrawTest:testWithdrawMany((bool,address,address,uint256,uint248)[]) (runs: 5096, μ: 1447473, ~: 1381991)
+OrderBookWithdrawTest:testWithdrawMany((bool,address,address,uint256,uint248)[]) (runs: 5096, μ: 1448254, ~: 1371051)
 OrderBookWithdrawTest:testWithdrawPartialVault(address,uint256,uint256,uint256) (runs: 5096, μ: 51929, ~: 51929)
-OrderBookWithdrawTest:testWithdrawReentrant(address,uint256,uint256,address,address,uint256) (runs: 5096, μ: 506060, ~: 507997)
+OrderBookWithdrawTest:testWithdrawReentrant(address,uint256,uint256,address,address,uint256) (runs: 5096, μ: 506329, ~: 507997)
 OrderBookWithdrawTest:testWithdrawZero(address,address,uint256) (runs: 5096, μ: 12787, ~: 12787)
\ No newline at end of file
diff --git a/src/concrete/OrderBook.sol b/src/concrete/OrderBook.sol
index 0c4b7cb49..b06daaa1c 100644
--- a/src/concrete/OrderBook.sol
+++ b/src/concrete/OrderBook.sol
@@ -39,6 +39,12 @@ error NotOrderOwner(address sender, address owner);
 /// @param bobToken The input or output of the other order that doesn't match a.
 error TokenMismatch(address aliceToken, address bobToken);
 
+/// Thrown when the input and output token decimals don't match, in either
+/// direction.
+/// @param aliceTokenDecimals The input or output decimals of one order.
+/// @param bobTokenDecimals The input or output decimals of the other order.
+error TokenDecimalsMismatch(uint8 aliceTokenDecimals, uint8 bobTokenDecimals);
+
 /// Thrown when the minimum input is not met.
 /// @param minimumInput The minimum input required.
 /// @param input The input that was achieved.
@@ -470,6 +476,16 @@ contract OrderBook is IOrderBookV3, ReentrancyGuard, Multicall, OrderBookV3Flash
                 );
             }
 
+            if (
+                alice.validOutputs[clearConfig.aliceOutputIOIndex].decimals
+                    != bob.validInputs[clearConfig.bobInputIOIndex].decimals
+            ) {
+                revert TokenDecimalsMismatch(
+                    alice.validOutputs[clearConfig.aliceOutputIOIndex].decimals,
+                    bob.validInputs[clearConfig.bobInputIOIndex].decimals
+                );
+            }
+
             if (
                 bob.validOutputs[clearConfig.bobOutputIOIndex].token
                     != alice.validInputs[clearConfig.aliceInputIOIndex].token
@@ -480,6 +496,16 @@ contract OrderBook is IOrderBookV3, ReentrancyGuard, Multicall, OrderBookV3Flash
                 );
             }
 
+            if (
+                bob.validOutputs[clearConfig.bobOutputIOIndex].decimals
+                    != alice.validInputs[clearConfig.aliceInputIOIndex].decimals
+            ) {
+                revert TokenDecimalsMismatch(
+                    alice.validInputs[clearConfig.aliceInputIOIndex].decimals,
+                    bob.validOutputs[clearConfig.bobOutputIOIndex].decimals
+                );
+            }
+
             // If either order is dead the clear is a no-op other than emitting
             // `OrderNotFound`. Returning rather than erroring makes it easier to
             // bulk clear using `Multicall`.
@@ -748,7 +774,7 @@ contract OrderBook is IOrderBookV3, ReentrancyGuard, Multicall, OrderBookV3Flash
             aliceOrderIOCalculation.order.validOutputs[aliceOrderIOCalculation.outputIOIndex].decimals, 0
         );
 
-        // Alice's input is her output * her IO ratio, rounded up.
+        // Alice's input is her bob-capped output * her IO ratio, rounded up.
         Input18Amount aliceInput18 = Input18Amount.wrap(
             Output18Amount.unwrap(aliceOutputMax18).fixedPointMul(aliceOrderIOCalculation.IORatio, Math.Rounding.Up)
         );

From 234f391ecad9f1de0cdb9fcadbfaec7e338222de Mon Sep 17 00:00:00 2001
From: thedavidmeister <thedavidmeister@gmail.com>
Date: Tue, 29 Aug 2023 20:33:47 +0400
Subject: [PATCH 8/9] lint

---
 src/concrete/OrderBook.sol | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/src/concrete/OrderBook.sol b/src/concrete/OrderBook.sol
index b06daaa1c..28178f623 100644
--- a/src/concrete/OrderBook.sol
+++ b/src/concrete/OrderBook.sol
@@ -780,6 +780,10 @@ contract OrderBook is IOrderBookV3, ReentrancyGuard, Multicall, OrderBookV3Flash
         );
         clearStateChange.aliceInput =
         // Use bob's output decimals as alice's input decimals.
+        //
+        // This is only safe if we have previously checked that the decimals
+        // match for alice and bob per token, otherwise bob could manipulate
+        // alice's intent.
         Input18Amount.unwrap(aliceInput18).scaleN(
             bobOrderIOCalculation.order.validOutputs[bobOrderIOCalculation.outputIOIndex].decimals, FLAG_ROUND_UP
         );

From ab8de9effe25a420dd995275086c13cf8e9da085 Mon Sep 17 00:00:00 2001
From: thedavidmeister <thedavidmeister@gmail.com>
Date: Tue, 29 Aug 2023 20:37:42 +0400
Subject: [PATCH 9/9] lint

---
 src/concrete/OrderBook.sol | 2 --
 1 file changed, 2 deletions(-)

diff --git a/src/concrete/OrderBook.sol b/src/concrete/OrderBook.sol
index 28178f623..41b8c5de5 100644
--- a/src/concrete/OrderBook.sol
+++ b/src/concrete/OrderBook.sol
@@ -23,8 +23,6 @@ import "../interface/unstable/IOrderBookV3OrderTaker.sol";
 import "../lib/LibOrder.sol";
 import "../abstract/OrderBookV3FlashLender.sol";
 
-import "forge-std/console2.sol";
-
 /// This will exist in a future version of Open Zeppelin if their main branch is
 /// to be believed.
 error ReentrancyGuardReentrantCall();