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

Store rollup node ArbSys block numbers if on an Arbitrum host chain #6

Merged
merged 2 commits into from
May 12, 2023

Conversation

PlasmaPower
Copy link
Contributor

This lets the nitro node do efficient log queries for the node creation event, even when running an L3.

Copy link
Member

@gzeoneth gzeoneth left a comment

Choose a reason for hiding this comment

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

We can also change createdAtBlock to return L1/L2 block number based on if it is a L2/L3 to optimize storage, createdAtBlock it is currently used in

  1. off-chain event lookup
  2. validatorAfk check (which should be replaced by deadlineBlock instead)
  3. time delta of minimumAssertionPeriod (can be removed in new protocol)

but LGTM otherwise

@PlasmaPower
Copy link
Contributor Author

Yeah, I had the same thought about the validatorAfk check but didn't realize the minimumAssertionPeriod is getting removed in the new challenge protocol. That's good to hear, but until then I think it'd make sense to merge this as-is.

@PlasmaPower PlasmaPower merged commit 87d5809 into develop May 12, 2023
@PlasmaPower PlasmaPower deleted the rollup-arbitrum-host branch May 12, 2023 04:38
@gzeoneth
Copy link
Member

Yeah, I had the same thought about the validatorAfk check but didn't realize the minimumAssertionPeriod is getting removed in the new challenge protocol. That's good to hear, but until then I think it'd make sense to merge this as-is.

because there is only 1 deterministic assertion to be made, so I think we don't need that anymore; not sure about potential dos tho

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