chore: more tests, added natspec

Author: igorroncevic

src/types/ComposableCowPoller.sol

@@ -1,10 +1,12 @@
 // SPDX-License-Identifier: GPL-3.0
 pragma solidity >=0.8.0 <0.9.0;
 
-import {IConditionalOrderGenerator} from "../interfaces/IConditionalOrder.sol";
+import {IConditionalOrderGenerator} from "src/interfaces/IConditionalOrder.sol";
 
 /// @title ComposableCowPoller - Just-in-time funding for composable conditional orders.
 contract ComposableCowPoller {
+    /// @notice Parameters for a JIT funding schedule.
+    /// @dev A schedule is uniquely identified by its funder, handler, owner, and salt.
     struct Schedule {
         IConditionalOrderGenerator handler; // the conditional-order handler to poll (e.g. the TWAP type)
         address funder; // source of funds; the only registrant
@@ -17,11 +19,23 @@ contract ComposableCowPoller {
     ///      order's `appData`.
     mapping(bytes32 => Schedule) public schedules;
 
+    /// @notice Thrown when someone other than the schedule funder registers or updates a schedule.
     error OnlyFunder();
 
+    /// @notice Emitted when a schedule is registered or updated.
+    /// @param id The deterministic key of the schedule.
+    /// @param owner The conditional-order owner and pull destination.
+    /// @param funder The token source that registered the schedule.
     event ScheduleRegistered(bytes32 indexed id, address indexed owner, address indexed funder);
 
-    /// @notice Deterministic, appData-independent schedule key.
+    /// @notice Computes the deterministic, appData-independent schedule key.
+    /// @dev `staticInput` is excluded because its appData can depend on this key.
+    ///      Use a different salt for concurrent schedules with the same funder, handler, and owner.
+    /// @param funder The token source.
+    /// @param handler The conditional-order generator.
+    /// @param owner The conditional-order owner.
+    /// @param salt The conditional-order salt.
+    /// @return The schedule key.
     function scheduleId(address funder, IConditionalOrderGenerator handler, address owner, bytes32 salt)
         public
         pure
@@ -30,8 +44,11 @@ contract ComposableCowPoller {
         return keccak256(abi.encode(funder, handler, owner, salt));
     }
 
-    /// @notice Register or update a schedule. Only the funds source may register, and the `id` is
-    ///         namespaced by `funder`.
+    /// @notice Registers or updates a schedule.
+    /// @dev Registering the same funder, handler, owner, and salt replaces the stored schedule.
+    ///      Only the funds source may register, and the ID is namespaced by the funder.
+    /// @param schedule The schedule to store.
+    /// @return id The deterministic key of the stored schedule.
     function register(Schedule calldata schedule) external returns (bytes32 id) {
         if (msg.sender != schedule.funder) revert OnlyFunder();
         id = scheduleId(schedule.funder, schedule.handler, schedule.owner, schedule.salt);

test/ComposableCowPoller.t.sol

@@ -1,13 +1,13 @@
 // SPDX-License-Identifier: GPL-3.0
 pragma solidity >=0.8.0 <0.9.0;
 
-import {IConditionalOrder, IValueFactory, BaseComposableCoWTest} from "./ComposableCoW.base.t.sol";
+import {IConditionalOrder, IValueFactory, BaseComposableCoWTest} from "test/ComposableCoW.base.t.sol";
 
-import {TWAP} from "../src/types/twap/TWAP.sol";
-import {TWAPOrder} from "../src/types/twap/libraries/TWAPOrder.sol";
-import {ComposableCowPoller} from "../src/types/ComposableCowPoller.sol";
-import {CurrentBlockTimestampFactory} from "../src/value_factories/CurrentBlockTimestampFactory.sol";
-import {IConditionalOrderGenerator} from "../src/interfaces/IConditionalOrder.sol";
+import {TWAP} from "src/types/twap/TWAP.sol";
+import {TWAPOrder} from "src/types/twap/libraries/TWAPOrder.sol";
+import {ComposableCowPoller} from "src/types/ComposableCowPoller.sol";
+import {CurrentBlockTimestampFactory} from "src/value_factories/CurrentBlockTimestampFactory.sol";
+import {IConditionalOrderGenerator} from "src/interfaces/IConditionalOrder.sol";
 
 /// @title ComposableCowPoller unit tests
 /// @notice Exercises registering a schedule for a composable TWAP created via `createWithContext`.
@@ -17,6 +17,7 @@ contract ComposableCowPollerTest is BaseComposableCoWTest {
     uint256 constant N = 3;
     uint256 constant FREQ = 1 hours;
     bytes32 constant SALT = keccak256("twap");
+    bytes32 constant SECOND_SALT = keccak256("second twap");
 
     ComposableCowPoller poller;
     IValueFactory currentBlockTimestampFactory;
@@ -36,19 +37,18 @@ contract ComposableCowPollerTest is BaseComposableCoWTest {
     }
 
     function _bundle() internal view returns (TWAPOrder.Data memory) {
-        return
-            TWAPOrder.Data({
-                sellToken: token0,
-                buyToken: token1,
-                receiver: address(0), // the owner itself
-                partSellAmount: PART,
-                minPartLimit: LIMIT,
-                t0: 0, // resolved from the cabinet via createWithContext
-                n: N,
-                t: FREQ,
-                span: 0, // valid for the whole epoch
-                appData: keccak256("dca.pull")
-            });
+        return TWAPOrder.Data({
+            sellToken: token0,
+            buyToken: token1,
+            receiver: address(0), // protocol shorthand for the Safe owner
+            partSellAmount: PART,
+            minPartLimit: LIMIT,
+            t0: 0, // resolved from the cabinet via createWithContext
+            n: N,
+            t: FREQ,
+            span: 0, // each part is valid for its full FREQ interval
+            appData: keccak256("dca.pull")
+        });
     }
 
     /// @dev Creates a JIT-funded TWAP: order via context, the funder funds + approves the poller,
@@ -57,20 +57,10 @@ contract ComposableCowPollerTest is BaseComposableCoWTest {
     ///      poller schedule key `id` (used for topUp/revoke).
     function _setupSchedule()
         internal
-        returns (
-            IConditionalOrder.ConditionalOrderParams memory params,
-            bytes32 ctx,
-            bytes32 id
-        )
+        returns (IConditionalOrder.ConditionalOrderParams memory params, bytes32 ctx, bytes32 id)
     {
         params = super.createOrder(twap, SALT, abi.encode(_bundle()));
-        _createWithContext(
-            address(safe1),
-            params,
-            currentBlockTimestampFactory,
-            bytes(""),
-            false
-        );
+        _createWithContext(address(safe1), params, currentBlockTimestampFactory, bytes(""), false);
         ctx = composableCow.hash(params);
 
         // Capital lives in the funder (the EOA), which approves the poller for the full notional.
@@ -82,39 +72,71 @@ contract ComposableCowPollerTest is BaseComposableCoWTest {
         // the funds source, the destination, the order's `salt` (so the poller can rebuild `ctx`
         // on-chain) and its `staticInput`. The key is appData-independent so the funding hook can
         // live in the order's own appData.
+        id = _register(SALT, abi.encode(_bundle()));
+
+        assertEq(
+            id,
+            poller.scheduleId(funder, IConditionalOrderGenerator(address(twap)), address(safe1), SALT),
+            "id matches the off-chain derivation"
+        );
+    }
+
+    function _register(bytes32 salt, bytes memory staticInput) internal returns (bytes32 id) {
         vm.prank(funder);
         id = poller.register(
             ComposableCowPoller.Schedule({
                 handler: IConditionalOrderGenerator(address(twap)),
                 funder: funder,
                 owner: address(safe1),
-                salt: SALT,
-                staticInput: abi.encode(_bundle())
+                salt: salt,
+                staticInput: staticInput
             })
         );
-
-        assertEq(
-            id,
-            poller.scheduleId(
-                funder,
-                IConditionalOrderGenerator(address(twap)),
-                address(safe1),
-                SALT
-            ),
-            "id matches the off-chain derivation"
-        );
     }
 
     /// @dev A registered schedule is stored under its appData-independent id.
     function test_register_storesSchedule() public {
-        (, , bytes32 id) = _setupSchedule();
-
-        (IConditionalOrderGenerator handler, address scheduleFunder, address owner, bytes32 salt,) =
-            poller.schedules(id);
+        (,, bytes32 id) = _setupSchedule();
+
+        (
+            IConditionalOrderGenerator handler,
+            address scheduleFunder,
+            address owner,
+            bytes32 salt,
+            bytes memory staticInput
+        ) = poller.schedules(id);
         assertEq(address(handler), address(twap), "handler stored");
         assertEq(scheduleFunder, funder, "funder stored");
         assertEq(owner, address(safe1), "owner stored");
         assertEq(salt, SALT, "salt stored");
+        assertEq(staticInput, abi.encode(_bundle()), "static input stored");
+    }
+
+    /// @dev Distinct salts allow concurrent schedules with the same funder, handler, and owner.
+    function test_register_storesSchedulesWithDifferentSalts() public {
+        bytes memory staticInput = abi.encode(_bundle());
+        bytes32 firstId = _register(SALT, staticInput);
+        bytes32 secondId = _register(SECOND_SALT, staticInput);
+
+        assertTrue(firstId != secondId, "different salts create different ids");
+
+        (,,, bytes32 firstSalt,) = poller.schedules(firstId);
+        (,,, bytes32 secondSalt,) = poller.schedules(secondId);
+        assertEq(firstSalt, SALT, "first schedule remains stored");
+        assertEq(secondSalt, SECOND_SALT, "second schedule stored");
+    }
+
+    /// @dev Registering the same key updates the one schedule stored under that id.
+    function test_register_replacesScheduleWithSameId() public {
+        bytes32 id = _register(SALT, abi.encode(_bundle()));
+        TWAPOrder.Data memory replacement = _bundle();
+        replacement.appData = keccak256("updated dca pull");
+        bytes memory updatedStaticInput = abi.encode(replacement);
+
+        assertEq(_register(SALT, updatedStaticInput), id, "same key keeps same id");
+
+        (,,,, bytes memory staticInput) = poller.schedules(id);
+        assertEq(staticInput, updatedStaticInput, "replacement input stored");
     }
 
     /// @dev Only the funds source may register a schedule that draws on its own funds.