Skip to content

dcr: Always fetch the current dir path. #2242

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

JoeGruffins
Copy link
Contributor

On ios devices the path will change between updates breaking decred. Never save the path and always check to ensure it is up to date.

closes #2162

MrCyjaneK
MrCyjaneK previously approved these changes May 1, 2025
@JoeGruffins
Copy link
Contributor Author

JoeGruffins commented May 1, 2025

This currently seems to break android.

The wallet name is different/wrong suddenly when going from main to this.

@MrCyjaneK
Copy link
Collaborator

what is the difference in path?

@JoeGruffins
Copy link
Contributor Author

JoeGruffins commented May 1, 2025

Main is saving the wallet at a random name whenever I restore.

I restored a wallet called "Dreary Domain" but it saved it at "/data/user/0/com.cakewallet.cake_wallet/app_flutter/wallets/decred/Enlightened Cone" and thats what is in the walletInfo.dirPath

It looks like restoreFromSeed is fed a directory that does not match the name from the beginning.

@MrCyjaneK
Copy link
Collaborator

MrCyjaneK commented May 1, 2025

Unrelated but sometimes the wallets are put in the shared seed wallet groups, they should all be single seed wallets

This is a separate issue that is being worked on currently.

@JoeGruffins
Copy link
Contributor Author

I realized I'm restoring the same wallet with the same seed, so not sure if its wrong atm actually.

@MrCyjaneK MrCyjaneK dismissed their stale review May 1, 2025 07:39

tested the change - it breaks main

@MrCyjaneK
Copy link
Collaborator

MrCyjaneK commented May 1, 2025

I'm not sure why decred is creating a different directory in addition to new one but this behavior exists both with current main and with this PR.

when fixing it we also need to make sure that people who created wallet previously can still open it on android

@JoeGruffins
Copy link
Contributor Author

JoeGruffins commented May 1, 2025

It looks like it is being fed a bad directory from higher up? Should I look into that?

@JoeGruffins
Copy link
Contributor Author

Moving the old wallets. I suppose this will panic if the dest dir is not empty... Should I handle that? It's unlikely. https://github.com/cake-tech/cake_wallet/compare/9ee811c6e8953342cb1b4843e927412b77856bd7..9f44cb702da1f6b3c38d87a26d1e6e65d9b6b0e1

@JoeGruffins
Copy link
Contributor Author

rename should just do this as well I think, I don't think the wallet name matters to the wallet.

@JoeGruffins JoeGruffins force-pushed the dirpath branch 3 times, most recently from c25c5cc to 3c46a10 Compare May 1, 2025 09:07
@JoeGruffins
Copy link
Contributor Author

Maybe this name isn't being supplied, so its making another. Not yet sure where that is:

if (name.isEmpty) {
name = await generateName();
}

@JoeGruffins
Copy link
Contributor Author

I made an issue with my assessment of this "bug" #2243

@JoeGruffins
Copy link
Contributor Author

ios is broken again, will fix tomorrow

@JoeGruffins JoeGruffins marked this pull request as ready for review May 2, 2025 05:56
@JoeGruffins JoeGruffins force-pushed the dirpath branch 2 times, most recently from 8315093 to 76013b9 Compare May 2, 2025 22:27
@JoeGruffins
Copy link
Contributor Author

Rebased. Should be ok now on both ios and android.

@JoeGruffins JoeGruffins force-pushed the dirpath branch 2 times, most recently from be7a294 to 4b572ab Compare May 10, 2025 23:38
@JoeGruffins
Copy link
Contributor Author

Renaming files now rather than copying.

@JoeGruffins
Copy link
Contributor Author

I want to stick a libwallet version bump in here, we found an unrelated bug.

@JoeGruffins
Copy link
Contributor Author

The libwallet change is only this decred/libwallet#27

We were not re encrypting the seed on password change.

@JoeGruffins
Copy link
Contributor Author

Something still isn't quite right. Please don't merge yet.

@JoeGruffins JoeGruffins force-pushed the dirpath branch 2 times, most recently from 02e0003 to 3a76600 Compare May 16, 2025 07:41
@JoeGruffins
Copy link
Contributor Author

JoeGruffins commented May 16, 2025

So using dirPath doesn't work because it might have been updated with the correct name but still pointing to a nonexistant directory. just path is untouched so using that instead. Another odd thing that only seems to happen in certain circumstances on ios, deleting the directory errors with not existing, even if I add a check one line above. Will happen if I start on a wallet created without seed, then open a restored wallet from there. Unsure what that is about... anyway, tested more now and I think the current changes are good https://github.com/cake-tech/cake_wallet/compare/02e00034e7dbbd7e40eb4a1e046698eb532e1e26..3a76600bf02d470e60808b5463d0cbf738c04d83

@OmarHatem28 OmarHatem28 requested a review from MrCyjaneK May 24, 2025 02:19
JoeGruffins and others added 3 commits June 5, 2025 07:38
On ios devices the path will change between updates breaking decred.
Never save the path and always check to ensure it is up to date.
Previous wallets were also not creating a directory in the correct
place. Move those when found.
Copy link
Collaborator

@MrCyjaneK MrCyjaneK left a comment

Choose a reason for hiding this comment

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

Works fine on iOS

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.

Decred wallet wedged on iOS
3 participants