Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Mm2.1 extended error #923

Merged
merged 15 commits into from
May 3, 2021
Merged

Mm2.1 extended error #923

merged 15 commits into from
May 3, 2021

Conversation

sergeyboyko0791
Copy link

  • Design and implement the unified MmError
  • Design and implement the MarketMaker2 RPC protocol v2
  • Add WithdrawError returned from "withdraw" RPC function
  • Add TradePreimageRpcError returned from "trade_preimage" PRC function
  • Make a compile time checking if an error type is tagged

@sergeyboyko0791
Add and implement MmError
537782b
@sergeyboyko0791
Add WithdrawError returned from 'MmCoin::withdraw'
89fb0a4 
* Collect 'available' and 'required' info within GenerateTxError
* Add other internal error types
@sergeyboyko0791
Add BalanceError returned from my_balance
163e2f7
@sergeyboyko0791
Add more error types
652c601 
* Add Web3RpcError,
* Refactor the ordered_mature_unspents, list_unspent_ordered methods to return an extended error
* Rename MmRpcError to UtxoRpcError
@sergeyboyko0791
Refactor MmError mapping
9b910fb 
* Separate mm_error into several modules
* Rename into_mm_and() to into_mm()
* add into_mm_fut
@sergeyboyko0791
Merge branch 'mm2.1' into mm2.1-extended-error
65f457b
@sergeyboyko0791
Implement MarketMaker2 RPC protocol v2
424d392 
* Add lp_protocol, dispatcher_v2, dispatcher_legacy modules
* Change the coins::withdraw function signature
* Add HttpStatusCode trait
* Refactor test_withdraw_and_send to use v2 protocol
* Add test_mm2rpc test_withdraw_legacy, test_withdraw_not_sufficient_balance
@sergeyboyko0791
Upgrade the trade_preimage RPC call to the mmrpc 2.0
44ba842 
* Move lp_swap::trade_preimage and dependent types to the separated lp_swap::trade_preimage module
* Change the trade_preimage_rpc function signature
* Extend CheckBalanceError and move it to the separated check_balance module
* Refactor test_maker_trade_preimage, test_taker_trade_preimage to use v2 protocol
* Add test_trade_preimage_not_sufficient_balance, test_trade_preimage_legacy, test_trade_preimage_dynamic_fee_not_sufficient_balance tests
@sergeyboyko0791
Merge branch 'mm2.1' into mm2.1-extended-error
8c15af5
@sergeyboyko0791
Remove boilerplate
7eacf31 
* Add lp_coinfind_or_err and CoinFindError
* Replace lp_coinfind with lp_coinfind_or_err in coins::withdraw, trade_preimage_rpc
@sergeyboyko0791
Move functions that check the trade balance to check_balance module
8a4ac2c 
* Move check_my_coin_balance_for_swap, check_other_coin_balance_for_swap, check_base_coin_balance_for_swap
@sergeyboyko0791
Make a compile time checking if an error type is tagged
575b881 
* Add ser_error, ser_error_derive crates
* Remove a runtime checking if an error type is tagged
* Fill out the mm_error documentation with the examples
@sergeyboyko0791
Fix rustfmt warnings
e4bd65b
artemii235
artemii235 reviewed Apr 27, 2021
View reviewed changes
Copy link
Member

@artemii235 artemii235 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a few questions 🙂

mm2src/coins/lp_coins.rs Outdated Show resolved Hide resolved
mm2src/common/mm_error/into_mm.rs Outdated Show resolved Hide resolved
mm2src/common/mm_error/mm_error.rs Outdated Show resolved Hide resolved
@sergeyboyko0791
Fix PR issues
fd0887c 
* Rename 'into_mm' to 'map_to_mm', 'into_mm_fut' to 'map_to_mm_fut'
* Remove CoinFindError::NoCoinsContext error variant, replace it with the panic
artemii235
artemii235 approved these changes Apr 28, 2021
View reviewed changes
Copy link
Member

@artemii235 artemii235 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!
@tonymorony Could you please try using MM2 from this PR in GUI and check that it works fine? The backward compatibility should not be broken, but worth additional testing just in case 🙂

@tonymorony tonymorony requested a review from smk762 April 29, 2021 10:09
@tonymorony
Copy link

@smk762 could you please test desktop GUI with builds from fd0887c commit (mm2 can be found in http://195.201.0.6/mm2.1-extended-error/ )

@smk762
Copy link

smk762 commented Apr 29, 2021

So far no problems with basic usage, will continue testing this week to see if anything comes up.

@artemii235
Copy link
Member

@smk762 Thanks for the update! Do I understand correctly that everything is fine and we can merge this PR? cc @tonymorony

tonymorony
tonymorony approved these changes May 3, 2021
View reviewed changes
Copy link

@tonymorony tonymorony left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can merge this one since there are no problems with the sanity check. If something will pop up during GUI release testing we can just hotfix on GUI or API side depends on the problem :)

@artemii235
Copy link
Member

Great, thanks 🙂

@artemii235 artemii235 merged commit a632a47 into mm2.1 May 3, 2021
@artemii235 artemii235 deleted the mm2.1-extended-error branch May 3, 2021 07:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tonymorony tonymorony tonymorony approved these changes

@artemii235 artemii235 artemii235 approved these changes

@smk762 smk762 Awaiting requested review from smk762

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@sergeyboyko0791 @tonymorony @smk762 @artemii235