Skip to content

Conversation

@weilu
Copy link
Contributor

@weilu weilu commented Jun 29, 2014

By simply marking an utxo as pending makes it impossible to differentiate incoming and outgoing pending utxos, which leads to erroneous results for getBalance. This PR fixes that by attaching the to property to a pending spending utxo.

getUnspentOutputs excludes such pending spending utxos because from a wallet point view, it should be able to trust spending transactions created by itself, and exclude any pending spent utxo from unspent transaction outputs.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.0%) when pulling f7af487 on weilu:pending-spending-utxo into df743e5 on bitcoinjs:master.

@dcousens
Copy link
Contributor

What do you define as an erroneous result for getBalance?

@kyledrake
Copy link
Contributor

+1 on this. I ran into this issue with Coinpunk as well. It's one of those problems you run into that proves to me you're actually working on a wallet ;)

It is theoretically possible to create transactions that fail due to an error, which could potentially gum up the wallet and create a couple bad transactions, but I think it's an acceptable risk.

@kyledrake
Copy link
Contributor

@dcousens If I'm understanding your question (and the issue), when the wallet itself sends a tx, it sends the change back to itself. You need a way to tell the wallet that it's a transaction that you created yourself, otherwise the wallet assumes it's untrustable and has to wait for at least one confirmation before that balance is shown, and it looks like you lost the amount that was in the output you used to create the transaction until confirmations come in. It's very confusing to users.

@dcousens
Copy link
Contributor

Right, so this is less a bug, and more an implementation of treating our own addresses as green addresses. Despite the fact that malleability could be a problem for re-spending change before confirmation?

Why not just getBalance, and getPendingBalance. A developer can then decide if they want to add the two for display, or show them separately.

@kyledrake
Copy link
Contributor

Let's get this wrapped up! It's the last thing.

dcousens added a commit that referenced this pull request Jul 1, 2014
wallet: reintroduce output.to to track pending spent utxo
@dcousens dcousens merged commit ab20feb into bitcoinjs:master Jul 1, 2014
@dcousens
Copy link
Contributor

dcousens commented Jul 1, 2014

@weilu despite merge, could you please clarify the above?

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants