From a1fa0e0bbf6b2d5f4861d2215f240d8adbfc9285 Mon Sep 17 00:00:00 2001 From: Tian Date: Wed, 25 Oct 2023 19:15:17 -0400 Subject: [PATCH] [CORE-709] do not error when completing a bridge with non-positive amount (#691) * do not error when completing a bridge with non-positive amount (cherry picked from commit 525873de3b9cd5b29262db3241698f21b6754ca0) # Conflicts: # protocol/x/bridge/keeper/complete_bridge_test.go --- protocol/x/bridge/keeper/complete_bridge.go | 19 +++--- .../x/bridge/keeper/complete_bridge_test.go | 68 +++++++++++-------- 2 files changed, 50 insertions(+), 37 deletions(-) diff --git a/protocol/x/bridge/keeper/complete_bridge.go b/protocol/x/bridge/keeper/complete_bridge.go index 4e7a4db885..4b2d5e5ddd 100644 --- a/protocol/x/bridge/keeper/complete_bridge.go +++ b/protocol/x/bridge/keeper/complete_bridge.go @@ -35,14 +35,17 @@ func (k Keeper) CompleteBridge( return err } - // Send coin from bridge module account to specified account. - if err = k.bankKeeper.SendCoinsFromModuleToAccount( - ctx, - types.ModuleName, - bridgeAccAddress, - sdk.Coins{bridge.Coin}, - ); err != nil { - return err + // If coin amount is positive, send coin from bridge module account to + // specified account. + if bridge.Coin.Amount.IsPositive() { + if err = k.bankKeeper.SendCoinsFromModuleToAccount( + ctx, + types.ModuleName, + bridgeAccAddress, + sdk.Coins{bridge.Coin}, + ); err != nil { + return err + } } // Emit metric on last completed bridge id. diff --git a/protocol/x/bridge/keeper/complete_bridge_test.go b/protocol/x/bridge/keeper/complete_bridge_test.go index 5b42caee98..e091240298 100644 --- a/protocol/x/bridge/keeper/complete_bridge_test.go +++ b/protocol/x/bridge/keeper/complete_bridge_test.go @@ -16,7 +16,7 @@ import ( func TestCompleteBridge(t *testing.T) { tests := map[string]struct { // Initial balance of bridge module account. - initialBalance sdk.Coin + initialModAccBalance sdk.Coin // Bridge event to complete. bridgeEvent types.BridgeEvent // Whether bridging is disabled. @@ -25,26 +25,25 @@ func TestCompleteBridge(t *testing.T) { // Expected error, if any. expectedError string // Expected balance of bridge module account after bridge completion. - expectedBalance sdk.Coin + expectedModAccBalance sdk.Coin }{ "Success": { - initialBalance: sdk.NewCoin("adv4tnt", sdkmath.NewInt(1_000)), - bridgeEvent: constants.BridgeEvent_Id0_Height0, // bridges 888 tokens. - expectedBalance: sdk.NewCoin("adv4tnt", sdkmath.NewInt(112)), // 1000 - 888. + initialModAccBalance: sdk.NewCoin("adv4tnt", sdkmath.NewInt(1_000)), + bridgeEvent: constants.BridgeEvent_Id0_Height0, // bridges 888 tokens. + expectedModAccBalance: sdk.NewCoin("adv4tnt", sdkmath.NewInt(112)), // 1000 - 888. }, - "Failure: coin amount is 0": { - initialBalance: sdk.NewCoin("adv4tnt", sdkmath.NewInt(1_000)), + "Success: coin amount is 0": { + initialModAccBalance: sdk.NewCoin("adv4tnt", sdkmath.NewInt(1_000)), bridgeEvent: types.BridgeEvent{ Id: 7, Address: constants.BobAccAddress.String(), Coin: sdk.NewCoin("adv4tnt", sdkmath.ZeroInt()), EthBlockHeight: 3, }, - expectedError: "invalid coin", - expectedBalance: sdk.NewCoin("adv4tnt", sdkmath.NewInt(1_000)), + expectedModAccBalance: sdk.NewCoin("adv4tnt", sdkmath.NewInt(1_000)), }, - "Failure: coin amount is negative": { - initialBalance: sdk.NewCoin("adv4tnt", sdkmath.NewInt(1_000)), + "Success: coin amount is negative": { + initialModAccBalance: sdk.NewCoin("adv4tnt", sdkmath.NewInt(1_000)), bridgeEvent: types.BridgeEvent{ Id: 7, Address: constants.BobAccAddress.String(), @@ -54,32 +53,31 @@ func TestCompleteBridge(t *testing.T) { }, EthBlockHeight: 3, }, - expectedError: "invalid coin", - expectedBalance: sdk.NewCoin("adv4tnt", sdkmath.NewInt(1_000)), + expectedModAccBalance: sdk.NewCoin("adv4tnt", sdkmath.NewInt(1_000)), }, "Failure: invalid address string": { - initialBalance: sdk.NewCoin("adv4tnt", sdkmath.NewInt(1_000)), + initialModAccBalance: sdk.NewCoin("adv4tnt", sdkmath.NewInt(1_000)), bridgeEvent: types.BridgeEvent{ Id: 4, Address: "not an address string", Coin: sdk.NewCoin("adv4tnt", sdkmath.NewInt(1)), EthBlockHeight: 2, }, - expectedError: "decoding bech32 failed", - expectedBalance: sdk.NewCoin("adv4tnt", sdkmath.NewInt(1_000)), + expectedError: "decoding bech32 failed", + expectedModAccBalance: sdk.NewCoin("adv4tnt", sdkmath.NewInt(1_000)), }, "Failure: bridge module account has insufficient balance": { - initialBalance: sdk.NewCoin("adv4tnt", sdkmath.NewInt(500)), - bridgeEvent: constants.BridgeEvent_Id0_Height0, // bridges 888 tokens. - expectedError: "insufficient funds", - expectedBalance: sdk.NewCoin("adv4tnt", sdkmath.NewInt(500)), + initialModAccBalance: sdk.NewCoin("adv4tnt", sdkmath.NewInt(500)), + bridgeEvent: constants.BridgeEvent_Id0_Height0, // bridges 888 tokens. + expectedError: "insufficient funds", + expectedModAccBalance: sdk.NewCoin("adv4tnt", sdkmath.NewInt(500)), }, "Failure: bridging is disabled": { - initialBalance: sdk.NewCoin("adv4tnt", sdkmath.NewInt(1_000)), - bridgeEvent: constants.BridgeEvent_Id0_Height0, // bridges 888 tokens. - bridgingDisabled: true, - expectedError: types.ErrBridgingDisabled.Error(), - expectedBalance: sdk.NewCoin("adv4tnt", sdkmath.NewInt(1_000)), // same as initial. + initialModAccBalance: sdk.NewCoin("adv4tnt", sdkmath.NewInt(1_000)), + bridgeEvent: constants.BridgeEvent_Id0_Height0, // bridges 888 tokens. + bridgingDisabled: true, + expectedError: types.ErrBridgingDisabled.Error(), + expectedModAccBalance: sdk.NewCoin("adv4tnt", sdkmath.NewInt(1_000)), // same as initial. }, } @@ -92,11 +90,11 @@ func TestCompleteBridge(t *testing.T) { DelayBlocks: bridgeKeeper.GetSafetyParams(ctx).DelayBlocks, }) require.NoError(t, err) - // Fund bridge module account with enought balance. + // Fund bridge module account with enough balance. err = bankKeeper.MintCoins( ctx, types.ModuleName, - sdk.NewCoins(tc.initialBalance), + sdk.NewCoins(tc.initialModAccBalance), ) require.NoError(t, err) @@ -116,17 +114,29 @@ func TestCompleteBridge(t *testing.T) { sdk.MustAccAddressFromBech32(tc.bridgeEvent.Address), tc.bridgeEvent.Coin.Denom, ) - require.Equal(t, tc.bridgeEvent.Coin.Denom, balance.Denom) - require.Equal(t, tc.bridgeEvent.Coin.Amount, balance.Amount) + expectedBalance := sdk.NewCoin(tc.bridgeEvent.Coin.Denom, sdkmath.ZeroInt()) + if tc.bridgeEvent.Coin.Amount.IsPositive() { + expectedBalance = tc.bridgeEvent.Coin + } + require.Equal(t, expectedBalance.Denom, balance.Denom) + require.Equal(t, expectedBalance.Amount, balance.Amount) } // Assert that bridge module account's balance is as expected. modAccBalance := bankKeeper.GetBalance( ctx, +<<<<<<< HEAD authtypes.NewModuleAddress(types.ModuleName), tc.bridgeEvent.Coin.Denom, ) require.Equal(t, tc.expectedBalance.Denom, modAccBalance.Denom) require.Equal(t, tc.expectedBalance.Amount, modAccBalance.Amount) +======= + types.ModuleAddress, + tc.bridgeEvent.Coin.Denom, + ) + require.Equal(t, tc.expectedModAccBalance.Denom, modAccBalance.Denom) + require.Equal(t, tc.expectedModAccBalance.Amount, modAccBalance.Amount) +>>>>>>> 525873de ([CORE-709] do not error when completing a bridge with non-positive amount (#691)) }) } }