Does not support 1/1 trust level

danwt commented 2 months ago

Summary of Impact

A Tendermint light client trust level of 1/1 is not possible to use, because the code will require a greater than (>) rather than greater than or equal (>=) when compared against 1/1 (100%) of voting power. That means people intending to use 1/1 will be forced to use a reduced fraction (e.g. 999/1000) which will have incorrect security characteristics.

Steps to Reproduce

Create a light client with 1/1 trust level. It will reject headers with signing of 100% of validator set.

Workarounds

None

Supporting Material/References

https://github.com/cometbft/cometbft/blob/v0.37.5/types/validator_set.go#L878-L886 https://github.com/cosmos/ibc-go/blob/1c05fa7dbd86a0c711bdd6575f0abbbc331d0600/modules/light-clients/07-tendermint/update.go#L108-L112

Impact

Users requiring 100% voting power signing will be forced to use a reduce level (99%), which could cause the light client to accept a header from a signer who is not trusted.

cason commented 2 weeks ago

If we run cometbft light with 1/1 or 3/3 thresholds we do not observe any issue. Can you be more specific and provide examples where this issue arises?

danwt commented 2 weeks ago

I'm sorry I don't have an example to hand but I think it was due to the > in this code

https://github.com/cometbft/cometbft/blob/v0.37.5/types/validator_set.go#L878-L886

as opposed to >=

cason commented 1 week ago

Yes, but this code is run only in some particular circumstances...

There two or three methods for validation, this is just one of them, and it is not the one we do at first.

If there is no example of how this can fail, I'd recommend closing this issue.

danwt commented 1 week ago

Sure, sorry, I don't have a replication to hand. But we did see this on a real execution between two chains

cometbft / cometbft

Does not support 1/1 trust level #4021