raiden-network / raiden

Raiden Network
https://developer.raiden.network
Other
1.84k stars 376 forks source link

Define a strategy to handle failed transactions because the account can't pay for the gas #1861

Open hackaugusto opened 6 years ago

hackaugusto commented 6 years ago

Problem Definition

Transactions may fail because the user's account doesn't have enough ether to pay for the gas (#1815). This will result in the smart contract proxies raising an exception: after the transaction is sent, it's execution will fail, and once the proxy polls the receipt an exception is raised.

This can happen under two circumstances:

  1. The smart contract proxies are used directly. This can be done when:
    • The user uses the proxies directly from the console
    • Some of the RaidenAPI's uses the proxies directly (e.g. deposit, it is not an action that must be recoverable therefor is not in the WAL)
    • Some of the REST APIs use the respective methods from RaidenAPI which are not state machine aware.
  2. The proxies are used on a raiden event handler.

For the first scenario it's okay to fail with an exception, for either case a transaction or error message can be shown to the user. But, for the second scenario, the state of the raiden node will be changed, and we cant recover.

Example: The user wants to close the channel and calls close, this action is recoverable therefor it's written to the WAL, this is important because a channel that will be closed must not mediated transfers anymore (the node which is calling close cannot update the balance proof), after the state change is saved to the WAL and handled by the state machine, the ContractSendChannelClose event will be emitted which is responsible to send the closing transaction on-chain. If the transaction fails because the account doesn't have enough gas some action must be taken:

Alternatively, we can look for strategies to ensure that the account will always have enough ether to pay for the transaction.

TODO

LefterisJP commented 6 years ago

When working on https://github.com/raiden-network/raiden/pull/1858 I thought of the same problem.

My 2 cents:

The user uses the proxies directly from the console

I would not worry about this case as this is not supported. The console in general is just for us and other hardcore people. "Here Be Dragons".

Some of the REST APIs use the respective methods from RaidenAPI which are not state machine aware.

Which ones are these?

The proxies are used on a raiden event handler. But, for the second scenario, the state of the raiden node will be changed, and we cant recover.

Yes this is the troubling case as you describe in your example.

The Raiden node should quit

That can cause them to lose funds as they would no longer be online to monitor channels. On the other hand, even if the node stays online but has no ETH to go on-chain in case of a dispute it will still lose money.

A thread must wait until the account has enough ether to pay for the transaction, and then the transaction must be retried

This would be the best solution in a combination with strategies designed to keep the raiden node with enough ETH.

We should somehow warn the user if he is about to:

How to warn? Emitting warning messages in the log and a pop up in the webui are the first things I can think of.

hackaugusto commented 6 years ago

How to warn? Emitting warning messages in the log and a pop up in the webui are the first things I can think of.

And how often? Should we add a queue of warning in the channel state and have REST APIs to retrieve it?

err508 commented 6 years ago

after the state change is saved to the WAL and handled by the state machine, the ContractSendChannelClose event will be emitted which is responsible to send the closing transaction on-chain

If the clients chain listener doesn't get a confirmation for the transaction after some blocks, could the client roll-back to pre-failed-transaction state via its WAL?

this is important because a channel that will be closed must not mediated transfers

If the roll-back would work, the channel could/should still be usable - as closing failed, the client can still close or update onchain

We should somehow warn the user

hackaugusto commented 6 years ago

If the clients chain listener doesn't get a confirmation for the transaction after some blocks, could the client roll-back to pre-failed-transaction state via its WAL?

No, the transaction must succeed or fail. And we need to handle the race cases.

If the roll-back would work, the channel could/should still be usable - as closing failed, the client can still close or update onchain

Yes, but only after the transaction is known to have failed and the channel is still open, during the period of time which the node is waiting for the transaction to be mined, transactions cannot be mediated.

rakanalh commented 6 years ago

An idea comes to mind that could help us ensure availability of ether when the user is a participant in a channel. It goes as follows: Given that the raiden node sits behind an ethereum node, we can constantly watch gas prices. However, as soon as the the user enters a channel, we should try to lock enough ether for performing the close transaction and possible other transactions (if needed). If the gas price fluctuates to a point where the locked ether is no longer enough, then that's where we can log and show some sticky notification in the UI about the user "Being in danger of losing money". The sense of urgency should play a role in our messaging so that the user is to blame if he/she does lose his/her funds. The follow up question would be: What if the user didn't have enough ether ever since the channel was created (Being a participant), then we should also show the warning and emphasize on that and possibly link to a page where this is explained as to why we think it's not for the best interest of this user.

Would this be a possible solution to consider?

LefterisJP commented 6 years ago

@rakanalh

we should try to lock enough ether

We don't need to lock Ether. As per our docs the user should dedicate an account to Raiden and fill it up with Ether.

If we replace "lock" with "make sure that he has enough Ether in the account" then the logic seems to be similar to what I suggested above so I think we are in agreement.

In essence to keep watching the user's account and the gas price and then warn him when action needs to be taken.

What if the user didn't have enough ether ever since the channel was created (Being a participant), then we should also show the warning and emphasize on that and possibly link to a page where this is explained as to why we think it's not for the best interest of this user.

I would say in this case we should deny the opening of a channel. The whole point of such a strategy would be to look ahead and see in what state would any action (open a channel, join a token network) leave the user's ETH and if that state is viable or not.

@err508

we could prohibit deposits to a channel decreasing the account balance below a dispute threshold + no warranties for cmd line users. That would be an OSXish approach.

From the two this is what I would prefer. The point of a strategy (which would be configurable -- but defaults here would matter) is to try and protect the user to not end up in situations where he can lose money if he makes no action.

e.g. if he is about to join a token network by opening 3 channels and after that this would leave him with no ETH to dispute, we should warn him and deny the action.

LefterisJP commented 6 years ago

Removed this from Red Eyes in favour of #1998

hackaugusto commented 6 years ago

From #2129

[...] This, however, doesn't fix the case where a node acc1 which has ether opens a channel with a node acc2 which doesn't have ether.

Raiden can handle the acc2 case describe above better, by having a counter of how many channel lifecycles the node's ether support. In the case above that would be 0, and if a new channel is opened with acc2 the channel must stay in an inactive state, this is quite important because acc2 shouldn't accept off-chain payments which it cannot withdraw on-chain.

To do the above we need a new state change, which will update the ChainState's counter, this means the function check_gas_reserve should run on every new block, and if the estimated number of channel lifecycles change the state change must be dispatched.

Note: The above is required if we don't trust/rely on MSs. If we realy on MSs to call update the account doesn't need any ether, since the MSs can call update transfer on the user's behalf and the user can pay the MS off-chain. We do however have a chicken egg problem if the off-chain payment to the MSs is done in a bidirectional channel were both the MSs and the current Node are participants, in this case the MSs cannot be trusted to close the payment channel and call update.

LefterisJP commented 6 years ago

state change must be dispatched.

@hackaugusto And so this state change would be something like UpdateInternalChannelLifecycleGasCounter and if this is set to 0 then we stop accepting off-chain transfers since we would not be able to ever go on-chain to call updateTransfer() on them?

But I guess at the same time the warnings implemented by #2031 would be kicking in before that and the user should eventually fill the account again with Ether. And the "safe time" he would have to do so is the channel's settlement period. Which should be safe in the current default value of 2-3 hours.

rakanalh commented 4 years ago

Going back to the issue's description, Raiden currently fails with InsufficientFunds if the transaction's gas estimation fails because of lack of funds. This will cause the node to crash if the proxy is used in the event handler because the exception is not handled there. In some Rest API handlers, the exception is handled but the context is wrong (example: deposit also raises InsufficientFunds if the account doesn't have enough balance from the used token).

I believe the action to be taken here is to raise RaidenUnrecoverableError in case insufficient eth balance is detected.