Skip to content

feat: taiyi-challenger implementation#591

Open
martines3000 wants to merge 18 commits intolu-bann:devfrom
martines3000:feat/challenger
Open

feat: taiyi-challenger implementation#591
martines3000 wants to merge 18 commits intolu-bann:devfrom
martines3000:feat/challenger

Conversation

@martines3000
Copy link
Copy Markdown
Contributor

@martines3000 martines3000 commented Apr 14, 2025

Implemented/Updated:

  1. Scripts to run challenger, preconfer and the scripts to send example typeA and typeB preconf requests
  2. Listening to commitment stream from preconfer
  3. Verifying tx inclusion (exclusion) and opening challenges on TaiyiChallenger

Missing:

  • Contract calls to TaiyiChallenger (missing deployment scripts)

Open questions:

  • What to do with Transaction responses ? (Do we want to store them, wait for the transaction execution or ignore them)
  • Should we use a separate Preconf request type which uses a subset of properties from the existing PreconfRequestTypeA (and PreconfRequestTypeB) types. Main reason for this would be that the underwriterSignedBlockspaceAllocation and underwriterSignedRawTx are not needed for opening a challenge as we only use the commitment signature and fields necessary to recover the signer address.

Todo:

  • Verify couple of types (hex or bytes)

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Apr 14, 2025

Important

Review skipped

Auto reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@chirag-bgh chirag-bgh self-requested a review April 15, 2025 16:18
@umbertov
Copy link
Copy Markdown
Contributor

umbertov commented May 2, 2025

Hey i'm not sure whether there is a concurrency issue here, among the two tasks
handle_challenge_creation and handle_challenge_submission. Each of these listens to newly published blocks, read/writes something from DB, and moves on to waiting for the next block.

But I think I see a sequential dependency among the challenge creation -> challenge submission:

  1. The handle_challenge_creation receives a block with slot number N, reads from the DB and checks if it has to create a challenge. If a challenge is created, it is inserted into the DB with the N key.
  2. The handle_challenge_submission receives the block at slot N, reads preconfirmation requests (stored at point 1) from the DB at the slot N and then checks inclusion of the preconf tx hashes inside the block at slot N.

What if a new block is published, and the handle_challenge_submission task runs before the handle_challenge_creation task? The submission task would not submit, and then later the creation task would insert a row in the DB which would never be processed.

If that's the case, then instead of the submission task listening for blocks, there should be a channel between the handle_challenge_creation and handle_challenge_submission tasks, where the creation task inserts rows into the DB and also uses the channel to send a notification (or the necessary data directly) to the submission task, which will listen for new messages on the channel with while let Some(x) = channel_rx.recv() instead of listening for blocks with while let Some(header) = stream.next().await.

I might be missing something but I've read the code a few times and i still haven't convinced myself of the opposite

@martines3000
Copy link
Copy Markdown
Contributor Author

@umbertov Hi, I checked your comments and I don't think this would cause any issues. Main reason is the following line:

https://github.com/martines3000/taiyi/blob/04bcb33c864781cecbc06da805de9babcf9efa6d/bin/taiyi-challenger/src/main.rs#L255-L258

  1. The handle_challenge_creation function checks if all required transactions are included and in case they are not it creates a challenge for slot current_slot + x, where x is the finalization window (32 blocks).
  2. The handle_challenge_submission functions prepares all necessary data for opening challenges on-chain and submits them.

@umbertov
Copy link
Copy Markdown
Contributor

umbertov commented May 5, 2025

@martines3000 i see: the challenge is submitted at a later slot wrt to the challenge creation, hence there is no problem.

thanks for the answer, i didn't notice that

@martines3000 martines3000 marked this pull request as ready for review May 6, 2025 06:50
@chirag-bgh chirag-bgh requested review from will-luban and zsluedem May 6, 2025 09:25
@chirag-bgh
Copy link
Copy Markdown
Contributor

@martines3000 Could you fix the lint?

Copy link
Copy Markdown
Contributor

@chirag-bgh chirag-bgh left a comment

Choose a reason for hiding this comment

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

Just a few nits.

Need to figure out the e2e-tests for this

@martines3000 martines3000 requested a review from chirag-bgh May 9, 2025 07:40
@martines3000
Copy link
Copy Markdown
Contributor Author

@chirag-bgh I resolved the mentioned nits.

Let me know if you find anything else that needs changing 👍.

In the meantime I will do the same improvements (extracting functions out from the main.rs file) in the other PR I opened.

Copy link
Copy Markdown
Contributor

@chirag-bgh chirag-bgh left a comment

Choose a reason for hiding this comment

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

Left some more comments, some refactoring stuffs

@chirag-bgh
Copy link
Copy Markdown
Contributor

@martines3000 Could you also add some tests for the databse related methods?

@chirag-bgh
Copy link
Copy Markdown
Contributor

@martines3000 Thanks for the uts, there's a conflict needed to be resolved.

@martines3000
Copy link
Copy Markdown
Contributor Author

@martines3000 Thanks for the uts, there's a conflict needed to be resolved.

I will resolve it. I am also preparing some unit tests for the db write/read. Should be finished soon (1h max)

@martines3000
Copy link
Copy Markdown
Contributor Author

@chirag-bgh Ok, I resolved the conflicts and added some basic unit tests for the DB read/writes.

@martines3000 martines3000 requested a review from chirag-bgh May 23, 2025 09:07
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.

3 participants