Refactor x/ibc to ADR 031 #7576

atheeshp · 2020-10-16T08:25:11Z

Description

closes: #7543

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.

Targeted PR against correct branch (see CONTRIBUTING.md)
Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
Code follows the module structure standards.
Wrote unit and integration tests
Updated relevant documentation (docs/) or specification (x/<module>/spec/)
Added relevant godoc comments.
Added a relevant changelog entry to the Unreleased section in CHANGELOG.md
Re-reviewed Files changed in the Github PR explorer
Review Codecov Report in the comment section below once CI passes

…00-x-ibc

codecov · 2020-10-16T08:31:18Z

Codecov Report

Merging #7576 into master will increase coverage by 0.04%.
The diff coverage is 65.67%.

@@            Coverage Diff             @@
##           master    #7576      +/-   ##
==========================================
+ Coverage   54.15%   54.20%   +0.04%     
==========================================
  Files         606      610       +4     
  Lines       38194    38363     +169     
==========================================
+ Hits        20685    20793     +108     
- Misses      15411    15464      +53     
- Partials     2098     2106       +8

fedekunze

the transfer messages are missing

…00-x-ibc

…o atheesh/7500-x-ibc

x/ibc/applications/transfer/keeper/msg_server.go

x/ibc/core/keeper/msg_server.go

fedekunze · 2020-10-18T10:16:26Z

shouldn't we migrate the handler tests to msg_server_test.go as well?

…00-x-ibc

colin-axner

It'd be nice if we could move the x/ibc/core/handler_test.go to x/ibc/core/keeper/msg_server_test.go and replace the handler(...) in each test with a direct call to the keepers function. This will give us a better view of code cov of each msg_server function

x/ibc/core/02-client/handler.go

x/ibc/core/02-client/handler_test.go

…00-x-ibc

colin-axner

ACK, thanks!

colin-axner · 2020-10-19T12:49:49Z

it looks like some of the tests started breaking in x/ibc/core. This is because handler_test.go had the testing suite. I'd just add a core_test.go file which contains all the variables/constants and the IBCTestSuite as previously defined in handler_test.go

…00-x-ibc

amaury1093

utACK

amaury1093 · 2020-10-19T16:17:16Z

@fedekunze would you be able to give another round of review 🙏 ?

aaronc

utACK

…heesh/7500-x-ibc

aaronc · 2020-10-19T17:56:09Z

FYI I just added the RegisterServices wiring to this PR as #7584 got merged in first.

@fedekunze I believe we have addressed your concerns. We're trying to get this merged by tonight for rc1 so if we don't hear from you soon I'll just dismiss your review as stale, and if we missed anything there's still time to address fixes before 0.40 final.

* WIP: Refactor x/ibc to ADR 031 * updated handler * removed unsued * fix * Add proto service for ibc/transfer * lint * remove old upgrade handler * added doc * review changes * fix tests * formatter * Add MsgServer wiring in RegisterServices Co-authored-by: Aaron Craelius <aaronc@users.noreply.github.com> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

* Refactor RegisterQueryServices -> RegisterServices (#7518) * Refactor RegisterQueryServices -> RegisterServices * Fix tests * Add ADR 031 BaseApp and codec infrastructure (#7519) * Refactor RegisterQueryServices -> RegisterServices * Cleaner proto files * Fix tests * Add MsgServer * Fix lint * Remove MsgServer from configurator for now * Remove useless file * Fix build * typo * Add router * Fix test * WIP * Add router * Remove test helper * Add beginning of test * Move test to simapp? * ServiceMsg implement sdk.Msg * Add handler by MsgServiceRouter * Correct signature * Add full test * use TxEncoder * Update baseapp/msg_service_router.go Co-authored-by: Aaron Craelius <aaron@regen.network> * Push changes * WIP on ServiceMsg unpacking * Make TestMsgService test pass * Fix tests * Tidying up * Tidying up * Tidying up * Add JSON test * Add comments * Tidying * Lint * Register MsgRequest interface * Rename * Fix tests * RegisterCustomTypeURL * Add changelog entries * Put in features * Update baseapp/msg_service_router.go Co-authored-by: Aleksandr Bezobchuk <alexanderbez@users.noreply.github.com> * Update baseapp/msg_service_router.go Co-authored-by: Aleksandr Bezobchuk <alexanderbez@users.noreply.github.com> * Update baseapp/msg_service_router.go Co-authored-by: Aleksandr Bezobchuk <alexanderbez@users.noreply.github.com> * Update baseapp/msg_service_router.go Co-authored-by: Aleksandr Bezobchuk <alexanderbez@users.noreply.github.com> * Update baseapp/msg_service_router.go Co-authored-by: Aleksandr Bezobchuk <alexanderbez@users.noreply.github.com> * Update baseapp/msg_service_router.go Co-authored-by: Aleksandr Bezobchuk <alexanderbez@users.noreply.github.com> * Update baseapp/msg_service_router.go Co-authored-by: Aleksandr Bezobchuk <alexanderbez@users.noreply.github.com> * Address review comments * Address nit * Fix lint * Update codec/types/interface_registry.go Co-authored-by: Marie Gauthier <marie.gauthier63@gmail.com> * godoc Co-authored-by: Aaron Craelius <aaronc@users.noreply.github.com> Co-authored-by: Aaron Craelius <aaron@regen.network> Co-authored-by: Aleksandr Bezobchuk <alexanderbez@users.noreply.github.com> Co-authored-by: Marie Gauthier <marie.gauthier63@gmail.com> * Refactor x/bank according to ADR 031 (#7520) * Refactor x/bank according to ADR 031 * Add comment * Update comment * Add comment * Add tests, address edge cases * Imports Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> * Refactor x/{gov, crisis} according to ADR 031 (#7533) * Refactor x/gov according to ADR 31 * fix tests * Refactor x/crisis according to ADR 031 * fix lint * lint * lint * review changes * lint * review change * fic doc Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> * Refactor x/distribution according to ADR 031 (#7524) * Refactor x/distribution according to ADR 31 * lint * removed unused * Apply suggestions from code review Co-authored-by: Marie Gauthier <marie.gauthier63@gmail.com> Co-authored-by: Aaron Craelius <aaron@regen.network> Co-authored-by: Marie Gauthier <marie.gauthier63@gmail.com> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> * [x/slashing] Implement Protobuf Msg Services (#7557) * Update x/slashing to use proto msg service * Fix proto-gen Co-authored-by: Aaron Craelius <aaron@regen.network> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> * Refactor x/auth/vesting to use ADR-031 (#7551) * update auth/vesting module to use proto msg services * rm accidental tmp files * Update x/auth/vesting/msg_server.go Co-authored-by: Marie Gauthier <marie.gauthier63@gmail.com> * golangci-lint fix Co-authored-by: Marie Gauthier <marie.gauthier63@gmail.com> Co-authored-by: Aaron Craelius <aaron@regen.network> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> * Refactor x/staking according to ADR 031 (#7556) * Refactor x/staking according to ADR 031 * lint * review changes * review changes * review changes Co-authored-by: Aaron Craelius <aaron@regen.network> * Add MsgServer to Configurator for ADR 031 wiring (#7584) * Add MsgServer to Configurator for ADR 031 wiring * Add docs, wire up evidence & staking * Add integration test * Add comments * Doc strings * Update types/module/configurator.go Co-authored-by: Aleksandr Bezobchuk <alexanderbez@users.noreply.github.com> * Update types/module/configurator.go Co-authored-by: Cory <cjlevinson@gmail.com> * Wire up vesting * Update CHANGELOG.md Co-authored-by: Aleksandr Bezobchuk <alexanderbez@users.noreply.github.com> Co-authored-by: Amaury Martiny <amaury.martiny@protonmail.com> Co-authored-by: Cory <cjlevinson@gmail.com> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> * Refactor x/evidence to ADR-031 (#7538) * Refactor x/evidence to ADR-031 * Add hash in response * Update changelog * Update x/evidence/keeper/keeper.go * Update proto/cosmos/evidence/v1beta1/tx.proto Co-authored-by: Marie Gauthier <marie.gauthier63@gmail.com> * Use msgServer struct Co-authored-by: Marie Gauthier <marie.gauthier63@gmail.com> * Refactor x/ibc to ADR 031 (#7576) * WIP: Refactor x/ibc to ADR 031 * updated handler * removed unsued * fix * Add proto service for ibc/transfer * lint * remove old upgrade handler * added doc * review changes * fix tests * formatter * Add MsgServer wiring in RegisterServices Co-authored-by: Aaron Craelius <aaronc@users.noreply.github.com> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> * add changelog entries for non-nil Result.Data fields in message responses Co-authored-by: Aaron Craelius <aaron@regen.network> Co-authored-by: Amaury Martiny <amaury.martiny@protonmail.com> Co-authored-by: Aaron Craelius <aaronc@users.noreply.github.com> Co-authored-by: Aleksandr Bezobchuk <alexanderbez@users.noreply.github.com> Co-authored-by: Marie Gauthier <marie.gauthier63@gmail.com> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> Co-authored-by: atheeshp <59333759+atheeshp@users.noreply.github.com>

atheeshp added 4 commits October 16, 2020 00:38

WIP: Refactor x/ibc to ADR 031

82e7b9e

Merge branch 'master' of github.com:cosmos/cosmos-sdk into atheesh/75…

d593918

…00-x-ibc

updated handler

60830b6

removed unsued

cb1747f

fix

d07a7e5

atheeshp requested review from aleem1314, amaury1093, anilcse, blushi and robert-zaremba October 16, 2020 09:18

atheeshp added 3 commits October 16, 2020 14:48

Merge branch 'master' into atheesh/7500-x-ibc

0303c13

Merge branch 'master' into atheesh/7500-x-ibc

56f2788

Merge branch 'master' into atheesh/7500-x-ibc

8456666

atheeshp marked this pull request as ready for review October 16, 2020 14:54

atheeshp requested review from aaronc, alessio and alexanderbez as code owners October 16, 2020 14:54

aaronc added the x/ibc label Oct 16, 2020

aaronc requested review from fedekunze, colin-axner and AdityaSripal October 16, 2020 14:57

atheeshp requested a review from clevinson October 16, 2020 14:57

fedekunze suggested changes Oct 16, 2020

View reviewed changes

atheeshp added 5 commits October 16, 2020 21:21

Merge branch 'master' of github.com:cosmos/cosmos-sdk into atheesh/75…

a8637e3

…00-x-ibc

Add proto service for ibc/transfer

d064c53

Merge branch 'master' of github.com:cosmos/cosmos-sdk into atheesh/75…

642aa7f

…00-x-ibc

Merge branch 'atheesh/7500-x-ibc' of github.com:cosmos/cosmos-sdk int…

bd3a3e6

…o atheesh/7500-x-ibc

lint

4a9b991

atheeshp requested a review from fedekunze October 16, 2020 16:21

remove old upgrade handler

5c65041

atheeshp requested a review from aaronc October 17, 2020 08:25

fedekunze reviewed Oct 18, 2020

View reviewed changes

x/ibc/applications/transfer/keeper/msg_server.go Show resolved Hide resolved

x/ibc/core/keeper/msg_server.go Show resolved Hide resolved

atheeshp added 2 commits October 19, 2020 14:37

added doc

4f3e42d

Merge branch 'master' of github.com:cosmos/cosmos-sdk into atheesh/75…

3536d15

…00-x-ibc

colin-axner reviewed Oct 19, 2020

View reviewed changes

x/ibc/core/02-client/handler.go Outdated Show resolved Hide resolved

x/ibc/core/02-client/handler_test.go Outdated Show resolved Hide resolved

atheeshp added 2 commits October 19, 2020 17:25

review changes

002772a

Merge branch 'master' of github.com:cosmos/cosmos-sdk into atheesh/75…

096c575

…00-x-ibc

atheeshp requested review from colin-axner and fedekunze October 19, 2020 11:57

colin-axner approved these changes Oct 19, 2020

View reviewed changes

atheeshp added 2 commits October 19, 2020 21:22

fix tests

1223c2d

Merge branch 'master' of github.com:cosmos/cosmos-sdk into atheesh/75…

2003d4f

…00-x-ibc

amaury1093 approved these changes Oct 19, 2020

View reviewed changes

formatter

b927c0d

aaronc approved these changes Oct 19, 2020

View reviewed changes

aaronc added 2 commits October 19, 2020 13:47

Merge branch 'master' of https://github.com/cosmos/cosmos-sdk into at…

ba0346a

…heesh/7500-x-ibc

Add MsgServer wiring in RegisterServices

bac9f1a

fedekunze approved these changes Oct 19, 2020

View reviewed changes

fedekunze added the A:automerge Automatically merge PR once all prerequisites pass. label Oct 19, 2020

Merge branch 'master' into atheesh/7500-x-ibc

897366d

mergify bot merged commit 155a6ad into master Oct 19, 2020

mergify bot deleted the atheesh/7500-x-ibc branch October 19, 2020 18:21

clevinson mentioned this pull request Oct 19, 2020

[backport/v0.40.x] Implement ADR031 #7599

Merged

9 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor x/ibc to ADR 031 #7576

Refactor x/ibc to ADR 031 #7576

atheeshp commented Oct 16, 2020 •

edited

Loading

codecov bot commented Oct 16, 2020 •

edited

Loading

fedekunze left a comment

fedekunze commented Oct 18, 2020

colin-axner left a comment •

edited

Loading

colin-axner left a comment

colin-axner commented Oct 19, 2020

amaury1093 left a comment

amaury1093 commented Oct 19, 2020

aaronc left a comment

aaronc commented Oct 19, 2020

Refactor x/ibc to ADR 031 #7576

Refactor x/ibc to ADR 031 #7576

Conversation

atheeshp commented Oct 16, 2020 • edited Loading

Description

codecov bot commented Oct 16, 2020 • edited Loading

Codecov Report

fedekunze left a comment

Choose a reason for hiding this comment

fedekunze commented Oct 18, 2020

colin-axner left a comment • edited Loading

Choose a reason for hiding this comment

colin-axner left a comment

Choose a reason for hiding this comment

colin-axner commented Oct 19, 2020

amaury1093 left a comment

Choose a reason for hiding this comment

amaury1093 commented Oct 19, 2020

aaronc left a comment

Choose a reason for hiding this comment

aaronc commented Oct 19, 2020

atheeshp commented Oct 16, 2020 •

edited

Loading

codecov bot commented Oct 16, 2020 •

edited

Loading

colin-axner left a comment •

edited

Loading