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

Fetch: Winning Settlement from Orderbook API #220

Merged
merged 21 commits into from
Apr 6, 2023

Conversation

bh2smith
Copy link
Contributor

@bh2smith bh2smith commented Mar 27, 2023

Part of #177

A standalone component of the internalized transfer workflow, we fetch the WinningSettlement Data from the Orderbook API. This PR adds a model for the relevant data, a fetching method and tests for all possible cases. Remains in Draft State because we may want to use txReceipt instead of txHash as function input.

Furthermore, there are some TODOs regarding whether we should attempt to fetch from the Barn Orderbook when Prod not available. It turns out that there are quite large barn batches and we probably shouldn't ignore them when it comes to accounting. I just don't like how lopsided all our requests are.

bh2smith and others added 13 commits March 27, 2023 15:30
Introducing transaction log parsing via ethers library with appropriate Contract artifacts. Namely, we introduce dependencies on @cowprotocol/contracts which contain the abi files for GPv2Settlement and iERC20 and suffice for us to decode relevant event data from settlement transactions. For us these are Transfer, Settlement and Trade.

We construct Event types out of each which only keep the parts that matter for our purposes (namely TradeEvent having owner as the only relevant field).
@bh2smith bh2smith requested a review from a team March 27, 2023 22:51
@socket-security
Copy link

socket-security bot commented Mar 27, 2023

New dependency changes detected. Learn more about Socket for GitHub ↗︎


👍 No new dependency issues detected in pull request

Bot Commands

To ignore an alert, reply with a comment starting with @SocketSecurity ignore followed by a space separated list of package-name@version specifiers. e.g. @SocketSecurity ignore [email protected] bar@* or ignore all packages with @SocketSecurity ignore-all

Pull request alert summary
Issue Status
Install scripts ✅ 0 issues
Native code ✅ 0 issues
Bin script shell injection ✅ 0 issues
Unresolved require ✅ 0 issues
Invalid package.json ✅ 0 issues
HTTP dependency ✅ 0 issues
Git dependency ✅ 0 issues
Potential typo squat ✅ 0 issues
Known Malware ✅ 0 issues
Telemetry ✅ 0 issues
Protestware/Troll package ✅ 0 issues

📊 Modified Dependency Overview:

➕ Added Package Capability Access +/- Transitive Count Publisher
[email protected] network, filesystem, environment +8 jasonsaayman
⬆️ Updated Package Version Diff Added Capability Access +/- Transitive Count Publisher
[email protected] 6.2.2...6.2.3 None +0/-0 ricmoo

@bh2smith bh2smith mentioned this pull request Mar 27, 2023
40 tasks
@bh2smith bh2smith force-pushed the fetch/winning-settlement branch from ffc32bf to c8d7eaf Compare March 28, 2023 11:45
Base automatically changed from action-to-db to slippage-v2 April 3, 2023 19:34
Comment on lines +20 to +21
// TODO - Note that old records have solver = ZeroAddress!
// We need to rectify this (use txReceipt or whatever Tenderly web-action provides)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We will have to get this from the tx.from field of receipt.

@bh2smith bh2smith changed the title Fetch: Winning settlement from Orderbook API Fetch: Winning Settlement from Orderbook API Apr 3, 2023
@bh2smith bh2smith marked this pull request as ready for review April 3, 2023 19:55
Copy link
Contributor

@gentrexha gentrexha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left a minor comment. Other than that, LGTM.

import axios from "axios";

// https://api.cow.fi/docs/#/
const PROD_API_URL =
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a comment, although I guess you are already aware, there are rate limits for the API. Not sure if that can potentially create issues.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would hope that the rate limit is not more than how frequently settlements are happening.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I would hope so as well, so maybe we don't need to worry about this after all.

Copy link
Contributor Author

@bh2smith bh2smith Apr 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally rate limits are more on par with numbers like N per second, so I think we're ok here. However you might have issues with the EBBO thing if you're gonna wake up and make a bunch of requests every 30 minutes.

@harisang
Copy link
Contributor

harisang commented Apr 6, 2023

Putting it here as a more general thought. With solver-driver collocation (which will probably happen in the next 2-3 months max), I believe we might not have anything related to the winning solution other than what actually happened onchain. Which means that looking at "uninternalized" calldata will be impossible, unless we explicitly enforce that the winner commits to submitting both versions of the data to the protocol. So I believe the backend team should be involved in this discussion.

@bh2smith
Copy link
Contributor Author

bh2smith commented Apr 6, 2023

Great point @harisang -- then perhaps we should get the ball rolling by tagging someone like, maybe, @sunce86 or @MartinquaXD (who has been working on this).

@bh2smith bh2smith merged commit c3d83cd into slippage-v2 Apr 6, 2023
@bh2smith bh2smith deleted the fetch/winning-settlement branch April 6, 2023 08:57
@github-actions github-actions bot locked and limited conversation to collaborators Apr 6, 2023
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.

4 participants