0x v2 support #106
0x v2 support #106
Conversation
| /// https://0x.org/docs/introduction/0x-cheat-sheet#0x-contracts | ||
| const DEFAULT_ALLOWANCE_TARGET: &str = "0x0000000000001fF3684f28c67538d4D072C22734"; |
There was a problem hiding this comment.
This address depends on the EVM hardfork at the time of deployment. I think we should probably future proof this and make this an optional config parameter which defaults to this value instead of having this always hardcoded constant here.
There was a problem hiding this comment.
This is dangerous to change without official version update (where api endpoints would be changed as well, similar to this PR), so I don't expect we will need to update this any time soon. That said, I would rather avoid adding configuration just because we might need it sometimes in the future, but probably won't. If we do, why not making it configurable when needed.
| }, | ||
| "issues": { | ||
| "allowance": { | ||
| "spender": "0xdef1c0ded9bec7f1a1670819833240f027b25eff", |
There was a problem hiding this comment.
Is this spender actually correct? I would have expected this to be equal to the default allowance target? Can you set an allowance on either contract?
There was a problem hiding this comment.
I just kept the value from previous test, but for the purpose of the test the correct value is not relevant.
| @@ -19,7 +21,13 @@ pub struct ZeroEx { | |||
| defaults: dto::Query, | |||
| } | |||
|
|
|||
| /// https://0x.org/docs/introduction/0x-cheat-sheet#0x-contracts | |||
| const DEFAULT_ALLOWANCE_TARGET: &str = "0x0000000000001fF3684f28c67538d4D072C22734"; | |||
There was a problem hiding this comment.
An eth::ContractAddress gets constructed each time in the code. Can we use this type instead of &str here?
There was a problem hiding this comment.
Besides latest @MartinquaXD 's comment, code wise looks good to me.
Support for v2 version of 0x API: https://0x.org/docs/api#tag/Swap/operation/swap::allowanceHolder::getQuote
Migration tips: https://0x.org/docs/upgrading/upgrading_to_swap_v2
There is no support for BUY orders so will have to exclude 0x from native price estimators list.
Tested on mainnet staging. Quote solved and verified.