Update docs to make auction lengths chain specific #221
Conversation
|
Visit the preview URL for this PR (updated for commit 821e218): https://oev-docs--pr221-auction-lengths-4cxltzdi.web.app (expires Mon, 15 Sep 2025 08:11:32 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: 6915b094b5ba83fde754632ba50c1ee9406d433f |
|
|
||
| The auction length depends on the chain. | ||
|
|
||
| | Chain | Length (seconds) | |
There was a problem hiding this comment.
This table needs to be updated based on our decision on which chains are to use the fast auction times. I'd keep the PR open until we have at least one chain for which we enable the fast auctions to avoid changing the docs drastically.
There was a problem hiding this comment.
I'm also wondering whether we should not have the table specify the auction length for each chain explicitly (and write a test for it).
There was a problem hiding this comment.
I like the idea of explicitly listing all chains. In terms of writing a test for this list of chains, I think we can use the @api3/dapi-management and @api3/contracts packages to get a list of supported chains and then add a test that parses the markdown table and compares the two.
There was a problem hiding this comment.
Following on from prev comment, the test wouldn't be able to assert that the auction lengths are correct, but just that the expected chains are present.
There was a problem hiding this comment.
I think having a check that all chains are listed is a reasonable start. Can you try hacking up a test for this?
…on length section
Conflicts: package.json pnpm-lock.yaml
|
|
||
| ```solidity | ||
| uint256(keccak256(abi.encodePacked(uint256(dAppId)))) % AUCTION_LENGTH_SECONDS; | ||
| uint256(keccak256(abi.encodePacked(uint256(dappId)))) % auctionLength; |
There was a problem hiding this comment.
👍 Yeah, the casing was incorrect.
| .map((chain) => chain.name) | ||
| .sort((a, b) => a.localeCompare(b)); | ||
|
|
||
| test('the auction length section lists all the supported chains', () => { |
There was a problem hiding this comment.
👍 Yeah, I imagined something like this
| 'in-depth', | ||
| 'oev-auctioneer.md' | ||
| ); | ||
| const content = fs.readFileSync(docPath, 'utf8'); |
There was a problem hiding this comment.
Can you make sure this errors out nicely in case the path doesn't exist (when we move the files arouind)
There was a problem hiding this comment.
This is quite a change 🤔 Is this only from the included deps?
There was a problem hiding this comment.
Yeah. It seems most of it gets added because of the @api3/api-integrations package (which is a dependency of @api3/dapi-management)
There was a problem hiding this comment.
Maybe if we do the test using javascript file we don't need the TS configuration? I think it's a bit of an overkill for the docs repo.
|
Take a look at my comments @mcoetzee and LMK your thoughts, but they can be worked on later. I'm merging this to document the Arbitrum auction length. |
Relates to https://github.com/api3dao/oev-auctioneer/issues/762
Note
We are keeping the PR open until we have enabled fast auctions for at least one chain