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

fix(wallet): detect base node change during txo_validation #4610

Conversation

sdbondi
Copy link
Member

@sdbondi sdbondi commented Sep 2, 2022

Description

Interrupt txo_validation_protocol and txo_validation_task if base node is changed

Motivation and Context

These long-running tasks continue to run using the same base node even if it has changed. This PR checks for base node changes and interrupts the tasks at the correct points.

Other tasks may also need to be interrupted in a similar way.

Ref #4599 - this may fix this issue, but more info is needed to confirm

How Has This Been Tested?

Manually, changing the base node and checking that the tasks end.

@sdbondi sdbondi force-pushed the wallet-base-node-change-interrupt branch 3 times, most recently from 37a75b2 to 3e9f236 Compare September 5, 2022 04:28
stringhandler
stringhandler previously approved these changes Sep 5, 2022
@stringhandler
Copy link
Collaborator

That test looks like it broke in this pr

@sdbondi
Copy link
Member Author

sdbondi commented Sep 5, 2022

Looks like there are some problems here that need to be figured out - #4599 needs more info because it's unclear if the symptom noted in the issue is because the txo validation has begun before the base node is switched or after

@sdbondi sdbondi force-pushed the wallet-base-node-change-interrupt branch from dc2bee8 to 047e846 Compare September 5, 2022 13:30
@sdbondi sdbondi marked this pull request as ready for review September 5, 2022 13:30
@sdbondi
Copy link
Member Author

sdbondi commented Sep 5, 2022

The base node watch is a little tricky because there is no way to say "I've seen the value" without awaiting on .changed() - has_changed() doesn't cut it (i.e. it will be changed again the next time the protocol starts . So instead I poll changed() with the main future and interrupt if base node has changed and that seems to work better

@sdbondi sdbondi force-pushed the wallet-base-node-change-interrupt branch from 047e846 to 5c1aba9 Compare September 5, 2022 13:59
@sdbondi sdbondi force-pushed the wallet-base-node-change-interrupt branch from 5c1aba9 to ecaebc0 Compare September 6, 2022 05:27
@stringhandler stringhandler merged commit 2a2a8b6 into tari-project:development Sep 6, 2022
@sdbondi sdbondi deleted the wallet-base-node-change-interrupt branch September 6, 2022 12:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants