Skip to content

feat: initial checkpoints #551

Merged
ChaoticTempest merged 2 commits intodevelopfrom
phuong/feat/checkpoints
Oct 30, 2025
Merged

feat: initial checkpoints #551
ChaoticTempest merged 2 commits intodevelopfrom
phuong/feat/checkpoints

Conversation

@ChaoticTempest
Copy link
Copy Markdown
Contributor

This is the initial work towards Checkpoints and PendingRequests and is merely a refactoring right now of what we have right now.

Changes

  • We use to use SignRespondTxId as the id for mapping into the sign_respond_tx_map, but this changes that to SignId since that will be more generic going forward for all chains.
  • SignatureGenerator takes in Checkpoints to set it for pending publish. We're still using the old channels to send the request for publishing, but these channels will likely be moved to checkpoints in the future.
  • RpcExecutor takes in Checkpoints to mark a request as completed (does nothing for now).

Copy link
Copy Markdown
Contributor

@volovyks volovyks left a comment

Choose a reason for hiding this comment

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

Looks good!

@@ -38,8 +34,9 @@ struct AbiField {
}

#[derive(Debug, Clone, PartialEq, Eq, Hash, serde::Serialize, serde::Deserialize)]
pub enum SignRespondTxStatus {
Pending,
pub enum PendingRequestStatus {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I do not understand these statuses.
My understanding:

  • PendingExecution
  • PendingPublishSignature
  • WaitingForOutcome
  • PendingPublishOutcome

Maybe the names are not the best, open for discussion.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I didn't care enough to change the Failed/Success ones. We can keep debating on what to name each of the status as we add them in on utilization

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Sure
cc @ppca

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

success/failed is needed to handle different logic to create the message we are sending in read_respond depending on whether the eth tx was success or failure. It is not to represent the status of the entire bidirectional request flow.

jakmeier
jakmeier previously approved these changes Oct 2, 2025
Copy link
Copy Markdown
Contributor

@jakmeier jakmeier left a comment

Choose a reason for hiding this comment

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

Great incremental change!!

Please address the comments by @volovyks and my own but I'm excited to get this merged soon. :)

jakmeier
jakmeier previously approved these changes Oct 9, 2025
volovyks
volovyks previously approved these changes Oct 13, 2025
Copy link
Copy Markdown
Contributor

@volovyks volovyks left a comment

Choose a reason for hiding this comment

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

The usage of PendingRequestStatus still has mixed purposes, but we can deal with it in the followup.

@ChaoticTempest ChaoticTempest dismissed stale reviews from volovyks and jakmeier via c655b39 October 16, 2025 22:30
@ChaoticTempest ChaoticTempest force-pushed the phuong/feat/checkpoints branch from c655b39 to 7c64db4 Compare October 29, 2025 22:47
@ChaoticTempest ChaoticTempest force-pushed the phuong/feat/checkpoints branch from 7c64db4 to 6f99ab0 Compare October 29, 2025 22:54
@ChaoticTempest
Copy link
Copy Markdown
Contributor Author

ChaoticTempest commented Oct 30, 2025

need another approval for this one. It's now working with the bidirectional test in #525. We do not need to wait for it to go in. This one can go in now

@ChaoticTempest ChaoticTempest merged commit fe2f2a3 into develop Oct 30, 2025
3 checks passed
@ChaoticTempest ChaoticTempest deleted the phuong/feat/checkpoints branch October 30, 2025 16:17
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.

4 participants