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

adr 8 callback packet data impl followups #3325

Merged
merged 36 commits into from
Mar 27, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
1bdb0e9
remove query client (#3227)
expertdicer Mar 14, 2023
be4c2d0
Rename ``IsBound`` to ``HasCapability`` (#3253)
expertdicer Mar 14, 2023
036565a
chore: add support for tendermint debug log level (#3279)
colin-axner Mar 14, 2023
80bab81
build(deps): bump cosmossdk.io/math from 1.0.0-beta.6.0.2023021617212…
dependabot[bot] Mar 14, 2023
2cb28d7
Write docker inspect output to diagnostics (#3291)
chatton Mar 14, 2023
02d8975
chore: fix dead links (#3293)
crodriguezvega Mar 14, 2023
796c7bc
build(deps): bump google.golang.org/protobuf from 1.29.0 to 1.29.1 (#…
dependabot[bot] Mar 15, 2023
71e8bf4
deps: bump SDK v0.47 (#3295)
crodriguezvega Mar 15, 2023
94d74f9
remove unnecessary import from doc
crodriguezvega Mar 15, 2023
b502f47
chore: remove support for v3 (#3294)
crodriguezvega Mar 17, 2023
6be595a
build(deps): bump actions/setup-go from 3 to 4 (#3307)
dependabot[bot] Mar 17, 2023
85faf55
imp: remove unnecessary defer func statements (#3304)
GNaD13 Mar 20, 2023
b4a386a
Remove gogoproto yaml tags from proto files (#3290)
hieuvubk Mar 20, 2023
b3f1b4d
add reasoning for migration to enable localhost
crodriguezvega Mar 20, 2023
cbb6859
Support configuration file for e2e tests (#3260)
chatton Mar 21, 2023
89ecf25
E2E fzf Test Selection Autocompletion (#3313)
chatton Mar 21, 2023
88f3c3c
post v7 release chores (#3310)
crodriguezvega Mar 21, 2023
5a67efc
chore: fix linter warnings (#3311)
crodriguezvega Mar 22, 2023
7d23e0f
ADR 008: IBC Actor Callbacks (#1976)
AdityaSripal Mar 22, 2023
8c16ce0
lint: fix spelling
colin-axner Mar 22, 2023
2818a57
chore: apply self review concerns
colin-axner Mar 22, 2023
8e8c64a
chore: rename CallbackPacketDataI to CallbackPacketData
colin-axner Mar 22, 2023
3cdc8b3
chore: finish applying remaining review suggestions
colin-axner Mar 22, 2023
be5a766
test: add remaining unit tests for transfer and ica
colin-axner Mar 22, 2023
767c5af
test: add unmarshaling test
colin-axner Mar 22, 2023
12c00ec
imp: address ADR 8 review suggestions (#3319)
colin-axner Mar 22, 2023
4961826
Bump interchain test (#3314)
chatton Mar 23, 2023
a02ec51
fix: remove codec registration
colin-axner Mar 23, 2023
6f25b8e
fix: build + linting
colin-axner Mar 23, 2023
f4c20e4
Only run e2e on R4R (#3330)
chatton Mar 23, 2023
8971ffc
fix fork workflows (#3328)
chatton Mar 23, 2023
1c6164b
Merge branch 'main' of github.com:cosmos/ibc-go into colin/callback-p…
colin-axner Mar 23, 2023
9435525
Revert "Merge branch 'main' of github.com:cosmos/ibc-go into colin/ca…
colin-axner Mar 23, 2023
7f88419
chore: add optional interface godoc
colin-axner Mar 23, 2023
935c5f0
Apply suggestions from code review
colin-axner Mar 27, 2023
a1252f3
chore: use backticks instead of escape characters in testing
colin-axner Mar 27, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
test: add remaining unit tests for transfer and ica
  • Loading branch information
colin-axner committed Mar 22, 2023
commit be5a766f6a98596758e82298c27652894241e168
2 changes: 1 addition & 1 deletion modules/apps/27-interchain-accounts/types/codec.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ func RegisterInterfaces(registry codectypes.InterfaceRegistry) {
registry.RegisterImplementations((*authtypes.GenesisAccount)(nil), &InterchainAccount{})

registry.RegisterImplementations(
(*exported.CallbackPacketDataI)(nil),
(*exported.CallbackPacketData)(nil),
&InterchainAccountPacketData{},
)
}
Expand Down
1 change: 1 addition & 0 deletions modules/apps/27-interchain-accounts/types/packet.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ func (iapd InterchainAccountPacketData) GetSourceCallbackAddress() string {
if !ok {
return ""
}

return callbackAddr
}

Expand Down
82 changes: 68 additions & 14 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() {
expSrcCbAddr := "srcCbAddr"
colin-axner marked this conversation as resolved.
Show resolved Hide resolved

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,7 +109,7 @@ func (suite *TypesTestSuite) TestGetCallbackAddresses() {
Data: []byte("data"),
Memo: "memo",
},
"",
false,
},
{
"memo is does not have callbacks in json struct",
colin-axner marked this conversation as resolved.
Show resolved Hide resolved
Expand All @@ -114,7 +118,7 @@ func (suite *TypesTestSuite) TestGetCallbackAddresses() {
Data: []byte("data"),
Memo: "{\"Key\": 10}",
},
"",
false,
},
{
"memo has callbacks in json struct but does not have src_callback_address key",
Expand All @@ -123,7 +127,7 @@ func (suite *TypesTestSuite) TestGetCallbackAddresses() {
Data: []byte("data"),
Memo: "{\"callbacks\": {\"Key\": 10}}",
},
"",
false,
},
{
"memo has callbacks in json struct but does not have string value for src_callback_address key",
Expand All @@ -132,21 +136,71 @@ func (suite *TypesTestSuite) TestGetCallbackAddresses() {
Data: []byte("data"),
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() {
Copy link
Member

Choose a reason for hiding this comment

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

Seems like an odd test to have for a no-op function

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If unresolved merge conflicts cause this to return a non empty string, I'd prefer the tests to catch the issue. It's quite easy for merge conflicts to slip through

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")
}
2 changes: 1 addition & 1 deletion modules/apps/transfer/types/codec.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ func RegisterInterfaces(registry codectypes.InterfaceRegistry) {
)

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

Expand Down
211 changes: 211 additions & 0 deletions modules/apps/transfer/types/packet_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package types_test

import (
"fmt"
"testing"

"github.com/stretchr/testify/require"
Expand Down Expand Up @@ -43,3 +44,213 @@ func TestFungibleTokenPacketDataValidateBasic(t *testing.T) {
}
}
}

func (suite *TypesTestSuite) TestGetSourceCallbackAddress() {
testCases := []struct {
name string
packetData types.FungibleTokenPacketData
expPass bool
}{
{
"memo is empty",
types.FungibleTokenPacketData{
Denom: denom,
Amount: amount,
Sender: addr1,
Receiver: addr2,
Memo: "",
},
false,
},
{
"memo is not json string",
types.FungibleTokenPacketData{
Denom: denom,
Amount: amount,
Sender: addr1,
Receiver: addr2,
Memo: "memo",
},
false,
},
{
"memo is does not have callbacks in json struct",
colin-axner marked this conversation as resolved.
Show resolved Hide resolved
types.FungibleTokenPacketData{
Denom: denom,
Amount: amount,
Sender: addr1,
Receiver: addr2,
Memo: "{\"Key\": 10}",
},
false,
},
{
"memo has callbacks in json struct but does not have src_callback_address key",
types.FungibleTokenPacketData{
Denom: denom,
Amount: amount,
Sender: addr1,
Receiver: addr2,
Memo: "{\"callbacks\": {\"Key\": 10}}",
},
false,
},
{
"memo has callbacks in json struct but does not have string value for src_callback_address key",
types.FungibleTokenPacketData{
Denom: denom,
Amount: amount,
Sender: addr1,
Receiver: addr2,
Memo: "{\"callbacks\": {\"src_callback_address\": 10}}",
Copy link
Contributor

Choose a reason for hiding this comment

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

don't think this is a big enough deal to change it, but I find using backticks can be more readable than requiring to escape quotes

e.g.

Memo: `{"callbacks": {"src_callback_address": 10}}`,

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh that's a lot cleaner. I will do this change when I get a chance (might not be until after this merge or monday)

},
false,
},
{
"memo has callbacks in json struct and properly formatted src_callback_address which does not match packet sender",
types.FungibleTokenPacketData{
Denom: denom,
Amount: amount,
Sender: addr1,
Receiver: addr2,
Memo: "{\"callbacks\": {\"src_callback_address\": \"testAddress\"}}",
},
false,
},
{
"valid src_callback_address specified in memo that matches sender",
types.FungibleTokenPacketData{
Denom: denom,
Amount: amount,
Sender: addr1,
Receiver: addr2,
Memo: fmt.Sprintf("{\"callbacks\": {\"src_callback_address\": \"%s\"}}", addr1),
},
true,
},
}

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

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

func (suite *TypesTestSuite) TestGetDestCallbackAddress() {
testCases := []struct {
name string
packetData types.FungibleTokenPacketData
expPass bool
}{
{
"memo is empty",
types.FungibleTokenPacketData{
Denom: denom,
Amount: amount,
Sender: addr1,
Receiver: addr2,
Memo: "",
},
false,
},
{
"memo is not json string",
types.FungibleTokenPacketData{
Denom: denom,
Amount: amount,
Sender: addr1,
Receiver: addr2,
Memo: "memo",
},
false,
},
{
"memo is does not have callbacks in json struct",
colin-axner marked this conversation as resolved.
Show resolved Hide resolved
types.FungibleTokenPacketData{
Denom: denom,
Amount: amount,
Sender: addr1,
Receiver: addr2,
Memo: "{\"Key\": 10}",
},
false,
},
{
"memo has callbacks in json struct but does not have dest_callback_address key",
types.FungibleTokenPacketData{
Denom: denom,
Amount: amount,
Sender: addr1,
Receiver: addr2,
Memo: "{\"callbacks\": {\"Key\": 10}}",
},
false,
},
{
"memo has callbacks in json struct but does not have string value for dest_callback_address key",
types.FungibleTokenPacketData{
Denom: denom,
Amount: amount,
Sender: addr1,
Receiver: addr2,
Memo: "{\"callbacks\": {\"dest_callback_address\": 10}}",
},
false,
},
{
"memo has callbacks in json struct and properly formatted dest_callback_address which does not match packet sender",
types.FungibleTokenPacketData{
Denom: denom,
Amount: amount,
Sender: addr1,
Receiver: addr2,
Memo: "{\"callbacks\": {\"dest_callback_address\": \"testAddress\"}}",
},
false,
},
{
"valid dest_callback_address specified in memo that matches sender",
types.FungibleTokenPacketData{
Denom: denom,
Amount: amount,
Sender: addr1,
Receiver: addr2,
Memo: fmt.Sprintf("{\"callbacks\": {\"dest_callback_address\": \"%s\"}}", addr2),
},
true,
},
}

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

if tc.expPass {
suite.Require().Equal(addr2, destCbAddr)
} else {
suite.Require().Equal("", destCbAddr)
}
})
}
}

func (suite *TypesTestSuite) TestUserDefinedGasLimit() {
packetData := types.FungibleTokenPacketData{
Denom: denom,
Amount: amount,
Sender: addr1,
Receiver: addr2,
Memo: "{\"callbacks\": {\"user_defined_gas_limit\": 100}}",
}

suite.Require().Equal(uint64(0), packetData.UserDefinedGasLimit(), "user defined gas limit does not return 0")
}
4 changes: 2 additions & 2 deletions modules/core/exported/codec.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import (
// RegisterInterfaces registers the CallbackPacketDataI interface.
func RegisterInterfaces(registry codectypes.InterfaceRegistry) {
registry.RegisterInterface(
"ibc.core.exported.v1.CallbackPacketDataI",
(*CallbackPacketDataI)(nil),
"ibc.core.exported.v1.CallbackPacketData",
(*CallbackPacketData)(nil),
)
}