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

Add strict requirements for changing 07-tendermint chainID #1308

Closed
3 tasks
colin-axner opened this issue Apr 28, 2022 · 1 comment
Closed
3 tasks

Add strict requirements for changing 07-tendermint chainID #1308

colin-axner opened this issue Apr 28, 2022 · 1 comment

Comments

@colin-axner
Copy link
Contributor

Summary

IBC only supports past/future misbehaviour detection for changing the chainID across upgrades given a strict format. That is, if the current chainID is in the revision format, the past/future revision must contain the same chainID base with a revision number less than (past) or greater than (future) to the current one. If the revision number is not a chainID, past/future revisions are not supported (at the moment).

However, the 07-tendermint client also allows arbitrary changing of the chainID, even if it breaks misbehaviour detection. We should increase the strictness of our allowed changing of the chain ID so misbehaviour detection does not become nullified

Within upgrade.go for 07-tendermint, after the height check, we should add the following check:

switch { 
case clienttypes.IsRevisionFormat(cs.ChainID):
    // check that the base of the cs.ChainID == base of upgradedClientState.ChainID
    oldChainIDBase := strings.TrimSuffix(chainID, fmt.Sprintf("-%d", clienttypes.ParseChainID(chainID)))
    newChainIDBase := chainID = strings.TrimSuffix(upgradedClientState.ChainID, fmt.Sprintf("-%d", clienttypes.ParseChainID(upgradedClientState.ChainID)))
    if oldChainIDBase != newChainIDBase {
        return err
    }

case cs.ChainID != upgradedClientState.ChainID
    // only allow upgrade from non-revision chainID to a chainID with "-0"
    if upgradedClientState.ChainID != clienttypes.SetRevisionNumber(cs.ChainID, 0) {
        return err
    }
}

For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged/assigned
@colin-axner
Copy link
Contributor Author

Discussing further with @AdityaSripal, increasing strictness of chainID changing partially breaks the network due to previous behaviour (in earlier releases).

We decided if a chainID goes away from the revision format, misbehaviour detection will no longer be supported. Again, if we supported misbehaviour detection for chains going away from the strict revision format, we would partially break the network

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

No branches or pull requests

1 participant