-
Notifications
You must be signed in to change notification settings - Fork 602
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
listunspent returns spent transactions #91
Comments
Thanks for the report. I'll ask @jrick to take a look at this. On a different note, one thing I noticed is you are specifying the port and certificate. While there is nothing wrong with that, since both are defaults, you can shorten that a bit to:
It also generally makes sense to put the RPC username and password into ~/.btcctl/btcctl.conf so you don't have to specify it each time. With that it would shorten to just |
I see, thanks! I've looked into this a bit more and it seems that the bug occurs when transactions have several outputs and more than one of the target addressees belongs to the wallet. It also calculates the balance incorrectly in this case. |
Does transaction 93d2b08415ca9fa0b203417e35aabcd645a293be83cbfc5ac44eac43c6b5703a appear in the wallet transaction history? |
@jrick How do I check? @davecgh Omitting the command-line options as you suggested does not work for me:
|
@goldmar Sorry I wasn't clear. You need $ btcctl -u rpcuser -P rpcpass --wallet --testnet help If you put the following in your [Application Options]
rpcuser=yourusername
rpcpass=yourpass Then you can omit the $ btcctl --wallet --testnet help |
|
@davecgh Thanks, it works now! I should have read your post more carefully. @jrick when I run that btcwallet crashes:
|
Thanks, that's very helpful. Will look into this. |
@goldmar Two more tests: Can you look again for that transaction in the output of 'btcctl --testnet --wallet listtransactions 99999'? (the large number is to override the default maximum 10 transactions returned). The previous crash was in the gettransaction handler, and this should avoid that codepath. Once that has been done, can you shutdown wallet, delete your ~/.btcwallet/testnet/tx.bin, and restart wallet? This will cause btcwallet to rebuild its transaction history again with a rescan. Essentially, I'm looking for whether a notification was missed or mishandled (in which case the transaction store is in some state inconsistant with reality) or if a bug in the transaction store is the cause. |
Here you go:
So yeah, the transaction is there! After deleting the file you mentioned and restarting btcwallet it correctly returns no unspent transactions:
But it still crashes on |
Would you happen to know whether the spending transaction (93d2b08415ca9fa0b203417e35aabcd645a293be83cbfc5ac44eac43c6b5703a) was created by this wallet, or was produced by some other wallet software and then seen on the blockchain and notified by btcd? (Are you sharing keys across multiple wallets?) |
The spending transaction was created using btcwallet. I used |
The gettransaction handler was attempting to lookup the "sent-to" address of an outgoing transaction from the transaction store (as a wallet credit). This is the incorrect address when sending to an address controlled by another wallet, and panics when there are no credits (for example, sending to another wallet without any change address). Instead, use the first non-change output address is used as the address of the "send" result. This fixes the panic reported when debugging issue #91. While here, fix the category strings used for wallet credits to support immature and generate (the categories for coinbase outputs).
As an update to this issue, I believe that I have found the cause. If a transaction is added to wallet that debits from previous unspent outputs, but those outputs are not already known at the callsite (as is the case for stuff sent with sendrawtransaction), the transaction store must search for UTXOs which are now spent by that tx. That function does not search through transaction outputs for unmined transactions, so if you spent unconfirmed coins in this manner they will eventually move to confirmed when the tx is mined, but will still be marked unspent. |
Thanks for the update! That's exactly what I was doing (using unconfirmed transactions as inputs to |
If a transaction is added that debits from previous transaction outputs, and those outputs are still unconfirmed, it is possible that if the credits were not already known (as is the case with transactions notified after a sendrawtransaction), only mined unspent transaction outputs would be searched and the unconfirmed unspent credits would be missed. This results in spent outputs still being marked unspent. This change fixes the above by also searching through unconfirmed transactions when the previous credits must be lookup up, rather than being pass from an AddDebits call. Fixes issue #91.
listunspent
seems to return spent transactions.E.g., the first one clearly is redeemed: http://blockexplorer.com/testnet/tx/93d2b08415ca9fa0b203417e35aabcd645a293be83cbfc5ac44eac43c6b5703a#i36014931
The text was updated successfully, but these errors were encountered: