Skip to content

Conversation

@fleupold
Copy link
Contributor

@fleupold fleupold commented Jan 5, 2024

Description

This PR introduces a conditional limit order type. It's not so much conditional, rather partially unconditionally, namely it doesn't care about what appDataHash the suggested trade has set. Instead it only defines some other key parameter of the order (tokens, amounts, validity, receiver) to provide basically the same security guarantees as regular CoW orders.

The advantage of not committing to an app data hash up front, is that it allows the order placer to specify arbitrary pre and post hooks on behalf of the user.
This can be useful if for instance the pre-hook depends on the address of the safe, which may depend the app data used (e.g. if the safe is only being created as part of the pre hook and wants to create the order as part of the initializer call).
In such a case we cannot know the pre-hook before knowing the address of the safe, but need would have to commit to a concrete app data before knowing the address of the Safe.

This order type breaks this circular dependency by removing the app data from the order parameters that need to be committed to up front and instead allowing any app data to be valid (as long as the other important parameters are respected).

Security wise this shouldn't be of huge concern as hooks aren't executed in a privileged context. The only griefing vector I see would be expensive pre-hook invocations benefiting the actor that placed the order on behalf of the safe. This would result in high fees, but since fees are part of the limit order check already, I don't think this is an issue in practice (any other party could place an order without such interactions which should be preferred by solvers as it generates more surplus).

Changes

  • Add Order type
  • Add test coverage for new order type

How to test

Unit tests

@github-actions
Copy link

github-actions bot commented Jan 5, 2024

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

@fleupold
Copy link
Contributor Author

fleupold commented Jan 5, 2024

I have read the CLA Document and I hereby sign the CLA

@mfw78 mfw78 requested a review from a team January 12, 2024 18:48
github-actions bot added a commit that referenced this pull request Jan 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants