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

[WIP1 - Wasabi Improvement Proposal] Refactoring Internals #2359

Open
nopara73 opened this issue Sep 28, 2019 · 1 comment
Open

[WIP1 - Wasabi Improvement Proposal] Refactoring Internals #2359

nopara73 opened this issue Sep 28, 2019 · 1 comment

Comments

@nopara73
Copy link
Collaborator

@nopara73 nopara73 commented Sep 28, 2019

Motivation

In the Meta issue, called "Roadmap? How to replace nopara73?" one of the todo is to "refactor internal data structures of Wasabi Wallet. I have been making numerous moves to facilitate this (#2188, #2352, #2194, #2195, #2350, #2319, #2318, #2317, #2259, #2258, #2226, #2216, #2201, #2198, #2197) but lately I've hit a roadblock, thus I believe a more conscious specification is due. Thus this issue.

High Level Roadmap

On a high level, I am planning to do 2 steps. Step 3 is optional and I expect anyone could easily implement enabled features if they wish/feel the need for them.

  1. Make relevant mempool and transaction serialization wallet independent.
  2. Make all wallets load by default and being monitored.
  3. Implement features those are enabled by step 2.

Benefits of Step 1.

If we make the mempool and transaction serialization and caching wallet independent, then

  • We will be able to send from one wallet to another, without the other wallet needing to be open. The other wallet will pick up the unconfirmed transaction right away instead of the need to wait for confirmation.
  • There may also be some unconfirmed transaction loss, this work would fix, but I doubt. We had this issue in the past, we don't seem to have it anymore.
  • We would be able to remove stuff from the walletservice, which is our largest low-level hotspot right now. (A hotspot is complex code that changes frequently. This is where bugs are thrive.)

These benefits are relatively small compared to the work needed, however it enables Step 2.

Benefits of Step 2.

If the mempool and transaction cache are wallet independent, we can implement loading all the wallets instantly at start.

  • We can remove the BlockchainState from KeyManager, which is causing currently our most terrific bug: incorrect wallet state.
  • We will be able to load all wallets instantly, because we won't search through the serialized blocks anymore, but we'll just go through the serialized transaction cache.
  • The user doesn't need to load the wallet, just keep running wasabi and unconfirmed transactions will not be lost.

Step 2 has amazing benefits in itself, but it also enables a host of other interesting features, which I am not planning to implement: this is Step 3.

Step 3

Step 3 is a host of features that Step 1 and 2 enables:

  • Multi-wallet loading.
  • Sending from one wallet to another with merge avoidance (#2280, #2129, #1628, #1476, #1010, #2129)
  • Mixing from one wallet to another (eg. mixing directly to hardware wallet.) (#1326)
  • Calculating anonymity sets cross-wallet. So if one sends from wallet1 to wallet2, then anonset wouldn't be reset to 1. So all your wallets could be considered just being one big wallet in this regard.
  • Preventing cross process launch without ruining functionality, because it'll be possible to mix with multiple wallets with the same software instance.
  • Preventing cross process launch will eliminate numerous possible mistakes. (Although there are still the question if the daemon should be able to run alongside the GUI, but that's for another time.)

There are numerous more benefits that I don't have on top of my mind.

Design of Step 1

My initial approach to this issue was to implement Step 1 quickly. I did, but the code wasn't as clean as I wanted to, so I decided to break the PR down. I did break it down and the 10th part of the PR has been merged. Now I'm not in a position anymore to break much more things down (although I may still be able to) so I decided to implement the thing finally. I failed to create clean enough code, so my new approach is to specify the design here and work towards it.

Right now I'm thinking about creating 4 classes:

  • TransactionStoreBase -> Have numerous unit tests.
  • MempoolStore -> Inherits from TransactionStore, contains MempoolService, which contains a MempoolBehavior creator.
  • ConfirmedTransactionStore -> Inherits from TransactionStore
  • AllTransactionStore -> This is contained by BitcoinStore and contains MempoolStore and ConfirmedTransactionStore. This will be the way to access all transactions. This is important, because some operation on the MempoolStore and ConfirmedTransactionStore requires cooperation, so this AllTransactionStore class is created to eliminate possible future mistakes.

Cross Process Tx Serialization

Finally I specified the design of TransactionStoreBase previously: #2216 (comment)

The complexity is needed for cross-process compatibility.

  1. We put a system wide async mutex for reading and writing the transaction file, this will ensure concurrency issues.
  2. When we want to add a transaction, we just append it to the file. Thus if two processes receive the same transaction, we will append it to the file twice. This results in multiple transactions.
  3. When the whole file is first loaded, uniqueness must be ensured: duplicate transactions are to be also removed from the file.
  4. When we are to remove a transaction, we'll read the file and remove all the occurrances of that transaction.

I created some drawings to make that logic better understandable:

image

@nopara73 nopara73 changed the title [WIP - Wasabi Improvement Proposal] Refactoring Wasabi Wallet Internals [WIP1 - Wasabi Improvement Proposal] Refactoring Wasabi Wallet Internals Sep 28, 2019
@nopara73 nopara73 changed the title [WIP1 - Wasabi Improvement Proposal] Refactoring Wasabi Wallet Internals [WIP1 - Wasabi Improvement Proposal] Refactoring Internals Sep 28, 2019
molnard added a commit that referenced this issue Sep 30, 2019
@nopara73 nopara73 mentioned this issue Oct 1, 2019
0 of 1 task complete
@nopara73 nopara73 removed the UX label Oct 4, 2019
@molnard molnard added the Epic label Oct 22, 2019
@NicolasDorier

This comment has been minimized.

@nopara73 nopara73 removed the Epic label Nov 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.