Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refine AS Punishment Criteria #10699

Merged
merged 16 commits into from
May 18, 2023
Merged

Conversation

nopara73
Copy link
Contributor

@nopara73 nopara73 commented May 15, 2023

Co-authored by @MaxHillebrand

This PR refines which coinjoin outputs' anonymity scores (AS) need to be punished and how much.

Current Behavior (master)

Currently, on the master branch, the following happens:

During anonymity calculation, in a WW2 coinjoin, if we see a unique, standard denomination output that >= second largest output in the coinjoin, then we punish its anonymity down to the smallest anonymity we can find in our inputs.

This PR

This PR changes the behavior mentioned above in 2 parts:

Part 1:

During anonymity calculation, in a WW2 coinjoin, if we see a unique, standard denomination output that >= second largest output in the coinjoin

In this PR the last criteria is that the output must be larger than the largest non-unique foreign output. In this way we catch more edge cases.

Part 2:

then we punish its anonymity down to the smallest anonymity we can find in our inputs.

In this PR we punish not to the smallest anonymity of all our inputs, but to the smallest anonymity of our own largest inputs of the coinjoins.

Impact

This is for whales and plebwhales (large plebs in small pleb rounds.) This is also for those with the privacy profile, because one occasion can invalidate a lot of work.

Not sure how large the impact of this PR. I tested with my main wallet and it did not change anon percentage. I also tested with some testnet wallets I had and it changed anon percentage for only one.

T6 					0% -> 0%
Random Wallet		32% -> 32%
Random Wallet 2		29% -> 29%
T5					98% -> 98%
T4					4% -> 5%

ToDo

  • nicer code
  • unit tests

MaxHillebrand
MaxHillebrand previously approved these changes May 15, 2023
Copy link
Collaborator

@MaxHillebrand MaxHillebrand left a comment

Choose a reason for hiding this comment

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

cACK

@turbolay
Copy link
Collaborator

turbolay commented May 15, 2023

EDIT: This comment is incorrect and was a misunderstanding, see Max's comment

Let's say I'm a whale, I have these inputs:

  • 100 BTC / 80 AS
  • 10 BTC / 1 AS (red coin)

All other are plebs, < 0.1 BTC (it's just for the example)

I have a non standard output of 109 BTC. Therefore we can deterministically link my 2 inputs together
On master it will inherit 1 AS, which is correct -> it had been deanonimized
On this PR it will inherit 80 AS, which doesn't seems correct

@MaxHillebrand
Copy link
Collaborator

On this PR it will inherit 80 AS, which doesn't seems correct

No, your two inputs are the two largest, so both will be included in the punishment, thus you would have 1 AS

@turbolay
Copy link
Collaborator

No, your two inputs are the two largest, so both will be included in the punishment, thus you would have 1 AS

I understood thanks for the clarification. Wording of the 2nd part is misleading: but to the smallest anonymity of our own largest inputs of the coinjoins.. It should be but to the smallest anonymity of our own inputs included in the punishment.

@nopara73 nopara73 marked this pull request as ready for review May 17, 2023 10:18
@nopara73 nopara73 merged commit acfc1ca into WalletWasabi:master May 18, 2023
0 of 4 checks passed
@nopara73 nopara73 deleted the largeststddenom branch May 18, 2023 07:07
@turbolay
Copy link
Collaborator

A PR changing AS computation with diff of +339/-80 was merged few hours only after marking it as ready for review without any reviews?

@lontivero lontivero added the affiliate Could affect affiliates compatibility label May 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affiliate Could affect affiliates compatibility size/L
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants