Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sane Defaults For Quote Parameters #249

Merged
merged 6 commits into from
Jun 8, 2022
Merged

Conversation

nlordell
Copy link
Contributor

@nlordell nlordell commented Jun 2, 2022

This PR makes more fields on quote parameters optional - making it so API clients can more easily quote:

  • In situations where they know even less about the order.
  • Populates some values like validTo with "sane" defaults.
  • Adds a validFor alternative so you can specify a duration for validity without having to compute Unix timestamps locally.

The hope is to reduce friction with Orderbook API integrations (you just need from, sell and buy tokens and amounts and trading side now).

The OpenAPI spec was updated accordingly.

Examples

Some examples of quotes that work now:

Minimum required parameters:

% curl -s \
  -X POST \
  -H 'Content-Type: application/json' \
  --data '@-' \
  'http://localhost:8080/api/v1/quote' \
  <<JSON | jq
{
  "from": "0x0001020304050607080910111213141516171819",
  "sellToken": "0xc778417e063141139fce010982780140aa0cd5ab",
  "buyToken": "0xa7D1C04fAF998F9161fC9F800a99A809b84cfc9D",
  "buyAmountAfterFee": "1000000000000000000",
  "kind": "buy"
}
JSON

{
  "quote": {
    "sellToken": "0xc778417e063141139fce010982780140aa0cd5ab",
    "buyToken": "0xa7d1c04faf998f9161fc9f800a99a809b84cfc9d",
    "receiver": null,
    "sellAmount": "900656973206412",
    "buyAmount": "1000000000000000000",
    "validTo": 1654236095,
    "appData": "0x0000000000000000000000000000000000000000000000000000000000000000",
    "feeAmount": "275521308410505",
    "kind": "buy",
    "partiallyFillable": false,
    "sellTokenBalance": "erc20",
    "buyTokenBalance": "erc20"
  },
  "from": "0x0001020304050607080910111213141516171819",
  "expiration": "2022-06-03T05:32:35.769737Z",
  "id": 0
}

Quoting with validity of 100 minutes (= 60*100 = 6000 seconds) instead of absolute value:

% curl -s \
  -X POST \
  -H 'Content-Type: application/json' \
  --data '@-' \
  'http://localhost:8080/api/v1/quote' \
  <<JSON | jq
{
  "from": "0x0001020304050607080910111213141516171819",
  "sellToken": "0xc778417e063141139fce010982780140aa0cd5ab",
  "buyToken": "0xa7D1C04fAF998F9161fC9F800a99A809b84cfc9D",
  "sellAmountAfterFee": "1000000000000000000",
  "validFor": 6000,
  "kind": "sell"
}
JSON

{
  "quote": {
    "sellToken": "0xc778417e063141139fce010982780140aa0cd5ab",
    "buyToken": "0xa7d1c04faf998f9161fc9f800a99a809b84cfc9d",
    "receiver": null,
    "sellAmount": "1000000000000000000",
    "buyAmount": "1107197235731981668071",
    "validTo": 1654240383,
    "appData": "0x0000000000000000000000000000000000000000000000000000000000000000",
    "feeAmount": "275521308190088",
    "kind": "sell",
    "partiallyFillable": false,
    "sellTokenBalance": "erc20",
    "buyTokenBalance": "erc20"
  },
  "from": "0x0001020304050607080910111213141516171819",
  "expiration": "2022-06-03T05:34:03.518281Z",
  "id": 1
}

Test Plan

CI to test new logic and adjust existing unit tests.

@nlordell nlordell requested a review from a team as a code owner June 2, 2022 16:15
@codecov-commenter
Copy link

codecov-commenter commented Jun 2, 2022

Codecov Report

Merging #249 (373e320) into main (79cba9c) will increase coverage by 0.06%.
The diff coverage is 96.15%.

❗ Current head 373e320 differs from pull request most recent head 2236c48. Consider uploading reports for the commit 2236c48 to get more accurate results

@@            Coverage Diff             @@
##             main     #249      +/-   ##
==========================================
+ Coverage   64.27%   64.34%   +0.06%     
==========================================
  Files         190      191       +1     
  Lines       39463    39684     +221     
==========================================
+ Hits        25364    25533     +169     
- Misses      14099    14151      +52     

@nlordell nlordell enabled auto-merge (squash) June 8, 2022 10:37
@nlordell nlordell merged commit 29cee54 into main Jun 8, 2022
@nlordell nlordell deleted the sane-defaults-for-quotes branch June 8, 2022 10:40
@github-actions github-actions bot locked and limited conversation to collaborators Jun 8, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants