Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
How to test
Unit tests