From 027485029f74a9cce3f486a352343a8a1f1a79b5 Mon Sep 17 00:00:00 2001 From: Michael Tsitrin <114929630+mtsitrin@users.noreply.github.com> Date: Tue, 18 Jun 2024 14:40:46 +0300 Subject: [PATCH] feat(eibc): auto create eibc demand order for rollapp packets (#944) --- ibctesting/eibc_test.go | 65 ++++++++---- x/delayedack/types/errors.go | 3 +- x/delayedack/types/packet_metadata.go | 12 +-- x/eibc/keeper/handler.go | 21 ++-- x/eibc/keeper/handler_test.go | 82 +++++++++++++++ x/eibc/keeper/msg_server_test.go | 137 +++++++++++++------------- 6 files changed, 212 insertions(+), 108 deletions(-) create mode 100644 x/eibc/keeper/handler_test.go diff --git a/ibctesting/eibc_test.go b/ibctesting/eibc_test.go index 304aa05c9..f40c62e1d 100644 --- a/ibctesting/eibc_test.go +++ b/ibctesting/eibc_test.go @@ -18,6 +18,7 @@ import ( channeltypes "github.com/cosmos/ibc-go/v6/modules/core/04-channel/types" ibctesting "github.com/cosmos/ibc-go/v6/testing" + "github.com/dymensionxyz/dymension/v3/app/apptesting" commontypes "github.com/dymensionxyz/dymension/v3/x/common/types" eibckeeper "github.com/dymensionxyz/dymension/v3/x/eibc/keeper" eibctypes "github.com/dymensionxyz/dymension/v3/x/eibc/types" @@ -68,61 +69,69 @@ func (suite *EIBCTestSuite) TestEIBCDemandOrderCreation() { name string amount string fee string - recipient string demandOrdersCreated int expectAck bool extraMemoData map[string]map[string]string + skipEIBCmemo bool }{ { "valid demand order", "1000000000", "150", - suite.hubChain.SenderAccount.GetAddress().String(), 1, false, map[string]map[string]string{}, + false, + }, + { + "valid demand order - fee is 0", + "1000000000", + "0", + 1, + false, + map[string]map[string]string{}, + false, + }, + { + "valid demand order - auto created", + "1000000000", + "0", + 1, + false, + map[string]map[string]string{}, + true, }, { "invalid demand order - negative fee", "1000000000", "-150", - suite.hubChain.SenderAccount.GetAddress().String(), 0, true, map[string]map[string]string{}, + false, }, { "invalid demand order - fee > amount", "1000", "1001", - suite.hubChain.SenderAccount.GetAddress().String(), - 0, - true, - map[string]map[string]string{}, - }, - { - "invalid demand order - fee is 0", - "1", - "0", - suite.hubChain.SenderAccount.GetAddress().String(), 0, true, map[string]map[string]string{}, + false, }, { "invalid demand order - fee > max uint64", "10000", "100000000000000000000000000000", - suite.hubChain.SenderAccount.GetAddress().String(), 0, true, map[string]map[string]string{}, + false, }, { "invalid demand order - PFM and EIBC are not supported together", "1000000000", "150", - suite.hubChain.SenderAccount.GetAddress().String(), 0, true, map[string]map[string]string{"forward": { @@ -130,6 +139,7 @@ func (suite *EIBCTestSuite) TestEIBCDemandOrderCreation() { "port": "transfer", "channel": "channel-0", }}, + false, }, } totalDemandOrdersCreated := 0 @@ -148,23 +158,36 @@ func (suite *EIBCTestSuite) TestEIBCDemandOrderCreation() { } eibcJson, _ := json.Marshal(memoObj) memo := string(eibcJson) - _ = suite.TransferRollappToHub(path, IBCSenderAccount, tc.recipient, tc.amount, memo, tc.expectAck) + + if tc.skipEIBCmemo { + memo = "" + } + + recipient := apptesting.CreateRandomAccounts(1)[0] + _ = suite.TransferRollappToHub(path, IBCSenderAccount, recipient.String(), tc.amount, memo, tc.expectAck) // Validate demand orders results eibcKeeper := ConvertToApp(suite.hubChain).EIBCKeeper demandOrders, err := eibcKeeper.ListAllDemandOrders(suite.hubChain.GetContext()) suite.Require().NoError(err) suite.Require().Equal(tc.demandOrdersCreated, len(demandOrders)-totalDemandOrdersCreated) totalDemandOrdersCreated = len(demandOrders) + amountInt, ok := sdk.NewIntFromString(tc.amount) suite.Require().True(ok) feeInt, ok := sdk.NewIntFromString(tc.fee) suite.Require().True(ok) if tc.demandOrdersCreated > 0 { - lastDemandOrder := demandOrders[len(demandOrders)-1] - suite.Require().True(ok) - suite.Require().Equal(tc.recipient, lastDemandOrder.Recipient) - suite.Require().Equal(amountInt.Sub(feeInt), lastDemandOrder.Price[0].Amount) - suite.Require().Equal(feeInt, lastDemandOrder.Fee[0].Amount) + var demandOrder *eibctypes.DemandOrder + for _, order := range demandOrders { + if order.Recipient == recipient.String() { + demandOrder = order + break + } + } + suite.Require().NotNil(demandOrder) + suite.Require().Equal(recipient.String(), demandOrder.Recipient) + suite.Require().Equal(amountInt.Sub(feeInt), demandOrder.Price[0].Amount) + suite.Require().Equal(feeInt, demandOrder.Fee.AmountOf(demandOrder.Price[0].Denom)) } }) } diff --git a/x/delayedack/types/errors.go b/x/delayedack/types/errors.go index 0b33ca192..c9670a80d 100644 --- a/x/delayedack/types/errors.go +++ b/x/delayedack/types/errors.go @@ -7,10 +7,9 @@ import ( var ( ErrCanOnlyUpdatePendingPacket = errorsmod.Register(ModuleName, 1, "can only update pending packet") ErrRollappPacketDoesNotExist = errorsmod.Register(ModuleName, 2, "rollapp packet does not exist") - ErrEmptyEpochIdentifier = errorsmod.Register(ModuleName, 4, "empty epoch identifier") ErrMismatchedStateRoots = errorsmod.Register(ModuleName, 5, "mismatched state roots") ErrMismatchedSequencer = errorsmod.Register(ModuleName, 6, "mismatched sequencer") - ErrMissingEIBCMetadata = errorsmod.Register(ModuleName, 7, "missing eibc metadata") ErrUnknownRequest = errorsmod.Register(ModuleName, 8, "unknown request") ErrInvalidType = errorsmod.Register(ModuleName, 9, "invalid type") + ErrBadEIBCFee = errorsmod.Register(ModuleName, 10, "provided eibc fee is invalid") ) diff --git a/x/delayedack/types/packet_metadata.go b/x/delayedack/types/packet_metadata.go index ba9ff5b85..b1e915203 100644 --- a/x/delayedack/types/packet_metadata.go +++ b/x/delayedack/types/packet_metadata.go @@ -29,12 +29,10 @@ func (e EIBCMetadata) ValidateBasic() error { return nil } -var ErrEIBCFeeNotPositiveInt = fmt.Errorf("eibc fee is not a positive integer") - func (e EIBCMetadata) FeeInt() (math.Int, error) { i, ok := sdk.NewIntFromString(e.Fee) - if !ok || !i.IsPositive() { - return math.Int{}, ErrEIBCFeeNotPositiveInt + if !ok || i.IsNegative() { + return math.Int{}, ErrBadEIBCFee } return i, nil } @@ -59,13 +57,13 @@ func ParsePacketMetadata(input string) (*PacketMetadata, error) { if err != nil { return nil, ErrMemoUnmarshal } - if memo[memoObjectKeyEIBC] == nil { - return nil, ErrMemoEibcEmpty - } if memo[memoObjectKeyPFM] != nil { // Currently not supporting eibc with PFM: https://github.com/dymensionxyz/dymension/issues/599 return nil, ErrMemoHashPFMandEIBC } + if memo[memoObjectKeyEIBC] == nil { + return nil, ErrMemoEibcEmpty + } var metadata PacketMetadata err = json.Unmarshal(bz, &metadata) if err != nil { diff --git a/x/eibc/keeper/handler.go b/x/eibc/keeper/handler.go index dc45e3a23..a342360b1 100644 --- a/x/eibc/keeper/handler.go +++ b/x/eibc/keeper/handler.go @@ -1,12 +1,12 @@ package keeper import ( - "errors" "fmt" sdk "github.com/cosmos/cosmos-sdk/types" transfertypes "github.com/cosmos/ibc-go/v6/modules/apps/transfer/types" channeltypes "github.com/cosmos/ibc-go/v6/modules/core/04-channel/types" + "github.com/pkg/errors" "github.com/dymensionxyz/dymension/v3/utils" commontypes "github.com/dymensionxyz/dymension/v3/x/common/types" @@ -61,16 +61,17 @@ func (k Keeper) EIBCDemandOrderHandler(ctx sdk.Context, rollappPacket commontype func (k *Keeper) CreateDemandOrderOnRecv(ctx sdk.Context, fungibleTokenPacketData transfertypes.FungibleTokenPacketData, rollappPacket *commontypes.RollappPacket, ) (*types.DemandOrder, error) { - packetMetaData, err := dacktypes.ParsePacketMetadata(fungibleTokenPacketData.Memo) - if errors.Is(err, dacktypes.ErrMemoUnmarshal) || errors.Is(err, dacktypes.ErrMemoEibcEmpty) { - ctx.Logger().Debug("skipping demand order creation - no eibc memo provided") - return nil, nil - } - if err != nil { - return nil, err + // zero fee demand order by default + eibcMetaData := dacktypes.EIBCMetadata{Fee: "0"} + + if fungibleTokenPacketData.Memo != "" { + packetMetaData, err := dacktypes.ParsePacketMetadata(fungibleTokenPacketData.Memo) + if err == nil { + eibcMetaData = *packetMetaData.EIBC + } else if !errors.Is(err, dacktypes.ErrMemoEibcEmpty) { + return nil, fmt.Errorf("parse packet metadata: %w", err) + } } - - eibcMetaData := packetMetaData.EIBC if err := eibcMetaData.ValidateBasic(); err != nil { return nil, fmt.Errorf("validate eibc metadata: %w", err) } diff --git a/x/eibc/keeper/handler_test.go b/x/eibc/keeper/handler_test.go new file mode 100644 index 000000000..cb169fd7f --- /dev/null +++ b/x/eibc/keeper/handler_test.go @@ -0,0 +1,82 @@ +package keeper_test + +import ( + sdk "github.com/cosmos/cosmos-sdk/types" + channeltypes "github.com/cosmos/ibc-go/v6/modules/core/04-channel/types" + dacktypes "github.com/dymensionxyz/dymension/v3/x/delayedack/types" +) + +func (suite *KeeperTestSuite) TestCreateDemandOrderOnRecv() { + tests := []struct { + name string + memo string + expectedErr bool + expectedFee string + expectedPrice string // considering bridging fee of 1% + }{ + { + name: "fee by memo - create demand order", + memo: `{"eibc":{"fee":"100"}}`, + expectedErr: false, + expectedFee: "100", + expectedPrice: "890", + }, + { + name: "empty memo - create demand order", + memo: "", + expectedErr: false, + expectedPrice: "990", + }, + { + name: "memo w/o eibc - create demand order", + memo: `{"notEIBC":{}}`, + expectedErr: false, + expectedPrice: "990", + }, + { + name: "bad memo - fail", + memo: "bad", + expectedErr: true, + }, + { + name: "PFM memo - fail", + memo: `{"forward":{}}`, + expectedErr: true, + }, + } + + // set 1% bridging fee + dackParams := dacktypes.NewParams("hour", sdk.NewDecWithPrec(1, 2)) // 1% + suite.App.DelayedAckKeeper.SetParams(suite.Ctx, dackParams) + + amt, _ := sdk.NewIntFromString(transferPacketData.Amount) + bridgeFee := suite.App.DelayedAckKeeper.BridgingFeeFromAmt(suite.Ctx, amt) + suite.Require().True(bridgeFee.IsPositive()) + + for _, tt := range tests { + suite.Run(tt.name, func() { + // modify the memo and set rollapp packet + transferPacketData.Memo = tt.memo + packet = channeltypes.NewPacket(transferPacketData.GetBytes(), 1, portid, chanid, cpportid, cpchanid, timeoutHeight, timeoutTimestamp) + rollappPacket.Packet = &packet + suite.App.DelayedAckKeeper.SetRollappPacket(suite.Ctx, *rollappPacket) + + // Create new demand order + order, err := suite.App.EIBCKeeper.CreateDemandOrderOnRecv(suite.Ctx, transferPacketData, rollappPacket) + if tt.expectedErr { + suite.Require().Error(err) + return + } + + suite.Require().NoError(err) + suite.Require().NotNil(order) + if tt.expectedFee != "" { + suite.Require().Equal(tt.expectedFee, order.Fee[0].Amount.String()) + suite.Require().Equal(tt.expectedPrice, order.Price[0].Amount.String()) + } else { + suite.Require().Len(order.Fee, 0) + suite.Require().Equal(tt.expectedPrice, order.Price[0].Amount.String()) + } + }) + } +} diff --git a/x/eibc/keeper/msg_server_test.go b/x/eibc/keeper/msg_server_test.go index 6b9039cab..de6368ef2 100644 --- a/x/eibc/keeper/msg_server_test.go +++ b/x/eibc/keeper/msg_server_test.go @@ -10,67 +10,7 @@ import ( types "github.com/dymensionxyz/dymension/v3/x/eibc/types" ) -func (suite *KeeperTestSuite) TestMsgFulfillOrderEvent() { - // Create and fund the account - testAddresses := apptesting.AddTestAddrs(suite.App, suite.Ctx, 2, sdk.NewInt(1000)) - eibcSupplyAddr := testAddresses[0] - eibcDemandAddr := testAddresses[1] - // Set the rollapp packet - suite.App.DelayedAckKeeper.SetRollappPacket(suite.Ctx, *rollappPacket) - // Create new demand order - demandOrder := types.NewDemandOrder(*rollappPacket, math.NewIntFromUint64(200), math.NewIntFromUint64(50), sdk.DefaultBondDenom, eibcSupplyAddr.String()) - err := suite.App.EIBCKeeper.SetDemandOrder(suite.Ctx, demandOrder) - suite.Require().NoError(err) - - tests := []struct { - name string - fulfillmentShouldFail bool - expectedPostFulfillmentEventsCount int - expectedPostFulfillmentEventsType string - expectedPostFulfillmentEventsAttributes []sdk.Attribute - }{ - { - name: "Test demand order fulfillment - success", - expectedPostFulfillmentEventsType: eibcEventType, - expectedPostFulfillmentEventsCount: 1, - expectedPostFulfillmentEventsAttributes: []sdk.Attribute{ - sdk.NewAttribute(types.AttributeKeyId, types.BuildDemandIDFromPacketKey(string(rollappPacketKey))), - sdk.NewAttribute(types.AttributeKeyPrice, "200"+sdk.DefaultBondDenom), - sdk.NewAttribute(types.AttributeKeyFee, "50"+sdk.DefaultBondDenom), - sdk.NewAttribute(types.AttributeKeyIsFulfilled, "true"), - sdk.NewAttribute(types.AttributeKeyPacketStatus, commontypes.Status_PENDING.String()), - }, - }, - { - name: "Failed fulfillment - ", - fulfillmentShouldFail: true, - expectedPostFulfillmentEventsCount: 0, - }, - } - - for _, tc := range tests { - suite.Ctx = suite.Ctx.WithEventManager(sdk.NewEventManager()) - expectedFee := "50" - if tc.fulfillmentShouldFail { - expectedFee = "30" - } - msg := types.NewMsgFulfillOrder(eibcDemandAddr.String(), demandOrder.Id, expectedFee) - _, err = suite.msgServer.FulfillOrder(suite.Ctx, msg) - if tc.fulfillmentShouldFail { - suite.Require().Error(err) - } else { - suite.Require().NoError(err) - } - suite.AssertEventEmitted(suite.Ctx, tc.expectedPostFulfillmentEventsType, tc.expectedPostFulfillmentEventsCount) - if tc.expectedPostFulfillmentEventsCount > 0 { - lastEvent, ok := suite.FindLastEventOfType(suite.Ctx.EventManager().Events(), tc.expectedPostFulfillmentEventsType) - suite.Require().True(ok) - suite.AssertAttributes(lastEvent, tc.expectedPostFulfillmentEventsAttributes) - } - } -} - -func (suite *KeeperTestSuite) TestMsgFulfillOrderWithoutEvents() { +func (suite *KeeperTestSuite) TestMsgFulfillOrder() { tests := []struct { name string demandOrderPacketKey string @@ -200,6 +140,67 @@ func (suite *KeeperTestSuite) TestMsgFulfillOrderWithoutEvents() { } } +// TestFulfillOrderEvent tests the event upon fulfilling a demand order +func (suite *KeeperTestSuite) TestFulfillOrderEvent() { + // Create and fund the account + testAddresses := apptesting.AddTestAddrs(suite.App, suite.Ctx, 2, sdk.NewInt(1000)) + eibcSupplyAddr := testAddresses[0] + eibcDemandAddr := testAddresses[1] + // Set the rollapp packet + suite.App.DelayedAckKeeper.SetRollappPacket(suite.Ctx, *rollappPacket) + // Create new demand order + demandOrder := types.NewDemandOrder(*rollappPacket, math.NewIntFromUint64(200), math.NewIntFromUint64(50), sdk.DefaultBondDenom, eibcSupplyAddr.String()) + err := suite.App.EIBCKeeper.SetDemandOrder(suite.Ctx, demandOrder) + suite.Require().NoError(err) + + tests := []struct { + name string + fulfillmentShouldFail bool + expectedPostFulfillmentEventsCount int + expectedPostFulfillmentEventsType string + expectedPostFulfillmentEventsAttributes []sdk.Attribute + }{ + { + name: "Test demand order fulfillment - success", + expectedPostFulfillmentEventsType: eibcEventType, + expectedPostFulfillmentEventsCount: 1, + expectedPostFulfillmentEventsAttributes: []sdk.Attribute{ + sdk.NewAttribute(types.AttributeKeyId, types.BuildDemandIDFromPacketKey(string(rollappPacketKey))), + sdk.NewAttribute(types.AttributeKeyPrice, "200"+sdk.DefaultBondDenom), + sdk.NewAttribute(types.AttributeKeyFee, "50"+sdk.DefaultBondDenom), + sdk.NewAttribute(types.AttributeKeyIsFulfilled, "true"), + sdk.NewAttribute(types.AttributeKeyPacketStatus, commontypes.Status_PENDING.String()), + }, + }, + { + name: "Failed fulfillment - no event", + fulfillmentShouldFail: true, + expectedPostFulfillmentEventsCount: 0, + }, + } + + for _, tc := range tests { + suite.Ctx = suite.Ctx.WithEventManager(sdk.NewEventManager()) + expectedFee := "50" + if tc.fulfillmentShouldFail { + expectedFee = "30" // wrong expected fee to fail the fulfillment msg + } + msg := types.NewMsgFulfillOrder(eibcDemandAddr.String(), demandOrder.Id, expectedFee) + _, err = suite.msgServer.FulfillOrder(suite.Ctx, msg) + if tc.fulfillmentShouldFail { + suite.Require().Error(err) + } else { + suite.Require().NoError(err) + } + suite.AssertEventEmitted(suite.Ctx, tc.expectedPostFulfillmentEventsType, tc.expectedPostFulfillmentEventsCount) + if tc.expectedPostFulfillmentEventsCount > 0 { + lastEvent, ok := suite.FindLastEventOfType(suite.Ctx.EventManager().Events(), tc.expectedPostFulfillmentEventsType) + suite.Require().True(ok) + suite.AssertAttributes(lastEvent, tc.expectedPostFulfillmentEventsAttributes) + } + } +} + func (suite *KeeperTestSuite) TestMsgUpdateDemandOrder() { // Create and fund the account testAddresses := apptesting.AddTestAddrs(suite.App, suite.Ctx, 2, sdk.NewInt(100_000)) @@ -217,34 +218,34 @@ func (suite *KeeperTestSuite) TestMsgUpdateDemandOrder() { testCases := []struct { name string - fee sdk.Int + newFee sdk.Int submittedBy string expectError bool expectedPrice sdk.Int }{ { name: "happy case", - fee: sdk.NewInt(400), + newFee: sdk.NewInt(400), submittedBy: eibcSupplyAddr.String(), expectError: false, expectedPrice: sdk.NewInt(590), }, { name: "happy case - zero eibc fee", - fee: sdk.NewInt(0), + newFee: sdk.NewInt(0), submittedBy: eibcSupplyAddr.String(), expectError: false, expectedPrice: sdk.NewInt(990), }, { name: "wrong owner", - fee: sdk.NewInt(400), + newFee: sdk.NewInt(400), submittedBy: testAddresses[1].String(), expectError: true, }, { name: "too high fee", - fee: sdk.NewInt(1001), + newFee: sdk.NewInt(1001), submittedBy: eibcSupplyAddr.String(), expectError: true, }, @@ -257,7 +258,7 @@ func (suite *KeeperTestSuite) TestMsgUpdateDemandOrder() { suite.Require().NoError(err) // try to update the demand order - msg := types.NewMsgUpdateDemandOrder(demandOrder.Id, tc.submittedBy, tc.fee.String()) + msg := types.NewMsgUpdateDemandOrder(demandOrder.Id, tc.submittedBy, tc.newFee.String()) _, err = suite.msgServer.UpdateDemandOrder(suite.Ctx, msg) if tc.expectError { suite.Require().Error(err, tc.name) @@ -267,7 +268,7 @@ func (suite *KeeperTestSuite) TestMsgUpdateDemandOrder() { // check if the demand order is updated updatedDemandOrder, err := suite.App.EIBCKeeper.GetDemandOrder(suite.Ctx, rollappPacket.Status, demandOrder.Id) suite.Require().NoError(err, tc.name) - suite.Assert().Equal(updatedDemandOrder.Fee.AmountOf(denom), tc.fee, tc.name) + suite.Assert().Equal(updatedDemandOrder.Fee.AmountOf(denom), tc.newFee, tc.name) suite.Assert().Equal(updatedDemandOrder.Price.AmountOf(denom), tc.expectedPrice, tc.name) } }