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

Open wallet should restart node if chain types don't match #12

Merged
merged 12 commits into from
Oct 6, 2022

Conversation

flomang
Copy link
Collaborator

@flomang flomang commented Oct 5, 2022

More bug fixes. Restart of node server will not work without global edits.

mimblewimble/grin#3737.

I also, fixed an overflow bug in that merge for new wallets.

node_url: None,
secret_path: None,
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Just an observation that we're going to have to think a bit about how much we store in the gui's config file vs the wallet's config file. Technically all of these fields (other than use_embedded_node) are already stored in the wallet's config file, so perhaps the gui code should be using/modifying (and allowing the user to configure) those instead of repeating them? This is fine for now to get us going since we don't have code in place to modify config files yet, (we'll want it soon,) but we should be keeping this in mind.

Copy link
Member

Choose a reason for hiding this comment

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

in any case, getting and using the chain type is better than an is_testnet flag

@yeastplume
Copy link
Member

All good, just an observation above and a small change requested to mimblewimble/grin#3737.
Really happy to see this, I'm going to merge and then reconcile with the work I was doing in #11. We're starting to get there with respect to wallet selection, creation and configuration.

@yeastplume yeastplume merged commit 2bfacb3 into mimblewimble:master Oct 6, 2022
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