cowprotocol/composable-cow

🐮 🧾 Composable Conditional Orders for CoW Protocol

composable-cow's Issues

chore: Add github templates

Problem

We have templates and we're not using them.

Solution

Add the templates

docs: move order specific docs

Problem

Documentation at a root level should be general in nature, and not necessarily address concrete order types.

Solution

Move concrete order type documentation to be side-by-side with their implementation. Expose documentation via the new docs structure in technical reference.

feat(twap): user-specified `appData`

Problem

Currently when a user creates a TWAP order, the appData is fixed as keccak256("twap"), not allowing for tracking of order flow potentially for referrals / analysis.

Solution

Allow for specifying the appData in the static input of a conditional order.

research: milkman price checkers

Problem

It may be desired for a user to avoid all trust assumptions, including the effectiveness of the batch auction mechanism, protecting their funds from malicious colluding solvers. Milkman price checkers enable this functionality somewhat (they are not general in nature - they have the assumption of sell order kinds).

So that all conditional orders can benefit, it would be possible to implement price checkers at the leaf-level, such that when writing conditional orders, a price checker can be used but this does not need to be considered by the implementation of the conditional order.

feat: watch-tower and sdk friendly extended interface

Problem

It's frustrating when wanting to implement a watch tower for conditional orders as the current interfaces do not provide any method of communicating their intent to the watch tower (such as try again at another block, another epoch etc).

Suggested solution

Revise the interface of custom errors to include parameterized custom errors capable of communicating target epochs, block numbers etc at which the watch tower should attempt to get a discrete order again - or even delete the conditional order from monitoring!

Alternatives considered

N/A

Additional context

This is the result of much iteration on the Tenderly Web3 actions watch-tower and limitations that have been observed in production.

Acceptance criteria

• Interface for "try at epoch"
• Interface for "try at block number"
• Communicate error messages (strings) from smart contracts accurately to the watch tower

bug: StopLoss doesn't check oracle data

Problem

The StopLoss order type doesn't validate the chainlink data.

Solution

1. Normalise the quote prices to ERC20 token decimals.
2. Check for stale data.
3. Make sure the data is sane (prices should be positive!)

Lack of input validation for number of twaps on CLI create-twap command

In create-twap command, n argument represents number of total swaps. Since they can never be less than 2, CLI should validate user input and throw error if its less than 2

Right now, CLI proposes an order without checking this

feat: interface function to hint when order in book

Problem

It became frustrating with our experience when optimising the tenderly-watch-tower that there may be times when the conditional order type wants to provide custom logic to the watch tower for how it should respond when an order is already in the book. For example, if the order is already in the book, in the event of a TWAP, it's best to wait until the next part.

Suggested solution

Given that this requires off-chain input (ie. the mere fact that the order is in the book), the suggested solution is to specify a function such as handle(address owner, address sender, bytes32 ctx, bytes calldata staticInput, bytes calldata offChainInput, uint8 offChainState). offChainState would be an enum and allow for extensibility if there become other cases that we would like to handle.

Alternatives considered

An alternative to this is implementing specific logic in the cow-sdk for each custom order type. Ideally we rule this out so that cow-sdk logic can try to be as order-agnostic as possible, with the specific instructions coming as a "source of truth" from conditional order contract.

Additional context

N/A

Acceptance criteria

• Interface is defined for handling abstract off-chain state.

research: time dependent trades

Problem

Multisigs take time to co-ordinate their actions which may result in hard-coded time constraints not being met if the multisig has difficulty co-ordinating signing.

Therefore, when updating the merkle root for the Safe, there could also be an associated timestamp written to state that would allow time relative order types as opposed to time absolute.

research: how to incentivise smart orders for correct polling

Problem

If a smart order author doesn't care about the expense incurred by the watch tower for maintaining their order, they can just ask the watch tower to poll every block.

Acceptance criteria

Clear guidance / mechanisms are available to incentivise smart order authors to correctly hint for their polling.

Possible Solutions

• Curated registries?

chore: use ERC165 support to detect ExtensibleFallbackHandler / SignatureVerifierMuxer

Problem

Safe recently removed NAME and VERSION from the CompatibilityFallbackHandler. As it's quite arbitrary the use of this with determining the signature formatting for getTradeableOrderWithSignature in ComposableCoW, this should be modified to a more idiomatic implementation.

Solution

Remove NAME and VERSION, and implement an IERC165 check on a safe's ExtensibleFallbackHandler.

Provide on-chain helper function to compute `H(params)`

In order to cancel a Composable CoW order one has to compute H(params) and pass this into the remove method. Whereas everything else can be fairly easily done using the Safe UI (and potentially a tool to encode calldata), computing this hash is non trivial and could therefore benefit from a helper view function that makes this computable via the Etherscan UI.

bug: GoodAfterTime order type sets receiver to address(0)

Problem

GoodAfterTime order type should have receiver in Data to align with other order types and flexibility..

Solution

Insert receiver into the GoodAfterTime.Data struct, allowing for maximum use cases.

chore: move actions to separate repo

Problem

The watch tower / Web3 actions are becoming a bit of a behemoth themselves. From a security perspective, the repository housing smart contracts should be compartmentalised for risk management purposes, and therefore having the web3 actions / CLI within the repository is antithetical to this.

Solution

Move the Web3 Actions and CLI to a separate repository.

Blocking

This is dependent on #42 as ABI from this repo will be pulled into the Web3 Actions/ CLI repo for consumption.

feat: provide watch-tower hints on order creation

Problem

When the watch-tower is indexing single orders, it will immediately attempt to poll on the same block at which the order was created. This may not be the desired effect.

Suggested solution

In the create and createWithContext functions within ComposableCoW, in ConditionalOrderCreated, emit the polling hints.

Alternatives considered

Rely on adapted interfaces in #73 to supply the block hints. Effectively the addition of on-creation hints will save one standard block polling cycle for the conditional order.

Additional context

This is only valid for single orders. As setRoot provides a root of many orders, these orders may have different logic, and therefore it's not practicable, or gas efficient to emit events for all orders within indicating when they should be polled.

Acceptance criteria

• ConditionalOrderCreated event updated to also emit watch tower hints for single orders.

feat: deployed abi json

Problem

Downstream consumers of deployed contracts need ABI artifacts.

Solution

Commit the deployed ABI JSON inside an out folder, that may be retrieved relative to the commit hash by downstream.

chore: Interface with errors supported by the watchtower

Background

The watchtower has a wide range of custom errors that can be used to tell it if and when an order should be recreated (see here). They are also documented here.
The only error that's available on an interface is OrderNotValid.

Details

It's helpful for developers to be able to refer to these errors without having to search for them and include them in their handler code.

Acceptance criteria

Either of:

• new interface with all custom errors accepted by the watchtower, or
• include the errors in IConditionalOrderGenerator.

chore: publish abi and deployment info

Problem
Downstream users of the contracts include:

1. Frontend: JavaScript
2. Backend: Rust, other smart contracts

Solution
Create a minimal NPM package, similar to that found here.

feat: parameterised errors

Problem
In the course of the ackee audit, some project consistency items have been higlighted. Amongst these are the use of error IConditionalOrder.OrderNotValid() for all conditions when the order isn't valid.

As orders may have complex / not easily rationalised logic, this creates onerous demand on the developer when debugging.

Solution

Change OrderNotValid to take a bytes4 parameter. This bytes4 parameter is itself an error selector native to the conditional order type that has been implemented.

chore: enable build size checking

Background

It's frustrating that the build sizes with the Vault are too large and require careful configuration for the CI/CD tests.

Details

To avoid this issue, it is desirable to abstract / refactor to remove the need to deploy the Vault in test artifacts.

Acceptance criteria

• CI/CD able to run with forge build --sizes