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

Check for old wallet data in correct location #2172

Merged
merged 1 commit into from Mar 6, 2024
Merged

Check for old wallet data in correct location #2172

merged 1 commit into from Mar 6, 2024

Conversation

luckysori
Copy link
Contributor

I believe some old apps do have a wallet file that indicates that the app should probably be migrated to the new wallet DB via a full sync.

But this is not the case for fresh 1.9.0 apps. For those, we need to consider the presence of the on_chain directory.

@luckysori luckysori requested a review from holzeis March 6, 2024 11:32
@luckysori luckysori self-assigned this Mar 6, 2024
@luckysori
fix(app): Check for old wallet data in correct location
b184698 
I believe some old apps do have a `wallet` file that indicates that
the app should probably be migrated to the new wallet DB via a full
sync.

But this is not the case for fresh 1.9.0 apps. For those, we need to
consider the presence of the `on_chain` directory.
@luckysori luckysori enabled auto-merge March 6, 2024 11:33
@holzeis
Copy link
Contributor

holzeis commented Mar 6, 2024

But this is not the case for fresh 1.9.0 apps. For those, we need to consider the presence of the on_chain directory.

Why isn't that for fresh 1.9.0 apps? did we change something there recently?

@holzeis holzeis linked an issue Mar 6, 2024 that may be closed by this pull request
Full sync was not run after upgrade from 1.9.0 to 1.10.0 #2164
Closed
@luckysori
Copy link
Contributor Author

luckysori commented Mar 6, 2024 •
edited

But this is not the case for fresh 1.9.0 apps. For those, we need to consider the presence of the on_chain directory.

Why isn't that for fresh 1.9.0 apps? did we change something there recently?

No idea. I assume that the location of the wallet directory has been like that for a long time. I don't really know what that other wallet file was for, but it's not present with 1.9.0.

Shall we just merge this though, @holzeis?

@holzeis
Copy link
Contributor

holzeis commented Mar 6, 2024

But this is not the case for fresh 1.9.0 apps. For those, we need to consider the presence of the on_chain directory.

Why isn't that for fresh 1.9.0 apps? did we change something there recently?

No idea. I assume that the location of the wallet directory has been like that for a long time. I don't really know what that other wallet file was for, but it's not present with 1.9.0.

Shall we just merge this though, @holzeis?

How about renaming the directory instead of deleting.. just to be on the safe side?

@luckysori
Copy link
Contributor Author

How about renaming the directory instead of deleting.. just to be on the safe side?

BDK is only storing stuff that can be found on-chain. If you have the same seed, you will find the same outputs and transactions after an on-chain sync.

holzeis
holzeis approved these changes Mar 6, 2024
View reviewed changes
Copy link
Contributor

@holzeis holzeis left a comment

Choose a reason for hiding this comment

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

Thank you !

@luckysori luckysori added this pull request to the merge queue Mar 6, 2024
Merged via the queue into main with commit 484faf6 Mar 6, 2024
20 checks passed
@luckysori luckysori deleted the fix/sync-on-upgrade branch March 6, 2024 12:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@holzeis holzeis holzeis approved these changes

Assignees

@luckysori luckysori

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Full sync was not run after upgrade from 1.9.0 to 1.10.0
2 participants
@luckysori @holzeis