Auto-freezing of address reuse below a threshold. #471

AdamISZ · 2019-12-15T22:02:07Z

Closes #274. Utxos are disabled if they are sent
to a reused address, and are below a threshold
set by the value min_sats_freeze_reuse in the
POLICY section of the config file. If the value
is -1, such utxos are always frozen irrespective of
the value.
Users are prompted with a warning level logging message
on CLI and a popup on Joinmarket-Qt. Such disabled utxos
can of course be re-enabled by the existing methods.

AdamISZ · 2019-12-15T22:11:39Z

A small technical point, but the function which does fast sync of addresses and indices in the wallet is renamed to sync_addresses_fast and the action of just getting addresses in-label is factored out as get_address_usages (this name was previously used for the whole thing; name was a bit misleading), since that action can be useful in isolation.
Notice also the small change that the set of used addresses returned from the latter function is now persisted as a class instance variable for the duration of a run and kept up to date in the always-called check_for_reuse. Obviously the main intent is to have a full list of usages at start up and then keep it up to date for long running bots.

There may be some exceptional case where a bot quits early and the utxo doesn't get frozen in time to be persisted for the next run. I'm not sure how much we need to worry about this, although it could certainly be looked at.

Tests OK on CLI and Qt; i tested below and above the threshold, but so far only on regtest.

jmclient/jmclient/wallet_service.py

chris-belcher · 2019-12-15T23:20:53Z

The code looks fine to me.

But I disagree with the use of the phrase "dust attack". Because in bitcoin the term "dust" is also used for very low valued UTXOs (for example 1 satoshi) which are uneconomic to spend. If a "dust attack" actually used "dust" then the attack would fail, because no wallet would actually spend the uneconomic UTXOs. Also the name is not very descriptive and almost negative-helpful in understanding the issue. I'd prefer the phrase "forced address reuse".
Also, feel free to ignore this suggestion but it might be useful to link the wiki page (https://en.bitcoin.it/wiki/Privacy#Forced_address_reuse) in the joinmarket.cfg explanation of min_sats_freeze_reuse, so that users can more easily read about what they're defending against.

AdamISZ · 2019-12-17T12:31:03Z

Good point; "dust attack" not only doesn't capture the phenomenon, but is even, by a reasonable argument, totally wrong.
But now I'm thinking about this, "forced address reuse" is a bit of a dodgy name too since the reuse is not forced .. and in fact that's the entire point of this PR :) It's more like "incentivized address reuse".

chris-belcher · 2019-12-17T14:24:55Z

In my mind the "forced" part refers to the sending to rather than spending from.

"incentivized address reuse" is also a good name, it captures the idea that these attacks economically are a bribe to the user in exchange for their privacy-relevant information.

chris-belcher · 2019-12-17T14:27:36Z

The algorithm behind this PR could be improved like this:

If an address has been fully-spent from and is now empty, then ignore any further deposits.
If an address already has UTXOs on it then spend all the UTXOs in one transaction.

If an address has UTXOs on it then this PR will freeze any additional UTXOs and keep them cluttering up the wallet and UTXO set, and leaving on the table the free money. Its probably better to spend them but make sure that all UTXOs from one address get spent in the same transaction. However I appreciate this might significantly complicate the code so maybe it's not worth it. EDIT: maybe it's not too complicated of an edit, you just need to access the wallet's list of UTXOs to tell whether to freeze a new UTXO or not, and in the input-choosing routine add code which spends all UTXOs on one address together. No extra lists or dicts are needed I think.

AdamISZ · 2019-12-17T15:00:58Z

If an address has been fully-spent from and is now empty, then ignore any further deposits.

I'm presuming you don't mean unconditionally do this? i.e. still with a filter as per the existing setup in the PR?

you just need to access the wallet's list of UTXOs to tell whether to freeze a new UTXO or not

I think I misunderstood your implicit context here, but the main thing that required any real coding in this PR is that we can't only use utxos to check usage, but have to check history to see if that first condition applies.

The "spend all at once" idea is interesting, but my spidey sense is tingling a bit that we're making things a bit more complicated. We're changing selection algos. I think what's in the PR is a decent first cut algorithm-wise.

Freezing utxos isn't that big of a deal after all; it's like 2 mouse clicks in the GUI to reverse it, and it'll happen extremely infrequently that this causes a disruption for a user (and only if they screw up).

AdamISZ · 2019-12-17T15:01:37Z

I should add test code for it though; I actually just forgot.

chris-belcher · 2019-12-17T21:43:29Z

I'm presuming you don't mean unconditionally do this? i.e. still with a filter as per the existing setup in the PR?

yes, I include the amount filter thing, I just didn't mention it.

you just need to access the wallet's list of UTXOs to tell whether to freeze a new UTXO or not
I think I misunderstood your implicit context here, but the main thing that required any real coding in this PR is that we can't only use utxos to check usage, but have to check history to see if that first condition applies.

For this part I meant: you check the wallet UTXO list to tell whether a newly-arriving UTXO on an already-used address is arriving to a spent-from-and-empty address or an address with UTXOs still unspent.

The "spend all at once" idea is interesting, but my spidey sense is tingling a bit that we're making things a bit more complicated.

Sure understood. It's worth noting that this feature already exists in Bitcoin Core 0.17 https://github.com/bitcoin/bitcoin/blob/master/doc/release-notes/release-notes-0.17.0.md#partial-spend-avoidance

AdamISZ · 2019-12-18T16:19:38Z

For this part I meant: you check the wallet UTXO list to tell whether a newly-arriving UTXO on an already-used address is arriving to a spent-from-and-empty address or an address with UTXOs still unspent.

Right, understood, we do need history (I was asking because you said we just need ... which made me think you were implying we only need utxo info, not history).
Don't really want to make things more complicated right now, as per previous comment. Can be added later; seem reasonable?

jmclient/test/test_walletservice.py

chris-belcher · 2019-12-19T13:19:11Z

jmclient/test/test_walletservice.py

+    load_program_config()
+    old_config = jm_single().config
+    def reset_config():
+        jm_single().config = old_config


Does this actually work? I would've thought wouldn't work because python uses pass-by-reference.

Oh, that was dumb of me, nice spot, thanks.
I was going to deepcopy to resolve this but it turns out it's not at all so simple for these configparser objects, so I've gone back to the less clever version, updating just this one changed variable. Seems to work fine as of 794726a

chris-belcher · 2019-12-19T13:20:02Z

Added some comments to the test code

AdamISZ · 2019-12-19T14:48:23Z

Should I change min_sats_freeze_reuse to max_sats_freeze_reuse? I guess so.

AdamISZ · 2019-12-21T14:11:59Z

Any more thoughts on this? I'll probably change the above var name but I think it's done apart from that, @chris-belcher ?

chris-belcher · 2019-12-21T14:39:24Z

Yes, remove and rephrase all mentions of the phrase "dust attack" from the code and config file.

AdamISZ · 2019-12-21T16:47:20Z

Replaced references to 'dust attack' with 'forced address reuse' and linked to the Privacy wiki in the config.
I renamed to max_sats_freeze_reuse since that value is the maximum value for the freeze action to occur so it's more logical imo.

AdamISZ · 2019-12-23T11:32:23Z

@chris-belcher OK after b9f97bc ? I'll squash and merge if so.

chris-belcher · 2019-12-23T11:40:54Z

Looks good to me

Closes #274. Utxos are disabled if they are sent to a reused address, and are below a threshold set by the value `max_sats_freeze_reuse` in the `POLICY` section of the config file. If the value is -1, such utxos are always frozen irrespective of the value. Users are prompted with a warning level logging message on CLI and a popup on Joinmarket-Qt. Such disabled utxos can of course be re-enabled by the existing methods. Also adds test case for address reuse freezing function.

d719ff2 Auto-freezing of address reuse below a threshold. (Adam Gibson)

chris-belcher reviewed Dec 15, 2019

View reviewed changes

jmclient/jmclient/wallet_service.py Show resolved Hide resolved

AdamISZ force-pushed the anti-dust-attack branch from 42a9b18 to 315e98e Compare December 18, 2019 16:05

chris-belcher reviewed Dec 19, 2019

View reviewed changes

jmclient/test/test_walletservice.py Outdated Show resolved Hide resolved

chris-belcher reviewed Dec 19, 2019

View reviewed changes

AdamISZ force-pushed the anti-dust-attack branch from c7f843f to 0d86205 Compare December 23, 2019 11:49

AdamISZ force-pushed the anti-dust-attack branch from 0d86205 to d719ff2 Compare December 23, 2019 12:20

AdamISZ added a commit that referenced this pull request Dec 23, 2019

Merge #471: Auto-freezing of address reuse below a threshold.

353b4aa

d719ff2 Auto-freezing of address reuse below a threshold. (Adam Gibson)

AdamISZ merged commit d719ff2 into master Dec 23, 2019

AdamISZ deleted the anti-dust-attack branch December 23, 2019 14:37

SomberNight mentioned this pull request Jan 21, 2021

Electrum lacks a clear way to handle dust attacks spesmilo/electrum#6960

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Auto-freezing of address reuse below a threshold. #471

Auto-freezing of address reuse below a threshold. #471

AdamISZ commented Dec 15, 2019

AdamISZ commented Dec 15, 2019

chris-belcher commented Dec 15, 2019 •

edited

AdamISZ commented Dec 17, 2019

chris-belcher commented Dec 17, 2019

chris-belcher commented Dec 17, 2019 •

edited

AdamISZ commented Dec 17, 2019

AdamISZ commented Dec 17, 2019

chris-belcher commented Dec 17, 2019

AdamISZ commented Dec 18, 2019

chris-belcher Dec 19, 2019

AdamISZ Dec 19, 2019

chris-belcher commented Dec 19, 2019

AdamISZ commented Dec 19, 2019

AdamISZ commented Dec 21, 2019

chris-belcher commented Dec 21, 2019

AdamISZ commented Dec 21, 2019

AdamISZ commented Dec 23, 2019

chris-belcher commented Dec 23, 2019

Auto-freezing of address reuse below a threshold. #471

Auto-freezing of address reuse below a threshold. #471

Conversation

AdamISZ commented Dec 15, 2019

AdamISZ commented Dec 15, 2019

chris-belcher commented Dec 15, 2019 • edited

AdamISZ commented Dec 17, 2019

chris-belcher commented Dec 17, 2019

chris-belcher commented Dec 17, 2019 • edited

AdamISZ commented Dec 17, 2019

AdamISZ commented Dec 17, 2019

chris-belcher commented Dec 17, 2019

AdamISZ commented Dec 18, 2019

chris-belcher Dec 19, 2019

Choose a reason for hiding this comment

AdamISZ Dec 19, 2019

Choose a reason for hiding this comment

chris-belcher commented Dec 19, 2019

AdamISZ commented Dec 19, 2019

AdamISZ commented Dec 21, 2019

chris-belcher commented Dec 21, 2019

AdamISZ commented Dec 21, 2019

AdamISZ commented Dec 23, 2019

chris-belcher commented Dec 23, 2019

chris-belcher commented Dec 15, 2019 •

edited

chris-belcher commented Dec 17, 2019 •

edited