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

fix: audit remediations for l1 contracts #457

Merged
merged 20 commits into from
Nov 4, 2024
Merged

fix: audit remediations for l1 contracts #457

merged 20 commits into from
Nov 4, 2024

Conversation

shaspitz
Copy link
Contributor

@shaspitz shaspitz commented Oct 30, 2024

@shaspitz shaspitz marked this pull request as ready for review November 1, 2024 05:07
Copy link
Member

@chrmatt chrmatt 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 to me.

Note: I've only looked at the "non-audit, but related commits". Let me know if I should look at something else.


Concretely, for validators to be slashable by the oracle, `slashPeriodSeconds` must be greater than:
Note _currently opted-in_ in this context, means the validator is opted-in with respect to the latest finalized L1 block state, in the worst case this is two L1 epochs ago.
Copy link
Member

Choose a reason for hiding this comment

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

That's strictly speaking not correct, since finalization could take longer than 2 epochs. I guess we consider everything from more than 2 epochs ago as finalized, even if it's technically not. Maybe not the right place to discuss this, but wanted to point it out.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is there a probabilistic/practical block threshold you'd suggest for assuming the vast majority of txes will be finalized?

I actually didn't know finalization could take longer than 2 epochs. A proof of stake protocol should be able to provide some sort of finalization guarantee..

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Asking cause we do need to make some sort of assumption of how long finalization can take in the worst case scenario here

Copy link
Member

@chrmatt chrmatt Nov 1, 2024

Choose a reason for hiding this comment

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

There is a block tag you can query for whether the block is already finalized or not. There is no guarantee in terms of number of block/slots etc. In theory, finalization could take arbitrarily long. But in practice, it is mostly happening after the 2 epochs. Even if not technically finalized, it already is highly unlikely to reorg much earlier due to how attestions work; I think the longest reorg ever since PoS was 7 blocks. So waiting 2 epochs is plenty in practice, but theoretically not guaranteed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Got it, and yea we're using the finalized block query around the repo. My question here was specific to protocol design and what we can use as a practical max reorg time for determining slashPeriodSeconds. Seems like 2 epochs works as that practical assumption

@shaspitz shaspitz merged commit 26ecd81 into main Nov 4, 2024
6 checks passed
@shaspitz shaspitz deleted the audit-fixes branch November 4, 2024 07:36
@shaspitz shaspitz restored the audit-fixes branch November 5, 2024 02:31
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