feat: pollFunds for just-in-time order funding by anxolin · Pull Request #125 · cowprotocol/composable-cow

anxolin · 2026-07-10T22:33:51Z

Part of #121. Follows on #124

What

This is the PR that adding the main feature.

Adds the just-in-time funding entry point (pollFunds). This is the function that will be called by the hooks to poll the funds.

The method will do a series of validations:

The schedule was registered,
The polling hasn't been done yet for the current order. For checking this, it will check with the pre-registered handler to generate an order, and from it, derices the orderUid. This is how we can reuse the logic from the handlers to be used for the polling too. For example, in TWAP makes sure we don't pull twice for the same leg.
It also checks the composable cow order has been authorised . This way, cancellations of composable cow orders will also work out of the box.

Topup vs pollFunds

Initially in my first draft I implemented a method to top up the account (adding whatever balance that is missing in order to be able to fullfull the order.sellAmount.

I decided to rename to pollFunds to move the funds unconditionally (don't check the balance. This way, we can reuse the same trader contract for more concurrent orders, and we don't run into race conditions where some order uses the funds of some other.

Test plan

forge test --match-contract ComposableCowPollerTest -vv

Test should pass

igorroncevic · 2026-07-14T11:57:33Z

        if (msg.sender != schedule.funder) revert OnlyFunder();
        id = scheduleId(schedule.funder, schedule.handler, schedule.owner, schedule.salt);
        schedules[id] = schedule;
+        delete lastFunded[id]; // reset funding history


Every successful registration clears lastFunded[id].

A realistic sequence is:

Alice creates a 10-day TWAP and gives the poller approval for 100 USDC.

On day 3, the poller pulls the intended 10 USDC into Alice's CowShed and records that pull.

Alice signs and sends another register transaction with the same ID.

The contract accepts it and runs delete lastFunded[id].

A call to pollFunds transfers another 10 USDC into CowShed, because it doesn't know about the old order anymore.

This is only weird UX, not an issue, but something to be documented, perhaps even reject overriding existing IDs to not disrupt ongoing orders, unless this was done for a reason.

If the user creates a new TWAP schedule without changing any of hte other parameters (salt, owner, funder, and handler), then they are creating a new TWAP schedule, and technically the old TWAP schedule, while still active, would not be able to receive new funds

Since there is a new TWAP schedule, that would mean that this new TWAP schedule would have its own funds pulling schedule, so it seems reasonable to me that

At this point, I think its OK to leave the existing functionality, and instead advise the user that salt shouldn't be an arbitrary value only used to prevent duplication; it should be the hash of the staticInput, with any appData related fields set to 0. This more or less completely prevents the collisions we are concerned about.

kaze-cow

im getting a lot of compilation warnings when I attempt to run forge build here in ComposableCowPoller.sol

I ran coverage tests and happy to confirm the ComposableCowPoller is fully covered

|------------------------------------------------------+------------------+------------------+----------------+-----------------|
| src/types/ComposableCowPoller.sol                    | 100.00% (21/21)  | 100.00% (24/24)  | 100.00% (4/4)  | 100.00% (4/4)   |
|------------------------------------------------------+------------------+------------------+----------------+-----------------|

Finally, as this is the last PR, I will confirm, I wasn't able to find any significant security issue with not including the staticInput in the schedule hashing; worst comes to worst, the tokens are sent from the user's wallet to their CoWShed unexpectedly. the funder has to set the schedule for any funds that can leave their wallet, so it seems safe. I would, however, recommend we augment salt to be renamed something like staticInputNormalizedHash or so as this would mostly prevent any possible issues with TWAP schedules or similar being refreshed, and the poller overwriting instead of creating a new schedule accordingly.

kaze-cow · 2026-07-17T06:40:08Z

+        );
+
+        // The order must still be authorised; `remove` disables the poller.
+        if (!composableCow.singleOrders(schedule.owner, ctx)) revert OrderNotLive();


Suggested change

if (!composableCow.singleOrders(schedule.owner, ctx)) revert OrderNotLive();

require(composableCow.singleOrders(schedule.owner, ctx), OrderNotLive());

technically its also possible that the order may be authorized through merkle hash, though I assume this functionality just hasn't been used so its ok to ignore this.

While I prefer this style of reverts, this is not a standard adopted in any CoW repo IIRC.

kaze-cow · 2026-07-17T06:43:57Z

 contract ComposableCowPoller {
+    using GPv2SafeERC20 for IERC20;
+
+    ComposableCoW public immutable composableCow;


This should be COMPOSABLE_COW due to solidity conventions of immutables

kaze-cow · 2026-07-17T06:51:08Z

+            schedule.handler.getTradeableOrder(schedule.owner, address(this), ctx, schedule.staticInput, bytes(""));
+
+        // Fund each order at most once.
+        bytes32 digest = GPv2Order.hash(order, composableCow.domainSeparator());


its weird here to use the domain separator of composableCow. Why not use the domain hash of the settlement contract instead? It can be included as an additional immutable property. That way, the order hash will match up with the one actually on the settlement contract.

ComposableCoW.domainSeparator() already is the settlement domain separator (see here)

kaze-cow · 2026-07-17T06:55:27Z

    mapping(bytes32 => Schedule) public schedules;

+    /// @dev `id => digest` of the order last funded, so each order is funded at most once.
+    mapping(bytes32 => bytes32) public lastFunded;


with our current pragmatic orders, I don't believe there is any reason why this strategy of only tracking the order last funded would be problematic. However, if a pragmatic order's next order could somehow be controlled or set by a value/field that a solver can manipulate, a single order could be reused/pulled multiple times by switching back and forth between two valid getTradableOrders

Good point, this is something I've also thought about and I've introduced a fix for this with the new mapping:
mapping(bytes32 id => mapping(bytes32 digest => bool isFunded)) funded

what this would allow is to keep track of older legs and if they had been funded, preventing any A -> B -> A (older legs) being replayed.

kaze-cow · 2026-07-17T06:59:49Z

+    /// @dev A single poll moves exactly the current part into the owner.
+    function test_pollFunds_fundsCurrentPart() public {
+        (, bytes32 ctx, bytes32 id) = _setupSchedule();
+        vm.warp(_t0(ctx));
+
+        assertEq(
+            token0.balanceOf(address(safe1)),
+            0,
+            "owner empty before pull"
+        );
+
+        poller.pollFunds(id);
+
+        assertEq(
+            token0.balanceOf(address(safe1)),
+            PART,
+            "owner funded with exactly one part"
+        );
+        assertEq(
+            token0.balanceOf(funder),
+            PART * N - PART,
+            "exactly one part left the funder"
+        );
+    }


this test is superfluous with the following test_pollFunds_movesFullAmountUnconditionally

kaze-cow · 2026-07-17T07:01:51Z

+    /// @notice Move the current order's `sellAmount` from the funder to the owner. Permissionless.
+    ///         The full amount always moves (no balance check), so one owner can serve several
+    ///         concurrent orders.
+    function pollFunds(bytes32 id) external {


this function could return false or true depending upon whether funds were ultimately transferred or not. This would mostly make the tests a bit more clear and offer a bit more surface in the off chance that a smart contract wants to use this contract and know if funds were transferred or not without relying on balance checks.

This is useful only if we have callers that need the result. I’d keep the current interface until that's the case.

kaze-cow · 2026-07-17T07:02:55Z

+        vm.expectRevert(bytes("GPv2: failed transferFrom"));
+        poller.pollFunds(id);
+
+        assertEq(poller.lastFunded(id), bytes32(0), "failed pull is not recorded");


technically this check is superfluous if the transaction is already known to have reverted

kaze-cow · 2026-07-17T07:05:16Z

+    ///      cannot be pulled until time advances into its window — even after the part settles and
+    ///      drains the owner. Without the per-order guard, the drained owner would be refilled
+    ///      immediately and that balance would be sold as the next part, a full interval early.
+    function test_pollFunds_cannotFundFuturePartEarly() public {


this test is superfluous with test_pollFunds_idempotentWithinPart. If you want to drive this part home, then would be better to test this in test_pollFunds_fundsEachPartAcrossSchedule by calling pollFunds twice

kaze-cow · 2026-07-17T07:10:50Z

        if (msg.sender != schedule.funder) revert OnlyFunder();
        id = scheduleId(schedule.funder, schedule.handler, schedule.owner, schedule.salt);
        schedules[id] = schedule;
+        delete lastFunded[id]; // reset funding history


If the user creates a new TWAP schedule without changing any of hte other parameters (salt, owner, funder, and handler), then they are creating a new TWAP schedule, and technically the old TWAP schedule, while still active, would not be able to receive new funds

Since there is a new TWAP schedule, that would mean that this new TWAP schedule would have its own funds pulling schedule, so it seems reasonable to me that

At this point, I think its OK to leave the existing functionality, and instead advise the user that salt shouldn't be an arbitrary value only used to prevent duplication; it should be the hash of the staticInput, with any appData related fields set to 0. This more or less completely prevents the collisions we are concerned about.

Track every funded digest across schedule updates, document salt construction, and remove redundant funding tests.

This was referenced Jul 10, 2026

Just in time polling of funds #121

Open

feat: Just in time polling of funds (PoC) #116

Closed

anxolin marked this pull request as draft July 10, 2026 22:37

anxolin force-pushed the feat/poller-register branch from 8fdcbe1 to 4e308b0 Compare July 10, 2026 23:12

anxolin force-pushed the feat/poller-topup branch from 3538e4f to 7a6dc89 Compare July 10, 2026 23:12

anxolin changed the title ~~feat: topUp for just-in-time order funding (3/3)~~ feat: topUp for just-in-time order funding Jul 10, 2026

anxolin force-pushed the feat/poller-topup branch from 7a6dc89 to 71c5297 Compare July 10, 2026 23:40

anxolin changed the title ~~feat: topUp for just-in-time order funding~~ feat: pollFunds for just-in-time order funding Jul 10, 2026

anxolin force-pushed the feat/poller-topup branch from 71c5297 to c4f5a50 Compare July 10, 2026 23:44

anxolin mentioned this pull request Jul 10, 2026

chore: deploy ComposableCowPoller #126

Open

anxolin marked this pull request as ready for review July 10, 2026 23:50

anxolin force-pushed the feat/poller-topup branch 2 times, most recently from adb0365 to 70ba19a Compare July 10, 2026 23:55

anxolin force-pushed the feat/poller-register branch from 4e308b0 to 6966303 Compare July 11, 2026 00:42

anxolin force-pushed the feat/poller-topup branch from 70ba19a to c71ed55 Compare July 11, 2026 00:42

anxolin force-pushed the feat/poller-register branch from 6966303 to 04c1bde Compare July 11, 2026 01:44

anxolin force-pushed the feat/poller-topup branch from c71ed55 to f1988b4 Compare July 11, 2026 01:44

anxolin force-pushed the feat/poller-register branch from 04c1bde to bf350d1 Compare July 13, 2026 09:17

anxolin force-pushed the feat/poller-topup branch from f1988b4 to d07ce51 Compare July 13, 2026 09:17

igorroncevic reviewed Jul 14, 2026

View reviewed changes

kaze-cow requested changes Jul 17, 2026

View reviewed changes

igorroncevic force-pushed the feat/poller-register branch from 21fe323 to 45755b8 Compare July 17, 2026 17:17

igorroncevic added a commit that referenced this pull request Jul 17, 2026

fix: harden poller replay protection (#125)

8da1bc9

Track every funded digest across schedule updates, document salt construction, and remove redundant funding tests.

igorroncevic force-pushed the feat/poller-topup branch from 0359eff to 8da1bc9 Compare July 17, 2026 17:17

igorroncevic requested a review from kaze-cow July 17, 2026 17:43

anxolin and others added 3 commits July 23, 2026 19:29

feat: pollFunds for just-in-time order funding

7da790c

fix: add safe transfer

81501ae

fix: harden poller replay protection (#125)

3fe9374

Track every funded digest across schedule updates, document salt construction, and remove redundant funding tests.

igorroncevic force-pushed the feat/poller-topup branch from 8da1bc9 to 3fe9374 Compare July 23, 2026 16:30

igorroncevic changed the base branch from feat/poller-register to feat/poller-revoke July 23, 2026 16:30

igorroncevic mentioned this pull request Jul 23, 2026

feat(poller): authorize funder CowShed #128

Draft

	if (!composableCow.singleOrders(schedule.owner, ctx)) revert OrderNotLive();
	require(composableCow.singleOrders(schedule.owner, ctx), OrderNotLive());

Uh oh!

Conversation

anxolin commented Jul 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

Topup vs pollFunds

Test plan

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kaze-cow left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

anxolin commented Jul 10, 2026 •

edited

Loading