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.

Sign up for GitHub

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

Jump to bottom

chore: adding sdk.Msg impl for ics27 MsgSubmitTx #2135

Merged
merged 42 commits into from
Aug 30, 2022
Merged

chore: adding sdk.Msg impl for ics27 MsgSubmitTx #2135

merged 42 commits into from
Aug 30, 2022

Conversation

charleenfei
Copy link
Contributor

@charleenfei charleenfei commented Aug 26, 2022 •
edited
Loading

Description

please first review #2086
closes: #2054

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

@charleenfei charleenfei marked this pull request as ready for review August 26, 2022 14:32
@AdityaSripal AdityaSripal mentioned this pull request Aug 29, 2022
Merged
9 tasks
@codecov-commenter
Copy link

Codecov Report

Merging #2135 (ba01304) into main (f14179d) will decrease coverage by 0.63%.
The diff coverage is 81.96%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2135      +/-   ##
==========================================
- Coverage   80.08%   79.44%   -0.64%     
==========================================
  Files         170      170              
  Lines       11838    11870      +32     
==========================================
- Hits         9480     9430      -50     
- Misses       1940     2023      +83     
+ Partials      418      417       -1
Impacted Files Coverage Δ
...nterchain-accounts/controller/keeper/msg_server.go 83.33% <0.00%> (-16.67%) ⬇️
...ps/27-interchain-accounts/genesis/types/genesis.go 0.00% <0.00%> (ø)
modules/apps/27-interchain-accounts/types/codec.go 79.54% <ø> (-0.89%) ⬇️
modules/apps/27-interchain-accounts/module.go 56.62% <42.85%> (+0.52%) ⬆️
...ps/27-interchain-accounts/controller/types/msgs.go 92.30% <93.33%> (+1.39%) ⬆️
...7-interchain-accounts/controller/keeper/genesis.go 90.00% <100.00%> (ø)
...27-interchain-accounts/controller/keeper/keeper.go 94.73% <100.00%> (ø)
...s/27-interchain-accounts/controller/types/codec.go 100.00% <100.00%> (+100.00%) ⬆️
...apps/27-interchain-accounts/host/keeper/genesis.go 89.47% <100.00%> (ø)
.../apps/27-interchain-accounts/host/keeper/keeper.go 83.33% <100.00%> (ø)
... and 2 more

seantking
seantking approved these changes Aug 29, 2022
View reviewed changes
Copy link
Contributor

@seantking seantking left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Super clean! Really nice work :-)

colin-axner
colin-axner reviewed Aug 30, 2022
View reviewed changes
Copy link
Contributor

@colin-axner colin-axner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work, left a minor suggestion for packet data validation

modules/apps/27-interchain-accounts/controller/types/msgs.go Outdated Show resolved Hide resolved
modules/apps/27-interchain-accounts/controller/types/errors.go Outdated Show resolved Hide resolved
colin-axner
colin-axner approved these changes Aug 30, 2022
View reviewed changes
Copy link
Contributor

@colin-axner colin-axner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK, left minor nits

modules/apps/27-interchain-accounts/controller/types/msgs.go Outdated Show resolved Hide resolved
modules/apps/27-interchain-accounts/controller/types/errors.go Outdated Show resolved Hide resolved
@charleenfei charleenfei enabled auto-merge (squash) August 30, 2022 15:15
@charleenfei charleenfei merged commit 41d69d8 into main Aug 30, 2022
@charleenfei charleenfei deleted the charly/submittx_sdkMsg branch August 30, 2022 17:03
@mergify mergify bot mentioned this pull request Sep 7, 2022
Merged
mergify bot pushed a commit that referenced this pull request Sep 7, 2022
@charleenfei @mergify
chore: adding sdk.Msg impl for ics27 MsgSubmitTx (#2135)
c20927e 
* add protos/rpcs, boilerplate

(cherry picked from commit 41d69d8)
colin-axner pushed a commit that referenced this pull request Sep 8, 2022
@mergify @charleenfei
chore: adding sdk.Msg impl for ics27 MsgSubmitTx (#2135) (#2217)
45948d0 
* add protos/rpcs, boilerplate

(cherry picked from commit 41d69d8)

Co-authored-by: Charly <charly@interchain.berlin>
@chatton chatton mentioned this pull request Nov 7, 2022
Merged
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@colin-axner colin-axner colin-axner approved these changes

@seantking seantking seantking approved these changes

@AdityaSripal AdityaSripal Awaiting requested review from AdityaSripal AdityaSripal is a code owner

@damiannolan damiannolan Awaiting requested review from damiannolan damiannolan is a code owner

@crodriguezvega crodriguezvega Awaiting requested review from crodriguezvega

@chatton chatton Awaiting requested review from chatton

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Implement sdk.Msg interface for MsgSendTx
5 participants
@charleenfei @codecov-commenter @seantking @colin-axner @damiannolan