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

RFC-009: Consensus Parameter Upgrades #7524

Merged
merged 8 commits into from Jan 14, 2022
Merged

RFC-009: Consensus Parameter Upgrades #7524

merged 8 commits into from Jan 14, 2022

Conversation

williambanfield
Copy link
Contributor

Related to #7503

@tychoish
Copy link
Contributor

tychoish commented Jan 6, 2022

Isn't there also an option where each parameter has a height/version information included in its self that would maintain compatibility but also allow parameters to change over the life of a change?

@williambanfield
Copy link
Contributor Author

williambanfield commented Jan 6, 2022

Isn't there also an option where each parameter has a height/version information included in its self that would maintain compatibility but also allow parameters to change over the life of a change?

Is there? I haven't heard of this if there is. From the sound of it, that would solve our problem quite nicely, I just maybe don't know about this feature yet?

Copy link
Contributor

@creachadair creachadair left a comment

Choose a reason for hiding this comment

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

Some very good points here. I'm not sure I have any compelling answers, but I left a few points for discussion.

docs/rfc/rfc-009-consensus-parameter-upgrades.md Outdated Show resolved Hide resolved
docs/rfc/rfc-009-consensus-parameter-upgrades.md Outdated Show resolved Hide resolved

#### Only Update HashedParams on Hash-Breaking Releases

An alternate solution to never hashing defaults is to not update the hashed
Copy link
Contributor

Choose a reason for hiding this comment

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

I think I understand this, but it seems subtle. Some hash-breaking changes are obvious, e.g., we change the value for some existing default. But what about adding a(nother) new consensus parameter? Presumably we have to choose a place for it in the global hashing order (e.g., "new parameters always go to the end" or something). Do we hash in its default value immediately?

Also (and this affects other implementation strategies too): What happens if we want to get rid of a consensus parameter? We can't get rid of it from the old blocks that committed to it, but we have to do something about the existing values in the hash order. Do we just leave it at its last-pinned value forever, or "zero" it before deleting, or something else? (We definitely can't remove it from the hash order entirely, or all future changes will be anchored to that merge)

Am I reading too much into it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think that if we always stick to 'defaults are never hashed' we will mostly be fine.

Old blocks will still validate because we effectively consider them as having had the defaults in the past. Removing consensus parameters is the same, they'll just stop being included in the hash.

We can't get rid of it from the old blocks that committed to it, but we have to do something about the existing values in the hash order.

Without a verification/hash-breaking upgrade, we cannot get rid of old consensus parameters. Tendermint would no longer know how to produce the hash of the old blocks so this would break blocksync etc. Verifying old blocks relies on getting consensus parameter updates from the application and hashing these into the block.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we can ever truly delete old consensus parameters, for the reason you say. When I say "get rid of" I really mean "agree to stop using". But then there is the question of how to tell nodes that parameter should no longer be used—otherwise it becomes a side channel.

My intuition is that "disabling" a parameter could be done by setting it to a designated "invalid" value and subsequently treating that as the default for subsequent blocks. That would be a hash-breaking change at the point of bugging the value, but presumably not thereafter.

proposes applying different hashing rules depending on the active block version.
The consensus parameter hash could be versioned in the same way. When different
block versions are used, a different set of consensus parameters will be included
in the hash.
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 compatible with the original "defaults" based proposal, in that we still need some rule for how to bootstrap new consensus parameters. 👍🏼

@williambanfield williambanfield added the S:automerge Automatically merge PR when requirements pass label Jan 14, 2022
@mergify mergify bot merged commit abb7c8c into master Jan 14, 2022
@mergify mergify bot deleted the wb/rfc-009 branch January 14, 2022 16:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S:automerge Automatically merge PR when requirements pass
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants