diff --git a/precompiles/bank/bank.go b/precompiles/bank/bank.go index d498125fa6..436364dfa6 100644 --- a/precompiles/bank/bank.go +++ b/precompiles/bank/bank.go @@ -151,7 +151,15 @@ func (c *Contract) Run(evm *vm.EVM, contract *vm.Contract, readOnly bool) ([]byt return err }) if execErr != nil { - return method.Outputs.Pack(false) + res, errPack := method.Outputs.Pack(false) + if errPack != nil { + return nil, errPack + } + + // Return the proper result (true/false) and the error message. + // This way we make bank compliant with smart contracts which would expect a true/false. + // And also with Go bindings which would expect an error. + return res, err } return res, nil diff --git a/precompiles/bank/method_deposit.go b/precompiles/bank/method_deposit.go index 3d139124e3..2f57944723 100644 --- a/precompiles/bank/method_deposit.go +++ b/precompiles/bank/method_deposit.go @@ -61,10 +61,17 @@ func (c *Contract) deposit( // Safety check: token has to be a valid whitelisted ZRC20 and not be paused. t, found := c.fungibleKeeper.GetForeignCoins(ctx, zrc20Addr.String()) - if !found || t.Paused { + if !found { return nil, &ptypes.ErrInvalidToken{ Got: zrc20Addr.String(), - Reason: "token is not a whitelisted ZRC20 or it's paused", + Reason: "token is not a whitelisted ZRC20", + } + } + + if t.Paused { + return nil, &ptypes.ErrInvalidToken{ + Got: zrc20Addr.String(), + Reason: "token is paused", } } @@ -86,7 +93,13 @@ func (c *Contract) deposit( } balance, ok := resBalanceOf[0].(*big.Int) - if !ok || balance.Cmp(amount) < 0 || balance.Cmp(big.NewInt(0)) <= 0 { + if !ok { + return nil, &ptypes.ErrUnexpected{ + Got: "ZRC20 balanceOf returned an unexpected type", + } + } + + if balance.Cmp(amount) < 0 || balance.Cmp(big.NewInt(0)) <= 0 { return nil, &ptypes.ErrInvalidAmount{ Got: balance.String(), } @@ -110,7 +123,13 @@ func (c *Contract) deposit( } allowance, ok := resAllowance[0].(*big.Int) - if !ok || allowance.Cmp(amount) < 0 || allowance.Cmp(big.NewInt(0)) <= 0 { + if !ok { + return nil, &ptypes.ErrUnexpected{ + Got: "ZRC20 allowance returned an unexpected type", + } + } + + if allowance.Cmp(amount) < 0 || allowance.Cmp(big.NewInt(0)) <= 0 { return nil, &ptypes.ErrInvalidAmount{ Got: allowance.String(), } @@ -152,8 +171,7 @@ func (c *Contract) deposit( } // 3. Interactions: create cosmos coin and send. - err = c.bankKeeper.MintCoins(ctx, types.ModuleName, coinSet) - if err != nil { + if err := c.bankKeeper.MintCoins(ctx, types.ModuleName, coinSet); err != nil { return nil, &ptypes.ErrUnexpected{ When: "MintCoins", Got: err.Error(), diff --git a/precompiles/bank/method_test.go b/precompiles/bank/method_test.go index ce1a2aec61..187b0c714b 100644 --- a/precompiles/bank/method_test.go +++ b/precompiles/bank/method_test.go @@ -98,7 +98,14 @@ func Test_Methods(t *testing.T) { ) success, err := ts.bankContract.Run(ts.mockEVM, ts.mockVMContract, false) - require.NoError(t, err) + require.Error(t, err) + require.ErrorAs( + t, + ptypes.ErrInvalidAmount{ + Got: "0", + }, + err, + ) res, err := ts.bankABI.Methods[DepositMethodName].Outputs.Unpack(success) require.NoError(t, err) @@ -127,7 +134,14 @@ func Test_Methods(t *testing.T) { ) success, err := ts.bankContract.Run(ts.mockEVM, ts.mockVMContract, false) - require.NoError(t, err) + require.Error(t, err) + require.ErrorAs( + t, + ptypes.ErrInvalidAmount{ + Got: "0", + }, + err, + ) res, err := ts.bankABI.Methods[DepositMethodName].Outputs.Unpack(success) require.NoError(t, err) @@ -158,7 +172,14 @@ func Test_Methods(t *testing.T) { ) success, err := ts.bankContract.Run(ts.mockEVM, ts.mockVMContract, false) - require.NoError(t, err) + require.Error(t, err) + require.ErrorAs( + t, + ptypes.ErrInvalidAmount{ + Got: "500", + }, + err, + ) res, err := ts.bankABI.Methods[DepositMethodName].Outputs.Unpack(success) require.NoError(t, err) @@ -205,7 +226,14 @@ func Test_Methods(t *testing.T) { ) success, err := ts.bankContract.Run(ts.mockEVM, ts.mockVMContract, false) - require.NoError(t, err) + require.Error(t, err) + require.ErrorAs( + t, + ptypes.ErrInvalidAmount{ + Got: "1000", + }, + err, + ) res, err := ts.bankABI.Methods[DepositMethodName].Outputs.Unpack(success) require.NoError(t, err) @@ -409,7 +437,15 @@ func Test_Methods(t *testing.T) { ) success, err = ts.bankContract.Run(ts.mockEVM, ts.mockVMContract, false) - require.NoError(t, err) + require.Error(t, err) + require.ErrorAs( + t, + ptypes.ErrInsufficientBalance{ + Requested: "501", + Got: "500", + }, + err, + ) res, err = ts.bankABI.Methods[WithdrawMethodName].Outputs.Unpack(success) require.NoError(t, err) diff --git a/precompiles/bank/method_withdraw.go b/precompiles/bank/method_withdraw.go index 81b7bcef4b..99cb4f763c 100644 --- a/precompiles/bank/method_withdraw.go +++ b/precompiles/bank/method_withdraw.go @@ -57,19 +57,26 @@ func (c *Contract) withdraw( // Safety check: token has to be a valid whitelisted ZRC20 and not be paused. t, found := c.fungibleKeeper.GetForeignCoins(ctx, zrc20Addr.String()) - if !found || t.Paused { + if !found { return nil, &ptypes.ErrInvalidToken{ Got: zrc20Addr.String(), - Reason: "token is not a whitelisted ZRC20 or it's paused", + Reason: "token is not a whitelisted ZRC20", + } + } + + if t.Paused { + return nil, &ptypes.ErrInvalidToken{ + Got: zrc20Addr.String(), + Reason: "token is paused", } } // Caller has to have enough cosmos coin balance to withdraw the requested amount. coin := c.bankKeeper.GetBalance(ctx, fromAddr, ZRC20ToCosmosDenom(zrc20Addr)) - if coin.Amount.IsNil() { + if !coin.IsValid() { return nil, &ptypes.ErrInsufficientBalance{ Requested: amount.String(), - Got: "nil", + Got: "invalid coin", } } @@ -103,39 +110,19 @@ func (c *Contract) withdraw( } balance, ok := resBalanceOf[0].(*big.Int) - if !ok || balance.Cmp(amount) == -1 { - return nil, &ptypes.ErrInvalidAmount{ - Got: "not enough bank balance", - } - } - - // 2. Effect: transfer balance. - - // function transfer(address recipient, uint256 amount) public virtual override returns (bool) - resTransferFrom, err := c.CallContract( - ctx, - &c.fungibleKeeper, - c.zrc20ABI, - zrc20Addr, - "transfer", - []interface{}{caller /* sender */, amount}, - ) - if err != nil { + if !ok { return nil, &ptypes.ErrUnexpected{ - When: "transferFrom", - Got: err.Error(), + Got: "ZRC20 balanceOf returned an unexpected type", } } - transferred, ok := resTransferFrom[0].(bool) - if !ok || !transferred { - return nil, &ptypes.ErrUnexpected{ - When: "transferFrom", - Got: "transaction not successful", + if balance.Cmp(amount) == -1 { + return nil, &ptypes.ErrInvalidAmount{ + Got: balance.String(), } } - // 3. Interactions: send to module and burn. + // 2. Effect: burn cosmos coin balance. if err := c.bankKeeper.SendCoinsFromAccountToModule(ctx, fromAddr, types.ModuleName, coinSet); err != nil { return nil, &ptypes.ErrUnexpected{ When: "SendCoinsFromAccountToModule", @@ -157,6 +144,38 @@ func (c *Contract) withdraw( } } + // 3. Interactions: send to module and burn. + + // function transfer(address recipient, uint256 amount) public virtual override returns (bool) + resTransfer, err := c.CallContract( + ctx, + &c.fungibleKeeper, + c.zrc20ABI, + zrc20Addr, + "transfer", + []interface{}{caller /* sender */, amount}, + ) + if err != nil { + return nil, &ptypes.ErrUnexpected{ + When: "transfer", + Got: err.Error(), + } + } + + transferred, ok := resTransfer[0].(bool) + if !ok { + return nil, &ptypes.ErrUnexpected{ + Got: "ZRC20 transfer returned an unexpected type", + } + } + + if !transferred { + return nil, &ptypes.ErrUnexpected{ + When: "transfer", + Got: "transaction not successful", + } + } + return method.Outputs.Pack(true) }