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(svm): relay root bundle event #683

Merged

Conversation

md0x
Copy link
Contributor

@md0x md0x commented Oct 23, 2024

Proposed changes in this PR:

  • Add relay root bundle event

Signed-off-by: Pablo Maldonado <[email protected]>
Copy link

linear bot commented Oct 23, 2024

@@ -23,6 +23,13 @@ pub struct EnabledDepositRoute {
pub enabled: bool,
}

#[event]
pub struct RelayRootBundleEvent {
Copy link
Contributor

Choose a reason for hiding this comment

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

this is named RelayedRootBundle in EVM. Also the ordering is different (rootBundleId, relayerRefundRoot, slowRelayRoot)

root_bundle_id: state.root_bundle_id,
});

// Finally, increment the root bundle id
Copy link
Contributor

Choose a reason for hiding this comment

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

not directly related to this PR, but I think its confusing to have prop named root_bundle_id in state account. Instead it should be something to resemble number of relayed bundles or next_root_bundle_id - Maybe add this as TODO here

md0x added 2 commits October 23, 2024 16:06
Signed-off-by: Pablo Maldonado <[email protected]>
Signed-off-by: Pablo Maldonado <[email protected]>
@md0x md0x requested a review from Reinis-FRP October 23, 2024 15:29
Copy link
Contributor

@Reinis-FRP Reinis-FRP left a comment

Choose a reason for hiding this comment

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

nice!

@md0x md0x merged commit 79bc5b1 into master Oct 24, 2024
9 checks passed
@md0x md0x deleted the pablo/acx-2922-instructionsadminrs-add-event-to-relay_root_bundle branch October 24, 2024 08:44
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.

2 participants