Skip to content
This repository was archived by the owner on Nov 2, 2018. It is now read-only.

Wallet implicit defrag#2692

Open
ChrisSchinnerl wants to merge 2 commits into
masterfrom
wallet-implicit-defrag
Open

Wallet implicit defrag#2692
ChrisSchinnerl wants to merge 2 commits into
masterfrom
wallet-implicit-defrag

Conversation

@ChrisSchinnerl

Copy link
Copy Markdown
Contributor

Once the wallet reaches a certain number of unspent outputs it will try to defrag them when spending coins. If no transactions are spent it will fall back to the defrag thread method.

Comment thread modules/wallet/transactionbuilder.go Outdated
}
sort.Sort(sort.Reverse(so))

// If we have too man unspent transactions we might as well do some

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Too man

@DavidVorick

Copy link
Copy Markdown
Member

Do we properly ignore dust outputs during transaction selection? For example, we don't want to be considering as a part of our defragging outputs that are smaller than the dust threshold - we should just consider that money to be gone.

I guess that's not super relevant to this PR, if we weren't doing it before, this isn't the right place to start doing it.

I waffled back and forth on whether selecting the smallest outputs first was the right move when doing a defrag, and I'm still not 100% convinced. The worry here is that if it's a large output, you'd consume every output in the wallet in one stroke, which would prevent you from sending different sets later on. The auto-defragger on the other hand is careful to make sure that you always have at least 10 large outputs available (the 10 largest).

I think what we should do instead here is after we're done collecting the outputs that we need to collect the transaction, check if we want to defrag, and then if we do, we should skip 10 outputs and then collect the remaining ones. That way the user always has 10 outputs available.


A big downside to defragging that I don't think we've discussed previously is privacy. Defragging a wallet correlates all of your outputs and destroys your privacy. I don't think most people care, but it's something we should continue to consider as we move forward.

@ChrisSchinnerl

Copy link
Copy Markdown
Contributor Author

@DavidVorick Yes, checkOutput considers dust and won't use dust outputs. I agree, skipping some of the largest outputs might be a good idea.
The wallet already has a setting that can be used to disable defrags. We probably just have to expose it.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants