UDC deposit and withdraw API #6821
Conversation
raiden/api/python.py
Outdated
| current_total_deposit = user_deposit.get_total_deposit( | ||
| address=self.address, block_identifier=confirmed_block_identifier | ||
| ) | ||
| addendum = new_total_deposit - current_total_deposit |
There was a problem hiding this comment.
I don't think the work "addendum" is used in the context of summing up numbers. Counter suggestion: deposit_increase.
raiden/api/python.py
Outdated
| if new_total_deposit <= current_total_deposit: | ||
| raise DepositMismatch("Total deposit did not increase.") | ||
|
|
||
| if whole_balance + addendum > UINT256_MAX: | ||
| raise DepositOverLimit("Deposit overflow.") | ||
|
|
||
| if whole_balance + addendum > whole_balance_limit: | ||
| msg = f"Deposit of {addendum} would have exceeded the UDC balance limit." | ||
| raise DepositOverLimit(msg) | ||
|
|
||
| if balance < addendum: | ||
| msg = f"Not enough balance to deposit. Available={balance} Needed={addendum}" | ||
| raise InsufficientFunds(msg) |
There was a problem hiding this comment.
Should these checks be in the contract proxy, instead? Do we ever want to call the contract with any of these violated or unkown?
See also https://github.com/raiden-network/raiden/blob/develop/docs/proxies.rst#state-modifying-case
There was a problem hiding this comment.
They are in both places at the moment. This is quite redundant, but I tried to be consistent with the other API functions. The channel deposit function does the same for example.
One advantage I see is that we can show more specific error codes like this. For example a 402 if the RDN balance is too low.
The guide for using the proxies also says for a raised BrokenPreconditionError:
This means there is a mistake in the Raiden codebase, and a check must be added before calling the proxy.
And yes, it would be good to hear someone else's opinion on that.
There was a problem hiding this comment.
I pointed this out repeatedly during our stand-up. Apparently nobody feels strong enough about this to comment. Feel free to proceed with the current approach.
There was a problem hiding this comment.
Alright then. Thanks for your help!
| if total_deposit is not None and planned_withdraw_amount is not None: | ||
| return api_error( | ||
| errors="Cannot deposit to UDC and plan a withdraw at the same time", | ||
| status_code=HTTPStatus.BAD_REQUEST, | ||
| ) |
There was a problem hiding this comment.
I don't like such multi-purpose functions, but since other API endpoints work in the same way, this seems to be the right thing to do for consistency's sake.
There was a problem hiding this comment.
Yep, I agree. We should probably improve in that sense in a v2 of the API.
This includes three POST requests: - deposit to UDC - plan withdraw from UDC - withdraw from UDC Resolves raiden-network#5497
manuelwedler
force-pushed
the
udc-api
branch
from
35ef6b5 to
eb0c335
Compare
Description
Fixes: #5497
This adds a
api/v1/user_depositendpoint as specified in the linked issue for interacting with the UDC.It has three possibilities for POST requests depending on the body parameter:
{total_deposit: <amount>}{planned_withdraw_amount: <amount>}{withdraw_amount: <amount>}This is needed for having a user-friendly interface to the UDC in the WebUI.