Implement the rpc handler method for RegisterAccount#2110
Conversation
…thod-for-registeraccount
…ccount and RegisterAccount call into
…thod-for-registeraccount
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
…thod-for-registeraccount
chatton
requested review from
AdityaSripal,
colin-axner,
damiannolan and
seantking
as code owners
colin-axner
left a comment
colin-axner
left a comment
There was a problem hiding this comment.
LGTM, could you add a msg_server_test.go? With simple success/fail tests cases?
modules/apps/27-interchain-accounts/controller/keeper/account.go
Outdated
Show resolved
Hide resolved
modules/apps/27-interchain-accounts/controller/keeper/account.go
Outdated
Show resolved
Hide resolved
modules/apps/27-interchain-accounts/controller/keeper/account.go
Outdated
Show resolved
Hide resolved
damiannolan
left a comment
damiannolan
left a comment
There was a problem hiding this comment.
Looks great! I agree with @colin-axner's suggestions. I think it may be nicer to just return the channelID string directly :)
seantking
left a comment
seantking
left a comment
There was a problem hiding this comment.
LGTM once @colin-axner changes are in.
| ibctesting "github.com/cosmos/ibc-go/v5/testing" | ||
| ) | ||
|
|
||
| func (suite *KeeperTestSuite) TestRegisterAccount() { |
There was a problem hiding this comment.
@colin-axner some of tests are the same or very similar as the existing tests for RegisterInterchainAccount such as this one let me know if you think it's overkill to have the test in multiple locations.
There was a problem hiding this comment.
When we deprecate RegisterInterchainAccount in favor of the message server API, we can just remove the others. I think it is fine to have duplicates for now
colin-axner
left a comment
colin-axner
left a comment
There was a problem hiding this comment.
ACK, nice work! Left just a few suggestions but otherwise looks great to me
modules/apps/27-interchain-accounts/controller/keeper/msg_server.go
Outdated
Show resolved
Hide resolved
modules/apps/27-interchain-accounts/controller/keeper/msg_server_test.go
Outdated
Show resolved
Hide resolved
| ibctesting "github.com/cosmos/ibc-go/v5/testing" | ||
| ) | ||
|
|
||
| func (suite *KeeperTestSuite) TestRegisterAccount() { |
There was a problem hiding this comment.
When we deprecate RegisterInterchainAccount in favor of the message server API, we can just remove the others. I think it is fine to have duplicates for now
…thod-for-registeraccount
Codecov Report
@@ Coverage Diff @@
## main #2110 +/- ##
==========================================
+ Coverage 80.03% 80.08% +0.04%
==========================================
Files 170 170
Lines 11823 11838 +15
==========================================
+ Hits 9463 9480 +17
+ Misses 1942 1940 -2
Partials 418 418
|
chatton
deleted the
cian/issue#2055-implement-the-rpc-handler-method-for-registeraccount
branch
(cherry picked from commit b7d8359)
(cherry picked from commit b7d8359) Co-authored-by: Cian Hatton <[email protected]>
5 participants
Description
The suggestions in this discussion have been pulled into this PR.
closes: #2055
Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.
docs/) or specification (x/<module>/spec/)godoccomments.Unreleasedsection inCHANGELOG.mdFiles changedin the Github PR explorerCodecov Reportin the comment section below once CI passes