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

feat(taiko-client): add swagger api for preconf server #18274

Merged
merged 3 commits into from
Oct 22, 2024

Conversation

RogerLamTd
Copy link
Contributor

No description provided.

@RogerLamTd RogerLamTd self-assigned this Oct 21, 2024
@RogerLamTd RogerLamTd marked this pull request as ready for review October 21, 2024 22:15
}
}
},
"/preconfBlocks": {
Copy link
Contributor

Choose a reason for hiding this comment

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

preconfTransactions

Copy link
Member

Choose a reason for hiding this comment

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

Renamed to POST /tentativeBlocks as suggested below

},
"/preconfBlocks": {
"post": {
"description": "Insert preconfirmation blocks by the given groups to the backend L2 execution engine, please note that\nthe AVS service should sort the groups and make sure all the groups are valid at first.",
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"description": "Insert preconfirmation blocks by the given groups to the backend L2 execution engine, please note that\nthe AVS service should sort the groups and make sure all the groups are valid at first.",
"description": "Insert a group of transactions into a tentative block for preconfirmation. If the group is the first for a block, a new tentative block will be created. Otherwise, the transactions will be appended to the existing tentative block. The API will fail if: 1) the block is not tentative, 2) any transaction in the group is invalid or a duplicate, 3) block-level parameters are invalid or do not match the current tentative block’s parameters, 4) the group ID is not exactly 1 greater than the previous one, or 5) the last group of the block indicates no further transactions are allowed.",

Copy link
Member

Choose a reason for hiding this comment

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

Updated

packages/taiko-client/docs/swagger.json Outdated Show resolved Hide resolved
},
"/preconfHead": {
"put": {
"description": "Resets the backend L2 execution engine preconfirmation head, please note that\nthe AVS service should make sure the new head height is from a valid preconfirmation head.",
Copy link
Contributor

@dantaik dantaik Oct 22, 2024

Choose a reason for hiding this comment

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

Suggested change
"description": "Resets the backend L2 execution engine preconfirmation head, please note that\nthe AVS service should make sure the new head height is from a valid preconfirmation head.",
"description": "Remove all tentative blocks from the blockchain beyond the specified block height, ensuring the latest block ID does not exceed the given height. This method will fail if the block with an ID one greater than the specified height is not a tentative block. If the specified block height is greater than the latest tentative block ID, the method will succeed without modifying the blockchain.",

Copy link
Member

Choose a reason for hiding this comment

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

Updated

// @Param body body CreateOrUpdateBlocksFromBatchResponseBodyRequestBody true "preconf blocks creation request body"
// @Accept json
// @Produce json
// @Success 200 {object} CreateOrUpdateBlocksFromBatchResponseBody
// @Router /perconfBlocks [post]
// @Router /preconfBlocks [post]
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// @Router /preconfBlocks [post]
// @Router /preconfTransactions [post]

Copy link
Contributor

Choose a reason for hiding this comment

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

Shall we use ResetPreconfirmationBlockHeight? If the only parameter is a block ID/height, we should use height, as "head" usually means a structure.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

After another thought: Lets name these two functions:

  • build_tentative_blocks (buildTentativeBlocks)
  • remove_tentative_blocks (removeTentativeBlocks)

Copy link
Member

Choose a reason for hiding this comment

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

After another thought: Lets name these two functions:

  • build_tentative_blocks (buildTentativeBlocks)
  • remove_tentative_blocks (removeTentativeBlocks)

Updated the method names and the URL path

@@ -75,6 +75,6 @@ func (s *PreconfAPIServer) configureMiddleware(corsOrigins []string) {
func (s *PreconfAPIServer) configureRoutes() {
s.echo.GET("/", s.HealthCheck)
s.echo.GET("/healthz", s.HealthCheck)
s.echo.POST("/perconfTransactions", s.CreateOrUpdateBlocksFromBatch)
s.echo.POST("/preconfTransactions", s.CreateOrUpdateBlocksFromBatch)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should use "preconfirm_transactions" or "preconfirmTransactions", and "remove_tentative_blocks"

Copy link
Member

Choose a reason for hiding this comment

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

Changed

@davidtaikocha davidtaikocha merged commit b891588 into preconf-driver-apis Oct 22, 2024
7 checks passed
@davidtaikocha davidtaikocha deleted the preconf-driver-apis-swagger branch October 22, 2024 05:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants