kallewoof
commented at 2:13 am on May 11, 2017:
member
(Note: putting this on hold until #12257 is merged.)
#10065 brings up a privacy issue where a user can send a bunch of near-dust transactions to an address, which would be picked up by the coin select code when the owner funded transactions, connecting multiple transactions and addresses to the same user.
This adds a (by default turned off) flag -avoidreuse. When enabled, the wallet will mark any addresses that were used to fund a transaction as “dirty” and will avoid using them in funding additional transactions, unless an “allow dirty” flag is set.
It also adds support to allow dirty addresses in sendtoaddress. More tweaks to other RPC commands is necessary but I wanted to keep the PR as small as possible.
Retroactive flagging of dirty addresses can be done by rescanning the chain.
luke-jr
commented at 2:48 am on May 11, 2017:
member
IMO this isn’t complete unless incoming transactions to dirty addresses (even before being used!) are hidden as well (or at least flagged in some visible manner).
Also, shouldn’t rescanning be sufficient?
kallewoof force-pushed
on May 11, 2017
kallewoof
commented at 2:52 am on May 11, 2017:
member
@luke-jr: Maybe I am misunderstanding you, but incoming transactions are irrelevant. The only thing that matters is when you spend from an address. Each time you do, that address is marked dirty and any UTXOs pointing to it are automatically considered dirty in this implementation. Am I missing a case?
Also, shouldn’t rescanning be sufficient?
Oops - yes, rescanning is sufficient. Updated OP, thanks.
Edit: my definition of address reuse has always been “spending from the same address 2+ times”, whereas @luke-jr’s definition seems to be “any UTXOs which send to an address that has already been sent to”. Both definitions would solve the issue in question, but the latter would mean people could no longer say “to support my work send BTC to [static address]” if they wished to use this feature. The question ultimately is “which one of the two definitions makes the most sense?”
kallewoof force-pushed
on May 11, 2017
kallewoof force-pushed
on May 11, 2017
jonasschnelli added the label
Wallet
on May 11, 2017
in
src/wallet/wallet.cpp:878
in
a05d03a7e8outdated
in
src/wallet/wallet.cpp:3642
in
a05d03a7e8outdated
3638@@ -3598,6 +3639,7 @@ std::string CWallet::GetWalletHelpString(bool showDebug)
3639 strUsage += HelpMessageOpt("-walletnotify=<cmd>", _("Execute command when a wallet transaction changes (%s in cmd is replaced by TxID)"));
3640 strUsage += HelpMessageOpt("-zapwallettxes=<mode>", _("Delete all wallet transactions and only recover those parts of the blockchain through -rescan on startup") +
3641 " " + _("(1 = keep tx meta data e.g. account owner and payment request information, 2 = drop tx meta data)"));
3642+ strUsage += HelpMessageOpt("-avoidreuse", _("Mark addresses which have been used to fund transactions in the past, and avoid reusing these in future funding, except when explicitly requested"));
Maybe add a AssertLockHeld(cs_wallet); somewhere above GetDestData?
in
src/wallet/rpcwallet.cpp:422
in
a05d03a7e8outdated
418@@ -416,12 +419,15 @@ UniValue sendtoaddress(const JSONRPCRequest& request)
419 " transaction, just kept in your wallet.\n"
420 "5. subtractfeefromamount (boolean, optional, default=false) The fee will be deducted from the amount being sent.\n"
421 " The recipient will receive less bitcoins than you enter in the amount field.\n"
422+ "6. allowdirty (boolean, optional, " + (GetBoolArg("-avoidreuse", false) ? "default=false" : "unavailable") + ") Allows spending from dirty addresses; addresses are considered\n"
Why unavailable? Someone may just have enabled -avoidreuse but hasn’t rescanned. Maybe keep a state somewhere if we have scanned with -avoidreuse up to the wallets bestblock.
It returns “unavailable” if you do not have the feature turned on. I don’t want people to think they can avoid using dirty coins if they are running without the flag. I.e. even by explicitly saying allowdirty=false, it will still use dirty coins when -avoidreuse is not enabled.
You are right though, that it will still use dirty coins unless you rescan. Feels like that should be mentioned somewhere, but not sure where.
kallewoof force-pushed
on May 11, 2017
kallewoof force-pushed
on May 11, 2017
gmaxwell
commented at 10:09 am on May 18, 2017:
contributor
I support the goal, but: If an address is paid 10 btc then 0.0001 btc and then a transaction spends the latter, A would be dirty and the 10 BTC are stuck. That seems sub-optimal.
The best idea I had to deal with that previously is that whenever you spend from A you make an effort to spend all payments to A or at least all non-dust payments to A. Then the only time you get dirty funds is when someone pays a non-negligible amount to an address after you’ve already spent to it.
If something is going to cause transactions to fail, we probably will need another kind of balance display to make the behavior explicable.
Independently from this (but also useful for it) we probably should have an icon in the transaction list for reused addresses. (Trefoil made of arrows? :P )
kallewoof
commented at 1:49 pm on May 18, 2017:
member
You are able to spend by using the allowdirty flag in sendtoaddress, and you can always make a raw transaction yourself. The intention of this is to give expert users a way to plug the gaping security hole that exists in the system – to make it intuitive and wonderful for the average user is not an aspiration at this point (as mentioned, in particular RPC commands need some work before this could ever be made a default-on feature).
The solutions you present are all solutions an expert user could make use of with this implementation. It would simply stop their clients from shooting them (privacy wise) in the foot automatically.
Edit: Re-reading, I realize you are talking about coin select algorithm. That’s an interesting idea. It would make sense to consider all “coins” going to A as a single coin for as long as you haven’t spent from A yet. That way you select on an all-or-nothing basis (perhaps excluding dust).
kallewoof force-pushed
on May 19, 2017
kallewoof force-pushed
on Jul 13, 2017
kallewoof
commented at 2:00 am on July 13, 2017:
member
Rebased (and code became slightly more clean thanks to new coin control being present).
kallewoof force-pushed
on Sep 28, 2017
kallewoof force-pushed
on Sep 28, 2017
kallewoof force-pushed
on Sep 28, 2017
kallewoof force-pushed
on Sep 28, 2017
kallewoof force-pushed
on Oct 11, 2017
kallewoof force-pushed
on Oct 11, 2017
kallewoof force-pushed
on Dec 5, 2017
in
src/wallet/coincontrol.h:31
in
f596a41ca2outdated
26@@ -27,6 +27,8 @@ class CCoinControl
27 boost::optional<CFeeRate> m_feerate;
28 //! Override the default confirmation target if set
29 boost::optional<unsigned int> m_confirm_target;
30+ //! Allows inclusion of dirty (previously used) addresses
31+ bool fAllowDirtyAddresses;
promag
commented at 1:04 pm on January 9, 2018:
member
Should creating transactions to dirty addresses fail?
Needs rebase.
Nit, commit messages don’t have period.
kallewoof force-pushed
on Jan 10, 2018
kallewoof
commented at 1:04 am on January 10, 2018:
member
@promag Thanks for the review! Fixed the default usage issue.
Edit: and the commit message dots.
kallewoof force-pushed
on Jan 11, 2018
kallewoof force-pushed
on Jan 19, 2018
kallewoof renamed this:
[wallet] Optional '-avoidreuse' flag which defaults to not reusing addresses in sends
[WIP] [wallet] Optional '-avoidreuse' flag which defaults to not reusing addresses in sends
on Feb 20, 2018
kallewoof closed this
on Mar 15, 2018
laanwj referenced this in commit
5f7575e263
on Jul 24, 2018
kallewoof reopened this
on Jul 24, 2018
DrahtBot
commented at 6:52 pm on July 24, 2018:
member
DrahtBot added the label
Needs rebase
on Jul 24, 2018
[wallet] Set 'dirty' DestData for used addressesccd5326d3b
[wallet] Exclude dirty coins in AvailableCoins resultsb032f70473
[wallet] Add allow_dirty_addresses to CCoinControl and adhere to this in AvailableCoinscab7197213
[wallet] [rpc] Add allowdirty option to sendtoaddress RPC and make dirty tracking optional as -avoidreuse flag8297ae1611
[doc] Add -avoidreuse documentation to -help and manpagesfd910c6141
[wallet] Make GetAvailableCredit (and thus getbalance RPC) adhere to dirty state6c178d0908
[test] Add test for avoidreuse featurec353877abd
kallewoof force-pushed
on Jul 25, 2018
kallewoof closed this
on Jul 25, 2018
kallewoof deleted the branch
on Jul 25, 2018
meshcollider referenced this in commit
44d8172323
on Jun 18, 2019
sidhujag referenced this in commit
6bcaa95d9d
on Jun 19, 2019
laanwj removed the label
Needs rebase
on Oct 24, 2019
Munkybooty referenced this in commit
3a9c562e6e
on Jun 15, 2021
Munkybooty referenced this in commit
c40f154def
on Jun 15, 2021
Munkybooty referenced this in commit
0f644d5f5f
on Jun 16, 2021
Munkybooty referenced this in commit
e2bf529053
on Jun 22, 2021
Munkybooty referenced this in commit
82659a4eb8
on Jun 24, 2021
Munkybooty referenced this in commit
640b1ceea2
on Jun 24, 2021
This is a metadata mirror of the GitHub repository
bitcoin/bitcoin.
This site is not affiliated with GitHub.
Content is generated from a GitHub metadata backup.
generated: 2025-06-16 12:13 UTC
This site is hosted by @0xB10C More mirrored repositories can be found on mirror.b10c.me