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

CoinsRegistry: CoinsByOutPoint -> CoinsByPrevOuts #11699

Conversation

kiminuo
Copy link
Collaborator

@kiminuo kiminuo commented Oct 16, 2023

CoinsByOutPoint (Dictionary<OutPoint, HashSet<SmartCoin>>) is not good especially because we have OutpointCoinCache (Dictionary<OutPoint, SmartCoin>)

@turbolay
Copy link
Collaborator

Just a quick note, it's not really by PrevOut, as it's by Inputs' PrevOuts. So the name might suggest that it's the same as the CoinsByOutPoint cache.
In fact, it should even be grouped by transactions, as the values are grouped by transaction anyway (This was the idea of #11516), but anyway, this name is better. The previously introduced comment is a great help

Copy link
Collaborator

@adamPetho adamPetho left a comment

Choose a reason for hiding this comment

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

LGTM

@kiminuo
Copy link
Collaborator Author

kiminuo commented Oct 16, 2023

Just a quick note, it's not really by PrevOut, as it's by Inputs' PrevOuts. So the name might suggest that it's the same as the CoinsByOutPoint cache.
In fact, it should even be grouped by transactions, as the values are grouped by transaction anyway (This was the idea of #11516), but anyway, this name is better. The previously introduced comment is a great help

I agree. I'll merge this but feel free to suggest a better name. I'm playing with Adam's PR to remove this altogether but I figured it's better to rename it anyway because the name is confusing to me. The new one is at least less confusing.

@kiminuo kiminuo merged commit d01ad85 into WalletWasabi:master Oct 16, 2023
6 of 7 checks passed
@kiminuo kiminuo deleted the feature/2023-10-16-CoinsRegistry-rename-variable branch October 16, 2023 20:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants