novitski / bitcoinj

Automatically exported from code.google.com/p/bitcoinj
Apache License 2.0
0 stars 0 forks source link

proper error handling for sending coins #425

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
I think it's time to track this before it gets lost:

completeTx/sendCoins/etc can go wrong in a variety of ways. Currently, this is 
indicated only by returning null. Let's make that (checked) exceptions. I can 
think of the following distinct cases:

- out of funds for SendRequest.amount
- out of funds for SendRequest.fee
- tx exceeds max size
- ??? tx already signed
- ??? outputs already spent

This is a long standing issue. Since bitcoinj 0.9, we are facing a small 
regression here because since the framework decides the fee, the app cannot any 
more check for enough funds in advance like it was done before.

A common first usecase for many people is sending 1 BTC to the freshly 
downloaded app, then sending the 1 BTC back from where it came. Since bitcoinj 
0.9, this fails without helpful error message. Hence, the number of "cannot 
send, useless" complaints has risen sharply.

Original issue reported on code.google.com by andreas....@gmail.com on 3 Jul 2013 at 9:58

GoogleCodeExporter commented 9 years ago
D'oh. For the case where the user is trying to send their entire balance, you 
could automatically point out that they need to attach a fee, or offer to let 
them do it for free (and maybe take a long time).

Original comment by hearn@google.com on 4 Jul 2013 at 10:21

GoogleCodeExporter commented 9 years ago
Yeah - I ended up writing a specific support note mentioning the need for a fee 
here:
https://multibit.org/en/help/v0.5/help_support_insufficientFundsOnSend.html

It seems a sufficiently common gotcha that it needs mentioning.

Original comment by jimburto...@gmail.com on 4 Jul 2013 at 10:40

GoogleCodeExporter commented 9 years ago
I can check for the "send all" case, but how is the user able to attach a fee 
if it is calculated by the framework?

Actually I think the bitcoinj could have an option to extract the fee from the 
amount rather than adding to it - I'll probably open a separate ticket.

Original comment by andreas....@gmail.com on 4 Jul 2013 at 10:33

GoogleCodeExporter commented 9 years ago
You can still control the fee that's set!

Long-run the payment protocol will arrive and for payments that use it, the 
receiver can pay the fee.

Original comment by hearn@google.com on 5 Jul 2013 at 8:24

GoogleCodeExporter commented 9 years ago
I don't understand. Surely I can set the fee but that's not going to work. Its 
the framework that knows how much fee to pay, not the app.

I can imagine for the last tx out of the wallet its even more important to set 
the correct fee, because it's more likely to consist of many small inputs. I 
don't even know if one single dust output will probably spoil the whole thing 
(are dust outputs counted into wallet.getBalance(AVAILABLE)?).

Original comment by andreas....@gmail.com on 5 Jul 2013 at 9:05

GoogleCodeExporter commented 9 years ago
Well, to empty a wallet there'd only be one output so it's not an issue unless 
you have less than the min send amount in your wallet.

What you can do is completeTx, examine the fee field afterwards, then you know 
what the framework calculated and can set it yourself, then run completeTx 
again. Or adjust the fee-per-kb.

Anyway, yes, we need a better error API here for sure.

Original comment by hearn@google.com on 5 Jul 2013 at 9:22

GoogleCodeExporter commented 9 years ago

Original comment by hearn@google.com on 11 Jul 2013 at 3:38

GoogleCodeExporter commented 9 years ago
Does this look OK?

https://code.google.com/r/hearn-bitcoinj/source/detail?r=52f822a2ccbb8aca09ed8e5
b94b3c3ff4c104ecd

Original comment by mh.in.en...@gmail.com on 26 Nov 2013 at 5:22

GoogleCodeExporter commented 9 years ago
Super! I added two comments, but generally I'm really happy about this 
progress. Maybe in future I will ask for more specific exceptions (and maybe 
some of them checked), but for now its good I think. Will adapt my 
bitcoinj-0.11 branch very soon.

Original comment by andreas....@gmail.com on 26 Nov 2013 at 9:26

GoogleCodeExporter commented 9 years ago
One thing I'd immediately wish for is some kind of feedback in the 
InsufficientFundsException what actually did fit. Example: The user has 1 BTC, 
tries to send 0.9999999 BTC. He will expect to have enough coins, although 
because of the fee he hasn't. This is something I'd like to tell to the user. 
In this special case, I also want to offer to empty the wallet.

Original comment by andreas....@gmail.com on 26 Nov 2013 at 10:33

GoogleCodeExporter commented 9 years ago
Comments addressed, and the exception now has info on how much money is missing.

https://code.google.com/r/hearn-bitcoinj/source/detail?r=35561c53a531732b0fb858d
c7ed88b3a66348224

How does that look?

Original comment by hearn@google.com on 27 Nov 2013 at 11:22

GoogleCodeExporter commented 9 years ago
This issue was closed by revision 70cd2ffb9681.

Original comment by hearn@google.com on 27 Nov 2013 at 2:26