Technical Debt Tracker - Githubissues

cosmos / ibc-go

Inter-Blockchain Communication Protocol (IBC) implementation in Golang.

https://ibc.cosmos.network/

MIT License

542 stars 580 forks source link

Technical Debt Tracker #145

Closed AdityaSripal closed 1 year ago

AdityaSripal commented 3 years ago

This issue is a tracker for technical debt accumulated after IBC 1.0 for the purpose of maintaining backward compatibility.

[x] Indirect Iteration Keys needed for efficient iteration while not breaking ICS-24 paths which are inefficient for iteration: https://github.com/cosmos/ibc-go/pull/125
[x] Maintaining FrozenHeight in clientState even though it will not be used except as a boolean (non-zero vs zero height): https://github.com/cosmos/ibc-go/pull/141
[x] Maintaining DEPRECATED packet_data attribute, users should switch to packet_data_hex: https://github.com/cosmos/ibc-go/pull/144
[x] IBC client/consensus states for upgrades are set in the Upgrade Keeper
[x] v0.42.0 SDK to ibc-go v1.0.0 migration code ie legacy/v100 ref: #205
[x] solo machine bypass for timestamp check #214

colin-axner commented 3 years ago

tendermint#6403 may influence which attribute should be deprecated between packet_data and packet_data_hex. We can discuss with relayers which is preferred when the time comes

crodriguezvega commented 1 year ago

All the PRs mentioned above haven been merged and I believe the upgrade keeper is already used to set client/consensus states. Thus I will close the issue now.

colin-axner commented 1 year ago

I believe the upgrade keeper is already used to set client/consensus states.

This is correct. The original issue is a bit hard to follow, but the intention was to note technical debt, so the fact that the client/consensus states being set by the upgrade keeper is actually a problem. Ideally they would exist under the client keeper.

I think a couple of the other issues mentioned are not addressed:

frozen height is still maintained in the client state (it should be a boolean)
duplicate event emission for packet_data is still maintained (should be removed)
Indirect Iteration Keys needed for efficient iteration while not breaking ICS-24 paths which are inefficient for iteration (we should upgrade connections to allow for efficient iteration of consensus keys, thus making the indirect iteration keys useless)

colin-axner commented 1 year ago

I think we should actually move this issue to a document. I do think it is useful to document areas of the code which have been deprecated and should be fixed/removed when possible