Skip to content

Commit

Permalink
adr 8 callback packet data impl followups (#3325)
Browse files Browse the repository at this point in the history
* remove query client (#3227)

* remove query client

* merge main

* go mod tidy

* Rename ``IsBound`` to ``HasCapability`` (#3253)

## Description



closes: #828


### Commit Message / Changelog Entry

```bash
imp(api!): rename `IsBound` to `HasCapability` for IBC application modules
```

see the [guidelines](https://github.com/cosmos/ibc-go/blob/main/docs/dev/pull-requests.md#commit-messages) for commit messages. (view raw markdown for examples)




---

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](https://github.com/cosmos/ibc-go/blob/main/docs/dev/pull-requests.md#pull-request-targeting)).
- [x] Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
- [x] Code follows the [module structure standards](https://github.com/cosmos/cosmos-sdk/blob/main/docs/docs/building-modules/10-structure.md) and [Go style guide](../docs/dev/go-style-guide.md).
- [ ] Wrote unit and integration [tests](https://github.com/cosmos/ibc-go/blob/main/testing/README.md#ibc-testing-package).
- [ ] Updated relevant documentation (`docs/`) or specification (`x/<module>/spec/`).
- [ ] Added relevant `godoc` [comments](https://blog.golang.org/godoc-documenting-go-code).
- [ ] Provide a [commit message](https://github.com/cosmos/ibc-go/blob/main/docs/dev/pull-requests.md#commit-messages) to be used for the changelog entry in the PR description for review.
- [ ] Re-reviewed `Files changed` in the Github PR explorer.
- [ ] Review `Codecov Report` in the comment section below once CI passes.

* chore: add support for tendermint debug log level (#3279)

* build(deps): bump cosmossdk.io/math from 1.0.0-beta.6.0.20230216172121-959ce49135e4 to 1.0.0-rc.0 (#3285)

Bumps [cosmossdk.io/math](https://github.com/cosmos/cosmos-sdk) from 1.0.0-beta.6.0.20230216172121-959ce49135e4 to 1.0.0-rc.0.
<details>
<summary>Commits</summary>
<ul>
<li>See full diff in <a href="https://github.com/cosmos/cosmos-sdk/commits/math/v1.0.0-rc.0">compare view</a></li>
</ul>
</details>
<br />


[![Dependabot compatibility score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=cosmossdk.io/math&package-manager=go_modules&previous-version=1.0.0-beta.6.0.20230216172121-959ce49135e4&new-version=1.0.0-rc.0)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores)

Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`.

[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)

---

<details>
<summary>Dependabot commands and options</summary>
<br />

You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it
- `@dependabot merge` will merge this PR after your CI passes on it
- `@dependabot squash and merge` will squash and merge this PR after your CI passes on it
- `@dependabot cancel merge` will cancel a previously requested merge and block automerging
- `@dependabot reopen` will reopen this PR if it is closed
- `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually
- `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)
- `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)
- `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)


</details>

* Write docker inspect output to diagnostics (#3291)

* chore: fix dead links (#3293)

## Description



closes: #XXXX


### Commit Message / Changelog Entry

```bash
type: commit message
```

see the [guidelines](https://github.com/cosmos/ibc-go/blob/main/docs/dev/pull-requests.md#commit-messages) for commit messages. (view raw markdown for examples)




---

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.

- [x] Targeted PR against correct branch (see [CONTRIBUTING.md](https://github.com/cosmos/ibc-go/blob/main/docs/dev/pull-requests.md#pull-request-targeting)).
- [ ] Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
- [ ] Code follows the [module structure standards](https://github.com/cosmos/cosmos-sdk/blob/main/docs/docs/building-modules/10-structure.md) and [Go style guide](../docs/dev/go-style-guide.md).
- [ ] Wrote unit and integration [tests](https://github.com/cosmos/ibc-go/blob/main/testing/README.md#ibc-testing-package).
- [ ] Updated relevant documentation (`docs/`) or specification (`x/<module>/spec/`).
- [ ] Added relevant `godoc` [comments](https://blog.golang.org/godoc-documenting-go-code).
- [ ] Provide a [commit message](https://github.com/cosmos/ibc-go/blob/main/docs/dev/pull-requests.md#commit-messages) to be used for the changelog entry in the PR description for review.
- [x] Re-reviewed `Files changed` in the Github PR explorer.
- [ ] Review `Codecov Report` in the comment section below once CI passes.

* build(deps): bump google.golang.org/protobuf from 1.29.0 to 1.29.1 (#3292)

* deps: bump SDK v0.47 (#3295)

Co-authored-by: Damian Nolan <damiannolan@gmail.com>
Co-authored-by: colin axnér <25233464+colin-axner@users.noreply.github.com>

* remove unnecessary import from doc

* chore: remove support for v3 (#3294)

* build(deps): bump actions/setup-go from 3 to 4 (#3307)

* imp: remove unnecessary defer func statements (#3304)

* Remove gogoproto yaml tags from proto files (#3290)

## Description
Refer from original issue, I removed all `yaml` tags in proto files.

closes: #3145 


### Commit Message / Changelog Entry

```bash
type: commit message
```

see the [guidelines](https://github.com/cosmos/ibc-go/blob/main/docs/dev/pull-requests.md#commit-messages) for commit messages. (view raw markdown for examples)




---

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](https://github.com/cosmos/ibc-go/blob/main/docs/dev/pull-requests.md#pull-request-targeting)).
- [ ] Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
- [ ] Code follows the [module structure standards](https://github.com/cosmos/cosmos-sdk/blob/main/docs/docs/building-modules/10-structure.md) and [Go style guide](../docs/dev/go-style-guide.md).
- [ ] Wrote unit and integration [tests](https://github.com/cosmos/ibc-go/blob/main/testing/README.md#ibc-testing-package).
- [ ] Updated relevant documentation (`docs/`) or specification (`x/<module>/spec/`).
- [ ] Added relevant `godoc` [comments](https://blog.golang.org/godoc-documenting-go-code).
- [ ] Provide a [commit message](https://github.com/cosmos/ibc-go/blob/main/docs/dev/pull-requests.md#commit-messages) to be used for the changelog entry in the PR description for review.
- [ ] Re-reviewed `Files changed` in the Github PR explorer.
- [ ] Review `Codecov Report` in the comment section below once CI passes.

* add reasoning for migration to enable localhost

* Support configuration file for e2e tests (#3260)

* E2E fzf Test Selection Autocompletion (#3313)

* post v7 release chores (#3310)

* chore: fix linter warnings (#3311)

* ADR 008: IBC Actor Callbacks (#1976)

* context and decision

* complete adr

* Apply suggestions from code review

Co-authored-by: Carlos Rodriguez <carlos@interchain.io>

* change from caller to generalized actor

* Apply suggestions from code review

Co-authored-by: colin axnér <25233464+colin-axner@users.noreply.github.com>

* create folder and scaffolded middleware

* add error handling and generify packetdata interface

* complete renaming

* add user defined gas limit and clarify pseudocode

* Clarify CallbackPacketData interface

imp: Add ADR 008: IBC Actor Callbacks

---------

Co-authored-by: Carlos Rodriguez <carlos@interchain.io>
Co-authored-by: colin axnér <25233464+colin-axner@users.noreply.github.com>

* lint: fix spelling

* chore: apply self review concerns

* chore: rename CallbackPacketDataI to CallbackPacketData

* chore: finish applying remaining review suggestions

* test: add remaining unit tests for transfer and ica

* test: add unmarshaling test

* imp: address ADR 8 review suggestions (#3319)



---------

Co-authored-by: Damian Nolan <damiannolan@gmail.com>

* Bump interchain test (#3314)

* fix: remove codec registration

* fix: build + linting

* Only run e2e on R4R (#3330)

* fix fork workflows (#3328)

* Revert "Merge branch 'main' of github.com:cosmos/ibc-go into colin/callback-packet-data-impl"

This reverts commit 1c6164b, reversing
changes made to 6f25b8e.

* chore: add optional interface godoc

* Apply suggestions from code review

Co-authored-by: Carlos Rodriguez <carlos@interchain.io>

* chore: use backticks instead of escape characters in testing

---------

Co-authored-by: Lặc <67097720+expertdicer@users.noreply.github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Cian Hatton <cian@interchain.io>
Co-authored-by: Carlos Rodriguez <carlos@interchain.io>
Co-authored-by: Damian Nolan <damiannolan@gmail.com>
Co-authored-by: GNaD13 <89174180+GNaD13@users.noreply.github.com>
Co-authored-by: Hieu Vu <72878483+hieuvubk@users.noreply.github.com>
Co-authored-by: Aditya <adityasripal@gmail.com>
  • Loading branch information
9 people authored Mar 27, 2023
1 parent 32b74ec commit f8298da
Show file tree
Hide file tree
Showing 11 changed files with 390 additions and 124 deletions.
6 changes: 0 additions & 6 deletions modules/apps/27-interchain-accounts/types/codec.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import (
sdk "github.com/cosmos/cosmos-sdk/types"
authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
"github.com/cosmos/gogoproto/proto"
"github.com/cosmos/ibc-go/v7/modules/core/exported"
)

// ModuleCdc references the global interchain accounts module codec. Note, the codec
Expand All @@ -22,11 +21,6 @@ var ModuleCdc = codec.NewProtoCodec(codectypes.NewInterfaceRegistry())
func RegisterInterfaces(registry codectypes.InterfaceRegistry) {
registry.RegisterImplementations((*authtypes.AccountI)(nil), &InterchainAccount{})
registry.RegisterImplementations((*authtypes.GenesisAccount)(nil), &InterchainAccount{})

registry.RegisterImplementations(
(*exported.CallbackPacketDataI)(nil),
&InterchainAccountPacketData{},
)
}

// SerializeCosmosTx serializes a slice of sdk.Msg's using the CosmosTx type. The sdk.Msg's are
Expand Down
37 changes: 22 additions & 15 deletions modules/apps/27-interchain-accounts/types/packet.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ var (
DefaultRelativePacketTimeoutTimestamp = uint64((time.Duration(10) * time.Minute).Nanoseconds())
)

var _ exported.CallbackPacketDataI = (*InterchainAccountPacketData)(nil)
var _ exported.CallbackPacketData = (*InterchainAccountPacketData)(nil)

// ValidateBasic performs basic validation of the interchain account packet data.
// The memo may be empty.
Expand Down Expand Up @@ -55,7 +55,7 @@ func (iapd InterchainAccountPacketData) GetBytes() []byte {
ADR-8 CallbackPacketData implementation
InterchainAccountPacketData implements CallbackPacketDataI interface. This will allow middlewares targetting specific VMs
InterchainAccountPacketData implements CallbackPacketDataI interface. This will allow middlewares targeting specific VMs
to retrieve the desired callback addresses for the ICA packet on the source and destination chains.
The Memo is used to set the desired callback addresses.
Expand All @@ -66,27 +66,32 @@ The Memo format is defined like so:
{
// ... other memo fields we don't care about
"callbacks": {
"src_callback_address": {contractAddrOnSrcChain},
"src_callback_address": {contractAddrOnSourceChain},
"dest_callback_address": {contractAddrOnDestChain},
"src_callback_msg": {jsonObjectForSrcChainCallback}, // optional field
"dest_callback_msg": {jsonObjectForDestChainCallback}, // optional field
}
// optional fields
"src_callback_msg": {jsonObjectForSourceChainCallback},
"dest_callback_msg": {jsonObjectForDestChainCallback},
}
}
```
*/

// GetSrcCallbackAddress returns the callback address on the source chain.
// ADR-8 middleware should callback on the sender address on the source chain
// if the sender address is an IBC Actor (i.e. smart contract that accepts IBC callbacks)
func (iapd InterchainAccountPacketData) GetSrcCallbackAddress() string {
// GetSourceCallbackAddress returns the source callback address provided in the packet data memo.
// If no callback address is specified, an empty string is returned.
//
// The memo is expected to specify the callback address in the following format:
// { "callbacks": { "src_callback_address": {contractAddrOnSourceChain}}
//
// ADR-8 middleware should callback on the returned address if it is a PacketActor
// (i.e. smart contract that accepts IBC callbacks).
func (iapd InterchainAccountPacketData) GetSourceCallbackAddress() string {
if len(iapd.Memo) == 0 {
return ""
}

jsonObject := make(map[string]interface{})
// the jsonObject must be a valid JSON object
err := json.Unmarshal([]byte(iapd.Memo), &jsonObject)
if err != nil {
return ""
Expand All @@ -101,17 +106,19 @@ func (iapd InterchainAccountPacketData) GetSrcCallbackAddress() string {
if !ok {
return ""
}

return callbackAddr
}

// GetDestCallbackAddress returns the callback address on the destination chain.
// ADR-8 middleware should callback on the receiver address on the destination chain
// if the receiver address is an IBC Actor (i.e. smart contract that accepts IBC callbacks)
// GetDestCallbackAddress returns an empty string. Destination callback addresses
// are not supported for ICS 27 since this feature is natively supported by
// interchain accounts host submodule transaction execution.
func (iapd InterchainAccountPacketData) GetDestCallbackAddress() string {
return ""
}

// UserDefinedGasLimit no-ops on this method to use relayer passed in gas
// UserDefinedGasLimit returns 0 (no-op). The gas limit of the executing
// transaction will be used.
func (fptd InterchainAccountPacketData) UserDefinedGasLimit() uint64 {
return 0
}
Expand Down
90 changes: 72 additions & 18 deletions modules/apps/27-interchain-accounts/types/packet_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package types_test

import (
"fmt"

"github.com/cosmos/ibc-go/v7/modules/apps/27-interchain-accounts/types"
)

Expand Down Expand Up @@ -83,11 +85,13 @@ func (suite *TypesTestSuite) TestValidateBasic() {
}
}

func (suite *TypesTestSuite) TestGetCallbackAddresses() {
func (suite *TypesTestSuite) TestGetSourceCallbackAddress() {
const expSrcCbAddr = "srcCbAddr"

testCases := []struct {
name string
packetData types.InterchainAccountPacketData
expSrcCallbackAddr string
name string
packetData types.InterchainAccountPacketData
expPass bool
}{
{
"memo is empty",
Expand All @@ -96,7 +100,7 @@ func (suite *TypesTestSuite) TestGetCallbackAddresses() {
Data: []byte("data"),
Memo: "",
},
"",
false,
},
{
"memo is not json string",
Expand All @@ -105,48 +109,98 @@ func (suite *TypesTestSuite) TestGetCallbackAddresses() {
Data: []byte("data"),
Memo: "memo",
},
"",
false,
},
{
"memo is does not have callbacks in json struct",
"memo does not have callbacks in json struct",
types.InterchainAccountPacketData{
Type: types.EXECUTE_TX,
Data: []byte("data"),
Memo: "{\"Key\": 10}",
Memo: `{"Key": 10}`,
},
"",
false,
},
{
"memo has callbacks in json struct but does not have src_callback_address key",
types.InterchainAccountPacketData{
Type: types.EXECUTE_TX,
Data: []byte("data"),
Memo: "{\"callbacks\": {\"Key\": 10}}",
Memo: `{"callbacks": {"Key": 10}}`,
},
"",
false,
},
{
"memo has callbacks in json struct but does not have string value for src_callback_address key",
types.InterchainAccountPacketData{
Type: types.EXECUTE_TX,
Data: []byte("data"),
Memo: "{\"callbacks\": {\"src_callback_address\": 10}}",
Memo: `{"callbacks": {"src_callback_address": 10}}`,
},
"",
false,
},
{
"memo has callbacks in json struct but does not have string value for src_callback_address key",
"memo has callbacks in json struct and properly formatted src_callback_address",
types.InterchainAccountPacketData{
Type: types.EXECUTE_TX,
Data: []byte("data"),
Memo: fmt.Sprintf(`{"callbacks": {"src_callback_address": "%s"}}`, expSrcCbAddr),
},
true,
},
}

for _, tc := range testCases {
tc := tc
suite.Run(tc.name, func() {
srcCbAddr := tc.packetData.GetSourceCallbackAddress()

if tc.expPass {
suite.Require().Equal(expSrcCbAddr, srcCbAddr)
} else {
suite.Require().Equal("", srcCbAddr)
}
})
}
}

func (suite *TypesTestSuite) TestGetDestCallbackAddress() {
testCases := []struct {
name string
packetData types.InterchainAccountPacketData
}{
{
"memo is empty",
types.InterchainAccountPacketData{
Type: types.EXECUTE_TX,
Data: []byte("data"),
Memo: "{\"callbacks\": {\"src_callback_address\": \"testAddress\"}}",
Memo: "",
},
},
{
"memo has dest callback address specified in json struct",
types.InterchainAccountPacketData{
Type: types.EXECUTE_TX,
Data: []byte("data"),
Memo: `{"callbacks": {"dest_callback_address": "testAddress"}}`,
},
"testAddress",
},
}

for _, tc := range testCases {
srcCbAddr := tc.packetData.GetSrcCallbackAddress()
suite.Require().Equal(tc.expSrcCallbackAddr, srcCbAddr, "%s testcase does not have expected src_callback_address", tc.name)
tc := tc
suite.Run(tc.name, func() {
destCbAddr := tc.packetData.GetDestCallbackAddress()
suite.Require().Equal("", destCbAddr)
})
}
}

func (suite *TypesTestSuite) TestUserDefinedGasLimit() {
packetData := types.InterchainAccountPacketData{
Type: types.EXECUTE_TX,
Data: []byte("data"),
Memo: `{"callbacks": {"user_defined_gas_limit": 100}}`,
}

suite.Require().Equal(uint64(0), packetData.UserDefinedGasLimit(), "user defined gas limit does not return 0")
}
13 changes: 3 additions & 10 deletions modules/apps/transfer/types/codec.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,13 @@ package types
import (
"bytes"

"github.com/cosmos/cosmos-sdk/x/authz"
"github.com/cosmos/gogoproto/jsonpb"
"github.com/cosmos/gogoproto/proto"
"github.com/cosmos/ibc-go/v7/modules/core/exported"

"github.com/cosmos/cosmos-sdk/codec"
codectypes "github.com/cosmos/cosmos-sdk/codec/types"
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/types/msgservice"
"github.com/cosmos/cosmos-sdk/x/authz"
"github.com/cosmos/gogoproto/jsonpb"
"github.com/cosmos/gogoproto/proto"
)

// RegisterLegacyAminoCodec registers the necessary x/ibc transfer interfaces and concrete types
Expand All @@ -30,11 +28,6 @@ func RegisterInterfaces(registry codectypes.InterfaceRegistry) {
&TransferAuthorization{},
)

registry.RegisterImplementations(
(*exported.CallbackPacketDataI)(nil),
&FungibleTokenPacketData{},
)

msgservice.RegisterMsgServiceDesc(registry, &_Msg_serviceDesc)
}

Expand Down
Loading

0 comments on commit f8298da

Please sign in to comment.