From 44814ffd0163db7f9d6eddd45b02284ad8e26750 Mon Sep 17 00:00:00 2001 From: Gjermund Garaba Date: Fri, 22 Nov 2024 13:54:10 +0100 Subject: [PATCH] fix abi encoding order --- modules/apps/transfer/types/abi.go | 19 +++-- modules/apps/transfer/types/abi_test.go | 30 ++++++- .../transfer/v2/keeper/msg_server_test.go | 58 +++++++++++-- .../04-channel/v2/types/commitment_test.go | 83 ++++++++++++++----- 4 files changed, 147 insertions(+), 43 deletions(-) diff --git a/modules/apps/transfer/types/abi.go b/modules/apps/transfer/types/abi.go index 1c1eedbffcf..a1a8be56efc 100644 --- a/modules/apps/transfer/types/abi.go +++ b/modules/apps/transfer/types/abi.go @@ -1,6 +1,7 @@ package types import ( + fmt "fmt" "math/big" "reflect" @@ -8,11 +9,12 @@ import ( "github.com/ethereum/go-ethereum/accounts/abi" ) +// TODO: Check if order matters in the struct type abiFungibleTokenPacketData struct { Denom string - Amount *big.Int Sender string Receiver string + Amount *big.Int Memo string } @@ -46,7 +48,7 @@ func DecodeABIFungibleTokenPacketData(abiEncodedData []byte) (*FungibleTokenPack // Unpack the data unpacked, err := parsedABI.Unpack(abiEncodedData) if err != nil { - return nil, err + return nil, fmt.Errorf("failed to unpack: %w", err) } unpackedData := reflect.ValueOf(unpacked[0]) @@ -65,12 +67,13 @@ func DecodeABIFungibleTokenPacketData(abiEncodedData []byte) (*FungibleTokenPack } func getFungibleTokenPacketDataABI() (abi.Arguments, error) { - abiType, err := abi.NewType("tuple", "", []abi.ArgumentMarshaling{ - {Name: "Denom", Type: "string"}, - {Name: "Amount", Type: "uint256"}, - {Name: "Sender", Type: "string"}, - {Name: "Receiver", Type: "string"}, - {Name: "Memo", Type: "string"}, + abiType, err := abi.NewType("tuple", "struct ICS20Lib.FungibleTokenPacketData", []abi.ArgumentMarshaling{ + // The order of the fields need to match the order in solidity + {Name: "Denom", Type: "string", InternalType: "string"}, + {Name: "Sender", Type: "string", InternalType: "string"}, + {Name: "Receiver", Type: "string", InternalType: "string"}, + {Name: "Amount", Type: "uint256", InternalType: "uint256"}, + {Name: "Memo", Type: "string", InternalType: "string"}, }) if err != nil { return abi.Arguments{}, err diff --git a/modules/apps/transfer/types/abi_test.go b/modules/apps/transfer/types/abi_test.go index df7017e1a4f..df22f9ad733 100644 --- a/modules/apps/transfer/types/abi_test.go +++ b/modules/apps/transfer/types/abi_test.go @@ -1,6 +1,7 @@ package types_test import ( + "encoding/hex" "testing" "github.com/stretchr/testify/require" @@ -8,19 +9,42 @@ import ( "github.com/cosmos/ibc-go/v9/modules/apps/transfer/types" ) -func TestABIFungibleTokenPacketData(t *testing.T) { +const solidityEncodedHex = "000000000000000000000000000000000000000000000000000000000000002000000000000000000000000000000000000000000000000000000000000000a000000000000000000000000000000000000000000000000000000000000000e0000000000000000000000000000000000000000000000000000000000000012000000000000000000000000000000000000000000000000000000000000f4240000000000000000000000000000000000000000000000000000000000000016000000000000000000000000000000000000000000000000000000000000000057561746f6d000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000673656e64657200000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000008726563656976657200000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000046d656d6f00000000000000000000000000000000000000000000000000000000" + +func TestEncodeABIFungibleTokenPacketData(t *testing.T) { data := types.FungibleTokenPacketData{ - Denom: "denom", + Denom: "uatom", Sender: "sender", Receiver: "receiver", - Amount: "100", + Amount: "1000000", Memo: "memo", } encodedData, err := types.EncodeABIFungibleTokenPacketData(data) require.NoError(t, err) + hexEncodedData := hex.EncodeToString(encodedData) + require.Equal(t, solidityEncodedHex, hexEncodedData) + decodedData, err := types.DecodeABIFungibleTokenPacketData(encodedData) require.NoError(t, err) require.Equal(t, data, *decodedData) } + +func TestDecodeABIFungibleTokenPacketData(t *testing.T) { + encodedData, err := hex.DecodeString(solidityEncodedHex) + require.NoError(t, err) + + data, err := types.DecodeABIFungibleTokenPacketData(encodedData) + require.NoError(t, err) + + expectedData := types.FungibleTokenPacketData{ + Denom: "uatom", + Sender: "sender", + Receiver: "receiver", + Amount: "1000000", + Memo: "memo", + } + + require.Equal(t, expectedData, *data) +} diff --git a/modules/apps/transfer/v2/keeper/msg_server_test.go b/modules/apps/transfer/v2/keeper/msg_server_test.go index f34eb42747f..33bd781188e 100644 --- a/modules/apps/transfer/v2/keeper/msg_server_test.go +++ b/modules/apps/transfer/v2/keeper/msg_server_test.go @@ -2,6 +2,7 @@ package keeper_test import ( "bytes" + "encoding/json" "time" sdkmath "cosmossdk.io/math" @@ -28,12 +29,22 @@ func (suite *KeeperTestSuite) TestMsgSendPacketTransfer() { expError error }{ { - "success", + "success: v2 payload", func() {}, nil, }, { - "success: ABI encoded payload", + "success: v1 payload", + func() { + ftpd := transfertypes.NewFungibleTokenPacketData(sdk.DefaultBondDenom, ibctesting.DefaultCoinAmount.String(), suite.chainA.SenderAccount.GetAddress().String(), suite.chainB.SenderAccount.GetAddress().String(), "") + bz, err := json.Marshal(ftpd) + suite.Require().NoError(err) + payload = channeltypesv2.NewPayload(transfertypes.ModuleName, transfertypes.ModuleName, transfertypes.V1, transfertypes.EncodingJSON, bz) + }, + nil, + }, + { + "success: v1 ABI encoded payload", func() { ftpd := transfertypes.NewFungibleTokenPacketData(sdk.DefaultBondDenom, ibctesting.DefaultCoinAmount.String(), suite.chainA.SenderAccount.GetAddress().String(), suite.chainB.SenderAccount.GetAddress().String(), "") bz, err := transfertypes.EncodeABIFungibleTokenPacketData(ftpd) @@ -110,20 +121,46 @@ func (suite *KeeperTestSuite) TestMsgRecvPacketTransfer() { path *ibctesting.Path packet channeltypesv2.Packet expectedAck channeltypesv2.Acknowledgement + sendPayload channeltypesv2.Payload ) testCases := []struct { - name string - malleate func() - expError error + name string + malleateSend func() + malleate func() + expError error }{ { - "success", + "success: v2 payload", + func() {}, + func() {}, + nil, + }, + { + "success: v1 payload", + func() { + ftpd := transfertypes.NewFungibleTokenPacketData(sdk.DefaultBondDenom, ibctesting.DefaultCoinAmount.String(), suite.chainA.SenderAccount.GetAddress().String(), suite.chainB.SenderAccount.GetAddress().String(), "") + bz, err := json.Marshal(ftpd) + suite.Require().NoError(err) + sendPayload = channeltypesv2.NewPayload(transfertypes.ModuleName, transfertypes.ModuleName, transfertypes.V1, transfertypes.EncodingJSON, bz) + }, + func() {}, + nil, + }, + { + "success: v1 ABI encoded payload", + func() { + ftpd := transfertypes.NewFungibleTokenPacketData(sdk.DefaultBondDenom, ibctesting.DefaultCoinAmount.String(), suite.chainA.SenderAccount.GetAddress().String(), suite.chainB.SenderAccount.GetAddress().String(), "") + bz, err := transfertypes.EncodeABIFungibleTokenPacketData(ftpd) + suite.Require().NoError(err) + sendPayload = channeltypesv2.NewPayload(transfertypes.ModuleName, transfertypes.ModuleName, transfertypes.V1, transfertypes.EncodingABI, bz) + }, func() {}, nil, }, { "failure: invalid destination channel on received packet", + func() {}, func() { packet.DestinationChannel = ibctesting.InvalidID }, @@ -131,6 +168,7 @@ func (suite *KeeperTestSuite) TestMsgRecvPacketTransfer() { }, { "failure: counter party channel does not match source channel", + func() {}, func() { packet.SourceChannel = ibctesting.InvalidID }, @@ -138,6 +176,7 @@ func (suite *KeeperTestSuite) TestMsgRecvPacketTransfer() { }, { "failure: receive is disabled", + func() {}, func() { expectedAck.AppAcknowledgements[0] = channeltypes.NewErrorAcknowledgement(transfertypes.ErrReceiveDisabled).Acknowledgement() suite.chainB.GetSimApp().TransferKeeperV2.SetParams(suite.chainB.GetContext(), @@ -173,9 +212,10 @@ func (suite *KeeperTestSuite) TestMsgRecvPacketTransfer() { bz := suite.chainA.Codec.MustMarshal(&ftpd) timestamp := suite.chainA.GetTimeoutTimestampSecs() - payload := channeltypesv2.NewPayload(transfertypes.ModuleName, transfertypes.ModuleName, transfertypes.V2, transfertypes.EncodingProtobuf, bz) + sendPayload = channeltypesv2.NewPayload(transfertypes.ModuleName, transfertypes.ModuleName, transfertypes.V2, transfertypes.EncodingProtobuf, bz) + tc.malleateSend() var err error - packet, err = path.EndpointA.MsgSendPacket(timestamp, payload) + packet, err = path.EndpointA.MsgSendPacket(timestamp, sendPayload) suite.Require().NoError(err) // by default, we assume a successful acknowledgement will be written. @@ -197,7 +237,7 @@ func (suite *KeeperTestSuite) TestMsgRecvPacketTransfer() { denom := transfertypes.Denom{ Base: sdk.DefaultBondDenom, Trace: []transfertypes.Hop{ - transfertypes.NewHop(payload.DestinationPort, packet.DestinationChannel), + transfertypes.NewHop(sendPayload.DestinationPort, packet.DestinationChannel), }, } diff --git a/modules/core/04-channel/v2/types/commitment_test.go b/modules/core/04-channel/v2/types/commitment_test.go index a77f93f451f..7790a9c117e 100644 --- a/modules/core/04-channel/v2/types/commitment_test.go +++ b/modules/core/04-channel/v2/types/commitment_test.go @@ -3,6 +3,7 @@ package types_test import ( "encoding/hex" "encoding/json" + fmt "fmt" "testing" "github.com/stretchr/testify/require" @@ -15,32 +16,68 @@ import ( // so that other implementations (such as the IBC Solidity) can replicate the // same commitment output. But it is also useful to catch any changes in the commitment. func TestCommitPacket(t *testing.T) { - transferData, err := json.Marshal(transfertypes.FungibleTokenPacketData{ - Denom: "uatom", - Amount: "1000000", - Sender: "sender", - Receiver: "receiver", - Memo: "memo", - }) - require.NoError(t, err) - packet := types.Packet{ - Sequence: 1, - SourceChannel: "channel-0", - DestinationChannel: "channel-1", - TimeoutTimestamp: 100, - Payloads: []types.Payload{ - { - SourcePort: transfertypes.PortID, - DestinationPort: transfertypes.PortID, - Version: transfertypes.V1, - Encoding: "application/json", - Value: transferData, + var packet types.Packet + testCases := []struct { + name string + malleate func() + expectedHash string + }{ + { + "json packet", + func() {}, // default is json packet + "450194f2ce25b12487f65593e106d91367a1e5c90b2efc03ed78265a54cfcebe", + }, + { + "abi packet", + func() { + transferData, err := transfertypes.EncodeABIFungibleTokenPacketData(transfertypes.FungibleTokenPacketData{ + Denom: "uatom", + Amount: "1000000", + Sender: "sender", + Receiver: "receiver", + Memo: "memo", + }) + fmt.Printf("transferData: %s\n", string(transferData)) + fmt.Printf("hex value: %s\n", hex.EncodeToString(transferData)) + require.NoError(t, err) + packet.Payloads[0].Value = transferData + packet.Payloads[0].Encoding = "application/x-abi" }, + "5d47f1550f95d29d73c0e0af6940be5837cbd311b3c72d99fd6cb640081e7495", }, } - commitment := types.CommitPacket(packet) - require.Equal(t, "450194f2ce25b12487f65593e106d91367a1e5c90b2efc03ed78265a54cfcebe", hex.EncodeToString(commitment)) - require.Len(t, commitment, 32) + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + transferData, err := json.Marshal(transfertypes.FungibleTokenPacketData{ + Denom: "uatom", + Amount: "1000000", + Sender: "sender", + Receiver: "receiver", + Memo: "memo", + }) + require.NoError(t, err) + packet = types.Packet{ + Sequence: 1, + SourceChannel: "channel-0", + DestinationChannel: "channel-1", + TimeoutTimestamp: 100, + Payloads: []types.Payload{ + { + SourcePort: transfertypes.PortID, + DestinationPort: transfertypes.PortID, + Version: transfertypes.V1, + Encoding: "application/json", + Value: transferData, + }, + }, + } + tc.malleate() + commitment := types.CommitPacket(packet) + require.Equal(t, tc.expectedHash, hex.EncodeToString(commitment)) + require.Len(t, commitment, 32) + }) + } } // TestCommitAcknowledgement is primarily used to document the expected commitment output