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

332/quote id on metadata #750

Merged

Conversation

alfetopito
Copy link
Collaborator

@alfetopito alfetopito commented Jun 27, 2022

Summary

Part of #332

Passing down quoteId received from quote call, if any

Note: There will be a follow up PR removing quoteId and sell|buyAmount from metadata in favor of slippageBips (see cowprotocol/cow-sdk#41)

To Test

  1. Fill in trade data (or do nothing, as it'll pre-fill with WETH-USDC pair)
  2. Observe the console logs. Filter by appData
  • It should contain id inside the quote object
    Screen Shot 2022-06-27 at 16 47 08
  1. Write down the quote id
    Screen Shot 2022-06-29 at 11 19 20
  2. Switch to the network tab
  3. Place an order and open it on the browser https://barn.api.cow.fi/mainnet/api/v1/orders/<orderId>
  • Check the appDataHash in the order matches the last console debug log
  1. On the network tab, check for the POST request where the order was placed. Check the request sub tab
  • It should contain the field quoteId matching the id noted on step 3
    Screen Shot 2022-06-29 at 11 19 35
  1. Back at the console tab, filter the console log for UploadToIpfs
  • Check the appData related to your order has been uploaded, like the image

Screen Shot 2022-06-08 at 15 10 57

  1. Search for the appDataHash using https://codepen.io/pacara/pen/BadZgjg?editors=0101
  • Check it exists and the fields match the last console debug log

@alfetopito alfetopito self-assigned this Jun 27, 2022
@alfetopito alfetopito requested review from a team June 27, 2022 15:50
@github-actions
Copy link
Contributor

CLA Assistant Lite All Contributors have signed the CLA.

@alfetopito alfetopito force-pushed the 332/generate-appDataHash-for-every-order branch from 42ecff3 to e625664 Compare June 29, 2022 11:04
@alfetopito alfetopito force-pushed the 332/quote-id-on-metadata branch from ecf57ba to 5ee8305 Compare June 29, 2022 11:06
@alfetopito alfetopito requested review from a team and anxolin June 29, 2022 11:38
@github-actions
Copy link
Contributor

  • 🔭 GP Swap: CoW Protocol v2 Swap UI

@elena-zh
Copy link

elena-zh commented Jun 29, 2022

Hey @alfetopito ,
I have noticed that sometimes appdata logs do not contain ID. It happens when a token pair is selected for the 1st time, and 2 logs appear: 1st without ID, the 2nd contains it. . Related to all networks. Is it OK?
image
no quote

Then, I have found a request in the Netowkr tab with the same ID, but the appData does not match there. See:
appdatahash
uploaded
app data network
image

It is in Rinkeby. I the screenshots you provided above the appdata matches. The issue is also related to GC and Ethereum networks

@elena-zh
Copy link

I have also noticed that there is no 'Fees exceed from amount' message in your PR
image

@alfetopito
Copy link
Collaborator Author

I have also noticed that there is no 'Fees exceed from amount' message in your PR image

I'll address that in this PR #748, still not fixed

@alfetopito
Copy link
Collaborator Author

Hey @alfetopito , I have noticed that sometimes appdata logs do not contain ID. It happens when a token pair is selected for the 1st time, and 2 logs appear: 1st without ID, the 2nd contains it. . Related to all networks. Is it OK? image no quote

Then, I have found a request in the Netowkr tab with the same ID, but the appData does not match there. See: appdatahash uploaded app data network image

It is in Rinkeby. I the screenshots you provided above the appdata matches. The issue is also related to GC and Ethereum networks

Regarding the quoteId issues, 2 comments:

The next PR is removing it from the metadata, so I'd ask you to ignore the issues here as it won't be there anymore.
Maybe it makes more sense to skip the test here and verify the issues solved or not there.

@alfetopito alfetopito merged commit 26d1c6b into 332/generate-appDataHash-for-every-order Jun 30, 2022
@alfetopito alfetopito deleted the 332/quote-id-on-metadata branch June 30, 2022 10:04
@github-actions github-actions bot locked and limited conversation to collaborators Jun 30, 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.

3 participants