Farming policies are broken on testnet

[LeeSmet]

• Some of them have minimal uptime 95, which is presumed to be 95% but should then be expressed as minimal uptime 950
• New nodes seem to get policy 3, whereas they should get policy 1. Policy 3 seems otherwise the same as policy 1, but "default" is not set to true for policy 3, while it is for policy 1 (and hence policy 1 should be chosen)

[DylanVerstraete]

I updated farming policy with ID 3 minimal uptime to 950.

Now we need to investigate in the code why this is happening and why the default policy is not chosen.

[DylanVerstraete]

policies on testnet now:

{
  version: 2
  id: 4
  name: farming_policy_non_certified_default_2
  cu: 2,400
  su: 1,000
  nu: 30
  ipv4: 5
  minimalUptime: 950
  policyCreated: 4,034,804
  policyEnd: 0
  immutable: false
  default: false
  nodeCertification: Diy
  farmCertification: NotCertified
}

tfgridModule.farmingPoliciesMap: PalletTfgridFarmingPolicy
{
  version: 2
  id: 3
  name: farming_policy_non_certified_default_1
  cu: 2,400
  su: 1,000
  nu: 30
  ipv4: 5
  minimalUptime: 950
  policyCreated: 4,034,184
  policyEnd: 0
  immutable: false
  default: false
  nodeCertification: Diy
  farmCertification: NotCertified
}

tfgridModule.farmingPoliciesMap: PalletTfgridFarmingPolicy
{
  version: 1
  id: 2
  name: farming_policy_certified_nodes_default
  cu: 3,000
  su: 1,250
  nu: 38
  ipv4: 6
  minimalUptime: 950
  policyCreated: 3,615,198
  policyEnd: 0
  immutable: false
  default: false
  nodeCertification: Certified
  farmCertification: NotCertified
}

tfgridModule.farmingPoliciesMap: PalletTfgridFarmingPolicy
{
  version: 1
  id: 1
  name: farming_policy_non_certified_default
  cu: 2,400
  su: 1,000
  nu: 30
  ipv4: 5
  minimalUptime: 950
  policyCreated: 3,615,198
  policyEnd: 0
  immutable: false
  default: true
  nodeCertification: Diy
  farmCertification: NotCertified
}

Findings:

• Default policies are only attached when there are limits specified on a policy
• Ordering was not taking into account of the ID of a policy (we can also order on created but ID is incremental so the last policy created will be taken anyhow)

[DylanVerstraete]

@LeeSmet the code right now does not really care if a policy is marked as default or not unless the policy has limits attached. I'm not sure why it's like that. Why did we add default policies in the first place again?

[DylanVerstraete]

So it seems it's documented here:

https://github.com/threefoldtech/tfchain/blob/development/substrate-node/pallets/pallet-tfgrid/src/types.rs#L161-L163

" Indicates if the farming policy is a default one. Meaning it will be used when there is no Farming policy defined on the farm itself"

Not sure why that is actually needed because when there is no farming policy attached to a farm, the code will pick the last one created with the right certification type (regardless of default or not) which makes sense to me

[LeeSmet]

from what I remember, a farm should get (one of) the default policies, UNLESS a farm has an explicit farming policy with limits) attached to it

[renauter]

Quick reminder on farming policy:

• If a farming policy is attached to a farm:
  It gives power for a farm to associate a specific policy (default or not, with some limit of usage), to a new node that is added to this farm.
  After the limit of usage is reached the last stored default farming policy is associated to each new node
• If farm has no policy attached, the last stored farming policy (which certifications are not more qualified than current node certifications / default or not) is considered at node creation

See code here:

tfchain/substrate-node/pallets/pallet-tfgrid/src/lib.rs

Line 2188 in f6ff986

// Set the farming policy as the last stored farming

So regarding the code it is possible that policy 3 is chosen instead of policy 1 in case farm has no policy attached.
And I would even say that it should be policy 4 if we consider the fix here #704

Question is what do we go for? Stay as it is (last "best" policy) or change it (last "best" default policy)?

[DylanVerstraete]

This default concept is making this harder then it should be. I would just go for the (last "best" policy) and remove the notion of default policies.

[mohamedamer453]

Verified on testnet.

I checked the uptime of the 4 farming policies that are available there and it was 950 for all of them.