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

Make sure default GRPC ports correctly map #5272

Closed
SWvheerden opened this issue Mar 28, 2023 · 0 comments · Fixed by #5291
Closed

Make sure default GRPC ports correctly map #5272

SWvheerden opened this issue Mar 28, 2023 · 0 comments · Fixed by #5291
Assignees
Labels
A-base_node Area - The Tari base node executable and libraries A-wallet Area - related to the wallet C-bug Category - fixes a bug, typically associated with an issue.

Comments

@SWvheerden
Copy link
Collaborator

SWvheerden commented Mar 28, 2023

Make sure every where we use default grpc port number, we map them to the correct default port:

        ApplicationType::BaseNode => match network {
            Network::MainNet => 18102u16,
            Network::StageNet => 18172u16,
            Network::NextNet => 18182u16,
            Network::Weatherwax => 18112,
            Network::Dibbler => 18122,
            Network::Esmeralda => 18142,
            Network::Igor => 18152,
            Network::LocalNet => 18162,
            _ => unreachable!("Network {} not supported", network),
        },
        ApplicationType::ConsoleWallet => match network {
            Network::MainNet => 18103u16,
            Network::StageNet => 18173u16,
            Network::NextNet => 18183u16,
            Network::Weatherwax => 18113,
            Network::Dibbler => 18123,
            Network::Esmeralda => 18143,
            Network::Igor => 18153,
            Network::LocalNet => 18163,
            _ => unreachable!("Network {} not supported", network),
        },

For example this is wrong:

impl Default for MergeMiningProxyConfig {
    fn default() -> Self {
        Self {
            override_from: None,
            monerod_url: StringList::default(),
            monerod_username: String::new(),
            monerod_password: String::new(),
            monerod_use_auth: false,
            base_node_grpc_address: "/ip4/127.0.0.1/tcp/18142".parse().unwrap(),
            console_wallet_grpc_address: "/ip4/127.0.0.1/tcp/18143".parse().unwrap(),
            console_wallet_grpc_authentication: GrpcAuthentication::default(),
            listener_address: "/ip4/127.0.0.1/tcp/18081".parse().unwrap(),
            submit_to_origin: true,
            wait_for_initial_sync_at_startup: true,
            check_tari_difficulty_before_submit: true,
            max_randomx_vms: 5,
            coinbase_extra: "tari_merge_mining_proxy".to_string(),
        }
    }
}

There should not be any hard coded grpc ports in the code base except for tests.

@SWvheerden SWvheerden added C-bug Category - fixes a bug, typically associated with an issue. A-base_node Area - The Tari base node executable and libraries A-wallet Area - related to the wallet labels Mar 28, 2023
SWvheerden added a commit that referenced this issue Apr 12, 2023
…5291)

Description
---
This PR does two jobs (sorry). Mostly because they're dependent tasks.

- Add `--network` to the miners for simple setting switches
- Add the network into the data directory path

Motivation and Context
---
This can be used to help select the default ports of communication for
the base node and wallet, and also easily keeping network id's and
folders separate.

How Has This Been Tested?
---
Manually

What process can a PR reviewer use to test or verify this change?
---
Run a base node, wallet, and miner. Set a base directory path as
something like `-b ./data/mytest` and the network to igor `--network
igor` and watch in amazement as the data directory `./data/mytest/igor/`
is created to store information.

Closes #5272
Closes #5273

Breaking Changes
---

- [x] None
- [ ] Requires data directory on base node to be deleted
- [ ] Requires hard fork
- [ ] Other - Please specify

<!-- Does this include a breaking change? If so, include this line as a
footer -->
<!-- BREAKING CHANGE: Description what the user should do, e.g. delete a
database, resync the chain -->

---------

Co-authored-by: SW van Heerden <swvheerden@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-base_node Area - The Tari base node executable and libraries A-wallet Area - related to the wallet C-bug Category - fixes a bug, typically associated with an issue.
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants