ibc-go
ibc-go copied to clipboard
Remove `AllowUpdateAfterMisbehaviour`/`AllowUpdateAfterExpiry` from 07-tendermint client state.
These have been marked as deprecated after 02-client refactor reference ADR. We could probably remove these in a fully compatible way by using [reserved}(https://protobuf.dev/programming-guides/proto3/#reserved) values.
Opening issue to keep track and discuss when to remove.
For Admin Use
- [x] Not duplicate issue
- [x] Appropriate labels applied
- [ ] Appropriate contributors tagged/assigned
Sorry for doing the issue before asking, but I've already done it.
better late than never to drop a comment @expertdicer :smile:! I see the PR is failing on e2es but don't have much time this month to see why (reserved might not be right approach?). I'll prioritize reviewing this when I get some additional time. Thanks!
I'm still trying to debug the err, still don't know why :sweat_smile:
It might be possible that the counterparty has set these fields to true which means in the connection handshake, our code needs to be able to populate these fields when decoding. Will the reserved field work correct in this case @DimitrisJim?
I looked into this a little. The SDK will fail to unmarshal unknown fields so when we mark existing fields as reserved the compiler sees these as unknown fields. We would need to require relayers to remove the fields from their grpc requests. Personally, I think it might be better to ice box this issue and wait until we restructure the client state for other reasons as well. What do you think @DimitrisJim?
sgtm! this seems quite unfortunate though, iirc reserved was supposed to be a clean way to remove certain field from the proto structures.