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

Remove ibc-go fork dependency #1616

Closed
4 tasks
Taztingo opened this issue Jul 5, 2023 · 0 comments · Fixed by #1618
Closed
4 tasks

Remove ibc-go fork dependency #1616

Taztingo opened this issue Jul 5, 2023 · 0 comments · Fixed by #1618
Assignees
Milestone

Comments

@Taztingo
Copy link
Contributor

Taztingo commented Jul 5, 2023

Summary

Our repo has a custom fork of ibc-go, and it allowed us to access the SendTransfer method of the ibc-go Keeper. We should be able to replace this with a MsgTransfer and remove our dependency on a custom ibc-go fork.

Our custom implementation checks the following on an ibc transfer and skips the IsSendEnabledCoins check.

Original:
https://github.com/cosmos/ibc-go/blob/fa3116f0bd885b0ef4d7e974cb2918f9b7e4ff87/modules/apps/transfer/keeper/msg_server.go#L18-L33

Ours:

if !ibcKeeper.GetSendEnabled(ctx) {
return false, ibctypes.ErrSendDisabled
}
if ibcKeeper.BankKeeper.BlockedAddr(sender) {
return false, sdkerrors.ErrUnauthorized.Wrapf("%s is not allowed to send funds", sender)
}

This check should not be an issue anymore because we have the Bypass Context:
https://github.com/provenance-io/provenance/blob/de83db67fbda994a6c63998d4496a95c45a01bcd/x/marker/keeper/marker.go#L709C3-L709C24

Problem Definition

Our go.mod contains a reference to our ibc-go repo, and we no longer need it.

Proposal

Update go.mod to point to ibc-go v6.1.1
Pass in and store a baseapp.IMsgServiceRouter to the Marker Keeper
Update the marker keeper's IbcTransfer method to send a MsgTransfer and use the IMsgServiceRouter


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant