threefoldtech / tfchain_graphql

Graphql for TFchain
Apache License 2.0
2 stars 3 forks source link

Fixing pricing-policy ID 0 for some farms #164

Closed sameh-farouk closed 5 months ago

sameh-farouk commented 5 months ago

There is an issue of data inconsistency caused by an old implementation of the update farm extrinsic. This allowed users to mess up with their farms' pricing policy ID. Later, a storage migration reset all the farms' pricing policy IDs to the default one. To solve this issue, I propose that the value of pricing policy ID on all farmUpdated events shouldn't persist since it was never meant to be altered in the first place, and with the current chain implantation, we are not allowing such change.

For full context see here https://github.com/threefoldtech/tfchain_graphql/issues/96#issue-1603754480

sameh-farouk commented 5 months ago

Are we sure pricing_policy_id on farm was always initialized to 1 ?

yes, I checked and farms was created with a hardcoded farming policy ID 1 since the initial implementation of this extrinsic.

If not we could just set savedFarm.pricingPolicyID = 1 to force instead of commenting savedFarm.pricingPolicyID = farmUpdatedEventParsed.pricingPolicyId

IMO, both are workable with the current implementation of tfgridmodule, but it is a matter of taste, I don't prefer to use a magic value here, and it is not the responsibility of the processor to decide about the value (and you could argue that my approach also decide about the value by ignoring it but this taste better for me :) ). Also, the approach I choose here by keeping the original pricing policy attached to the farm when it was created, will still be functioning properly even if we decide to change the hardcoded value on farm creation to something else.