CoinsRegistry: Introduce CoinsByPubKeys
#11571
Conversation
| public bool RegisterToHdPubKey() | ||
| => HdPubKey.Coins.Add(this); | ||
|
|
||
| public bool UnregisterFromHdPubKey() | ||
| => HdPubKey.Coins.Remove(this); |
There was a problem hiding this comment.
Functionality was moved to CoinsRegistry.
| @@ -246,10 +281,10 @@ public async Task ConfirmedDoubleSpendAsync() | |||
| int doubleSpendReceived = 0; | |||
| transactionProcessor.WalletRelevantTransactionProcessed += (s, e) => | |||
| { | |||
| var doubleSpents = e.SuccessfullyDoubleSpentCoins; | |||
| if (doubleSpents.Any()) | |||
| var doubleSpends = e.SuccessfullyDoubleSpentCoins; | |||
| public class TransactionProcessorTests | ||
| { | ||
| [Fact] | ||
| public async Task TransactionDoesNotCointainCoinsForTheWalletAsync() | ||
| public async Task TransactionDoesNotContainCoinsForTheWalletAsync() |
| @@ -68,8 +67,6 @@ public double AnonymitySet | |||
| private set => RaiseAndSetIfChanged(ref _anonymitySet, value); | |||
| } | |||
|
|
|||
| public HashSet<SmartCoin> Coins { get; } = new HashSet<SmartCoin>(); | |||
There was a problem hiding this comment.
Replaced with private Dictionary<HdPubKey, HashSet<SmartCoin>> CoinsByPubKeys { get; } = new(); in CoinsRegistry.
| if (!CoinsByPubKeys.TryGetValue(coin.HdPubKey, out HashSet<SmartCoin>? coinsOfPubKey)) | ||
| { | ||
| coinsOfPubKey = new(); | ||
| CoinsByPubKeys.Add(coin.HdPubKey, coinsOfPubKey); | ||
| } | ||
|
|
||
| coinsOfPubKey.Add(coin); |
There was a problem hiding this comment.
This is slightly longer but we can have a helper to make it shorter. But that would be a good change for another PR I guess.
There was a problem hiding this comment.
Not sure it is required. Especially with #11516 where the CoinsRegistry would basically be a succession of Dictionary.TryGetValue calls
EDIT: I'm answering to your comment, not talking about the code.
| { | ||
| coinsOfPubKey.Remove(coin); | ||
|
|
||
| if (coinsOfPubKey.Count == 0) |
There was a problem hiding this comment.
If a single HdPubKey is used a lot, then we should allocate less than now.
| if (!CoinsByPubKeys.TryGetValue(coin.HdPubKey, out HashSet<SmartCoin>? coinsOfPubKey)) | ||
| { | ||
| coinsOfPubKey = new(); | ||
| CoinsByPubKeys.Add(coin.HdPubKey, coinsOfPubKey); | ||
| } | ||
|
|
||
| coinsOfPubKey.Add(coin); |
There was a problem hiding this comment.
Not sure it is required. Especially with #11516 where the CoinsRegistry would basically be a succession of Dictionary.TryGetValue calls
EDIT: I'm answering to your comment, not talking about the code.
Alternative to #11534
The idea of this PR is to remove
HdPubKey.Coinsand move it toCoinsRegistrywhich feels like a better place to put it conceptually.