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

params: drop evidence.MaxAgeNumBlocks in the next major release #2942

Closed
melekes opened this issue Apr 30, 2024 · 3 comments
Closed

params: drop evidence.MaxAgeNumBlocks in the next major release #2942

melekes opened this issue Apr 30, 2024 · 3 comments
Labels
consensus enhancement New feature or request wontfix This will not be worked on

Comments

@melekes
Copy link
Contributor

melekes commented Apr 30, 2024

After PBTS is enabled by default for all chains, we should be able to remove evidence.MaxAgeNumBlocks (see tendermint/tendermint#2653 (comment)). That's because a timewarp attack (+1/3 cabal modifying block's time) will no longer be possible.

@melekes melekes added enhancement New feature or request needs-triage This issue/PR has not yet been triaged by the team. consensus and removed needs-triage This issue/PR has not yet been triaged by the team. labels Apr 30, 2024
@andynog
Copy link
Contributor

andynog commented Apr 30, 2024

My understanding is that PBTS will not be enabled by default.

If enable is not enforced then I believe you might still need to keep evidence.MaxAgeNumBlocks around, or do a big refactoring of the evidence codebase logic to check if PBTS is enabled or disabled (or when it will be enabled) and then use MaxAgeNumBlocks in the logic or not.

But since PBTS can be enabled for a future height via FeatureParams.PbtsEnableHeight I guess this just complicates the logic for evidence validation.

@cason
Copy link
Contributor

cason commented May 10, 2024

But block times can still be arbitrarily long, due to failures, asynchrony, etc.

So we probably still need to have the requirement of both time interval ("real" time) and blocks committed ("logic" time).

@melekes
Copy link
Contributor Author

melekes commented May 10, 2024

But block times can still be arbitrarily long, due to failures, asynchrony, etc.

So we probably still need to have the requirement of both time interval ("real" time) and blocks committed ("logic" time).

You mean 1/3+ cabal will still be able to delay block commit for arbitrary time. Therefore we still need MaxAgeNumBlocks?

@melekes melekes closed this as completed Jun 10, 2024
@melekes melekes added the wontfix This will not be worked on label Jun 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
consensus enhancement New feature or request wontfix This will not be worked on
Projects
Status: Done
Development

No branches or pull requests

3 participants