Skip to content

Conversation

@weilu
Copy link
Contributor

@weilu weilu commented Jun 18, 2014

  • utxo.receive renamed to utxo.from
  • getUnspentOutputs now includes the pending field
  • do not overestimate fees when network has dustSoftThreshold

estimateFeePadChangeOutput was adding an output with amount zero, which works for bitcoin as it does not care about the amount when estimating tx fee. It causes a consistent overestimation when it comes to networks with a soft dust threshold used for applying additional fees when the output amount is below that threshold.

Now with the proposed modification there's a chance that we could be underestimating -- when the change amount happens to fall below the soft dust threshold. But for lite & doge the slight underestimation is acceptable?

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 79a17d6 on weilu:utxo into db5a6d0 on bitcoinjs:master.

@dcousens
Copy link
Contributor

+1 LGTM. As discussed on IRC, would be nice to be able to improve that input selection algorithm, but that is out of scope of this PR.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.0%) when pulling a99e825 on weilu:utxo into db5a6d0 on bitcoinjs:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.0%) when pulling 82b1d8f on weilu:utxo into db5a6d0 on bitcoinjs:master.

dcousens added a commit that referenced this pull request Jun 21, 2014
Wallet fee & utxo fixups
@dcousens dcousens merged commit f4940cc into bitcoinjs:master Jun 21, 2014
@kyledrake
Copy link
Contributor

Sorry about the delay on this one, at Bitcoin in the Beltway, just finished up a talk. +1 on this.

I wanted to bring up a question regarding the fees though. I'm confused as to why we have moved the fee calculation code from the Transaction to the Wallet object. I have done transactions without even using a wallet before, it seems like something that should be in TransactionBuilder when @dcousens refactors. I'm running on 26 hours without sleep, so apologies if I'm talking crazy talk.

@dcousens
Copy link
Contributor

In regards to Transaction, fees are just the unspent remainder of outputs.
There is no meaningful way to handle fees without a wallet like abstraction
(aka change handling). Whether you use our wallet specifically is
irrelevant.

Also, I think the proposed rename for network.estimateFee was
network.minimumFee.

And TransactionBuilder is only capable of displaying the fee paid if all
input (aka prevOut) values are known.

On Jun 22, 2014 12:25 PM, "Kyle Drake" notifications@github.com wrote:

Sorry about the delay on this one, at Bitcoin in the Beltway, just
finished up a talk. +1 on this.

I wanted to bring up a question regarding the fees though. I'm confused as
to why we have moved the fee calculation code from the Transaction to the
Wallet object. I have done transactions without even using a wallet before,
it seems like something that should be in TransactionBuilder when
@dcousens https://github.com/dcousens refactors. I'm running on 26
hours without sleep, so apologies if I'm talking crazy talk.


Reply to this email directly or view it on GitHub
#219 (comment)
.

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