Clean the Label Chaos

[molnard]

📜 Preamble

Clean the Label Chaos by @molnard

📖 TL;DR - Too Long, Didn't Read

The labeling system is an essential element of privacy in Wasabi. Coin selectors are basing selection on them or users when manual selection is done. It must work perfectly. Currently, it works more or less but it is like a wild octopus, grabbing information from here and there - nobody dares to touch it and fix current issues. The idea is to have a separate module just for labels, plus unit tests.

🌟 Specification, Scope, and Features

• Refactoring the label system by removing it from SmartCoin and storing it in a separate module.
• The module will be optimized for speed and memory usage.
• The module should be designed in a wallet-independent manner since we have cross wallet label system.
• Keep in mind label exporting as a feature in the future.

⚔️ Motivation

As I mentioned earlier, the labeling system is an essential element. It should work perfectly and be easily maintainable. Currently, it has a few issues that nobody dares to touch - because it might break other things. A lot of time and resources are spent on this because every time there is an issue, it is very hard to understand how it actually works.

• Stucked issues.
• Not maintainable.

📜 Rationale

The rationale behind this is not the solidest one. However, if we think about dev time spent on label issues and the effectiveness of fixing them perfectly is calling attention. It is not possible to shoot down issues one by one because the underlying business logic is flawed. So we can go all-in or don't touch it at all.

🌌 Backward Compatibility

Yes, it will be an essential part of migrating old labels to the new system in a safe way. We have to make sure to not loose any label. This is important.

📚 Reference Implementation (optional)

• all AS 1 coins inherit label after sending 2 tx's with private coins with similar label #10193
• [bug] [tesnet(?) edge case] Coins gains label of other wallets after opening that wallet #10980
• coin selection: non-private coin of same pocket as recipient label not selected when additional coin needs to be added for fee #11436
• Pocket selection / Label management: No option to manage the labels #11038
• Speed Up TX: Labeling CPFP txs #11153
• Speed Up Transaction: transaction gained labels after speeding up (no new UTXOs were added) #11241
• semi-private and private coins having labels #11313
• Pocket selection questions/issues #8080
• Lables #9806
• Send and receive transaction labels are the same when using wallets on the same machine #6056

🧑‍🏫 Team involvements

Lead: ?
Code team.

💪 Subtasks

The goal of the refactor is to free up SmartTransaction and SmartCoin from all the label-related data objects and move them into one place called LabelManager. That is it. We agreed with Lucas that the current concept is flawed but refactoring would lead to refactoring the whole transaction processing which is out of the scope of this Quest!

🕐 Overall time estimation: 8 weeks

⛔ What NOT to do

• Create a database or a file for LabelManager, use only memory.
• Remove information from files. Do not change the structure of the wallet file or transactions.dat.
• Try to make the solution more robust against reorg and RBF. The same behavior that we have now is OK.
• Thinking about any more recursive generation of total labels or anything like that. That would lead very far. It is enough to gather all the logic into LabelManager as a first step.

✅ Todo:

• Create a class called LabelManager PoC
  • Fill up LabelManager with the data the same way we do now with SmartCoin and SmartTransaction labels. Yes, it will be a duplication for now.
  • Store stuff in memory only (with the consideration of later it will be a database).
  • The data in LabelManager should look like a list of [PubKey, LabelArray, Wallet] There might be one or more labels or no labels at all. Similarly what you can see in the wallet file.
  • The public API of the class is like: "Give me the labels for this Pubkey".
  • Make sure that new information is processed as well, when we receive a new tx it should be processed in this LabelManager as well. For example, when I get a tx, the created coins will inherit the labels from the spent coins, etc.
  • Make sure to take care that the SmartTransaction itself can have labels as well. Check the Transactions.dat file.
• Remove all label-related stuff from SmartCoin and move into LabelManager. All the places where it has been referenced, now have to use LabelManager to get the information.
• Remove all label-related stuff from SmartTransaction and move into LabelManager. All the places where it has been referenced, now have to use LabelManager to get the information.
• Integrate Cluster calculation into LabelManager and remove Cluster class from HdPubKey class.
• Make sure that AnonScore is incorporated.
• Make sure that all the Label, Cluster, and related information is coming from LabelManager only.
• If it is working and makes sense try to add a few Unit Tests.
• If the concept makes sense and everyone likes it try to break down the PoC if required and create the final PR.

[turbolay]

Remove information from files. Do not change the structure of the wallet file or transactions.dat.

Why? The Labels have absolutely nothing to do in the wallet.json file, and having them there creates problems. In fact, it causes exactly what I'm trying to fix in #12668, but without success.

Because the real solution for #12668 is to remove the labels from the wallet.json file!!!

The problem is simple: When we resync, we want to recompute the state. But, we do not start from a clean state of the KeyManager. We start from a pre-existing state. It makes no sense, and the only reason why we are doing this is to preserve the labels, but what are those labels doing there in the first place??? We should have a Labels repository database and stop mixing everything together.

[molnard]

We are on the same page. This is moving label handling into a manager. In my imagination, this is the first step. Once we have done that, we can proceed with what you are suggesting. I set those boundaries to define the scope of our work.

BTW the issue was moved here from the release game repo to have an entry point for the problem itself.