Grant Proposal – Milkman

Grant Proposal – Milkman Development & Audit

Author: @charlesndalton, contributor at Yearn Finance


About you

Contributor to Yearn Finance and member of the Yearn strategy team. At charlesndalton on GitHub and Twitter.

Additional links

  • Milkman HackMD
  • Forum isn’t allowing more than two links, but both the milkman repo and the milkman-bot repo are pinned on my GitHub

Grant Category

Developer tools (SDK)

Grant Description

Summary:
This is a proposal to fund a smart contract system which would allow smart contracts to route their order flow through the CoW Protocol.

Problem being solved:

At Yearn, we need to do a lot of token swaps. An increasing number of those token swaps are being routed through the CoW Protocol. However, we have encountered a frequent issue when swapping through a Gnosis Safe.

Although it is technically possible for smart contracts to sign order UIDs via the setPresignature function on the settlement contract, these UIDs still need to be generated off-chain. At first, we would first generate the order UIDs via the API and then pass them into multisig requests, but often the request’s minOut would go out of range by the time the multisig transaction was executed (e.g., because the price of the token we’re selling went down), meaning that we would need to repeat this process (generate another request, and submit another multisig transaction).

Milkman:

Obviously, this is non-optimal. As a result, we’ve had a few discussions with the CoW team over what the best solution is. We needed something that provided the following properties:

  • Easy-to-use
  • Trustless:
    • Doesn’t add any new trust assumptions (e.g., trusted keeper)
    • Doesn’t weaken any existing trust assumptions (e.g., setting minOut to 0, which places more trust in solvers and Flashbots RPC)

The solution is called Milkman, and it sits as a layer on top of CoW protocol. A technical description of the design & implementation is in the HackMD and codebase.

I’ve been working on Milkman since July, and have high conviction that it would be a useful public good for the CoW ecosystem. Some testing and QA has been done (you can check out 0x9d763Cca6A8551283478CeC44071d72Ec3FD58Cb and 0x2aa7ff04460cddc61a2b466c9a2924603863a030 for some iterations that we tested with production Yearn swaps), but I would like to see Milkman as a production-grade smart contract system that can be utilized by DAOs to securely route their order flow through CoW protocol. For this, we need (1) an audit of the core contract, (2) a good set of ‘price checkers’, peripheral contracts needed for reducing trust in solvers and Flashbots, and (3) a complete testing suite, which tests all edge cases and many types of tokens. If approved, this grant would fund these three areas.

Grant Goals and impact:

The goal is to build a system that allows DAOs, gnosis safes, and other contracts to easily route their order flow through CoW Protocol.

Again, the problem that Milkman solves isn’t theoretical: it’s one that we encountered. You can imagine that Milkman will open up use-cases which weren’t previously possible. For example, you can imagine borrow/lend platforms using CoW to sell their collateral instead of either (1) slow collateral auctions or (2) first-come-first-serve liquidations, where a large chunk of value is extracted away from users in the form of MEV. More concretely, Felix mentioned in TG that Nouns DAO had expressed interest in using Milkman for DAO-authored swaps.

Milestones

  • Completed Milkman contract, which allows users to 1. request swaps and 2. cancel swaps that haven’t been picked up
    • Audit by yAcademy
  • Completed price checkers
    • UniV2, which includes Uniswap V2 and Sushiswap
    • Curve
    • Uniswap V3
    • Balancer
    • Where it makes sense, meta-price checkers that combine multiple lower-level price checkers (e.g., for ANGLE → DAI, ANGLE → ETH on Sushiswap and ETH → DAI on Uniswap V2)
  • Useful peripherals, such as a contract that allows off-chain services to query for the state of swaps
  • Off-chain bot, written in Rust, that pairs orders against Milkman swap requests and handles other scenarios (e.g., order doesn’t go through first try)
  • Documentation and code examples that help developers submit orders through Milkman
  • Documentation for keepers to run the bot

For all of the above, this includes unit and integration testing.

Grant timeline

The biggest dependency in the timeline is getting the audit. Assuming that yAcademy can complete the audit by the end of September, all of the above milestones should be completed by mid-October.

Funding request

$45,000, half paid up-front and half paid once the code and testing is up to the standards of member(s) of the CoW team (maybe @nlordell?).

Budget breakdown

  • $16k (33% of total): quoted cost of 7-day-long yAcademy audit
  • $32k (66% of total): development / labor cost

Any cost overruns of the audit would be paid out of the development cost, so it’s a $45k flat fee w/ no strings attached.

Gnosis Chain Address (to receive the grant):

If okay, I’d prefer not putting this on a public forum, but would be happy to share with any members of the CoW team

Referral:

This project has been the culmination of discussions between myself, Nicholas Lordello, Felix Leupold, and Poolpi Tako. I’m not sure if this counts as a referral :man_shrugging:

Terms and conditions:

By applying for this grant, I agree to be bound by the CowDAO Participation Agreement and the COWDAO Grant Terms and Conditions.

Many thanks for reviewing this proposal :pray:.

3 Likes

I like this idea and commend you on building this solution.

Question: The funding request is predicated on the assumption that the audit will pass without medium/low severity exploits. If the audit does uncover something that requires remediation, how will the re-audit cost impact the grant request? Or will that come out of the dev/labor cost?

3 Likes

Thanks @charlesndalton!
Appreciate the time and effort that already went into thinking, developing and testing this solution.
Admittedly I do not fully understand the technical solution and for that I would like to get @fleupold and @nlordell thoughts.

Generally, making it easier for people or smart contracts to interact and submit orders to CoW Protocol is of the highest priority. Fully permissionless and MEV protected on-chain trade execution is something that indeed I believe many protocols and DAOs would like to leverage.

I would suggest to add into the proposal items for writing full documentation and code examples for Milkman so new developers can onboard easily.

3 Likes

:pray:

Yep, any cost overruns would come out of dev/labor cost

1 Like

Yep, 100%. Added this to the milestones

1 Like

Hey @charlesndalton, thanks for the proposal and all the work that already went into this! In fact I spoke with a contributor (@davidbrai) from Noun DAO and they might be interested in using such a system if it was production ready.

As for the scope, the CoW Swap development team could potentially also provide an audit of the codebase from G0 if we wanted to reduce the size of the grant.

Regarding the proposed design, I see a small issue with regards to how the fee is provided (basically up to the Keeper). If the keeper decided to set a 0 fee the order would likely be ignored by the system (or only matched as part of CoWs). Admittedly - other than griefing - this is not really a big attack vector since the keeper needs to pay every 50 blocks for this type of censoring.

We are working on allowing orders with 0 fee to be placed (fee will be taken from the surplus), but that is still 2-3 months out. With the native ETH flow coming online in ~4 weeks, we should be able to index the smart contract order even from an onchain event (rather than relying on the keeper submitting it to the API), but here again the correct fee amount remains a challenge if I’m not mistaken.

I wonder if the same style of order would be better implemented using EIP1271 where isValidSignature(uid, raw_order_data) is used to verify that the limit price of the raw order hashes to uid and the limit price is in line with some price checker? One difference I see is that isValidSignature is a view function so it wouldn’t allow to do any accounting via state updates (e.g. for replay protection & the like). EIP1271 is already available in staging and close to being shipped into production.

To summarize, I think having this type of infrastructure available for DAOs is hugely important for the protocol. My only concern is that we might have a “better way” of achieving this given the features we are currently developing. Would love to also hear thoughts from @nlordell and @Cowry

2 Likes

:+1:

Yeah, this is one of those things that’s the responsibility of the price checker. The core contract passes in the fee + the non-fee amount (src), so the price checker can run arbitrary logic to ensure that the fee is in line as well. One implementation I’ve been toying with is using Chainlink’s “fast gas” oracle so that it can calculate a reasonable fee and then reject orders with an unreasonable fee (in this case, too small, but would also help for too big).

There’s a trade-off here though between gas & reducing this grief, so IMO price checkers with & without this feature should exist; users can pick whether they want the feature.

Another option is having fee as a separate param that’s passed into the price checker. So far, it hasn’t seemed worth it, but maybe it is :man_shrugging:

Yep, as you mention, the problem here is replay attacks. It was a while ago, but we discussed this possibility:

As Nicholas pointed out, we could also use EIP1271 with proxy contracts, with 1 proxy per trade. This could simplify some things (e.g., you don’t need to worry about swap state). The main motivation behind using settlement.preSignature over EIP1271 is gas efficiency.

Comparing EIP1271 w/ EIP1167 minimal proxies vs current design:

  • EIP1167 minimal proxy contract is 45 bytes, so ~40k gas, and there’s overhead cost associated with it (700 gas per call IIRC).
  • Current design does SSTORE in settlement.setPreSignature(UID, true), but we can get this refunded if we want by running settlement.setPreSignature(UID, false) inside the proveExecuted() function (src)
  • Current design needs nonce counter, which adds 5k gas per transaction (1 SLOAD + 1 hot SSTORE)

So at present time, it looks like the non-EIP1271 route makes more sense. But I may be wrong and am happy to change my opinion based on new information / previously unconsidered viewpoints!

A question for @fleupold or @nlordell:

I see that the input to your EIP1271 implementation is GPv2Trade.Data.signature, of type bytes. It looks like this data is originally coming from a solver’s input to GPv2Settlement.settle(). Is there any way to pass arbitrary order data into signature?

If so, we’d be able to cut out the process of keepers submitting on-chain transactions, although they still may need to generate off-chain orders so that the system is aware. In this case, I’d be for Milkman doing EIP1271, since it would cut down the complexity for end-users.

Is there any way to pass arbitrary order data into signature?

Yes, signature bytes are arbitrary data that is specified at order creation time. Conceivably, the keepers could update off-chain orders with updated bytes instead of transactions. One concern is still around replay protection (i.e. the mechanism for invaliding the previous order is off-chain and more trust-based, since the previous signature bytes would still be valid).

Yes, signature bytes are arbitrary data that is specified at order creation time. Conceivably, the keepers could update off-chain orders with updated bytes instead of transactions.

Awesome!

One concern is still around replay protection (i.e. the mechanism for invaliding the previous order is off-chain and more trust-based, since the previous signature bytes would still be valid).

Yep, I think we would need to use proxy contracts with 1 order per proxy if we went down this route. This creates a trade-off, since proxies would assumedly consume more gas. I’m going to try a parallel implementation on a different branch & will report back the gas difference between the two implementations.

One other idea I had - I noticed that the priceChecker is a “trusted” entity (since its allowed to update order prices).

If you wanted to not deploy proxy contracts, you could also have signature validation be contingent on a signed priceChecker message, and trust your priceChecker won’t submit signatures when it shouldn’t. Specifically, you could design an:

function isValidSignature(bytes32 hash, bytes memory signature) view returns (bytes4) {
    (bytes32 orderHash, uint256 validFrom, bytes32 r, bytes32 s, uint32 v) = abi.decode(signature);
    require(orderHash == hash);
    require(block.number >= validFrom && block.number < validFrom + 50);
    require(ecrecover(orderHash, r, s, v) == priceChecker);
    return ERC1271_MAGIC_BYTES;
}

This way, you would run your off-chain price checker and have it submit updated prices every 50 blocks. This way updating prices is gasless, and this should be replay-protection-free since the order hash would be the same, and the CoW protocol contract would ensure that it doesn’t get replayed.

From an API side, this would require a ERC-1271 signature updating support. I’d have to think a bit more about whether or not there are any issues with having something like that, but implementing that from a technical perspective is not that hard.

Also, I just double checked the gas overhead price for deploying a minimal proxy contract:

% % curl -s -X POST $NODE_URL -H "Content-Type: application/json" --data '{"jsonrpc":"2.0","id":0,"method":"eth_estimateGas","params":[{"data":"0x3d602d80600a3d3981f3363d3d373d3d3d363d73eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee5af43d82803e903d91602b57fd5bf3"},"latest"]}' | jq -r '.result'
0xf5bf

So the overhead of a minimal proxy contract is ~62k gas.

1 Like

So I think it’s important to make a distinction between two things. On one hand, you have the keepers, which are off-chain bots that send API requests to CoW and submit transactions to Ethereum. On the other, you have price checkers, which are on-chain and permisionless smart contracts which run logic to ensure that whatever the keeper supplied as a _minOut is reasonable.

A very dumb example price checker is this one, which checks that _minOut is at least 90% of what you’d get from selling to Sushiswap.

Could the additional amount be coming from the 21k fixed transaction cost?

:man_facepalming: of course!

Ah OK. I confused the two.

So keepers use on-chain price checkers to “prove” that the price they are updating to is “valid” when pairing swaps.

Right, exactly. The user specifies a price checker & Milkman only signs the order if their specified price checker allows it

@nlordell @fleupold

I implemented a partial EIP-1271-compliant solution using proxy contracts here. The gas numbers are as following.

  • For the original implementation, the user-initiated swap request is expected to cost on average 119k gas. The EIP-1271 version has a cost of 169k gas – an increase of 42%.
  • The EIP-1271 version should make this back because it doesn’t have the setPreSignature SSTORE (although this could theoretically be refunded) nor the the fixed 21k from the extra keeper transaction.

Based on those numbers, it seems like an EIP-1271-based Milkman may be better than what I had originally planned. Do you agree?

If so, I’m happy to implement this solution once we get the grant sorted out

Looks like it would work! One difference is that (as long as the priceChecker call executes without reverting) a signature will stay valid for the duration of the Milkman order (i.e there is no unpairSwap equivalent). I don’t see any issues with this (since this will just end up having multiple valid orders open where only one can execute) but it is a small difference.

Also, unless I’m missing something, I think there is an SSTORE (0 -> nonzero) optimization that can be made:

    function initialize(IERC20 _fromToken, bytes32 _swapHash) external {
        require(!isOriginal); // dev: should only be called on order contracts
        require(!isInitialized); // dev: cannot re-initialize an order contract

        isInitialized = true;

        _fromToken.approve(gnosisVaultRelayer, type(uint256).max);

        swapHash = _swapHash;
    }

To

function initialize(IERC20 _fromToken, bytes32 _swapHash) external {
        require(!isOriginal); // dev: should only be called on order contracts
        require(swapHash == bytes32(0)); // dev: cannot re-initialize an order contract

        swapHash = _swapHash;

        _fromToken.approve(gnosisVaultRelayer, type(uint256).max);
    }

Otherwise, it looks like it would work!

Nice one! Just tested this, and it brings it down to 148k gas