raiden icon indicating copy to clipboard operation
raiden copied to clipboard

Possible protocol ambiguities for withdraw

Open palango opened this issue 3 years ago • 2 comments

Reference: https://github.com/raiden-network/light-client/issues/2177 Especially this comment

Christian saw some the linked problem during testing withdraws on the light client. See the description below. We suspect that python and light client handle a part of withdrawal differently and as it looks like the python client is not answering a valid WithdrawRequest I'm opening this issue.

From our side, what I could see that happened, is expected:

  • after some transfers and successful withdraws
  • LC was requested a new withdraw; it did persist it, receive a confirmation but was killed before signing/broadcasting the transaction
    • This is expected, we do persist the confirmation, but don't retry the tx; it shouldn't be fatal, it's not an error to NOT send a withdraw tx, and just let it expire
  • A transfer was successfully made after that, indicating the clients were still in sync
  • Then a new withdraw was requested, while the previous one was still pending; it shouldn't be a big deal either, we do support parallel withdraw requests (total_withdraw is monotonic, so it should be safe to sign confirmations to partner's requests as long as their balance [not including pending withdraws] allows the requested withdraw), but looks like python didn't like receiving a new WithdrawRequest without the previous one either getting confirmed or expiring.
    • @palango can you confirm raiden-py doesn't like the second withdraw request while the first one was still pending? Any reason why it shouldn't?
    • An alternative explanation for the request not being accepted by raiden-py is that it took into account the balance minus the pending withdraw, while it should not subtract the pending value since withdraw is a monotonic value, and later, larger withdraws can supersede smaller ones, as long as partner has enough balance. We took some time to get it to work properly

From my understanding the python client supports multiple parallel withdraws.

If somebody has ideas about the cause of this please let me know, otherwise I'll try to write a test reproducing this.

palango avatar Oct 09 '20 14:10 palango

We found the problem. It's basically this part of the test: https://github.com/raiden-network/raiden/blob/9ddcfea09f36aea9346e8b22926fe35edfdad27b/raiden/tests/unit/test_channelstate.py#L1712-L1735

Before that a withdraw of 20T was requested. Then the same withdraw over 20T is tried again. There should be no harm in confirming it again, as the total_withdraw is a monotone value.

See also https://github.com/raiden-network/light-client/issues/2177#issuecomment-706225757 and below.

palango avatar Oct 09 '20 15:10 palango

Not only same value retries, but any value which is inside the [0, totalWithdrawable] range (where totalWithdrawable is balance without deducing the offchain_total_withdraw=largest pending total_withdraw amount) should be accepted. total_withdraw is a monotonic value, it's valid to request values smaller or larger than a previous pending or even already mined withdraws (less than on-chain is useless, but not invalid).

To illustrate this, imagine partner had a balance o 20T, had all 20 available for withdraw, sent a WithdrawRequest for all of it, we signed and sent a WithdrawConfirmation. Instead of withdrawing it on-chain, while this withdraw request was still pending, they deposit +10 tokens. In this case, their balance (how many they can transfer to us) is 10, until they can expire this withdraw and get back to balance=30. But instead of transfering, they want now to withdraw everything. They could send us a request now (even while the previous request was still valid) of 30T (or 25, or 15, but not 31). LC accepts it, PC doesn't.

If it's of some use, this is the algorithm we use to calculate these balances.

andrevmatos avatar Oct 09 '20 16:10 andrevmatos