libitx / txforge

Modern Bitcoin transaction builder, capable of supporting any non-standard and custom script type.
Apache License 2.0
63 stars 21 forks source link

Updating dust value to align with https://github.com/bitcoin-sv/bitco… #7

Closed F1r3Hydr4nt closed 3 years ago

F1r3Hydr4nt commented 3 years ago

…in-sv/releases/tag/v1.0.5

As can be seen here: https://github.com/bitcoin-sv/bitcoin-sv/blob/782c043fb39060bfc12179792a590b6405b91f3c/src/test/transaction_tests.cpp#L651 And here: https://github.com/bitcoin-sv/bitcoin-sv/blob/782c043fb39060bfc12179792a590b6405b91f3c/test/functional/dustrelayfee.py

The minimum dust threshold has been reduced to 135 and miners running bsv-1.0.5 are mining txs with values just higher than that e.g 140 satoshis here: https://whatsonchain.com/tx/75386e3f14e1507355fb9780b24aa70c42c9338e9e1a7b479bffb4d57e94c0c5

Perhaps you would prefer it to be added to the default options to allow for overriding? If so, let me know.

Thanks

libitx commented 3 years ago

I must confess I’m a little confused by the new dust rules. What’s the difference between the 135 threshold and the 140 figure? At what size are dust outputs spendable without needing to consolidate?

My thinking is the default behaviour of txforge should be that it always builds transactions that are accepted by miners and its outputs are spendable without the user needing to worry about dust. For those wanting to create ultra low dust outputs where consolidation transactions will be necessary to spend the dust, then that should be opted in to by overriding the dust limit on the forge instance.

F1r3Hydr4nt commented 3 years ago

I thought as much and you're correct in that consolidation will be necessary to spend the dust limit utxos so I understand your UX concerns... Option B is certainly the route that should be taken now, overriding the dust limit in the constructor options or otherwise.

The difference between those values is 5 satoshis, I did not attempt to test 135 sats (we thought it was 150 actually, then surprised at 140, until confirming 135 in the C++ test).

Not every miner has updated to 1.0.5 too so this sledgehammer approach isn't for everyone.

Would it simply be the case of adding the limit to the defaults and overriding in the options? Is there any example of this in the repo? If not I'll figure it out in the morning and come back to you.

Thanks

F1r3Hydr4nt commented 3 years ago

This seems to work and have confirmed the 135 Satoshi dust limit here too: https://whatsonchain.com/tx/1e791c5a811f4dca8d0463830b6daba02ff4546ca373d335357d9a054c4df003

libitx commented 3 years ago

Looks good, thanks. 👍

I've merged this in. I'll get a release done soon

F1r3Hydr4nt commented 3 years ago

Thank you for the wonderful library! :+1:

libitx commented 3 years ago

@freddiehonohan FYI your PR has been superseded by #8 which replaces the dust limit with a function that calculates the correct dust threshold for each output.

No need to pass in dustLimit option any more. Version 0.2.0 is out today.

F1r3Hydr4nt commented 3 years ago

Cool will take a look thanks