fix(DCA): dca allocation bug (#360)

ConjunctiveNormalForm · alanhwu · codyborn · web-flow · commit 687d9e122082 · 2026-01-23T16:08:03.000-05:00
* fix exclusivity edge-case * add tests * full test snapshot * add exclusivityEndBlock to cosigner data * forge fmt * fuzz testing exclusivity * fix(DCA): allocation bug (#358) * dca allocation fix * new test * unuseful comment * remove recipients check * forge fmt * view -> pure * test: exactIn sweep rounding remainder * forge fmt * revert: undo last two commits * docs: comments for clarity around fix code --------- Co-authored-by: Alan Wu <alanwu100@gmail.com> * ci: integrate Nethermind Audit Agent for automated security scanning (#357) * ci: integrate Nethermind Audit Agent for automated security scanning Add GitHub Actions workflow to automatically scan pull requests using Nethermind's Audit Agent. The workflow triggers on PRs to main and performs quick security scans on v4 contract files, providing early vulnerability detection in the development process. Resolves: PROTO-1020 Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * Update .github/workflows/audit-agent.yml Co-authored-by: semgrep-code-uniswap[bot] <212759065+semgrep-code-uniswap[bot]@users.noreply.github.com> * Update .github/workflows/audit-agent.yml Co-authored-by: semgrep-code-uniswap[bot] <212759065+semgrep-code-uniswap[bot]@users.noreply.github.com> --------- Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com> Co-authored-by: semgrep-code-uniswap[bot] <212759065+semgrep-code-uniswap[bot]@users.noreply.github.com> * ci: integrate Nethermind Audit Agent for automated security scanning (#357) * ci: integrate Nethermind Audit Agent for automated security scanning Add GitHub Actions workflow to automatically scan pull requests using Nethermind's Audit Agent. The workflow triggers on PRs to main and performs quick security scans on v4 contract files, providing early vulnerability detection in the development process. Resolves: PROTO-1020 Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * Update .github/workflows/audit-agent.yml Co-authored-by: semgrep-code-uniswap[bot] <212759065+semgrep-code-uniswap[bot]@users.noreply.github.com> * Update .github/workflows/audit-agent.yml Co-authored-by: semgrep-code-uniswap[bot] <212759065+semgrep-code-uniswap[bot]@users.noreply.github.com> --------- Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com> Co-authored-by: semgrep-code-uniswap[bot] <212759065+semgrep-code-uniswap[bot]@users.noreply.github.com> * fix(v4 reactor): exclusivity edge case (#356) * fix exclusivity edge-case * add tests * full test snapshot * add exclusivityEndBlock to cosigner data * forge fmt * fuzz testing exclusivity --------- Co-authored-by: Alan Wu <alanwu100@gmail.com> Co-authored-by: Cody Born <cody.born@uniswap.org> Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com> Co-authored-by: semgrep-code-uniswap[bot] <212759065+semgrep-code-uniswap[bot]@users.noreply.github.com>
diff --git a/src/v4/hooks/dca/DCAHook.sol b/src/v4/hooks/dca/DCAHook.sol
@@ -393,32 +393,57 @@ contract DCAHook is IPreExecutionHook, IDCAHook {
         DCAOrderCosignerData memory cosignerData,
         OutputToken[] memory outputs
     ) internal pure {
-        // Aggregate outputs per recipient and compute totalOutput
+        // Aggregate outputs and compute totalOutput
         uint256 totalOutput = 0;
-        // Use a temporary in-memory structure to tally by recipient (no memory mapping in Solidity):
-        // Approach: loop once to total output; for each allocation, loop outputs to sum matching recipient.
         uint256 outputsLength = outputs.length;
         for (uint256 i = 0; i < outputsLength; i++) {
             // token already checked equals intent.outputToken in _beforeTokenTransfer
             totalOutput += outputs[i].amount;
         }
 
         uint256 allocationsLength = intent.outputAllocations.length;
+        uint256[] memory expected = new uint256[](allocationsLength);
+        uint256 sumExpected = 0;
+
+        // Select a deterministic recipient to receive any rounding remainder for EXACT_OUT.
+        // We choose the allocation with the highest bps; ties pick the first max due to strict `>`.
+        uint256 maxBps = 0;
+        uint256 maxBpsIndex = 0;
+
+        for (uint256 i = 0; i < allocationsLength; i++) {
+            uint256 bps = uint256(intent.outputAllocations[i].basisPoints);
+            if (bps > maxBps) {
+                maxBps = bps;
+                maxBpsIndex = i;
+            }
+            // Floor(totalOutput * bps / BPS). Sum of floors may be < totalOutput.
+            expected[i] = Math.mulDiv(totalOutput, bps, BPS);
+            sumExpected += expected[i];
+        }
+
+        if (!intent.isExactIn) {
+            // EXACT_OUT requires expected[] to sum exactly to totalOutput; otherwise allocation checks can be unfillable.
+            uint256 remainder = totalOutput - sumExpected;
+            if (remainder > 0) {
+                expected[maxBpsIndex] += remainder;
+            }
+        }
+
         for (uint256 i = 0; i < allocationsLength; i++) {
             address rcpt = intent.outputAllocations[i].recipient;
-            uint256 expected = Math.mulDiv(totalOutput, uint256(intent.outputAllocations[i].basisPoints), BPS);
             uint256 actual = 0;
             for (uint256 j = 0; j < outputsLength; j++) {
                 if (outputs[j].recipient == rcpt) actual += outputs[j].amount;
             }
             if (intent.isExactIn) {
                 // Allow ±1 wei for integer division rounding
-                if (!(actual + 1 >= expected && actual <= expected + 1)) {
-                    revert AllocationMismatch(rcpt, actual, expected);
+                uint256 exp = expected[i];
+                if (!(actual + 1 >= exp && actual <= exp + 1)) {
+                    revert AllocationMismatch(rcpt, actual, exp);
                 }
             } else {
-                if (actual != expected) {
-                    revert AllocationMismatch(rcpt, actual, expected);
+                if (actual != expected[i]) {
+                    revert AllocationMismatch(rcpt, actual, expected[i]);
                 }
             }
         }
diff --git a/src/v4/lib/ExclusivityLib.sol b/src/v4/lib/ExclusivityLib.sol
@@ -48,7 +48,7 @@ library ExclusivityLib {
         uint256 exclusivityOverrideBps,
         uint256 blockNumberish,
         address filler
-    ) internal view {
+    ) internal pure {
         _handleExclusiveOverride(order, exclusive, exclusivityEnd, exclusivityOverrideBps, blockNumberish, filler);
     }
 
@@ -66,7 +66,7 @@ library ExclusivityLib {
         uint256 exclusivityOverrideBps,
         uint256 currentPosition,
         address filler
-    ) internal view {
+    ) internal pure {
         // if the filler has fill right, we proceed with the order as-is
         if (hasFillingRights(exclusive, exclusivityEnd, currentPosition, filler)) {
             return;
@@ -99,7 +99,7 @@ library ExclusivityLib {
     /// @dev if the order has active exclusivity and the current filler is not the exclusive address, returns false
     function hasFillingRights(address exclusive, uint256 exclusivityEnd, uint256 currentPosition, address filler)
         internal
-        view
+        pure
         returns (bool)
     {
         return exclusive == address(0) || currentPosition > exclusivityEnd || exclusive == filler;
diff --git a/test/v4/hooks/dca/DCAHookHarness.sol b/test/v4/hooks/dca/DCAHookHarness.sol
@@ -12,6 +12,7 @@ import {
     PermitData
 } from "../../../../src/v4/hooks/dca/DCAStructs.sol";
 import {ResolvedOrder} from "../../../../src/v4/base/ReactorStructs.sol";
+import {OutputToken} from "../../../../src/base/ReactorStructs.sol";
 import {IPermit2} from "permit2/src/interfaces/IPermit2.sol";
 import {IReactor} from "../../../../src/v4/interfaces/IReactor.sol";
 
@@ -67,6 +68,15 @@ contract DCAHookHarness is DCAHook {
         _validateChunkSize(intent, cosignerData, inputAmount);
     }
 
+    /// @notice Exposes the internal _validateOutputDistribution function for testing
+    function validateOutputDistribution(
+        DCAIntent memory intent,
+        DCAOrderCosignerData memory cosignerData,
+        OutputToken[] memory outputs
+    ) external pure {
+        _validateOutputDistribution(intent, cosignerData, outputs);
+    }
+
     /// @notice Helper to create a basic DCA intent for testing
     function createTestIntent(address swapper, uint96 nonce, bool isExactIn, uint256 minChunk, uint256 maxChunk)
         external
diff --git a/test/v4/hooks/dca/DCAHook_validateOutputDistribution.t.sol b/test/v4/hooks/dca/DCAHook_validateOutputDistribution.t.sol
@@ -0,0 +1,89 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+pragma solidity ^0.8.0;
+
+import {Test} from "forge-std/Test.sol";
+import {IPermit2} from "permit2/src/interfaces/IPermit2.sol";
+import {DeployPermit2} from "../../../util/DeployPermit2.sol";
+import {DCAHookHarness} from "./DCAHookHarness.sol";
+import {IReactor} from "../../../../src/v4/interfaces/IReactor.sol";
+import {
+    DCAIntent,
+    DCAOrderCosignerData,
+    OutputAllocation,
+    PrivateIntent,
+    FeedInfo
+} from "../../../../src/v4/hooks/dca/DCAStructs.sol";
+import {OutputToken} from "../../../../src/base/ReactorStructs.sol";
+import {IDCAHook} from "../../../../src/v4/interfaces/IDCAHook.sol";
+
+contract DCAHook_validateOutputDistributionTest is Test, DeployPermit2 {
+    DCAHookHarness hook;
+    IPermit2 permit2;
+    address constant REACTOR_ADDRESS = address(0x2345);
+    IReactor constant REACTOR = IReactor(REACTOR_ADDRESS);
+
+    address constant SWAPPER = address(0x1234);
+    uint96 constant NONCE = 42;
+    address constant COSIGNER = address(0x5678);
+    address constant RECIPIENT_A = address(0xAAAA);
+    address constant RECIPIENT_B = address(0xBBBB);
+
+    function setUp() public {
+        permit2 = IPermit2(deployPermit2());
+        hook = new DCAHookHarness(permit2, REACTOR);
+    }
+
+    function _createExactOutIntent() internal view returns (DCAIntent memory) {
+        OutputAllocation[] memory allocations = new OutputAllocation[](2);
+        allocations[0] = OutputAllocation({recipient: RECIPIENT_A, basisPoints: 5000});
+        allocations[1] = OutputAllocation({recipient: RECIPIENT_B, basisPoints: 5000});
+
+        PrivateIntent memory privateIntent = PrivateIntent({
+            totalAmount: 0, exactFrequency: 0, numChunks: 0, salt: bytes32(0), oracleFeeds: new FeedInfo[](0)
+        });
+
+        return DCAIntent({
+            swapper: SWAPPER,
+            nonce: NONCE,
+            chainId: block.chainid,
+            hookAddress: address(hook),
+            isExactIn: false,
+            inputToken: address(0x1111),
+            outputToken: address(0x2222),
+            cosigner: COSIGNER,
+            minPeriod: 0,
+            maxPeriod: 0,
+            minChunkSize: 1,
+            maxChunkSize: 10_000,
+            minPrice: 0,
+            deadline: block.timestamp + 1 days,
+            outputAllocations: allocations,
+            privateIntent: privateIntent
+        });
+    }
+
+    function test_validateOutputDistribution_exactOut_remainderAssignedToFirstMaxBpsRecipient() public {
+        DCAIntent memory intent = _createExactOutIntent();
+        DCAOrderCosignerData memory cosignerData = hook.createTestCosignerData(SWAPPER, NONCE, 101, 0, 0);
+
+        // 50/50 split with odd total: remainder should go to first max-bps recipient (RECIPIENT_A).
+        OutputToken[] memory outputs = new OutputToken[](2);
+        outputs[0] = OutputToken({token: intent.outputToken, amount: 51, recipient: RECIPIENT_A});
+        outputs[1] = OutputToken({token: intent.outputToken, amount: 50, recipient: RECIPIENT_B});
+
+        hook.validateOutputDistribution(intent, cosignerData, outputs);
+    }
+
+    function test_validateOutputDistribution_exactOut_remainderOnOtherRecipient_reverts() public {
+        DCAIntent memory intent = _createExactOutIntent();
+        DCAOrderCosignerData memory cosignerData = hook.createTestCosignerData(SWAPPER, NONCE, 101, 0, 0);
+
+        // Remainder incorrectly assigned to RECIPIENT_B should revert.
+        OutputToken[] memory outputs = new OutputToken[](2);
+        outputs[0] = OutputToken({token: intent.outputToken, amount: 50, recipient: RECIPIENT_A});
+        outputs[1] = OutputToken({token: intent.outputToken, amount: 51, recipient: RECIPIENT_B});
+
+        vm.expectRevert(abi.encodeWithSelector(IDCAHook.AllocationMismatch.selector, RECIPIENT_A, 50, 51));
+        hook.validateOutputDistribution(intent, cosignerData, outputs);
+    }
+}
