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.

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

REST-API: Performing a deposit with total_deposit less than current total_deposit returns 200 response #2630

Closed
Dominik1999 opened this Issue Sep 28, 2018 · 5 comments

Comments

Projects
None yet
4 participants
@Dominik1999
Contributor

Dominik1999 commented Sep 28, 2018

Raiden node returns 200 SUCCESS when setting total_deposit to something smaller than current total_deposit (withdrawing)

This test runs successfully with a Raiden node.


scenario:
  serial:
         name: Scenario F11 (Depositing negative amount II)
         tasks:
           - open_channel: {from: 0, to: 1, total_deposit: 99_000_000_000_000_000_000}
           - assert: {from: 0, to: 1, total_deposit: 99_000_000_000_000_000_000, balance: 99_000_000_000_000_000_000, state: "opened"} 
           - deposit: {from: 0, to: 1, total_deposit: 1, expected_http_status: 200}
           - assert: {from: 0, to: 1, total_deposit: 99_000_000_000_000_000_000, balance: 99_000_000_000_000_000_000, state: "opened"}

Therefore, it is possible to set total_deposit to 1 token (the Raiden Node returns code 200) without changing the status of total_deposit.

I used my own token with 18 decimals.

@ulope ulope removed their assignment Sep 28, 2018

@ulope ulope changed the title from Red Eyes Release - Depositing total_amount less than current total_amount leads to 200 code SUCCESS to REST-API: Performing a deposit with total_deposit less than current total_deposit returns 200 response Sep 28, 2018

@ulope

This comment has been minimized.

Collaborator

ulope commented Sep 28, 2018

The reason is this:

raiden/raiden/api/python.py

Lines 399 to 401 in 49832df

if total_deposit <= channel_state.our_state.contract_balance:
# no action required
return

@Dominik1999

This comment has been minimized.

Contributor

Dominik1999 commented Sep 28, 2018

ok then, this is no bug and the return code 200 is what we expect? Then I close this again

@palango

This comment has been minimized.

Collaborator

palango commented Sep 28, 2018

ok then, this is no bug and the return code 200 is what we expect?

From my understanding we should return 409 CONFLICT in this case

@ulope

This comment has been minimized.

Collaborator

ulope commented Sep 28, 2018

Yes, this is definitely a bug IMO.

Esp. since we catch the DepositMismatch in the rest api layer:

raiden/raiden/api/rest.py

Lines 1046 to 1050 in 49832df

except DepositMismatch as e:
return api_error(
errors=str(e),
status_code=HTTPStatus.CONFLICT,
)

which describes exactly the case you're testing:

class DepositMismatch(RaidenRecoverableError):
""" Raised when the requested deposit is lower than actual channel deposit
Used when a *user* tries to deposit a given amount of token in a channel,
but the on-chain amount is already higher.
"""
pass

@Dominik1999

This comment has been minimized.

Contributor

Dominik1999 commented Sep 28, 2018

ok nice, I found my first bug then

@LefterisJP LefterisJP self-assigned this Sep 30, 2018

LefterisJP added a commit to LefterisJP/raiden that referenced this issue Sep 30, 2018

@palango palango added the minor bug label Oct 1, 2018

hackaugusto added a commit to hackaugusto/raiden that referenced this issue Oct 5, 2018

hackaugusto added a commit to hackaugusto/raiden that referenced this issue Oct 5, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment