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

fix: make launchpad work with development #3777

Closed
wants to merge 47 commits into from

Conversation

stringhandler
Copy link
Collaborator

@stringhandler stringhandler commented Jan 31, 2022

Description

Update launchpad to work on windows and with the latest development.
Also fixes the default configs for merge mining and mining node

Motivation and Context

Code was out of date after validator_node branch merge

How Has This Been Tested?

manually, cargo test

philipr-za and others added 16 commits January 28, 2022 14:41
Some fixes to the launchpad docker files to get them working under dibbler
* Add Dibbler to possible networks, and remove Weatherwax
* Make `config.toml` minimla to ease maintenance
* Migrate some netwrok-specific config variables to environment
  variables
* Update front-end to default to dibbler
* Remove torrc (It's 100% configured in the code)
* Fix `tauri.conf.json` so that `cargo tauri dev` works
* Copy assets over in dev mode (tauri.conf.json)
* Bump version

Note: I sometimes get random file access permissions in MacOs, but have
not been able to identify the precise reason. So watch out for this and
report back any issues.
Most apps need tor configuration, so pull this out into a helper method
and re-use where applicable.
These changes let you specify mining node parameters in the
`[mining_node]` configuration section.

Before this, settings would be overwritten from the `global` config
struct. This is not what we want, because it prevents general Tari app
configuration. A classic example is that the base node GRPC address is
overwritten from the `base_node.grpc_base_node_address` config variable.

This typically is fine on a local machine setup, but what if the base
node and miner are on different networks (or in different docker
containers)?

`grpc_base_node_address` will be `0.0.0.0:18149` to let the node listen
for GRPC connections from other networks. But the mining_node
`base_node_addr` must be `/dns4/base_node_url/tcp/18149`, and NOT
`0.0.0.0:18149` as it is currently forced into.

The overwriting also undoes the work of the `DefaultConfigLoader` trait
which elegantly handles sub-configs and negates the need to carry
everything in a `GlobalConfiguration` struct.
This commit migrates config settings for mining node 100% to the
DefaultConfigLoader. This makes the global variables unnessecary, and
they are removed.
This commit removes the global config variables in favour of the "magic"
config traits that Maxim wrote.

See the new test in applications/tari_merge_mining_proxy/src/config.rs
to see how this is awesome.
* Maybe controversial: Allow any string to be specified as "network" in
  config files, and not just official network names. This would let you
set up say "base_node.monday" and "base_node.tuesday" configuration
settings and select them at run time. Probably better is to change the
name of the key from netowrk to say, "config" though; because the
root-level network field MUST select a valid network.

* Rearrange the order of docker file steps so that the dependency
  compiled objects are more likely to be cached.
# Conflicts:
#	applications/launchpad/backend/assets/config.toml
#	applications/launchpad/backend/src/docker/settings.rs
#	applications/launchpad/docker_rig/base_node.Dockerfile
#	applications/launchpad/docker_rig/console_wallet.Dockerfile
#	applications/launchpad/docker_rig/mm_proxy.Dockerfile
#	applications/launchpad/docker_rig/sha3_miner.Dockerfile
#	applications/launchpad/versions.txt
#	common/src/configuration/global.rs
#	common/src/configuration/mod.rs
@stringhandler stringhandler marked this pull request as ready for review February 8, 2022 11:56
sdbondi
sdbondi previously approved these changes Feb 8, 2022
Copy link
Member

@sdbondi sdbondi left a comment

Choose a reason for hiding this comment

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

Nice - not sure if committing the node identities was intentional

dibbler/wallet/config/dibbler/tari_console_wallet_id.json Outdated Show resolved Hide resolved
@stringhandler stringhandler force-pushed the philipr-za-philip-launchpad-todo branch from 18a49e3 to cdbbebd Compare February 9, 2022 12:47
@stringhandler stringhandler force-pushed the philipr-za-philip-launchpad-todo branch from cdbbebd to 534b9ef Compare February 9, 2022 14:29
# Conflicts:
#	Cargo.lock
#	applications/launchpad/backend/Cargo.toml
#	applications/tari_app_utilities/src/initialization.rs
#	applications/tari_base_node/Cargo.toml
#	applications/tari_base_node/src/bootstrap.rs
#	applications/tari_base_node/src/command_handler.rs
#	applications/tari_base_node/src/main.rs
#	applications/tari_console_wallet/src/automation/commands.rs
#	applications/tari_console_wallet/src/init/mod.rs
#	applications/tari_merge_mining_proxy/src/main.rs
#	applications/tari_merge_mining_proxy/src/proxy.rs
#	applications/tari_validator_node/src/comms.rs
#	applications/tari_validator_node/src/main.rs
#	base_layer/wallet/src/config.rs
#	base_layer/wallet/src/wallet.rs
#	base_layer/wallet/tests/wallet.rs
#	base_layer/wallet_ffi/src/lib.rs
#	common/config/presets/base_node.toml
#	common/config/presets/common.toml
#	common/config/presets/merge_mining_proxy.toml
#	common/src/configuration/common_config.rs
#	common/src/configuration/global.rs
#	common/src/configuration/utils.rs
#	common/src/configuration/validator_node_config.rs
#	common/src/configuration/wallet_config.rs
#	integration_tests/helpers/baseNodeProcess.js
#	integration_tests/helpers/config.js
#	integration_tests/helpers/mergeMiningProxyProcess.js
#	integration_tests/helpers/miningNodeProcess.js
#	integration_tests/helpers/stratumTranscoderProcess.js
#	integration_tests/helpers/validatorNodeProcess.js
#	integration_tests/helpers/walletProcess.js
# Conflicts:
#	applications/tari_base_node/Cargo.toml
#	applications/tari_base_node/src/commands/command_handler.rs
#	applications/tari_base_node/src/main.rs
#	applications/tari_validator_node/src/dan_node.rs
# Conflicts:
#	Cargo.lock
#	applications/launchpad/backend/Cargo.toml
#	applications/tari_base_node/Cargo.toml
#	applications/tari_base_node/src/bootstrap.rs
#	applications/tari_base_node/src/commands/command/mod.rs
#	applications/tari_base_node/src/main.rs
#	applications/tari_console_wallet/src/init/mod.rs
#	applications/tari_merge_mining_proxy/src/proxy.rs
#	applications/tari_validator_node/src/comms.rs
#	base_layer/wallet/tests/wallet.rs
#	base_layer/wallet_ffi/src/lib.rs
#	common/Cargo.toml
#	common/config/presets/common.toml
#	common/config/presets/console_wallet.toml
#	common/src/configuration/common_config.rs
#	common/src/configuration/global.rs
# Conflicts:
#	Cargo.lock
#	applications/launchpad/backend/Cargo.toml
#	applications/launchpad/gui-vue/package-lock.json
#	applications/launchpad/gui-vue/package.json
#	applications/tari_app_utilities/src/initialization.rs
#	applications/tari_stratum_transcoder/Cargo.toml
#	applications/tari_validator_node/src/comms.rs
#	common/examples/base_node_init.rs
#	common/src/configuration/bootstrap.rs
#	common/src/configuration/utils.rs
#	common/src/configuration/writer.rs
aviator-app bot pushed a commit that referenced this pull request Apr 11, 2022
Description
---

This PR builds from work done on #3777 by @philipr-za @mikethetike @CjS77 

- refactors configuration to map directly to config structs on all apps
- refactor and simplify p2p configs
- split out various transport configs
- split out peer seeds from other p2p config so that it is common to all apps and overridable per network

FFI Breaking changes:
- network added to params for wallet_create
- `transport_type_destroy` renamed to `transport_config_destroy`. `transport_type_destroy` is still available but deprecated.
- uses TransportConfig instead of TransportType struct
- `network` argument removed from `comms_config_create`

Motivation and Context
---
Previous config was brittle and could easily break app configs when they are changed. This PR greatly reduces config maintenance by removing any mapping and default setting when changing configs. 

TODOs:
- update launchpad
- integration tests (in progress)

How Has This Been Tested?
---
Manually, testing each app
Existing tests
Code compiles
@stringhandler stringhandler deleted the philipr-za-philip-launchpad-todo branch April 12, 2022 15:00
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.

4 participants