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

Introduce PeriodicRunner in WasabiSynchronizer #12269

Merged
merged 13 commits into from
Jan 19, 2024

Conversation

Szpoti
Copy link
Collaborator

@Szpoti Szpoti commented Jan 17, 2024

Built upon #12262, this PR introduces PeriodicRunner in WasabiSynchronizer as a base class.

With this change we can rework the running logic of WasabiSynchronizer which before was basically a "PeriodicRunner by hand".
Now that the old base class was changed, we can use PeriodicRunner instead to run WasabiSynchronizer regularly.
With these changes, WasabiSynchronizer was registered as a HostedService.

The action logic is the same, a few logical details was tweaked so it fits and behaves the same as before.

@Szpoti Szpoti changed the title Introduce PeriodicRunner in WasabiSynchronize Introduce PeriodicRunner in WasabiSynchronizer Jan 17, 2024
@Szpoti
Copy link
Collaborator Author

Szpoti commented Jan 17, 2024

Arguably WasabiSynchronizer could be added to HostedServices so it's handled with all similar services as well.

WalletWasabi.Daemon/Global.cs Outdated Show resolved Hide resolved
WalletWasabi.Daemon/Global.cs Outdated Show resolved Hide resolved
WalletWasabi/Services/WasabiSynchronizer.cs Outdated Show resolved Hide resolved
@lontivero
Copy link
Collaborator

cACK. It looks very good to me. I will review it asap.

@@ -132,7 +133,7 @@ private static ITransactionBroadcasterModel CreateBroadcaster(Network network)

private static IAmountProvider CreateAmountProvider()
{
return new AmountProvider(Services.Synchronizer);
return new AmountProvider(Services.HostedServices.Get<WasabiSynchronizer>());
Copy link
Collaborator

Choose a reason for hiding this comment

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

Theoretical question:
Isn't it problematic that we require a non-static object inside a static function?

I tested and nothing is broken, just a question.

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.

Overally LGTM, I couldn't break the wallet.

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

@molnard molnard merged commit 1744db3 into WalletWasabi:master Jan 19, 2024
5 of 7 checks passed
@molnard molnard deleted the refactorWasabiSynchronizer branch January 19, 2024 13:57
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

4 participants