Update SolverEngine API docs #2192
Conversation
| @@ -67,16 +67,6 @@ impl Solutions { | |||
| buy_token_balance: BuyTokenBalance::Erc20, | |||
| signing_scheme, | |||
| signature, | |||
| pre_interactions: trade | |||
There was a problem hiding this comment.
Remove pre-interactions for jit orders (for some reason we didn't populate post interactions...
I don't remember if it was a conscious decision to not populate post interactions for JIT orders but they are not strictly necessary, right?. The solver is the one that builds and uses a JIT order. With that information they should be able to execute anything that would make sense as a post-interaction as part of the regular interactions in the middle.
This is not the case for pre-interactions, though.
There was a problem hiding this comment.
Sure, but I don't think this nuanced reasoning justifies this inconsistency (technically a self-hosted driver could also execute whatever call they want before settle).
Practically, no one is using this order type (and for sure not with pre-interactions), so I still think it's in the spirit of #simplification to remove it.
There was a problem hiding this comment.
technically a self-hosted driver could also execute whatever call they want before settle
How would they ensure that their interaction happens in the same block as their settlement?
Wouldn't they need tight block builder integration for that?
I'm fine with removing it. Just wanted to point possible reasons for why it is like it is.
There was a problem hiding this comment.
Right, it's not trivial (sending bundles can work) or using a Safe or the like as the msg.sender for the settle call and atomically do other calls (in a multicall)
crates/solvers/openapi.yml
Outdated
| "sellToken" for sell orders, and "buyToken" for buy orders. | ||
| allOf: | ||
| - $ref: "#/components/schemas/TokenAmount" |
There was a problem hiding this comment.
The executedAmount also appears to be outdated.
It's actually part of JitTrade and Fulfillment and should probably not be listed on the Trade. Also it's not a TokenAmount but a simple U256.
There was a problem hiding this comment.
Since JitTrade and Fulfillment are flattened this actually checks out (cf the example responses)
Will still put it on each type. TokenAmount is just a placeholder for U256 though.
|
Leaving a comment here not to forget to port the changes into https://github.com/cowprotocol/solvers |
😩 ugh, I don't know why we actually have the API documentation in the solvers crate and not in the driver (who kind of defines it). Also, the better long term solution is probably to switch to something like utoipa to have this automatically generated. |
Ok, I can probably integrate it into the |
Description
Docs were slightly out of date
Changes
Liquidity)How to test
Past into https://editor.swagger.io/ and compare line by line with the rust structs
Related Issues
Fixes #2182