-
Notifications
You must be signed in to change notification settings - Fork 3
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
priliminary OEV Network docs #12
Conversation
Visit the preview URL for this PR (updated for commit 04d6ca8): https://oev-docs--pr12-new-oev-docs-0y2wddya.web.app (expires Wed, 10 Jul 2024 04:23:48 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: 6915b094b5ba83fde754632ba50c1ee9406d433f |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In general 👍 LGTM - gj.
I read all of the content and commented on everything that stand out to me.
docs/reference/oev-network/searchers/understanding-auctioneer.md
Outdated
Show resolved
Hide resolved
docs/reference/oev-network/searchers/understanding-auctioneer.md
Outdated
Show resolved
Hide resolved
Co-authored-by: Emanuel Tesař <[email protected]>
Co-authored-by: Emanuel Tesař <[email protected]>
Co-authored-by: Emanuel Tesař <[email protected]>
Co-authored-by: Emanuel Tesař <[email protected]>
Co-authored-by: Emanuel Tesař <[email protected]>
Co-authored-by: Emanuel Tesař <[email protected]>
Co-authored-by: Emanuel Tesař <[email protected]>
Co-authored-by: Emanuel Tesař <[email protected]>
Co-authored-by: Emanuel Tesař <[email protected]>
Co-authored-by: Emanuel Tesař <[email protected]>
Co-authored-by: Emanuel Tesař <[email protected]>
Co-authored-by: Emanuel Tesař <[email protected]>
Co-authored-by: Emanuel Tesař <[email protected]>
Co-authored-by: Emanuel Tesař <[email protected]>
Co-authored-by: Emanuel Tesař <[email protected]>
Co-authored-by: Emanuel Tesař <[email protected]>
Co-authored-by: Emanuel Tesař <[email protected]>
Co-authored-by: Emanuel Tesař <[email protected]>
Co-authored-by: Emanuel Tesař <[email protected]>
Co-authored-by: Emanuel Tesař <[email protected]>
Co-authored-by: Emanuel Tesař <[email protected]>
Co-authored-by: Emanuel Tesař <[email protected]>
Co-authored-by: Emanuel Tesař <[email protected]>
Co-authored-by: Emanuel Tesař <[email protected]>
Co-authored-by: Emanuel Tesař <[email protected]>
docs/reference/oev-network/searchers/understanding-auctioneer.md
Outdated
Show resolved
Hide resolved
docs/reference/oev-network/searchers/understanding-auctioneer.md
Outdated
Show resolved
Hide resolved
docs/reference/oev-network/searchers/understanding-auctioneer.md
Outdated
Show resolved
Hide resolved
docs/reference/oev-network/searchers/understanding-auctioneer.md
Outdated
Show resolved
Hide resolved
docs/reference/oev-network/searchers/understanding-auctioneer.md
Outdated
Show resolved
Hide resolved
docs/reference/oev-network/searchers/understanding-auctioneer.md
Outdated
Show resolved
Hide resolved
I made some suggestions, looking good! |
Another note: |
I've noticed that there's no specific section on reporting fulfilments (see https://oev-docs--pr12-new-oev-docs-0y2wddya.web.app/reference/oev-network/searchers/submit-bids.html#performing-the-oracle-update-using-the-awarded-bid ). There are sections for placing a bid, executing the award, etc. |
Co-authored-by: Aaron Scheiner <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I used ChatGPT (v4o) to review and edit two of the pages - specifically the "OEV Network" and "Auction Cycle" pages. It's much more useful (and probably accurate) than me trying to point out any of the small syntax/grammar/spelling errors. I think the results are also quite a bit more readable. Let me know what you think and if I should keep going
const api3Contracts = require('@api3/contracts'); | ||
|
||
const oevNetworkProvider = new JsonRpcProvider(OEV_NETWORK_RPC_URL); | ||
const OevAuctionHouseArtifact = await hre.artifacts.readArtifact( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be nice if this could be copy-pasted. It currently depends on hre
, which isn't obvious to readers where it comes from. It also depends on the OevAuctionHouse
contract being present somewhere and an undefined OEV_NETWORK_RPC_URL
variable
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Ashar2shahid Can you resolve all the PR suggestions that are done? They clutter the PR.
Some generic feedback:
- When you go to https://oev-docs--pr12-new-oev-docs-0y2wddya.web.app/ there is no mention of OEV network. There should be a button for that and potentially hide other buttons (such as Airnode, etc...).
docs/reference/oev-network/overview/assets/oev-auction-sequence.png
Outdated
Show resolved
Hide resolved
docs/reference/oev-network/searchers/understanding-auctioneer.md
Outdated
Show resolved
Hide resolved
docs/reference/oev-network/searchers/understanding-auctioneer.md
Outdated
Show resolved
Hide resolved
docs/reference/oev-network/searchers/understanding-auctioneer.md
Outdated
Show resolved
Hide resolved
docs/reference/oev-network/searchers/understanding-auctioneer.md
Outdated
Show resolved
Hide resolved
Co-authored-by: Emanuel Tesař <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 LGTM
Feel free to merge anytime and I'll do final pass and create a new PR with my suggestions. @dcroote wanted to do the same, but we don't want to extend the scope of this PR more.
No description provided.