diem / reference-wallet

A reference wallet.
Apache License 2.0
15 stars 13 forks source link

[Bug] BalanceError in execute_trade #14

Closed Srtake closed 3 years ago

Srtake commented 3 years ago

πŸ› Bug

Hello I tried to add 10,000 Coin1 in Reference Wallet to test the function, but got nothing after I clicking the confirm button. The message showed on the frontend told me my request was successful, but I found there were error messages in backend output.

I tried to check codes of this function to learn what happened, and found that BalanceError only prevents the cover process code from executing, but payment charge has already finished at that time. Here is what I learn:

Steps to reproduce

  1. Click "transfer" button
  2. Click "add" button
  3. Add 10,000 Coin1
  4. Click "review" button
  5. Click "confirm" button
  6. A successful message is showed, but no Coin1 got

Error messages

backend-worker_1         | Traceback (most recent call last):
backend-worker_1         |   File "./wallet/services/order.py", line 182, in execute_trade
backend-worker_1         |     payment_type=TransactionType.INTERNAL,
backend-worker_1         |   File "./wallet/services/transaction.py", line 349, in internal_transaction
backend-worker_1         |     f"Balance {account_service.get_account_balance_by_id(account_id=sender_id).total[currency]} "
backend-worker_1         | wallet.types.BalanceError: Balance 7001000000 is less than amount needed 10000000000

Expected Behavior

Correctly add the balance after confirming the transfer, or show a error message to the user.

System information

Ubuntu 18.04

syam585 commented 3 years ago

πŸ› Bug

Hello I tried to add 10,000 Coin1 in Reference Wallet to test the function, but got nothing after I clicking the confirm button. The message showed on the frontend told me my request was successful, but I found there were error messages in backend output.

I tried to check codes of this function to learn what happened, and found that BalanceError only prevents the cover process code from executing, but payment charge has already finished at that time. Here is what I learn:

  • In backend/wallet/services/order.py, execute_trade() first checks whether the inventory account has enough balance, and buys from LP if not.
  • Then it tries to process internal transaction. In internal_transaction(), the second balance check is executed, and the function raises BalanceError exception if not.
  • The execute_trade() function catches the exception, updates database table, and returns a False.
  • In execute_order(), cover_order() function is skipped since we get a False from execute_trade().
  • But, the outer process_order() function and ExecuteQuoteView class never handle the exception, and the latter class responses a HTTP 200 to the frontend.

Steps to reproduce

  1. Click "transfer" button
  2. Click "add" button
  3. Add 10,000 Coin1
  4. Click "review" button
  5. Click "confirm" button
  6. A successful message is showed, but no Coin1 got

Error messages

backend-worker_1         | Traceback (most recent call last):
backend-worker_1         |   File "./wallet/services/order.py", line 182, in execute_trade
backend-worker_1         |     payment_type=TransactionType.INTERNAL,
backend-worker_1         |   File "./wallet/services/transaction.py", line 349, in internal_transaction
backend-worker_1         |     f"Balance {account_service.get_account_balance_by_id(account_id=sender_id).total[currency]} "
backend-worker_1         | wallet.types.BalanceError: Balance 7001000000 is less than amount needed 10000000000

Expected Behavior

Correctly add the balance after confirming the transfer, or show a error message to the user.

System information

Ubuntu 18.04

syam585 commented 3 years ago

Acc

syam585 commented 3 years ago

πŸ› Bug

Hello I tried to add 10,000 Coin1 in Reference Wallet to test the function, but got nothing after I clicking the confirm button. The message showed on the frontend told me my request was successful, but I found there were error messages in backend output.

I tried to check codes of this function to learn what happened, and found that BalanceError only prevents the cover process code from executing, but payment charge has already finished at that time. Here is what I learn:

  • In backend/wallet/services/order.py, execute_trade() first checks whether the inventory account has enough balance, and buys from LP if not.
  • Then it tries to process internal transaction. In internal_transaction(), the second balance check is executed, and the function raises BalanceError exception if not.
  • The execute_trade() function catches the exception, updates database table, and returns a False.
  • In execute_order(), cover_order() function is skipped since we get a False from execute_trade().
  • But, the outer process_order() function and ExecuteQuoteView class never handle the exception, and the latter class responses a HTTP 200 to the frontend.

Steps to reproduce

  1. Click "transfer" button
  2. Click "add" button
  3. Add 10,000 Coin1
  4. Click "review" button
  5. Click "confirm" button
  6. A successful message is showed, but no Coin1 got

Error messages

backend-worker_1         | Traceback (most recent call last):
backend-worker_1         |   File "./wallet/services/order.py", line 182, in execute_trade
backend-worker_1         |     payment_type=TransactionType.INTERNAL,
backend-worker_1         |   File "./wallet/services/transaction.py", line 349, in internal_transaction
backend-worker_1         |     f"Balance {account_service.get_account_balance_by_id(account_id=sender_id).total[currency]} "
backend-worker_1         | wallet.types.BalanceError: Balance 7001000000 is less than amount needed 10000000000

Expected Behavior

Correctly add the balance after confirming the transfer, or show a error message to the user.

System information

Ubuntu 18.04

yanivmo commented 3 years ago

Thank you for reporting this issue and supplying such a thorough explanation. We have introduced several changes recently that should address this issue as well. I am closing it, but if you still manage to reproduce it, do not hesitate to reopen.