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

ICA OnChanOpenAck fixes #314

Closed
5 tasks
colin-axner opened this issue Aug 5, 2021 · 4 comments
Closed
5 tasks

ICA OnChanOpenAck fixes #314

colin-axner opened this issue Aug 5, 2021 · 4 comments

Comments

@colin-axner
Copy link
Contributor

colin-axner commented Aug 5, 2021

Summary

  • matches spec (missing version check)
  • test active channel is correctly set

For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged/assigned
@colin-axner colin-axner added this to the Interchain Accounts milestone Aug 5, 2021
@seantking seantking self-assigned this Aug 5, 2021
@seantking
Copy link
Contributor

We don't actually pass Version into the ack step. We have access to the counter-party version but is it even necessary to check this? Maybe it should be removed from the spec.

@colin-axner
Copy link
Contributor Author

We don't actually pass Version into the ack step. We have access to the counter-party version but is it even necessary to check this? Maybe it should be removed from the spec.

Yes it is. The version set on our end is the counterpartyVersion, thus it must match our required version. Otherwise we may run into issues later if we try to create a new version and existing channels have random version strings

@seantking
Copy link
Contributor

seantking commented Aug 11, 2021

Right ok, gotcha. Thanks 👍

Agree, let's add it. Secondary thought: if we end up going with the approach whereby we pass the address back as the version string, could this become somewhat less necessary? Or perhaps we pass back a version like ics27-1-{interchain-account address}

@colin-axner
Copy link
Contributor Author

Yup, the version string should be something like ics27-1-{interchain-account address}. It could also be a protobuf definition with a Version and AccountAddress field, in which case we still want to check the application version

faddat pushed a commit to notional-labs/ibc-go that referenced this issue Feb 23, 2022
Replace sdk.AccAddress with bech32 string
faddat pushed a commit to notional-labs/ibc-go that referenced this issue Mar 1, 2022
* fix mutex

* fix timeout bug
faddat pushed a commit to notional-labs/ibc-go that referenced this issue Mar 1, 2022
* Merge PR cosmos#314: Fix timeout determination

* fix mutex

* fix timeout bug

* fix GetSignBytes

* address other calls to GetSignBytes

Co-authored-by: Jack Zampolin <jack.zampolin@gmail.com>
faddat pushed a commit to notional-labs/ibc-go that referenced this issue Mar 1, 2022
* WIP contextual codecs

* Push changes

* WIP

* Merge PR cosmos#319: Remove GetSignBytes usage

* Merge PR cosmos#314: Fix timeout determination

* fix mutex

* fix timeout bug

* fix GetSignBytes

* address other calls to GetSignBytes

Co-authored-by: Jack Zampolin <jack.zampolin@gmail.com>

* Working version

* Update retries for better errors

* Update import paths and address cosmos#292

* Address cosmos#293

* Address cosmos#295

* Address cosmos#315

* Add defensive key checks per cosmos#297

* Add flags for timeout offsets

* Update retries to have better and more explainatory errors

* WIP acks

* Update to remove replace

* update to sdk master

* Update sdk and make changes

* WIP timeouts and acks, strategy refactor

* WIP cleanup

* update to v0.40.0-rc2

* Refactor path generation and status

* Merge PR cosmos#274: feat: allow building a shared library

Co-authored-by: colin axnér <25233464+colin-axner@users.noreply.github.com>
Co-authored-by: Michael FIG <mfig@agoric.com>
CosmosCar pushed a commit to caelus-labs/ibc-go that referenced this issue Nov 6, 2023
* fix: make `TestValidatorSetHandling` even more stable
* docs: update CHANGELOG-PENDING.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

No branches or pull requests

3 participants