feat(registry): Add node_reward_type to NodeRecord

[max-dfinity]

This adds support for node_reward_type to NodeRecord and to AddNodePayload.

It adds it as a string to AddNodePayload, but as an enum with validation to NodeRecord.

[sasa-tomic]

LGTM, Ty!

[rumenov]

we add the node type into the node record. Naming would be flexible. We here suggest to call it node_remuneration_type but I can see in the context of UTOPIA that they could have different node specs and it would be useful to have a record in the registry on which machine spec a particular node has.

For UTOPIA I would definitely prefer it to be a called node_type, as the remuneration will be done differently, yet having a record in the registry to be able to distinguish between machine specs would be of great benefit.

Is there a design doc to read about the UTOPIA renumaration ?

[rumenov]

just rubber stamping

[yvonneanne]

we add the node type into the node record. Naming would be flexible. We here suggest to call it node_remuneration_type but I can see in the context of UTOPIA that they could have different node specs and it would be useful to have a record in the registry on which machine spec a particular node has.

For UTOPIA I would definitely prefer it to be a called node_type, as the remuneration will be done differently, yet having a record in the registry to be able to distinguish between machine specs would be of great benefit.

Is there a design doc to read about the UTOPIA renumaration ?

How UTOPIA owners will cover the cost of running the nodes is up to them and NOT part of the product we will offer them in the near future. Therefore there is no design doc for this topic.

[rumenov]

we add the node type into the node record. Naming would be flexible. We here suggest to call it node_remuneration_type but I can see in the context of UTOPIA that they could have different node specs and it would be useful to have a record in the registry on which machine spec a particular node has.

For UTOPIA I would definitely prefer it to be a called node_type, as the remuneration will be done differently, yet having a record in the registry to be able to distinguish between machine specs would be of great benefit.

Is there a design doc to read about the UTOPIA renumaration ?

How UTOPIA owners will cover the cost of running the nodes is up to them and NOT part of the product we will offer them in the near future. Therefore there is no design doc for this topic.

Then how do you know you will need node_type for UTOPIA as part of the renumaration for UTOPIA ? :)

[yvonneanne]

we add the node type into the node record. Naming would be flexible. We here suggest to call it node_remuneration_type but I can see in the context of UTOPIA that they could have different node specs and it would be useful to have a record in the registry on which machine spec a particular node has.

For UTOPIA I would definitely prefer it to be a called node_type, as the remuneration will be done differently, yet having a record in the registry to be able to distinguish between machine specs would be of great benefit.

Is there a design doc to read about the UTOPIA renumaration ?

How UTOPIA owners will cover the cost of running the nodes is up to them and NOT part of the product we will offer them in the near future. Therefore there is no design doc for this topic.

Then how do you know you will need node_type for UTOPIA as part of the renumaration for UTOPIA ? :)

I don't think this will be needed for remuneration in UTOPIA. That's why I wouldn't want remuneration to be part of the name of the field. As Sasa says in the comment I refer to, having different types for nodes with different specs is useful, e.g., when creating subnet.

[max-dfinity]

@sasa-tomic @rumenov @yvonneanne Would something like "node_hardware_type" be a reasonable name? I think node_type is too generic (given @andrewbattat's point about how many different "type" ideas refer to nodes. I'm open to suggestions here, and I now agree node_remuneration_type is not a good name either.

[yvonneanne]

@sasa-tomic @rumenov @yvonneanne Would something like "node_hardware_type" be a reasonable name? I think node_type is too generic (given @andrewbattat's point about how many different "type" ideas refer to nodes. I'm open to suggestions here, and I now agree node_remuneration_type is not a good name either.

"node_hardware_type" seems very reasonable (and useful) to me.

[andrewbattat]

"node_hardware_type" seems very reasonable (and useful) to me.

This is the Node Provider Machine Hardware Guide, so we'll just have to make sure it's updated to reflect all the different hardware sub-types.

[max-dfinity]

@sasa-tomic would this still be consistent with the node remuneration? (i.e. if we used the node type specifications?)

…

On Wed, Oct 9, 2024 at 6:57 AM Andrew Battat ***@***.***> wrote: "node_hardware_type" seems very reasonable (and useful) to me. This is the Node Provider Machine Hardware Guide <https://wiki.internetcomputer.org/wiki/Node_Provider_Machine_Hardware_Guide>, so we'll just have to make sure it's updated to reflect all the different hardware sub-types. — Reply to this email directly, view it on GitHub <#1608 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AX4HWTSNUEDKE4TRRFDXWITZ2UY33AVCNFSM6AAAAABOS6MX5WVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDIMBSGQZDIMZUGQ> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[sasa-tomic]

@sasa-tomic would this still be consistent with the node remuneration? (i.e. if we used the node type specifications?)

I think this will just add to the confusion. We have only 2 hardware types (gen1 and gen2) used in production and we have the following remuneration node_types: type0, type1, type1.1, type2, type3, type3.1, and will likely have more in the future, all assigned to the same gen1/gen2 node types based on arbitrary agreements reached.
If we start using node_hardware_type to refer to remuneration types in the registry, then the term node_hardware_type will imply that it refers to gen1/gen2 based on the variable name, but will actually refer to to remuneration typeX.Y in the context of remuneration. I think that would be quite confusing.

So I think it would be clearer to have a more generic name that would be used for remuneration on the IC Mainnet, and for whatever they want on UTOPIA.

[yvonneanne]

@sasa-tomic would this still be consistent with the node remuneration? (i.e. if we used the node type specifications?)

I think this will just add to the confusion. We have only 2 hardware types (gen1 and gen2) used in production and we have the following remuneration node_types: type0, type1, type1.1, type2, type3, type3.1, and will likely have more in the future, all assigned to the same gen1/gen2 node types based on arbitrary agreements reached. If we start using node_hardware_type to refer to remuneration types in the registry, then the term node_hardware_type will imply that it refers to gen1/gen2 based on the variable name, but will actually refer to to remuneration typeX.Y in the context of remuneration. I think that would be quite confusing.

So I think it would be clearer to have a more generic name that would be used for remuneration on the IC Mainnet, and for whatever they want on UTOPIA.

I assumed this type is exactly to differentiate between the two hardware types, Gen1 and Gen2 (and more types in the future)? (https://dfinity.atlassian.net/browse/IC-797)

If this is not the case (do we distinguish Gen1 and Gen2 by the chip_id field in this case?) and we use this type exclusively for remuneration, then remuneration should be part of the name.

[sasa-tomic]

I just had a chat with @yvonneanne to discuss this change. Here is the summary:

• we need a way to represent the node_hardware_type in the registry, but this is not of critical importance and can be done separately (i.e. would enable distinguishing gen1 vs gen2 nodes)
• we need a way to represent the node_rewards_type or whatever the name, associated with each node record somehow, to enable us to calculate precisely how much rewards each NP should get -- this is what this PR is about (e.g. type1, type3, type3.1, ...)

The field should be optional to cover for the expected utopia use cases. For the Mainnet, we can make sure in SetupOS that all newly deployed Mainnet nodes have this value set.
In the interest of simplicity, it seems easiest to include this information in the node record, as was done in this PR. @rumenov already approved the PR as is, so I suppose the change is acceptable to him.

One option suggested by Rosti earlier was to have a map of rewardable nodes somewhere else. However, that option seems to add a bit more complexity, and thus doesn't seem to be preferred by the NNS team. I also don't see a huge benefit in this option, other than not polluting the node record with even more fields. As a downside, it adds more room for error (e.g. a node could exist in the rewardable nodes map, but not in the list of active node records).

So the only open question is the name of the field in the NodeRecord. I don't mind personally. All options are okay with me, as long as they are not confusing (e.g. by saying it's a hardware description when it's a reward-related field). So either name it with a reward/remuneration (former is simpler so better by me), or alternatively just plain node_type would also be okay. I delegate the decision to @max-dfinity since he's the code owner.