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

Rename IBCAccountPacketData, Remove TxRaw #456

Merged
merged 2 commits into from
Oct 4, 2021
Merged

Rename IBCAccountPacketData, Remove TxRaw #456

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
df17bed
rename IBCAccountPacketData, remove txRaw type
colin-axner Sep 30, 2021
caeb970
Update proto/ibc/applications/interchain_accounts/v1/types.proto
colin-axner Sep 30, 2021
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
29 changes: 7 additions & 22 deletions docs/ibc/proto-docs.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,8 @@
- [Query](#ibc.applications.interchain_accounts.v1.Query)

- [ibc/applications/interchain_accounts/v1/types.proto](#ibc/applications/interchain_accounts/v1/types.proto)
- [IBCAccountPacketData](#ibc.applications.interchain_accounts.v1.IBCAccountPacketData)
- [IBCTxBody](#ibc.applications.interchain_accounts.v1.IBCTxBody)
- [IBCTxRaw](#ibc.applications.interchain_accounts.v1.IBCTxRaw)
- [InterchainAccountPacketData](#ibc.applications.interchain_accounts.v1.InterchainAccountPacketData)

- [Type](#ibc.applications.interchain_accounts.v1.Type)

Expand Down Expand Up @@ -399,22 +398,6 @@ Query defines the gRPC querier service.



<a name="ibc.applications.interchain_accounts.v1.IBCAccountPacketData"></a>

### IBCAccountPacketData
Packet data is comprised of raw transaction & type of transaction


| Field | Type | Label | Description |
| ----- | ---- | ----- | ----------- |
| `type` | [Type](#ibc.applications.interchain_accounts.v1.Type) | | |
| `data` | [bytes](#bytes) | | |






<a name="ibc.applications.interchain_accounts.v1.IBCTxBody"></a>

### IBCTxBody
Expand All @@ -430,15 +413,17 @@ Body of a tx for an ics27 IBC packet



<a name="ibc.applications.interchain_accounts.v1.IBCTxRaw"></a>
<a name="ibc.applications.interchain_accounts.v1.InterchainAccountPacketData"></a>

### IBCTxRaw
Raw tx body
### InterchainAccountPacketData
InterchainAccountPacketData is comprised of araw transaction,type of transaction and optional memo field.


| Field | Type | Label | Description |
| ----- | ---- | ----- | ----------- |
| `body_bytes` | [bytes](#bytes) | | |
| `type` | [Type](#ibc.applications.interchain_accounts.v1.Type) | | |
| `data` | [bytes](#bytes) | | |
| `memo` | [string](#string) | | |



Expand Down
4 changes: 2 additions & 2 deletions modules/apps/27-interchain-accounts/ibc_module.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ func (im IBCModule) OnRecvPacket(
) ibcexported.Acknowledgement {
ack := channeltypes.NewResultAcknowledgement([]byte{byte(1)})

var data types.IBCAccountPacketData
var data types.InterchainAccountPacketData
if err := types.ModuleCdc.UnmarshalJSON(packet.GetData(), &data); err != nil {
ack = channeltypes.NewErrorAcknowledgement(fmt.Sprintf("cannot unmarshal ICS-27 interchain account packet data: %s", err.Error()))
}
Expand Down Expand Up @@ -127,7 +127,7 @@ func (im IBCModule) OnAcknowledgementPacket(
if err := types.ModuleCdc.UnmarshalJSON(acknowledgement, &ack); err != nil {
return sdkerrors.Wrapf(sdkerrors.ErrUnknownRequest, "cannot unmarshal ICS-27 interchain account packet acknowledgment: %v", err)
}
var data types.IBCAccountPacketData
var data types.InterchainAccountPacketData
if err := types.ModuleCdc.UnmarshalJSON(packet.GetData(), &data); err != nil {
return sdkerrors.Wrapf(sdkerrors.ErrUnknownRequest, "cannot unmarshal ICS-27 interchain account packet data: %s", err.Error())
}
Expand Down
6 changes: 1 addition & 5 deletions modules/apps/27-interchain-accounts/keeper/keeper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,11 +83,7 @@ func (k Keeper) SerializeCosmosTx(cdc codec.BinaryCodec, data interface{}) ([]by
Messages: msgAnys,
}

txRaw := &types.IBCTxRaw{
BodyBytes: cdc.MustMarshal(txBody),
}

bz, err := cdc.Marshal(txRaw)
bz, err := cdc.Marshal(txBody)
if err != nil {
return nil, err
}
Expand Down
24 changes: 9 additions & 15 deletions modules/apps/27-interchain-accounts/keeper/relay.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import (

// TODO: implement middleware functionality, this will allow us to use capabilities to
// manage helper module access to owner addresses they do not have capabilities for
func (k Keeper) TrySendTx(ctx sdk.Context, portID string, data interface{}) ([]byte, error) {
func (k Keeper) TrySendTx(ctx sdk.Context, portID string, data interface{}, memo string) ([]byte, error) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

should we take in the type as well so we don't have to change the api later?

Copy link
Contributor

Choose a reason for hiding this comment

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

Great point. I guess right now it's just hardcoded to always be EXECUTE. I guess there is no way to interpret the type based on the data (which the more I think about it may be anything, not just a list of messages).

I think it probably makes sense to let the caller pass in the type here. 👍

// Check for the active channel
activeChannelId, found := k.GetActiveChannel(ctx, portID)
if !found {
Expand All @@ -30,7 +30,7 @@ func (k Keeper) TrySendTx(ctx sdk.Context, portID string, data interface{}) ([]b
destinationPort := sourceChannelEnd.GetCounterparty().GetPortID()
destinationChannel := sourceChannelEnd.GetCounterparty().GetChannelID()

return k.createOutgoingPacket(ctx, portID, activeChannelId, destinationPort, destinationChannel, data)
return k.createOutgoingPacket(ctx, portID, activeChannelId, destinationPort, destinationChannel, data, memo)
}

func (k Keeper) createOutgoingPacket(
Expand All @@ -40,6 +40,7 @@ func (k Keeper) createOutgoingPacket(
destinationPort,
destinationChannel string,
data interface{},
memo string,
) ([]byte, error) {
if data == nil {
return []byte{}, types.ErrInvalidOutgoingData
Expand All @@ -61,9 +62,10 @@ func (k Keeper) createOutgoingPacket(
return []byte{}, channeltypes.ErrSequenceSendNotFound
}

packetData := types.IBCAccountPacketData{
packetData := types.InterchainAccountPacketData{
Type: types.EXECUTE_TX,
Data: txBytes,
Memo: memo,
}

// timeoutTimestamp is set to be a max number here so that we never recieve a timeout
Expand All @@ -85,17 +87,9 @@ func (k Keeper) createOutgoingPacket(
}

func (k Keeper) DeserializeTx(_ sdk.Context, txBytes []byte) ([]sdk.Msg, error) {
var txRaw types.IBCTxRaw

err := k.cdc.Unmarshal(txBytes, &txRaw)
if err != nil {
return nil, err
}

var txBody types.IBCTxBody

err = k.cdc.Unmarshal(txRaw.BodyBytes, &txBody)
if err != nil {
if err := k.cdc.Unmarshal(txBytes, &txBody); err != nil {
return nil, err
}

Expand Down Expand Up @@ -189,7 +183,7 @@ func (k Keeper) ComputeVirtualTxHash(txBytes []byte, seq uint64) []byte {
}

func (k Keeper) OnRecvPacket(ctx sdk.Context, packet channeltypes.Packet) error {
var data types.IBCAccountPacketData
var data types.InterchainAccountPacketData

if err := types.ModuleCdc.UnmarshalJSON(packet.GetData(), &data); err != nil {
return sdkerrors.Wrapf(types.ErrUnknownPacketData, "cannot unmarshal ICS-27 interchain account packet data")
Expand All @@ -213,7 +207,7 @@ func (k Keeper) OnRecvPacket(ctx sdk.Context, packet channeltypes.Packet) error
}
}

func (k Keeper) OnAcknowledgementPacket(ctx sdk.Context, packet channeltypes.Packet, data types.IBCAccountPacketData, ack channeltypes.Acknowledgement) error {
func (k Keeper) OnAcknowledgementPacket(ctx sdk.Context, packet channeltypes.Packet, data types.InterchainAccountPacketData, ack channeltypes.Acknowledgement) error {
switch ack.Response.(type) {
case *channeltypes.Acknowledgement_Error:
if k.hook != nil {
Expand All @@ -232,7 +226,7 @@ func (k Keeper) OnAcknowledgementPacket(ctx sdk.Context, packet channeltypes.Pac
}
}

func (k Keeper) OnTimeoutPacket(ctx sdk.Context, packet channeltypes.Packet, data types.IBCAccountPacketData) error {
func (k Keeper) OnTimeoutPacket(ctx sdk.Context, packet channeltypes.Packet, data types.InterchainAccountPacketData) error {
if k.hook != nil {
k.hook.OnTxFailed(ctx, packet.SourcePort, packet.SourceChannel, k.ComputeVirtualTxHash(data.Data, packet.Sequence), data.Data)
}
Expand Down
26 changes: 7 additions & 19 deletions modules/apps/27-interchain-accounts/keeper/relay_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ func (suite *KeeperTestSuite) TestTrySendTx() {
suite.SetupTest() // reset
path = NewICAPath(suite.chainA, suite.chainB)
suite.coordinator.SetupConnections(path)
memo := "memo"

err := suite.SetupICAPath(path, TestOwnerAddress)
suite.Require().NoError(err)
Expand All @@ -89,7 +90,7 @@ func (suite *KeeperTestSuite) TestTrySendTx() {

tc.malleate()

_, err = suite.chainA.GetSimApp().ICAKeeper.TrySendTx(suite.chainA.GetContext(), portID, msg)
_, err = suite.chainA.GetSimApp().ICAKeeper.TrySendTx(suite.chainA.GetContext(), portID, msg, memo)

if tc.expPass {
suite.Require().NoError(err)
Expand Down Expand Up @@ -124,28 +125,15 @@ func (suite *KeeperTestSuite) TestOnRecvPacket() {
txBytes, err := suite.chainA.GetSimApp().ICAKeeper.SerializeCosmosTx(suite.chainA.Codec, msg)
suite.Require().NoError(err)

data := types.IBCAccountPacketData{Type: types.EXECUTE_TX,
data := types.InterchainAccountPacketData{Type: types.EXECUTE_TX,
Data: txBytes}
packetData = data.GetBytes()
}, true,
},
{
"Cannot deserialize txBytes", func() {
txBytes = []byte("invalid tx bytes")
data := types.IBCAccountPacketData{Type: types.EXECUTE_TX,
Data: txBytes}
packetData = data.GetBytes()
}, false,
},
{
"Cannot deserialize txBytes: invalid IBCTxRaw", func() {
txBody := []byte("invalid tx body")
txRaw := &types.IBCTxRaw{
BodyBytes: txBody,
}

txBytes = suite.chainB.Codec.MustMarshal(txRaw)
data := types.IBCAccountPacketData{Type: types.EXECUTE_TX,
data := types.InterchainAccountPacketData{Type: types.EXECUTE_TX,
Data: txBytes}
packetData = data.GetBytes()
}, false,
Expand All @@ -155,13 +143,13 @@ func (suite *KeeperTestSuite) TestOnRecvPacket() {
txBytes = []byte{}
// Type here is an ENUM
// Valid type is types.EXECUTE_TX
data := types.IBCAccountPacketData{Type: 100,
data := types.InterchainAccountPacketData{Type: 100,
Data: txBytes}
packetData = data.GetBytes()
}, false,
},
{
"Cannot unmarshal interchain account packet data into types.IBCAccountPacketData", func() {
"Cannot unmarshal interchain account packet data into types.InterchainAccountPacketData", func() {
packetData = []byte{}
}, false,
},
Expand All @@ -179,7 +167,7 @@ func (suite *KeeperTestSuite) TestOnRecvPacket() {
// build packet data
txBytes, err := suite.chainA.GetSimApp().ICAKeeper.SerializeCosmosTx(suite.chainA.Codec, msg)
suite.Require().NoError(err)
data := types.IBCAccountPacketData{Type: types.EXECUTE_TX,
data := types.InterchainAccountPacketData{Type: types.EXECUTE_TX,
Data: txBytes}
packetData = data.GetBytes()
}, false,
Expand Down
2 changes: 1 addition & 1 deletion modules/apps/27-interchain-accounts/types/packet.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,6 @@ import (
sdk "github.com/cosmos/cosmos-sdk/types"
)

func (packet IBCAccountPacketData) GetBytes() []byte {
func (packet InterchainAccountPacketData) GetBytes() []byte {
return sdk.MustSortJSON(ModuleCdc.MustMarshalJSON(&packet))
}
Loading