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

priliminary OEV Network docs #12

Merged
merged 40 commits into from
Jul 3, 2024
Merged

priliminary OEV Network docs #12

merged 40 commits into from
Jul 3, 2024

Conversation

Ashar2shahid
Copy link
Collaborator

No description provided.

Copy link

github-actions bot commented Jun 21, 2024

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

Copy link
Collaborator

@Siegrift Siegrift left a 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/overview/oev-network.md Outdated Show resolved Hide resolved
docs/reference/oev-network/overview/oev-network.md Outdated Show resolved Hide resolved
docs/reference/oev-network/overview/auction-cycle.md Outdated Show resolved Hide resolved
docs/reference/oev-network/overview/auction-cycle.md Outdated Show resolved Hide resolved
Ashar2shahid and others added 25 commits June 23, 2024 21:09
@aquarat
Copy link
Contributor

aquarat commented Jun 24, 2024

I made some suggestions, looking good!

@aquarat
Copy link
Contributor

aquarat commented Jun 24, 2024

Another note:
If you look at the preview site: https://oev-docs--pr12-new-oev-docs-0y2wddya.web.app/reference/oev-network/
I feel like this page needs something, like a pretty graphic or something. It's one of the user's first entry points into the docs of OEV.

@aquarat
Copy link
Contributor

aquarat commented Jun 24, 2024

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.

Copy link
Member

@andreogle andreogle left a 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

docs/reference/oev-network/overview/oev-network.md Outdated Show resolved Hide resolved
docs/reference/oev-network/searchers/arguments.md Outdated Show resolved Hide resolved
docs/reference/oev-network/searchers/arguments.md Outdated Show resolved Hide resolved
const api3Contracts = require('@api3/contracts');

const oevNetworkProvider = new JsonRpcProvider(OEV_NETWORK_RPC_URL);
const OevAuctionHouseArtifact = await hre.artifacts.readArtifact(
Copy link
Member

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

Copy link
Collaborator

@Siegrift Siegrift left a 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:

  1. 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/dapps/index.md Show resolved Hide resolved
docs/reference/oev-network/overview/auction-cycle.md Outdated Show resolved Hide resolved
Copy link
Collaborator

@Siegrift Siegrift left a 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.

@Ashar2shahid Ashar2shahid merged commit b2dff67 into main Jul 3, 2024
2 checks passed
@Ashar2shahid Ashar2shahid deleted the new_oev_docs branch July 3, 2024 04:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants