-
Notifications
You must be signed in to change notification settings - Fork 296
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
Counterparty chains don't have ICS23 0.10.0 support. #3045
Comments
Hi, thanks for the writeup @hdevalence. For clarification, the concern with opening connections occurs when the I'm curious if there's a fix related to the Assuming the above context doesn't help because you are concerned with creating a light client with the prehash_key_before_comparison key set to true before the counterparty has upgraded to v0.10, then indeed you would need counterparties to upgrade. You could potentially allow counterparties to run the light clients without timeout support in the meantime and then perform an IBC software upgrade to change counterparty proof specs to set the prehash key to true. You can see documentation on this in the IBC client breaking upgrades where changing the proof specs is supported functionality:
This approach would not be very clean though as it would try to upgrade all counterparty clients to using the new field in ics23 v0.10 at the same moment, it is also a feature that has not been used very frequently so it may take some effort UX wise |
My current understanding w.r.t ICS23 0.10.0 support: If the counterparties (cosmos SDK) had accepted unknown fields as we expected, we would have the situation where:
Therefore, to enable timeouts in that scenario, the counterparty would have to:
Alternatively, in the situation we find ourselves in, where unknown fields are rejected, we have this situation:
To enable timouts in that scenario, the counterparty has to:
Additionally, there are security questions around allowing the The ideal situation is where counterparties upgrade to 0.10.0 before launch, this way there is no period of time where timeouts do not work, and no client upgrades required. |
Tracking issue for Osmosis: osmosis-labs/osmosis#6482 |
Osmosis support may have landed: osmosis-labs/osmosis#6653 |
Still a blocker for Osmosis: osmosis-labs/osmosis#6482 (comment) |
Osmosis is working as of the latest testnet release v21 |
Marking this as done, we have a critical mass. |
We discovered a critical issue with ICS23 support on counterparty chains that will disrupt our ability to connect to them unless we work with other ecosystem participants.
ICS23 provides a generic mechanism for non-inclusion proofs: a non-inclusion proof for key
k
is a pair of inclusion proofs for keysk1
,k2
, a check thatk1
andk2
are adjacent in the tree, and a check thatk1 < k < k2
. The problem is that this mechanism only works on ordered trees like the IAVL tree where the key ordering corresponds to the tree structure. On a sparse merkle tree like our JMT, this isn't the case -- sparsity is achieved by using the hashH(k)
of the key, so keys are randomly ordered.To fix this, we upstreamed a
prehash_key_before_comparison
field into the ICS23 spec. This provides a backwards-compatible extension supporting trees like the JMT: if the field is absent, it'sfalse
, and noninclusion proofs are verified as before; if it's present, it'strue
, and the noninclusion proof verifier checks thatH(k1)
andH(k2) are adjacent and that
H(k1) < H(k) < H(k2)`.Because all proto fields are optional and proto implementations are supposed to ignore unknown fields, this extension should also have been forwards-compatible, in the sense that a client could be created on a counterparty chain with
prehash_key_before_comparison = true
, with the following behavior:0.10.0
or greater, it parses theprehash_key_before_comparison
field and verifies non-inclusion proofs correctly;prehash_key_before_comparison
field, and all non-inclusion proofs fail to verify.In the latter case, packet timeouts don't work, but because the on-chain data is correct, as soon as the counterparty chain updates their ICS23 version, they would start working without any migrations to chain state.
However, what actually happens is that the Cosmos SDK rejects unknown fields: https://github.com/cosmos/cosmos-sdk/blob/a3a049bc2577632b94ff2236c2b01a250e85b16d/x/auth/tx/decoder.go#L27
To fix this, we need to urgently coordinate with the rest of the ecosystem to move them onto versions of
ibc-go
that useics23 0.10.0
or greater. This is included by default inv7.1.0
, but that version is much newer than many counterparty chains, so there will be longer upgrade latency. For instance, Osmosis is onv4
. Ideally, we could convince theibc-go
maintainers to release backport releases that bump the ICS23 version.The text was updated successfully, but these errors were encountered: