Skip to content
This repository has been archived by the owner on May 13, 2022. It is now read-only.

configurable thresholds for utxo merge policies #414

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

configurable thresholds for utxo merge policies #414

wants to merge 1 commit into from

Conversation

adlai
Copy link
Contributor

@adlai adlai commented Feb 2, 2016

see #91 and #173, specifically this comment

utACK 😅

@adlai adlai force-pushed the merge-policies branch 2 times, most recently from 190347c to 5d5cc57 Compare February 4, 2016 09:44
@adlai
Copy link
Contributor Author

adlai commented Feb 4, 2016

rebased + removed unicode

@adlai adlai force-pushed the merge-policies branch 2 times, most recently from ce9346e to ac009e7 Compare February 4, 2016 11:20
@adlai adlai mentioned this pull request Feb 7, 2016
Open
@chris-belcher
Copy link
Collaborator

Untested but I agree with it.

Aside: The comments in the default joinmarket.cfg file seem to be getting quite large and wordy, I wonder if it's worth documenting the options on the wiki instead, with only a link in .cfg

Bikeshed: the default threshold should be higher for privacy IMO. Use bitcoin.select() up to about 30 UTXOs, switch to gradual around 40, greedy at 50 and greediest at 60+. My yield generator wallet has about 65 UTXOs if anyone is wondering. Having lots of UTXOs is great for privacy, its like owning a total of 20,000 CHF in a mixture of banknote sizes from 5 CHF all the way up to 1000 CHF notes. Spending them leaks a lower bound of your balance.

@adlai
Copy link
Contributor Author

adlai commented Feb 10, 2016

Re: bikeshed, bumped the numbers up and amended the comment.

@chris-belcher
Copy link
Collaborator

Has anyone tried this yet? It could be used on someone's yield generator.

@chris-belcher
Copy link
Collaborator

I'd say lets merge this into develop and just fix it if it crashes when being used, develop is allowed to be slightly unstable.

@AdamISZ
Copy link
Member

AdamISZ commented Apr 10, 2016

I'd say lets merge this into develop and just fix it if it crashes when being used, develop is allowed to be slightly unstable.

I sort of agree, but I just wrote #496 and I think it applies here. If we want to make a new release, is it needed to add this?

If it is, please provide test coverage of some sort.

@chris-belcher
Copy link
Collaborator

chris-belcher commented Apr 10, 2016 •
edited
Loading

Yes fair enough, write a test

This is still worth merging as a default because JoinMarket takers dont really have a way to handle very large amounts of UTXOs yet so this would be good to help prevent them happening in the first place.

This was referenced May 25, 2016
Open
Closed
@AdamISZ AdamISZ force-pushed the develop branch 3 times, most recently from 97d2603 to 47479d5 Compare September 13, 2016 11:23
adlai
adlai commented Jan 3, 2017
View reviewed changes
joinmarket/support.py Outdated
return selectors[i](unspent, value)
except IndexError: # luser configured more levels than algos
log.debug("I'm sorry, Dave, but I can't let you merge that!")
return selectors[-1](unspent, value) # just use the greediest one
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/1/0/?

@adlai adlai force-pushed the merge-policies branch 2 times, most recently from 24dc385 to 139fd73 Compare January 3, 2017 02:27
@adlai
Copy link
Contributor Author

adlai commented Jan 3, 2017 •
edited
Loading

rebased onto current develop. travis failure is mysteriously inscrutable, to me at least.

test pending.

@chris-belcher
Copy link
Collaborator

That test failure is confusing to me as well. Got no idea.

@AdamISZ
Copy link
Member

AdamISZ commented Jan 11, 2017

@adlai Looks like a circular dependency because you imported bitcoin into support.py

@adlai adlai force-pushed the merge-policies branch 5 times, most recently from 75eac86 to 35d0cf2 Compare February 14, 2017 11:46
@adlai adlai force-pushed the merge-policies branch 17 times, most recently from ea31242 to 76567cd Compare April 17, 2017 04:41
@coveralls
Copy link

Coverage Status

Coverage increased (+0.2%) to 77.473% when pulling 76567cd on adlai:merge-policies into 8c2b6d8 on JoinMarket-Org:develop.

@adlai adlai force-pushed the merge-policies branch 2 times, most recently from 84c9b7a to 3b3bca9 Compare April 17, 2017 05:55
@coveralls
Copy link

Coverage Status

Coverage increased (+0.2%) to 77.503% when pulling 3b3bca9 on adlai:merge-policies into 8c2b6d8 on JoinMarket-Org:develop.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.2%) to 77.488% when pulling 3b3bca9 on adlai:merge-policies into 8c2b6d8 on JoinMarket-Org:develop.

@adlai adlai force-pushed the merge-policies branch 2 times, most recently from 31a260e to 099973b Compare October 3, 2017 08:11
@coveralls
Copy link

Coverage Status

Coverage increased (+0.4%) to 78.552% when pulling 099973b on adlai:merge-policies into 46fe3e8 on JoinMarket-Org:develop.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.4%) to 78.49% when pulling 729df6c on adlai:merge-policies into 46fe3e8 on JoinMarket-Org:develop.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.4%) to 78.552% when pulling 729df6c on adlai:merge-policies into 46fe3e8 on JoinMarket-Org:develop.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@adlai @chris-belcher @AdamISZ @coveralls