-
Notifications
You must be signed in to change notification settings - Fork 586
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
refactor: channel handshake version improvements #1283
Merged
seantking
merged 23 commits into
main
from
sean/issue#722-channel-handshake-improvements
May 10, 2022
Merged
Changes from 1 commit
Commits
Show all changes
23 commits
Select commit
Hold shift + click to select a range
5b0bc50
refactor: returning version from OnChanOpenInit
seantking b7088f8
refactor: update tests and add version to proto resp
seantking 6f20ad9
refactor: adding version to msg server resp
seantking 31a2831
refactor: remove unncessary if & update version on Endpoint.Ack
seantking 0178c09
fix: ics29 OnChanOpenInit remake versionMetaData before returning
seantking 8f8ce78
chore: update godoc
seantking f83b472
Merge branch 'main' into sean/issue#722-channel-handshake-improvements
seantking 2b0cca4
test: adding check for expected version string
seantking 2d7f0b8
test: adding test case for passing empty version string to ics20 onCh…
seantking 534e00c
Update modules/apps/29-fee/ibc_module.go
seantking 8f08760
chore: comment
seantking 7d6246c
Merge branch 'main' into sean/issue#722-channel-handshake-improvements
seantking a03916d
chore: changelog
seantking 0db8c86
Merge branch 'main' into sean/issue#722-channel-handshake-improvements
seantking ce656c4
fix: ica now discards auth module version
seantking 092d3fe
chore: update changelog
seantking d30f887
adding default version for ics29
seantking 1a44c0b
fix: using transfer module directly rather than calling full middlewa…
seantking 15d7092
fix testing bug
colin-axner 35f71cf
Merge branch 'main' into sean/issue#722-channel-handshake-improvements
seantking 0b85196
refactor: test now uses bool for isFeeEnabled rather than direct check
seantking 0286c64
docs: updating comment and migration docs
seantking 28d5622
Merge branch 'main' into sean/issue#722-channel-handshake-improvements
seantking File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
test: adding test case for passing empty version string to ics20 onCh…
…anOpenInit
- Loading branch information
commit 2d7f0b862b24806f5fd493b2b117b88e85d95300
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you also add a test case when
channel.Version
is not empty but it's a string that does not match the expectedtypes.Version
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have this test case already, no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh, sorry. I missed it.