From 5099c03b61a19a5eae7ed7bb1154d14a330c0cb7 Mon Sep 17 00:00:00 2001 From: Justin Tieri <37750742+jtieri@users.noreply.github.com> Date: Tue, 19 Mar 2024 14:21:55 -0500 Subject: [PATCH] fix: mint and transfer funds back to escrow account on timeout or ack error (#173) * chore: fix deps * chore: fix func sig --- .github/workflows/golangci-lint.yaml | 2 +- .../packetforward/keeper/keeper.go | 43 +++++++++++-------- .../packetforward/types/expected_keepers.go | 2 + .../test/mock/bank_keeper.go | 28 ++++++++++++ 4 files changed, 57 insertions(+), 18 deletions(-) diff --git a/.github/workflows/golangci-lint.yaml b/.github/workflows/golangci-lint.yaml index c2883b12..c96b2814 100644 --- a/.github/workflows/golangci-lint.yaml +++ b/.github/workflows/golangci-lint.yaml @@ -25,6 +25,6 @@ jobs: - name: golangci-lint uses: golangci/golangci-lint-action@v2 with: - version: v1.52 + version: v1.55.2 working-directory: ${{ matrix.workdir }} args: --timeout=5m \ No newline at end of file diff --git a/middleware/packet-forward-middleware/packetforward/keeper/keeper.go b/middleware/packet-forward-middleware/packetforward/keeper/keeper.go index 1d093535..20374688 100644 --- a/middleware/packet-forward-middleware/packetforward/keeper/keeper.go +++ b/middleware/packet-forward-middleware/packetforward/keeper/keeper.go @@ -141,41 +141,40 @@ func (k *Keeper) WriteAcknowledgementForForwardedPacket( } } + amount, ok := sdk.NewIntFromString(data.Amount) + if !ok { + return fmt.Errorf("failed to parse amount from packet data for forward refund: %s", data.Amount) + } + + denomTrace := transfertypes.ParseDenomTrace(fullDenomPath) + token := sdk.NewCoin(denomTrace.IBCDenom(), amount) + + escrowAddress := transfertypes.GetEscrowAddress(packet.SourcePort, packet.SourceChannel) + refundEscrowAddress := transfertypes.GetEscrowAddress(inFlightPacket.RefundPortId, inFlightPacket.RefundChannelId) + + newToken := sdk.NewCoins(token) + if transfertypes.SenderChainIsSource(packet.SourcePort, packet.SourceChannel, fullDenomPath) { // funds were moved to escrow account for transfer, so they need to either: // - move to the other escrow account, in the case of native denom // - burn - - amount, ok := sdk.NewIntFromString(data.Amount) - if !ok { - return fmt.Errorf("failed to parse amount from packet data for forward refund: %s", data.Amount) - } - denomTrace := transfertypes.ParseDenomTrace(fullDenomPath) - token := sdk.NewCoin(denomTrace.IBCDenom(), amount) - - escrowAddress := transfertypes.GetEscrowAddress(packet.SourcePort, packet.SourceChannel) - if transfertypes.SenderChainIsSource(inFlightPacket.RefundPortId, inFlightPacket.RefundChannelId, fullDenomPath) { // transfer funds from escrow account for forwarded packet to escrow account going back for refund. - - refundEscrowAddress := transfertypes.GetEscrowAddress(inFlightPacket.RefundPortId, inFlightPacket.RefundChannelId) - if err := k.bankKeeper.SendCoins( - ctx, escrowAddress, refundEscrowAddress, sdk.NewCoins(token), + ctx, escrowAddress, refundEscrowAddress, newToken, ); err != nil { return fmt.Errorf("failed to send coins from escrow account to refund escrow account: %w", err) } } else { // transfer the coins from the escrow account to the module account and burn them. - if err := k.bankKeeper.SendCoinsFromAccountToModule( - ctx, escrowAddress, transfertypes.ModuleName, sdk.NewCoins(token), + ctx, escrowAddress, transfertypes.ModuleName, newToken, ); err != nil { return fmt.Errorf("failed to send coins from escrow to module account for burn: %w", err) } if err := k.bankKeeper.BurnCoins( - ctx, transfertypes.ModuleName, sdk.NewCoins(token), + ctx, transfertypes.ModuleName, newToken, ); err != nil { // NOTE: should not happen as the module account was // retrieved on the step above and it has enough balace @@ -183,6 +182,16 @@ func (k *Keeper) WriteAcknowledgementForForwardedPacket( panic(fmt.Sprintf("cannot burn coins after a successful send from escrow account to module account: %v", err)) } } + } else { + // Funds in the escrow account were burned, + // so on a timeout or acknowledgement error we need to mint the funds back to the escrow account. + if err := k.bankKeeper.MintCoins(ctx, transfertypes.ModuleName, newToken); err != nil { + return fmt.Errorf("cannot mint coins to the %s module account: %v", transfertypes.ModuleName, err) + } + + if err := k.bankKeeper.SendCoinsFromModuleToAccount(ctx, transfertypes.ModuleName, refundEscrowAddress, newToken); err != nil { + return fmt.Errorf("cannot send coins from the %s module to the escrow account %s: %v", transfertypes.ModuleName, refundEscrowAddress, err) + } } } diff --git a/middleware/packet-forward-middleware/packetforward/types/expected_keepers.go b/middleware/packet-forward-middleware/packetforward/types/expected_keepers.go index 6c59d74a..5bb154d0 100644 --- a/middleware/packet-forward-middleware/packetforward/types/expected_keepers.go +++ b/middleware/packet-forward-middleware/packetforward/types/expected_keepers.go @@ -32,6 +32,8 @@ type DistributionKeeper interface { // BankKeeper defines the expected bank keeper type BankKeeper interface { SendCoins(ctx sdk.Context, fromAddr sdk.AccAddress, toAddr sdk.AccAddress, amt sdk.Coins) error + SendCoinsFromModuleToAccount(ctx sdk.Context, senderModule string, recipientAddr sdk.AccAddress, amt sdk.Coins) error SendCoinsFromAccountToModule(ctx sdk.Context, senderAddr sdk.AccAddress, recipientModule string, amt sdk.Coins) error + MintCoins(ctx sdk.Context, moduleName string, amt sdk.Coins) error BurnCoins(ctx sdk.Context, moduleName string, amt sdk.Coins) error } diff --git a/middleware/packet-forward-middleware/test/mock/bank_keeper.go b/middleware/packet-forward-middleware/test/mock/bank_keeper.go index 89850591..c0391f38 100644 --- a/middleware/packet-forward-middleware/test/mock/bank_keeper.go +++ b/middleware/packet-forward-middleware/test/mock/bank_keeper.go @@ -48,6 +48,20 @@ func (mr *MockBankKeeperMockRecorder) BurnCoins(arg0, arg1, arg2 interface{}) *g return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "BurnCoins", reflect.TypeOf((*MockBankKeeper)(nil).BurnCoins), arg0, arg1, arg2) } +// MintCoins mocks base method. +func (m *MockBankKeeper) MintCoins(arg0 types.Context, arg1 string, arg2 types.Coins) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "MintCoins", arg0, arg1, arg2) + ret0, _ := ret[0].(error) + return ret0 +} + +// MintCoins indicates an expected call of MintCoins. +func (mr *MockBankKeeperMockRecorder) MintCoins(arg0, arg1, arg2 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "MintCoins", reflect.TypeOf((*MockBankKeeper)(nil).MintCoins), arg0, arg1, arg2) +} + // SendCoins mocks base method. func (m *MockBankKeeper) SendCoins(arg0 types.Context, arg1, arg2 types.AccAddress, arg3 types.Coins) error { m.ctrl.T.Helper() @@ -75,3 +89,17 @@ func (mr *MockBankKeeperMockRecorder) SendCoinsFromAccountToModule(arg0, arg1, a mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SendCoinsFromAccountToModule", reflect.TypeOf((*MockBankKeeper)(nil).SendCoinsFromAccountToModule), arg0, arg1, arg2, arg3) } + +// SendCoinsFromModuleToAccount mocks base method. +func (m *MockBankKeeper) SendCoinsFromModuleToAccount(arg0 types.Context, arg1 string, arg2 types.AccAddress, arg3 types.Coins) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "SendCoinsFromModuleToAccount", arg0, arg1, arg2, arg3) + ret0, _ := ret[0].(error) + return ret0 +} + +// SendCoinsFromModuleToAccount indicates an expected call of SendCoinsFromModuleToAccount. +func (mr *MockBankKeeperMockRecorder) SendCoinsFromModuleToAccount(arg0, arg1, arg2, arg3 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SendCoinsFromModuleToAccount", reflect.TypeOf((*MockBankKeeper)(nil).SendCoinsFromModuleToAccount), arg0, arg1, arg2, arg3) +}