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 ElectrumX server settings #178

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

QuickMythril
Copy link

This adds two new settings for each of the supported foreign coins. These settings allow users to specify ElectrumX servers to use (or wallet servers for Piratechain), and whether or not to use the default servers provided by the Core. This gives users more control over their node, and allows easier testing for issues with specific wallets or servers. As an example, the following lines could be added to settings.json to disable default servers and connect only to qortal.link:
"useDogecoinDefaults": false, "dogecoinServers": ["electrum.qortal.link:54002,SSL"]

@kennycud
Copy link

kennycud commented Jun 8, 2024

The code that parses the settings is identical for each coin and should be encapsulated in a utility class in another file.

@QuickMythril
Copy link
Author

The code that parses the settings is identical for each coin and should be encapsulated in a utility class in another file.

Sure, that would be a nice improvement, but it's not necessary to do that before adding this feature. If someone wants to clean up or consolidate the code at some point in the future they can do that any time, as they always have been able to. I tried to do that before with AlphaX's code for the wallets and address books and was told not to change anything. This seems like a difference in code style guidelines between the members of the official "Qortal Development Group" which doesn't involve me anymore.

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.

2 participants