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

Feat: Combine '--mine' and '--slave' into one, make relevant account-related flags optional #494

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

tuanha-98
Copy link
Contributor

@tuanha-98 tuanha-98 commented Dec 16, 2024

Description

This pull request introduces two key changes to improve flexibility and usability:

1. Make relevant account-related flags optional

  • A non-mining node does not require an account. Therefore, the --keystore, --password and --unlock flags can be optional and should only be required when a mining node lacks an account.

2. Combine --mine and --slave flags into one

  • Currently, the --mine flag is defined but not used in any scenario. The --slave flag is used when a master node wants to become a full node. To make this clearer, we can combine these into a single flag. If a full node wants to act as a master node, it can stake and enable the --mine flag along with relevant account-related flags. Conversely, if a master node wants to operate as a full node, it can disable the --mine flag and the associated account-related flags.

Changes overview

Masternode (Mining node)
Fullnode (Non-mining node)

Current

Use --mine Use --slave Require Account
Masternode
Fullnode

Propose

Use --mine Use --slave Require Account
Masternode Deprecate
Fullnode Deprecate

Shh: whisper.DefaultConfig,
TomoX: tomox.DefaultConfig,
Node: defaultNodeConfig(),
// StakeEnable: true,
Copy link
Collaborator

Choose a reason for hiding this comment

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

When code is not used anymore, please remove instead of comment

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Noted, I’ll remove unused code instead of commenting it out.

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