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

Add revalidation check to Output Manager service #1953

Merged
merged 1 commit into from
Jun 9, 2020

Conversation

philipr-za
Copy link
Contributor

@philipr-za philipr-za commented Jun 4, 2020

Description

Outputs can become marked as invalid by the Output Manager service when they do not appear in the base nodes blockchain. Generally this means that they were re-orged out or have been spent by another copy of the wallet. However they can be reported as invalid incorrectly in some case like, for example, if the Base Node is not fully synced.

This PR adds in an addition check that the Output Manager does on startup that will see if any of the invalid outputs has become valid again. If it has that output will changed back into a spendable output.

How Has This Been Tested?

The test of the UTXO query has been updated to include checking this revalidation behaviour.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Feature refactor (No new feature or functional changes, but performance or technical debt improvements)
  • New Tests
  • Documentation

Checklist:

  • I'm merging against the development branch.
  • I ran cargo-fmt --all before pushing.
  • I have squashed my commits into a single commit.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.

@philipr-za philipr-za force-pushed the philip-revalidate-utxos branch 2 times, most recently from f0dec64 to 96451a9 Compare June 5, 2020 12:39
@philipr-za philipr-za changed the title [DNM] Add revalidation check to Output Manager service Add revalidation check to Output Manager service Jun 5, 2020
@philipr-za philipr-za force-pushed the philip-revalidate-utxos branch from 96451a9 to c909725 Compare June 9, 2020 09:02
Copy link
Member

@sdbondi sdbondi left a comment

Choose a reason for hiding this comment

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

LGTM - What other triggers make sense for querying invalid outputs? Connecting to a new base node with a higher chain tip? (falls under #1805)

One comment to resolve then I'll approve

base_layer/wallet/src/output_manager_service/service.rs Outdated Show resolved Hide resolved
Outputs can become marked as invalid by the Output Manager service when they do not appear in the base nodes blockchain. Generally this means that they were re-orged out or have been spent by another copy of the wallet. However they can be reported as invalid incorrectly in some case like, for example, if the Base Node is not fully synced.

This PR adds in an addition check that the Output Manager does on startup that will see if any of the invalid outputs has become valid again. If it has that output will changed back into a spendable output.
@philipr-za philipr-za force-pushed the philip-revalidate-utxos branch from c909725 to 6458a3d Compare June 9, 2020 09:54
@CjS77 CjS77 merged commit d8085bd into development Jun 9, 2020
@CjS77 CjS77 deleted the philip-revalidate-utxos branch June 9, 2020 14:48
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.

3 participants