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 epoch number to Tendermint light clients, update height & timeout logic #6531

Closed
cwgoes opened this issue Jun 29, 2020 · 6 comments
Closed
Assignees

Comments

@cwgoes
Copy link
Contributor

cwgoes commented Jun 29, 2020

Per cosmos/ibc#439, blocked on upstream spec updates & review.

@cwgoes cwgoes added the x/ibc label Jun 29, 2020
@cwgoes cwgoes added this to the IBC 1.0 milestone Jun 29, 2020
@AdityaSripal
Copy link
Member

If a chain decides to upgrade and reset his height, should IBC consider misbehaviour across the upgrades?

If the chain updates at (e, h) to (e+1, 1). Does having two valid signed headers for height (e, h+1) and (e+1, 1) count as Misbehaviour?

@cwgoes
Copy link
Contributor Author

cwgoes commented Jul 7, 2020

Hmm. If the light client knows that the upgrade will happen at (e, h) it can refuse to accept (e, h + 1), so there is no danger of a fork, but it still clearly is "misbehaviour". However, I think we want to follow whatever rules Tendermint would use in this instance (which we need to think about, actually).

@AdityaSripal
Copy link
Member

One important thing to note is that the validators do not attest to the epoch number. Thus, a relayer can take a signedHeader h and submit it as height (e, h) or (e', h) with no issues. I'm not sure if there is are any other attacks here, but at the very least, we must enforce in IBC that all upgrades change the chain-id.

Otherwise it may be possible for a header h in an epoch e to be mistaken for a header h in epoch e+1 if the chain-id doesn't change and the validator-set is similar enough, since the relayer may simply resubmit the old header for (e, h) and simply increment the epoch number.

@cwgoes
Copy link
Contributor Author

cwgoes commented Jul 8, 2020

Ah yes, I see - for that reason, we should probably tie the epoch number to the chain ID (as in, parse it from the chain ID).

@fedekunze
Copy link
Collaborator

@AdityaSripal can we close this issue?

@colin-axner
Copy link
Contributor

only thing left is parsing epoch from chain-id but we have an issue for this #7194

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 a pull request may close this issue.

4 participants