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.

Sign up for GitHub

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

Jump to bottom

Store banned coins in file #10964

Merged
merged 10 commits into from Jul 21, 2023
Merged

Store banned coins in file #10964

merged 10 commits into from Jul 21, 2023

Conversation

Szpoti
Copy link
Collaborator

@Szpoti Szpoti commented Jul 1, 2023

Similar to #10957

This approach adds CoinPrison to CoinJoinManager, so we can get and call CoinPrison directly in CoinJoinManager for banning.

Also introduces an even subscription in Global to WalletManager which loads in banned state of coins. This function can later on be deleted when we get rid of SmartCoin.IsBanned.

@Szpoti Szpoti requested review from molnard, adamPetho and turbolay and removed request for molnard and adamPetho July 1, 2023 13:20
@Szpoti Szpoti mentioned this pull request Jul 1, 2023
Closed
turbolay
turbolay approved these changes Jul 2, 2023
View reviewed changes
Copy link
Collaborator

@turbolay turbolay left a comment

Choose a reason for hiding this comment

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

Everything in this solution LGTM.
I have some nits on naming, like exc that could be renamed ex, or banDeadlineTime which is not a perfect name. But nothing important enough to not approve this PR.

adamPetho
adamPetho requested changes Jul 4, 2023
View reviewed changes
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.

cACK

WalletWasabi.Daemon/Global.cs Outdated Show resolved Hide resolved
WalletWasabi.Daemon/Global.cs Outdated Show resolved Hide resolved
WalletWasabi/WabiSabi/Client/Banning/CoinPrison.cs Outdated Show resolved Hide resolved
WalletWasabi/WabiSabi/Client/Banning/CoinPrison.cs Outdated Show resolved Hide resolved
WalletWasabi/WabiSabi/Client/Banning/CoinPrison.cs Outdated Show resolved Hide resolved
WalletWasabi/WabiSabi/Client/Banning/CoinPrison.cs Outdated Show resolved Hide resolved
WalletWasabi/WabiSabi/Client/Banning/CoinPrison.cs Outdated Show resolved Hide resolved
WalletWasabi/WabiSabi/Client/Banning/CoinPrison.cs Outdated Show resolved Hide resolved
WalletWasabi/WabiSabi/Client/CoinJoinProgressEvents/CoinBanned.cs Show resolved Hide resolved
WalletWasabi/WabiSabi/Client/CoinJoinManager.cs Outdated Show resolved Hide resolved
molnard
molnard requested changes Jul 5, 2023
View reviewed changes
Copy link
Collaborator

@molnard molnard left a comment

Choose a reason for hiding this comment

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

Fix CF-s as well.

WalletWasabi/WabiSabi/Client/Banning/CoinPrison.cs Outdated Show resolved Hide resolved
@Szpoti
Copy link
Collaborator Author

Szpoti commented Jul 13, 2023

Fixed

molnard
molnard requested changes Jul 19, 2023
View reviewed changes
Copy link
Collaborator

@molnard molnard left a comment

Choose a reason for hiding this comment

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

cACK - almost done gj

WalletWasabi/WabiSabi/Client/Banning/CoinPrison.cs Outdated Show resolved Hide resolved
WalletWasabi/WabiSabi/Client/Banning/CoinPrison.cs Outdated Show resolved Hide resolved
WalletWasabi/WabiSabi/Client/Banning/CoinPrison.cs Show resolved Hide resolved
WalletWasabi/WabiSabi/Client/CoinJoinProgressEvents/CoinBanned.cs Show resolved Hide resolved
molnard
molnard previously approved these changes Jul 19, 2023
View reviewed changes
Copy link
Collaborator

@molnard molnard left a comment

Choose a reason for hiding this comment

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

tACK

@Szpoti Szpoti requested a review from adamPetho July 19, 2023 13:14
molnard
molnard approved these changes Jul 21, 2023
View reviewed changes
Copy link
Collaborator

@molnard molnard left a comment

Choose a reason for hiding this comment

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

still tACK

@molnard molnard merged commit b0d520c into zkSNACKs:master Jul 21, 2023
6 of 7 checks passed
@molnard molnard deleted the storeCoinInFile branch July 21, 2023 11:08
@MarnixCroes MarnixCroes mentioned this pull request Aug 3, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@turbolay turbolay turbolay approved these changes

@molnard molnard molnard approved these changes

@adamPetho adamPetho adamPetho approved these changes

Assignees
No one assigned
Labels
size/L
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@Szpoti @molnard @turbolay @adamPetho