feat(NODE-1486): Add node_remuneration_type to config.ini #1787
Conversation
| --node_remuneration_type node_remuneration_type | ||
| (optional) The node remuneration type used for node rewards | ||
|
|
There was a problem hiding this comment.
The reason I left node_remuneration_type optional in build-bootstrap-config-image (and generate-guestos-config), even though it's required in setupOS, is for backwards compatibility. This script runs on reboot, so after a HostOS upgrade on mainnet, node_remuneration_type will not be in config.ini, and therefore the field should be optional.
But note that once we are on the new config, this script as well as generate-guestos-config.sh and bootstrap-ic-node.sh will all be removed.
There was a problem hiding this comment.
I totally agree with that. We should make it non-optional only in SetupOS!
(and also in the registry at some point, but that shouldn't be a big concern for you)
There was a problem hiding this comment.
once we are on the new config, check-remuneration.sh will be removed, as all config validation will be contained in the config-tool, so this is only temporary 👍
| log_and_halt_installation_on_error 1 "Configuration error: node_remuneration_type is not set" | ||
| fi | ||
|
|
||
| if [[ ! "$node_remuneration_type" =~ ^type[0-9]+(\.[0-9])?$ ]]; then |
There was a problem hiding this comment.
must be of the format: typeX or typeX.Y
There was a problem hiding this comment.
Every time we update build-bootstrap-config-image.sh We need to update the scripts that call build-bootstrap. These are:
- rs/ic_os/launch-single-vm/src/main.rs (Node team tool)
- rs/tests/driver/src/driver/bootstrap.rs (farm)
- rs/tests/driver/src/driver/boundary_node.rs (farm boundary-node)
- testnet/tools/build-guestos-configs.sh (static testnets)
We can leave rs/ic_os/launch-single-vm/src/main.rs. It is a development tool that does not need networking and therefore won’t need registration (correct me if I’m wrong @Bownairo )
For the other three, should we just hardcode a value of type2.1 @sasa-tomic ?
There was a problem hiding this comment.
type3.1 would be more realistic, but either is okay.
There was a problem hiding this comment.
Sounds good! I'll hard-code a value of type3.1 for the other three!
There was a problem hiding this comment.
So you think a value of 3.1 should be provided for the other three?
Sure, sounds like a good idea (for tests).
There was a problem hiding this comment.
I updated
- rs/tests/driver/src/driver/bootstrap.rs (farm)
- testnet/tools/build-guestos-configs.sh (static testnets)
I didn't touch rs/tests/driver/src/driver/boundary_node.rs (farm boundary-node) because the boundary guestos is not NNS managed.
ic-os/components/boundary-guestos/opt/ic/bin/bootstrap-ic-node.sh
Outdated
Show resolved
Hide resolved
ic-os/components/boundary-guestos/opt/ic/bin/bootstrap-ic-node.sh
Outdated
Show resolved
Hide resolved
@Bownairo I believe so! I opened a discussion regarding this here: https://github.com/dfinity/ic/pull/1787/files#r1783622982 |
NODE-1486
/boot/config/remuneration.confon GuestOS to be used in node registration (the work to update registration to make use of remuneration.conf will be covered by DRE)