RFC: add consolidate stragegy #58
Conversation
|
up |
| var utxoValue = utils.uintOrNaN(utxo.value) | ||
|
|
||
| // skip detrimental input | ||
| if (utxoFee > utxo.value) { |
There was a problem hiding this comment.
If I have a 1000 sat utxo and it adds a 1000 sat fee to my transaction, this will add it...
Is this assumption also made in other parts of the library? If so that's fine, but if not, I would perhaps add a parameter for the user to define a percentage of fee that is the cutoff.
IMO if we have a utxo of 1000 value, the highest acceptable fee should be 999 and the lowest should be 0. The user of this API should be able to decide whether they want to skip above 10 or skip above 900.
There was a problem hiding this comment.
This is copypasted from
Line 20 in becfd26
|
So whats the general feedback? Should I add more tests (which ones would be best, besides coverage?), amend documentation and submit a PR? |
|
My general feedback is that this library needs a big overhaul... but I keep putting it off. As for this work in progress, I would say coverage definitely needs to get up higher for branches... 100% is a pretty severe threshold though. I would be fine with merging this if the coverage gets up... but I make no guarantees it will survive when I take a chainsaw to this library and mold it into something better (hopefully). But I will definitely publish a version with it if you can get coverage and documentation up. Thx |
|
there is no demand for this ATM. |
After discussion here #43 I believe this is a better way to implement consolidation strategy. While I happily use
splitforsendMaxfunctionality, I think slightly adjustedaccumulativestrategy will allow tech-savvy users to make regular BTC transactions (utilizing batch send when needed) while sweeping all their inputs for consolidation.In this PR I basically copied
accumulativestrategy while fixing the loop to add all inputs and not stop when fee+amount requirements are satisfied. Added couple of tests, but probably not enough.Did not change any documentation. Looking for an ACK before proceeding.