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

MsgTransfer For Eureka #7957

Merged
merged 13 commits into from
Feb 24, 2025
Merged

MsgTransfer For Eureka #7957

merged 13 commits into from
Feb 24, 2025

Conversation

AdityaSripal
Copy link
Member

@AdityaSripal AdityaSripal commented Feb 11, 2025

Description

closes: #7958


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 the 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 and Go style guide.
  • Wrote unit and integration tests.
  • Updated relevant documentation (docs/).
  • Added relevant godoc comments.
  • Provide a conventional commit message to follow the repository standards.
  • Include a descriptive changelog entry when appropriate. This may be left to the discretion of the PR reviewers. (e.g. chores should be omitted from changelog)
  • Re-reviewed Files changed in the GitHub PR explorer.
  • Review SonarCloud Report in the comment section below once CI passes.

// ClientKeeper defines the expected IBC client keeper
type ClientKeeper interface {
GetClientConsensusState(ctx sdk.Context, clientID string) (connection ibcexported.ConsensusState, found bool)
Copy link
Member Author

Choose a reason for hiding this comment

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

nonsensical method not used

Comment on lines 39 to 44
_, isIBCV2 = k.clientKeeperV2.GetClientCounterparty(ctx, msg.SourceChannel)
if !isIBCV2 {
var found bool
channel, found = k.channelKeeper.GetChannel(ctx, msg.SourcePort, msg.SourceChannel)
if !found {
return nil, errorsmod.Wrapf(channeltypes.ErrChannelNotFound, "port ID (%s) channel ID (%s)", msg.SourcePort, msg.SourceChannel)
Copy link
Member Author

Choose a reason for hiding this comment

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

i could choose to not have clientKeeperV2 in transfer keeper and just send the channelID from the msg directly to chanKeeperV2 and have it error if counterparty doesn't exist. But then we get a counterparty not found error over the channel not found error

if isIBCV2 {
encoding := msg.Encoding
if encoding == "" {
encoding = types.EncodingJSON
Copy link
Member Author

Choose a reason for hiding this comment

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

default to JSON since that is expected ics20-v1 behaviour

sender.String(), payload,
)

res, err := k.channelKeeperV2.SendPacket(ctx, msg)
Copy link
Member Author

Choose a reason for hiding this comment

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

SendPacket will call transfer module's OnSendPacket

@@ -61,6 +61,27 @@ func NewMsgTransfer(
}
}

// NewMsgTransferWithEncoding creates a new MsgTransfer instance
// with the provided encoding
func NewMsgTransferWithEncoding(
Copy link
Member Author

Choose a reason for hiding this comment

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

Allow users to pass in custom encoding

@@ -289,6 +290,15 @@ func (suite *TransferTestSuite) TestOnAckPacket() {
suite.Require().True(ok)
originalCoin := sdk.NewCoin(tc.sourceDenomToTransfer, amount)

msg := types.NewMsgTransferWithEncoding(
Copy link
Member Author

Choose a reason for hiding this comment

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

Use the transfer endpoint rather than channelKeeper.SendPacket

ics4Wrapper porttypes.ICS4Wrapper
channelKeeper types.ChannelKeeper
channelKeeperV2 types.ChannelKeeperV2
clientKeeperV2 types.ClientKeeperV2
Copy link
Member Author

Choose a reason for hiding this comment

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

I could remove this if desired

Copy link
Contributor

Choose a reason for hiding this comment

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

It would be nice to have less stuff to put in here, but what would that mean for the code?

@gjermundgaraba
Copy link
Contributor

I reviewed this yesterday, and came to the conclusion that I want to have a more indepth disucssion on how we do this one, because I am not too comfortable with the flag paths needed to make this work.

@AdityaSripal
Copy link
Member Author

refactored to have most of the logic pulled out in 2 different fns. If its still not desirable, we can revisit the approach

@AdityaSripal
Copy link
Member Author

Not sure why there's so much proto-gen diffs

Copy link
Contributor

@gjermundgaraba gjermundgaraba left a comment

Choose a reason for hiding this comment

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

I think it looks good now, but left a few comments and questions we should resolve before merging

ics4Wrapper porttypes.ICS4Wrapper
channelKeeper types.ChannelKeeper
channelKeeperV2 types.ChannelKeeperV2
clientKeeperV2 types.ClientKeeperV2
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be nice to have less stuff to put in here, but what would that mean for the code?

Copy link

@gjermundgaraba gjermundgaraba added this pull request to the merge queue Feb 24, 2025
Merged via the queue into main with commit 9f677ba Feb 24, 2025
58 of 59 checks passed
@gjermundgaraba gjermundgaraba deleted the aditya/msgtransfer-eureka branch February 24, 2025 08:27
mergify bot pushed a commit that referenced this pull request Feb 24, 2025
* initial progress

* progress

* initial build and existing tests pass

* testing

* documentation for tests

* refactor

* fix regression

* Update modules/apps/transfer/types/packet.go

Co-authored-by: Gjermund Garaba <[email protected]>

* remove clientKeeperv2 necessity in code

* remove clientkeeperv2 in transfer keeper

---------

Co-authored-by: Gjermund Garaba <[email protected]>
(cherry picked from commit 9f677ba)
@mergify mergify bot mentioned this pull request Feb 24, 2025
10 tasks
gjermundgaraba pushed a commit that referenced this pull request Feb 24, 2025
* initial progress

* progress

* initial build and existing tests pass

* testing

* documentation for tests

* refactor

* fix regression

* Update modules/apps/transfer/types/packet.go

Co-authored-by: Gjermund Garaba <[email protected]>

* remove clientKeeperv2 necessity in code

* remove clientkeeperv2 in transfer keeper

---------

Co-authored-by: Gjermund Garaba <[email protected]>
(cherry picked from commit 9f677ba)

Co-authored-by: Aditya <[email protected]>
mergify bot pushed a commit that referenced this pull request Feb 26, 2025
* initial progress

* progress

* initial build and existing tests pass

* testing

* documentation for tests

* refactor

* fix regression

* Update modules/apps/transfer/types/packet.go

Co-authored-by: Gjermund Garaba <[email protected]>

* remove clientKeeperv2 necessity in code

* remove clientkeeperv2 in transfer keeper

---------

Co-authored-by: Gjermund Garaba <[email protected]>
(cherry picked from commit 9f677ba)

# Conflicts:
#	modules/apps/transfer/types/tx.pb.go
mergify bot pushed a commit that referenced this pull request Feb 26, 2025
* initial progress

* progress

* initial build and existing tests pass

* testing

* documentation for tests

* refactor

* fix regression

* Update modules/apps/transfer/types/packet.go

Co-authored-by: Gjermund Garaba <[email protected]>

* remove clientKeeperv2 necessity in code

* remove clientkeeperv2 in transfer keeper

---------

Co-authored-by: Gjermund Garaba <[email protected]>
(cherry picked from commit 9f677ba)
gjermundgaraba added a commit that referenced this pull request Feb 26, 2025
* MsgTransfer For Eureka (#7957) (#8001)

* initial progress

* progress

* initial build and existing tests pass

* testing

* documentation for tests

* refactor

* fix regression

* Update modules/apps/transfer/types/packet.go

Co-authored-by: Gjermund Garaba <[email protected]>

* remove clientKeeperv2 necessity in code

* remove clientkeeperv2 in transfer keeper

---------

Co-authored-by: Gjermund Garaba <[email protected]>
(cherry picked from commit 9f677ba)

Co-authored-by: Aditya <[email protected]>

* Remove 29-fee (#8000) (#8002)

* rm 29-fee

* rm docs

* lint

* add back test

* doc link fix

(cherry picked from commit 972afc2)

Co-authored-by: Aditya <[email protected]>

* feat: add cli commands for client v2 counterparty (#7997) (#8007)

* feat: add client counterparty cli commands

* chore: make wasm simapp possible to run more than once

* add query to cli

* lint

(cherry picked from commit eeb8b8a)

Co-authored-by: Gjermund Garaba <[email protected]>

* chore: add deprecated functions for backwards compat (#8005) (#8009)

* chore: add back some deprecated functions

* Added DenomPathFromHash as deprecated function

* fix docstring

(cherry picked from commit 7bd0c6d)

Co-authored-by: Gjermund Garaba <[email protected]>

* chore: add backport mergify tasks (#7989) (#8015)

* chore: add backport mergify tasks

* add wasm ibc v10 to mergify backports

(cherry picked from commit 11488cf)

Co-authored-by: Gjermund Garaba <[email protected]>

* chore: use ibc v2 name consistently (#8006) (#8017)

(cherry picked from commit a71577c)

Co-authored-by: Gjermund Garaba <[email protected]>

* callbacks: Error on RecvPacket callback execution (#8014) (#8020)

* change behavior of recvPacket

* fix tests

* error on malformed callback data

* CHANGELOG

---------

Co-authored-by: Gjermund Garaba <[email protected]>
(cherry picked from commit c5a36e4)

Co-authored-by: Aditya <[email protected]>

* chore(deps): bump google.golang.org/protobuf from 1.36.4 to 1.36.5 (#7920) (#8021)

* chore(deps): bump google.golang.org/protobuf from 1.36.4 to 1.36.5

Bumps google.golang.org/protobuf from 1.36.4 to 1.36.5.

---
updated-dependencies:
- dependency-name: google.golang.org/protobuf
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>

* tidy

---------

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Gjermund Garaba <[email protected]>
(cherry picked from commit af5b171)

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* chore(deps): bump golangci/golangci-lint-action from 6.2.0 to 6.4.1 (#7969) (#8022)

Bumps [golangci/golangci-lint-action](https://github.com/golangci/golangci-lint-action) from 6.2.0 to 6.4.1.
- [Release notes](https://github.com/golangci/golangci-lint-action/releases)
- [Commits](golangci/golangci-lint-action@v6.2.0...v6.4.1)

---
updated-dependencies:
- dependency-name: golangci/golangci-lint-action
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: srdtrk <[email protected]>
(cherry picked from commit 1f11107)

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Gjermund Garaba <[email protected]>

* chore: remove remnants of ics20-2 (#8016) (#8045)

* chore: remove remnants of ics20-2

* rename FungibleTokenPacketDataV2 to make it clear that its an internal type

* fix broken link

(cherry picked from commit 40f3240)

Co-authored-by: Gjermund Garaba <[email protected]>

* chore(deps): bump JamesIves/github-pages-deploy-action (#7991) (#8048)

Bumps [JamesIves/github-pages-deploy-action](https://github.com/jamesives/github-pages-deploy-action) from 4.7.2 to 4.7.3.
- [Release notes](https://github.com/jamesives/github-pages-deploy-action/releases)
- [Commits](JamesIves/github-pages-deploy-action@v4.7.2...v4.7.3)

---
updated-dependencies:
- dependency-name: JamesIves/github-pages-deploy-action
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
(cherry picked from commit 205e6af)

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* chore(deps): bump golangci/golangci-lint-action from 6.4.1 to 6.5.0 (#7983) (#8049)

Bumps [golangci/golangci-lint-action](https://github.com/golangci/golangci-lint-action) from 6.4.1 to 6.5.0.
- [Release notes](https://github.com/golangci/golangci-lint-action/releases)
- [Commits](golangci/golangci-lint-action@v6.4.1...v6.5.0)

---
updated-dependencies:
- dependency-name: golangci/golangci-lint-action
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Gjermund Garaba <[email protected]>
(cherry picked from commit 272c09b)

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* feat: remove channel upgradability (#8008) (#8053)

* feat: remove channel upgradability

* lint

* basic migration setup

* feat: remove channel upgradeability

* fix docs

* register v10 migration in app module

* error on upgrading channels + code review fixes

(cherry picked from commit f7b24e1)

Co-authored-by: Gjermund Garaba <[email protected]>

---------

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Co-authored-by: Aditya <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
gjermundgaraba added a commit that referenced this pull request Feb 26, 2025
* chore: update sonarqube gh action (#7985) (#7988)

(cherry picked from commit fcb6809)

Co-authored-by: Gjermund Garaba <[email protected]>

* test: fix ica compatibility tests (#7976) (#7994)

* test: fix ica compatibility tests

* lint

* fix event search

* lint

(cherry picked from commit 22efa6c)

Co-authored-by: Gjermund Garaba <[email protected]>

* docs: document ICS23 MerklePath (#7993) (#7995)

* proto docs

* more go docs

(cherry picked from commit 0e8a6c9)

Co-authored-by: Aditya <[email protected]>

* MsgTransfer For Eureka (#7957) (#8001)

* initial progress

* progress

* initial build and existing tests pass

* testing

* documentation for tests

* refactor

* fix regression

* Update modules/apps/transfer/types/packet.go

Co-authored-by: Gjermund Garaba <[email protected]>

* remove clientKeeperv2 necessity in code

* remove clientkeeperv2 in transfer keeper

---------

Co-authored-by: Gjermund Garaba <[email protected]>
(cherry picked from commit 9f677ba)

Co-authored-by: Aditya <[email protected]>

* Remove 29-fee (#8000) (#8002)

* rm 29-fee

* rm docs

* lint

* add back test

* doc link fix

(cherry picked from commit 972afc2)

Co-authored-by: Aditya <[email protected]>

* feat: add cli commands for client v2 counterparty (#7997) (#8007)

* feat: add client counterparty cli commands

* chore: make wasm simapp possible to run more than once

* add query to cli

* lint

(cherry picked from commit eeb8b8a)

Co-authored-by: Gjermund Garaba <[email protected]>

* chore: add deprecated functions for backwards compat (#8005) (#8009)

* chore: add back some deprecated functions

* Added DenomPathFromHash as deprecated function

* fix docstring

(cherry picked from commit 7bd0c6d)

Co-authored-by: Gjermund Garaba <[email protected]>

* chore: add backport mergify tasks (#7989) (#8015)

* chore: add backport mergify tasks

* add wasm ibc v10 to mergify backports

(cherry picked from commit 11488cf)

Co-authored-by: Gjermund Garaba <[email protected]>

* chore: use ibc v2 name consistently (#8006) (#8017)

(cherry picked from commit a71577c)

Co-authored-by: Gjermund Garaba <[email protected]>

* callbacks: Error on RecvPacket callback execution (#8014) (#8020)

* change behavior of recvPacket

* fix tests

* error on malformed callback data

* CHANGELOG

---------

Co-authored-by: Gjermund Garaba <[email protected]>
(cherry picked from commit c5a36e4)

Co-authored-by: Aditya <[email protected]>

* chore(deps): bump google.golang.org/protobuf from 1.36.4 to 1.36.5 (#7920) (#8021)

* chore(deps): bump google.golang.org/protobuf from 1.36.4 to 1.36.5

Bumps google.golang.org/protobuf from 1.36.4 to 1.36.5.

---
updated-dependencies:
- dependency-name: google.golang.org/protobuf
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>

* tidy

---------

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Gjermund Garaba <[email protected]>
(cherry picked from commit af5b171)

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* chore(deps): bump golangci/golangci-lint-action from 6.2.0 to 6.4.1 (#7969) (#8022)

Bumps [golangci/golangci-lint-action](https://github.com/golangci/golangci-lint-action) from 6.2.0 to 6.4.1.
- [Release notes](https://github.com/golangci/golangci-lint-action/releases)
- [Commits](golangci/golangci-lint-action@v6.2.0...v6.4.1)

---
updated-dependencies:
- dependency-name: golangci/golangci-lint-action
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: srdtrk <[email protected]>
(cherry picked from commit 1f11107)

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Gjermund Garaba <[email protected]>

* chore: remove remnants of ics20-2 (#8016) (#8045)

* chore: remove remnants of ics20-2

* rename FungibleTokenPacketDataV2 to make it clear that its an internal type

* fix broken link

(cherry picked from commit 40f3240)

Co-authored-by: Gjermund Garaba <[email protected]>

* chore(deps): bump JamesIves/github-pages-deploy-action (#7991) (#8048)

Bumps [JamesIves/github-pages-deploy-action](https://github.com/jamesives/github-pages-deploy-action) from 4.7.2 to 4.7.3.
- [Release notes](https://github.com/jamesives/github-pages-deploy-action/releases)
- [Commits](JamesIves/github-pages-deploy-action@v4.7.2...v4.7.3)

---
updated-dependencies:
- dependency-name: JamesIves/github-pages-deploy-action
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
(cherry picked from commit 205e6af)

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* chore(deps): bump golangci/golangci-lint-action from 6.4.1 to 6.5.0 (#7983) (#8049)

Bumps [golangci/golangci-lint-action](https://github.com/golangci/golangci-lint-action) from 6.4.1 to 6.5.0.
- [Release notes](https://github.com/golangci/golangci-lint-action/releases)
- [Commits](golangci/golangci-lint-action@v6.4.1...v6.5.0)

---
updated-dependencies:
- dependency-name: golangci/golangci-lint-action
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Gjermund Garaba <[email protected]>
(cherry picked from commit 272c09b)

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* feat: remove channel upgradability (#8008) (#8053)

* feat: remove channel upgradability

* lint

* basic migration setup

* feat: remove channel upgradeability

* fix docs

* register v10 migration in app module

* error on upgrading channels + code review fixes

(cherry picked from commit f7b24e1)

Co-authored-by: Gjermund Garaba <[email protected]>

---------

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Co-authored-by: Aditya <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Enable using MsgTransfer for Eureka connections
2 participants