Dust accounting: per-(user, token) credit ledger realized on the next token move (#2673)

src/concrete/raindex/RaindexV6.sol

@@ -213,6 +213,20 @@ contract RaindexV6 is IRaindexV6, IMetaV1_2, ReentrancyGuard, Multicall, Raindex
     mapping(address owner => mapping(address token => mapping(bytes32 vaultId => Float balance))) internal
         sVaultBalances;
 
+    /// @dev Per `(user, token)` ledger of the sub-base-unit remainder that the
+    /// orderbook holds on behalf of `user` but has not yet moved, because a
+    /// lossy float->fixed-decimal conversion in `pullTokens`/`pushTokens` cannot
+    /// transfer a fraction of a base unit. The credit is always non-negative and
+    /// strictly less than one base unit: whenever it reaches a whole base unit it
+    /// is realized into the next token move for that `(user, token)`. It is
+    /// denominated in token units (a `Float`) so it is independent of the token's
+    /// decimals. Every credit is backed by real tokens the orderbook already
+    /// holds (a pull rounds the transfer up, a push rounds it down), so realizing
+    /// it only ever releases provable excess and never dips into a vault balance.
+    // Solhint and slither disagree on this. Slither wins.
+    //solhint-disable-next-line private-vars-leading-underscore
+    mapping(address user => mapping(address token => Float credit)) internal sDustCredit;
+
     /// @inheritdoc IRaindexV6
     function vaultBalance2(address owner, address token, bytes32 vaultId) external view override returns (Float) {
         return _vaultBalance(owner, token, vaultId);
@@ -1160,26 +1174,73 @@ contract RaindexV6 is IRaindexV6, IMetaV1_2, ReentrancyGuard, Multicall, Raindex
     /// never pulls in less than the vault accounting credits. `pushTokens` rounds
     /// DOWN for the same protocol-favoring reason. Together this keeps the
     /// orderbook solvent — its real token balance is always >= the sum of the
-    /// vault balances — but each lossy conversion leaves a sub-token-unit residue
-    /// in the contract that is not attributed to any vault and has no recovery
-    /// path. Such dust is under one base unit per conversion and can never cause
-    /// insolvency.
+    /// vault balances. The sub-base-unit residue each lossy conversion would
+    /// otherwise strand is instead booked into `sDustCredit[account][token]` and
+    /// realized into the next move for the same `(account, token)`: the pull is
+    /// reduced by any whole base unit of standing credit, and the fresh over-pull
+    /// is added to it. The credit is always backed by tokens the orderbook holds
+    /// for `account`, so this is exactly conservative and never insolvent. Near
+    /// the Float coefficient ceiling, where the credit subtraction and conversion
+    /// can truncate the pull below the requested base units, the pull is clamped
+    /// up to the full requested amount so solvency still holds.
     function pullTokens(address account, address token, Float amount) internal returns (uint256, uint8) {
         uint8 decimals = _safeDecimals(token);
         if (amount.lt(LibDecimalFloat.FLOAT_ZERO)) {
             revert NegativePull();
         }
 
-        (uint256 amount18, bool lossless) = LibDecimalFloat.toFixedDecimalLossy(amount, decimals);
-        // Round truncation up when pulling.
-        if (!lossless) {
-            // This needs to be checked math as an overflow would cause tokens
-            // to silently not be pulled (wraps to 0).
-            ++amount18;
-        }
-        if (amount18 > 0) {
+        // The orderbook already holds the standing credit (token units) for
+        // `account` from prior over-pulls / under-pushes, so it only needs to
+        // pull in the part of `amount` that the credit does not already cover.
+        Float credit = sDustCredit[account][token];
+        Float effective = amount.sub(credit);
+
+        uint256 amount18 = 0;
+        bool clamped = false;
+        // A non-positive `effective` means the credit already covers the whole
+        // pull, so no tokens move; `toFixedDecimalLossy` also rejects negatives.
+        if (effective.gt(LibDecimalFloat.FLOAT_ZERO)) {
+            bool lossless;
+            (amount18, lossless) = LibDecimalFloat.toFixedDecimalLossy(effective, decimals);
+            // Round truncation up when pulling.
+            if (!lossless) {
+                // This needs to be checked math as an overflow would cause tokens
+                // to silently not be pulled (wraps to 0).
+                ++amount18;
+            }
+
+            // Near the Float coefficient ceiling the `amount.sub(credit)` above
+            // can drop the sub-unit credit AND `toFixedDecimalLossy` can report a
+            // spuriously-lossless conversion that truncates `effective` DOWN by
+            // more than one base unit, so the round-up never fires and the pull
+            // falls short of the vault obligation it books. `obligation` is the
+            // full requested `amount` in base units (what a zero-credit pull would
+            // take); the standing credit can only ever reduce the pull by under one
+            // base unit, so when the rounded `amount18` lands two or more base units
+            // below `obligation` the credit was dropped. Clamp up to `obligation`
+            // so the orderbook is never short; the credit is then carried forward
+            // below rather than re-derived from the unreliable `effective`.
+            //slither-disable-next-line unused-return
+            (uint256 obligation,) = LibDecimalFloat.toFixedDecimalLossy(amount, decimals);
+            if (amount18 + 1 < obligation) {
+                amount18 = obligation;
+                clamped = true;
+            }
+
             IERC20(token).safeTransferFrom(account, address(this), amount18);
         }
+
+        // The new credit is the transferred amount minus what `effective`
+        // required: the over-pull when tokens moved (rounded up, so non-negative
+        // and under one base unit), or the leftover credit when none did. It is
+        // backed by tokens the orderbook holds for `account`. A clamped pull moved
+        // exactly `amount` in base units (the standing credit was sub-resolution
+        // beside it and could not reduce the pull), so the standing credit carries
+        // forward unchanged.
+        //slither-disable-next-line unused-return
+        (Float pulled,) = LibDecimalFloat.fromFixedDecimalLossyPacked(amount18, decimals);
+        sDustCredit[account][token] = clamped ? credit : pulled.sub(effective);
+
         return (amount18, decimals);
     }
 
@@ -1188,17 +1249,31 @@ contract RaindexV6 is IRaindexV6, IMetaV1_2, ReentrancyGuard, Multicall, Raindex
     ///
     /// A lossy float->fixed-decimal conversion is truncated (rounded DOWN) here
     /// so the contract never sends more than the vault accounting debits. See
-    /// `pullTokens` for the full rounding policy and its solvency / residual-dust
-    /// tradeoff.
+    /// `pullTokens` for the full rounding policy. The sub-base-unit residue each
+    /// lossy conversion would otherwise strand is booked into
+    /// `sDustCredit[account][token]` and realized into the next move for the same
+    /// `(account, token)`: the push is increased by any whole base unit of
+    /// standing credit, and the fresh under-push is added to it.
     function pushTokens(address account, address token, Float amountFloat) internal returns (uint256, uint8) {
         uint8 decimals = _safeDecimals(token);
 
         if (amountFloat.lt(LibDecimalFloat.FLOAT_ZERO)) {
             revert NegativePush();
         }
 
+        // The orderbook also owes `account` the standing credit from prior
+        // under-pushes, so the total it may send is `amountFloat + credit`.
+        Float effective = amountFloat.add(sDustCredit[account][token]);
+
+        //slither-disable-next-line unused-return
+        (uint256 amount,) = LibDecimalFloat.toFixedDecimalLossy(effective, decimals);
+        // The shortfall truncated away is the new credit. It is non-negative
+        // because the transfer was rounded down, and under one base unit because
+        // truncation drops less than one base unit.
         //slither-disable-next-line unused-return
-        (uint256 amount,) = LibDecimalFloat.toFixedDecimalLossy(amountFloat, decimals);
+        (Float pushed,) = LibDecimalFloat.fromFixedDecimalLossyPacked(amount, decimals);
+        sDustCredit[account][token] = effective.sub(pushed);
+
         if (amount > 0) {
             IERC20(token).safeTransfer(account, amount);
         }

src/lib/deploy/LibRaindexDeploy.sol

@@ -123,6 +123,15 @@ library LibRaindexDeploy {
     bytes32 constant RAINDEX_DEPLOYED_CODEHASH_0_1_9 =
         0x774ca4c194abf0720bca6c033580762029a80291feefd0a4b7855b16bcbbf7a9;
 
+    /// The deployed address of the `RaindexV6` contract at the published `0.1.10`
+    /// tag. (Changed in 0.1.10 — per-(user,token) dust-credit ledger.)
+    address constant RAINDEX_DEPLOYED_ADDRESS_0_1_10 = 0x66F51a9C29480491C9282D99cf324e9f419A4e1d;
+
+    /// The runtime code hash of the `RaindexV6` contract at the published `0.1.10`
+    /// tag.
+    bytes32 constant RAINDEX_DEPLOYED_CODEHASH_0_1_10 =
+        0x670aa891cb8fcb7166ecf8031238f6287ead1ac018d6353af04dfd720b146e75;
+
     /// The address of the `RaindexV6SubParser` contract when deployed with
     /// the rain standard zoltu deployer.
     address constant SUB_PARSER_DEPLOYED_ADDRESS = SUB_PARSER_ADDR;
@@ -212,6 +221,15 @@ library LibRaindexDeploy {
     bytes32 constant SUB_PARSER_DEPLOYED_CODEHASH_0_1_9 =
         0x704aadc1ed56f63ff918ab219e6681a5d2851d774e2ee136bbe7904ea3b2fdcd;
 
+    /// The deployed address of the `RaindexV6SubParser` contract at the
+    /// published `0.1.10` tag. (Unchanged from `0.1.2`.)
+    address constant SUB_PARSER_DEPLOYED_ADDRESS_0_1_10 = 0x09Bc7AF266012F44fb41D8Bd682da931666605e1;
+
+    /// The runtime code hash of the `RaindexV6SubParser` contract at the
+    /// published `0.1.10` tag.
+    bytes32 constant SUB_PARSER_DEPLOYED_CODEHASH_0_1_10 =
+        0x704aadc1ed56f63ff918ab219e6681a5d2851d774e2ee136bbe7904ea3b2fdcd;
+
     /// The address of the `RouteProcessor4` contract when deployed with the
     /// rain standard zoltu deployer.
     address constant ROUTE_PROCESSOR_DEPLOYED_ADDRESS = ROUTE_PROCESSOR_ADDR;
@@ -301,6 +319,15 @@ library LibRaindexDeploy {
     bytes32 constant ROUTE_PROCESSOR_DEPLOYED_CODEHASH_0_1_9 =
         0xeb3745a79c6ba48e8767b9c355b8e7b79f9d6edeca004e4bb91be4de515a7eeb;
 
+    /// The deployed address of the `RouteProcessor4` contract at the published
+    /// `0.1.10` tag. (Unchanged from `0.1.0`.)
+    address constant ROUTE_PROCESSOR_DEPLOYED_ADDRESS_0_1_10 = 0x6E2d0e71d900474b262E545Bc4C98b71ab368d21;
+
+    /// The runtime code hash of the `RouteProcessor4` contract at the published
+    /// `0.1.10` tag.
+    bytes32 constant ROUTE_PROCESSOR_DEPLOYED_CODEHASH_0_1_10 =
+        0xeb3745a79c6ba48e8767b9c355b8e7b79f9d6edeca004e4bb91be4de515a7eeb;
+
     /// The address of the `GenericPoolRaindexV6ArbOrderTaker` contract when
     /// deployed with the rain standard zoltu deployer.
     address constant GENERIC_POOL_ARB_ORDER_TAKER_DEPLOYED_ADDRESS = GENERIC_POOL_ARB_OT_ADDR;
@@ -393,6 +420,16 @@ library LibRaindexDeploy {
     bytes32 constant GENERIC_POOL_ARB_ORDER_TAKER_DEPLOYED_CODEHASH_0_1_9 =
         0xc4492cb22d918a3f0d41f13e533744ef03a2e7f744ec0a43b79d97a04c537544;
 
+    /// The deployed address of the `GenericPoolRaindexV6ArbOrderTaker` contract
+    /// at the published `0.1.10` tag. (Changed in 0.1.10 — embeds the new raindex
+    /// address.)
+    address constant GENERIC_POOL_ARB_ORDER_TAKER_DEPLOYED_ADDRESS_0_1_10 = 0xe2a825c9C2b52F7c73f30080680DdB90f36ed9c3;
+
+    /// The runtime code hash of the `GenericPoolRaindexV6ArbOrderTaker` contract
+    /// at the published `0.1.10` tag.
+    bytes32 constant GENERIC_POOL_ARB_ORDER_TAKER_DEPLOYED_CODEHASH_0_1_10 =
+        0xd3453fd739378f1ccce8181a1ba3554db33e3fc13f6eeda83d2abc716c45d89b;
+
     /// The address of the `RouteProcessorRaindexV6ArbOrderTaker` contract
     /// when deployed with the rain standard zoltu deployer.
     address constant ROUTE_PROCESSOR_ARB_ORDER_TAKER_DEPLOYED_ADDRESS = RP_ARB_OT_ADDR;
@@ -494,6 +531,17 @@ library LibRaindexDeploy {
     bytes32 constant ROUTE_PROCESSOR_ARB_ORDER_TAKER_DEPLOYED_CODEHASH_0_1_9 =
         0xc3fb3bb0355fee22e54cade57f5ff828478d59f34fdd8057bc0bc6a11b3d48fc;
 
+    /// The deployed address of the `RouteProcessorRaindexV6ArbOrderTaker`
+    /// contract at the published `0.1.10` tag. (Changed in 0.1.10 — embeds the
+    /// new raindex address.)
+    address constant ROUTE_PROCESSOR_ARB_ORDER_TAKER_DEPLOYED_ADDRESS_0_1_10 =
+        0xDd436bd0e92389e1c45E6Af5F0C2D989c626d49e;
+
+    /// The runtime code hash of the `RouteProcessorRaindexV6ArbOrderTaker`
+    /// contract at the published `0.1.10` tag.
+    bytes32 constant ROUTE_PROCESSOR_ARB_ORDER_TAKER_DEPLOYED_CODEHASH_0_1_10 =
+        0xebca76aecd91efeee0840b70028dda00c90b06266c1bea04d5367a58d5e7f751;
+
     /// The address of the `GenericPoolRaindexV6FlashBorrower` contract when
     /// deployed with the rain standard zoltu deployer.
     address constant GENERIC_POOL_FLASH_BORROWER_DEPLOYED_ADDRESS = GENERIC_POOL_FB_ADDR;
@@ -586,6 +634,16 @@ library LibRaindexDeploy {
     bytes32 constant GENERIC_POOL_FLASH_BORROWER_DEPLOYED_CODEHASH_0_1_9 =
         0xed618063fd6ffc7558e4588ea6357356e781d660fa22caa292a2f9aeaa21996f;
 
+    /// The deployed address of the `GenericPoolRaindexV6FlashBorrower` contract
+    /// at the published `0.1.10` tag. (Changed in 0.1.10 — embeds the new raindex
+    /// address.)
+    address constant GENERIC_POOL_FLASH_BORROWER_DEPLOYED_ADDRESS_0_1_10 = 0x94e275d0eAf27c28C4737b91f8CD9D1DD9132019;
+
+    /// The runtime code hash of the `GenericPoolRaindexV6FlashBorrower` contract
+    /// at the published `0.1.10` tag.
+    bytes32 constant GENERIC_POOL_FLASH_BORROWER_DEPLOYED_CODEHASH_0_1_10 =
+        0x3438f85dace78d284e0eb33f8a245750498a37dfde867325a0760cf022e8fdb7;
+
     uint256 constant RAINDEX_START_BLOCK_ARBITRUM = 473030678;
     uint256 constant RAINDEX_START_BLOCK_BASE = 47278140;
     uint256 constant RAINDEX_START_BLOCK_FLARE = 62835540;

test/concrete/raindex/PrecisionAttackMutableDecimalsToken.sol

@@ -0,0 +1,25 @@
+// SPDX-License-Identifier: LicenseRef-DCL-1.0
+// SPDX-FileCopyrightText: Copyright (c) 2020 Rain Open Source Software Ltd
+pragma solidity =0.8.25;
+
+import {ERC20} from "@openzeppelin-contracts-5.6.1/token/ERC20/ERC20.sol";
+
+contract PrecisionAttackMutableDecimalsToken is ERC20 {
+    uint8 public dec;
+
+    constructor(uint8 d) ERC20("Mut", "MUT") {
+        dec = d;
+    }
+
+    function setDecimals(uint8 d) external {
+        dec = d;
+    }
+
+    function decimals() public view override returns (uint8) {
+        return dec;
+    }
+
+    function mint(address to, uint256 amount) external {
+        _mint(to, amount);
+    }
+}

test/concrete/raindex/RaindexV6.deposit.entask.t.sol

@@ -26,12 +26,15 @@ contract RaindexV6DepositEnactTest is RaindexV6ExternalRealTest {
         // ReentrancyGuard.REENTRANCY_GUARD_STORAGE
         bytes32 reentrancyGuardStorage = 0x9b779b17422d0df92223018b32b4d1fa46e071723d6817e2486d003becc55f00;
 
-        assertEq(reads.length, 5);
+        // pullTokens reads sDustCredit (2 SLOADs of the Float slot) and writes it
+        // back (1 SSTORE), adding 2 reads and 1 write over the pre-dust-credit
+        // 5 reads (reentrancy x3 + vault balance x2) / 3 writes.
+        assertEq(reads.length, 7);
         assertEq(reads[0], reentrancyGuardStorage);
         assertEq(reads[1], reentrancyGuardStorage);
         assertEq(reads[reads.length - 1], reentrancyGuardStorage);
 
-        assertEq(writes.length, 3);
+        assertEq(writes.length, 4);
         assertEq(writes[0], reentrancyGuardStorage);
         assertEq(writes[writes.length - 1], reentrancyGuardStorage);
     }

test/concrete/raindex/RaindexV6.deposit.t.sol

@@ -199,11 +199,12 @@ contract RaindexV6DepositTest is RaindexV6ExternalMockTest {
             assertEq(vm.getRecordedLogs().length, 1, "logs");
             // - reentrancy guard x3
             // - vault balance floats x2
-            // - token decimals x2
-            assertTrue(reads.length == 5, "reads");
-            // // - reentrancy guard x2
-            // // - vault balance x1
-            assertTrue(writes.length == 4 || writes.length == 3, "writes");
+            // - dust credit floats x2 (sDustCredit SLOAD in pullTokens)
+            assertTrue(reads.length == 7, "reads");
+            // - reentrancy guard x2
+            // - vault balance x1
+            // - dust credit x1 (sDustCredit SSTORE in pullTokens)
+            assertTrue(writes.length == 4, "writes");
             assertTrue(
                 iRaindex.vaultBalance2(actions[i].depositor, actions[i].token, actions[i].vaultId)
                     .eq(actions[i].amount.add(vaultBalanceBefore)),