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

WasabiSynchronizer: Add InitialRequestTcs #11343

Conversation

kiminuo
Copy link
Collaborator

@kiminuo kiminuo commented Aug 22, 2023

Follow-up to #11303

Tries to simplify the wallet loading a bit as explained in #11303 (comment)

Thinking about it, I don't think we need https://github.com/kiminuo/WalletWasabi/blob/925ca7c3283a90137b785ec83e7140bc3731ead3/WalletWasabi.Fluent/Models/Wallets/WalletLoadWorkflow.cs#L76-L81 now.

Review with whitespace off.

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.

tACK on TestNet

ichthus1604
ichthus1604 previously approved these changes Aug 22, 2023
Copy link
Collaborator

@ichthus1604 ichthus1604 left a comment

Choose a reason for hiding this comment

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

tACK.

@ichthus1604
Copy link
Collaborator

@kiminuo

I'm interested in removing this: https://github.com/kiminuo/WalletWasabi/blob/925ca7c3283a90137b785ec83e7140bc3731ead3/WalletWasabi.Fluent/Models/Wallets/WalletLoadWorkflow.cs#L106-L109

Is there a way to guarantee that LastResponse will be set before triggering the TCS? Can be in another PR if you like.

…allet

# Conflicts:
#	WalletWasabi.Fluent/Models/Wallets/WalletLoadWorkflow.cs
@kiminuo
Copy link
Collaborator Author

kiminuo commented Aug 23, 2023

Unfortunately, there was conflict with #11246. Resolved.

Is there a way to guarantee that LastResponse will be set before triggering the TCS? Can be in another PR if you like.

I would like to fix it in a next step.

var processedCount = GetCurrentProcessedCount();
UpdateProgress(processedCount);
})
.Subscribe(
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Just fix whitespace

Copy link
Collaborator

@ichthus1604 ichthus1604 left a comment

Choose a reason for hiding this comment

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

tACK.

@ichthus1604 ichthus1604 merged commit cbd7dd5 into WalletWasabi:master Aug 24, 2023
6 of 7 checks passed
@kiminuo kiminuo deleted the feature/2023-08-22-Synchronizer-and-load-wallet branch August 24, 2023 15:49
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