[Upgrades] v0.0.12 upgrade #1043
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,154 @@ | ||
| package upgrades | ||
|
|
||
| import ( | ||
| "context" | ||
|
|
||
| "cosmossdk.io/math" | ||
| storetypes "cosmossdk.io/store/types" | ||
| upgradetypes "cosmossdk.io/x/upgrade/types" | ||
| cosmosTypes "github.com/cosmos/cosmos-sdk/types" | ||
| "github.com/cosmos/cosmos-sdk/types/module" | ||
| "github.com/pokt-network/poktroll/app/keepers" | ||
| sharedtypes "github.com/pokt-network/poktroll/x/shared/types" | ||
| ) | ||
|
|
||
| const Upgrade_0_0_12_PlanName = "v0.0.12" | ||
|
|
||
| // Upgrade_0_0_12 handles the upgrade to release `v0.0.12`. To be issued on both Alpha and Beta TestNets. | ||
okdas marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| var Upgrade_0_0_12 = Upgrade{ | ||
| PlanName: Upgrade_0_0_12_PlanName, | ||
| CreateUpgradeHandler: func(mm *module.Manager, | ||
| keepers *keepers.Keepers, | ||
| configurator module.Configurator, | ||
| ) upgradetypes.UpgradeHandler { | ||
| // Parameter configurations aligned with repository config.yml specifications | ||
okdas marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| // These values reflect the delta between v0.0.11 and current main branch | ||
okdas marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| // Reference: | ||
| // - Comparison: https://github.com/pokt-network/poktroll/compare/v0.0.11..main | ||
|
There was a problem hiding this comment. Should we be using That way this will be "evergreen" There was a problem hiding this comment. TLDR: I want to use Complex answer, feel free to skip: We're supposed to create a release and use artifacts (binaries & images) produced by that release to upgrade validators, full nodes, seeds, and relayminers. The release can be created off any branch or any commit. It MUST include the upgrade plan code (introduced by this PR). Now, the code in the But we are a small team, and we can track that drift to changes to upgrade, if necessary, before the release. So I want to compare to There was a problem hiding this comment. Moving conversation to discord to get this over the finish line. https://discord.com/channels/824324475256438814/1332065342746525778/1336434350660784239 |
||
| // - Direct diff: `git diff v0.0.11..main -- config.yml` | ||
| // | ||
| // Note: Parameter updates are derived from config.yml modifications, which serves | ||
| // as the source of truth for all parameter changes. | ||
okdas marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| const ( | ||
| supplierStakingFee = 1000000 | ||
| serviceTargetNumRelays = 100 | ||
| tokenomicsGlobalInflationPerClaim = 0.1 | ||
okdas marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| ) | ||
|
|
||
| applyNewParameters := func(ctx context.Context) (err error) { | ||
| logger := cosmosTypes.UnwrapSDKContext(ctx).Logger() | ||
| logger.Info("Starting parameter updates", "upgrade_plan_name", Upgrade_0_0_12_PlanName) | ||
|
|
||
| // Set supplier module staking_fee to 1000000upokt, in line with the config.yml in the repo. | ||
| // Verify via: | ||
| // $ poktrolld q supplier params --node=... | ||
| supplierParams := keepers.SupplierKeeper.GetParams(ctx) | ||
| supplierParams.MinStake = &cosmosTypes.Coin{ | ||
| Denom: "upokt", | ||
| Amount: math.NewInt(supplierStakingFee), | ||
| } | ||
| err = keepers.SupplierKeeper.SetParams(ctx, supplierParams) | ||
| if err != nil { | ||
| logger.Error("Failed to set supplier params", "error", err) | ||
| return err | ||
| } | ||
| logger.Info("Successfully updated supplier params", "new_params", supplierParams) | ||
|
|
||
| // Add service module `target_num_relays` parameter, in line with the config.yml in the repo. | ||
| // Verify via: | ||
| // $ poktrolld q service params --node=... | ||
| serviceParams := keepers.ServiceKeeper.GetParams(ctx) | ||
| serviceParams.TargetNumRelays = serviceTargetNumRelays | ||
| err = keepers.ServiceKeeper.SetParams(ctx, serviceParams) | ||
| if err != nil { | ||
| logger.Error("Failed to set service params", "error", err) | ||
| return err | ||
| } | ||
| logger.Info("Successfully updated service params", "new_params", serviceParams) | ||
|
|
||
| // Add tokenomics module `global_inflation_per_claim` parameter, in line with the config.yml in the repo. | ||
| // Verify via: | ||
| // $ poktrolld q tokenomics params --node=... | ||
| tokenomicsParams := keepers.TokenomicsKeeper.GetParams(ctx) | ||
| tokenomicsParams.GlobalInflationPerClaim = tokenomicsGlobalInflationPerClaim | ||
| err = keepers.TokenomicsKeeper.SetParams(ctx, tokenomicsParams) | ||
| if err != nil { | ||
| logger.Error("Failed to set tokenomics params", "error", err) | ||
| return err | ||
| } | ||
| logger.Info("Successfully updated tokenomics params", "new_params", tokenomicsParams) | ||
| return nil | ||
| } | ||
|
|
||
| // Helper function to update all suppliers' RevShare to 100%. This is necessary to ensure that | ||
| // we have that value populated before suppliers are connected. | ||
okdas marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| updateSuppliersRevShare := func(ctx context.Context) error { | ||
| logger := cosmosTypes.UnwrapSDKContext(ctx).Logger() | ||
| suppliers := keepers.SupplierKeeper.GetAllSuppliers(ctx) | ||
| logger.Info("Updating all suppliers to have a 100% revshare to the supplier", | ||
okdas marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| "num_suppliers", len(suppliers)) | ||
|
|
||
| for _, supplier := range suppliers { | ||
| for _, service := range supplier.Services { | ||
| // Add warning log if we're overwriting existing revshare settings. We can't get the previous | ||
| // revshare percentage due to a protobuf change, but we can at least log the number of previous revshares. | ||
okdas marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| if len(service.RevShare) > 1 { | ||
|
There was a problem hiding this comment. You also need to check if the address is different There was a problem hiding this comment. Changed the logic a bit. If more than 1 supplier: log and overwrite, |
||
| logger.Warn( | ||
| "Overwriting existing revenue share configuration", | ||
Olshansk marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| "supplier", supplier.OperatorAddress, | ||
okdas marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| "service", service.ServiceId, | ||
| "previous_revshare_count", len(service.RevShare), | ||
Olshansk marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| ) | ||
| } | ||
| service.RevShare = []*sharedtypes.ServiceRevenueShare{ | ||
|
There was a problem hiding this comment. Ideally we would be able to loop over the old revshare map by unmashalling to the old proto type then create the new But since we don't have proto versioning and we marked the old (float32) value as reserved, this is the next best option offered to us. There was a problem hiding this comment. @red-0ne Really appreciate this feedback & input. @okdas Can you #PUC what the "best" approach would be an "why" - essentially paragraphsing @red-0ne's comments. In particular, explaining why we're not doing it would help. I'm thinking:
|
||
| { | ||
| Address: supplier.OperatorAddress, | ||
| RevSharePercentage: uint64(100), | ||
| }, | ||
| } | ||
| } | ||
| keepers.SupplierKeeper.SetSupplier(ctx, supplier) | ||
| logger.Info("Updated supplier", | ||
| "supplier", supplier.OperatorAddress) | ||
|
|
||
| } | ||
| return nil | ||
| } | ||
|
|
||
| // Returns the upgrade handler for v0.0.12 | ||
| return func(ctx context.Context, plan upgradetypes.Plan, vm module.VersionMap) (module.VersionMap, error) { | ||
| logger := cosmosTypes.UnwrapSDKContext(ctx).Logger() | ||
| logger.Info("Starting upgrade handler", "upgrade_plan_name", Upgrade_0_0_12_PlanName) | ||
|
|
||
| err := applyNewParameters(ctx) | ||
okdas marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| if err != nil { | ||
| logger.Error("Failed to apply new parameters", | ||
| "upgrade_plan_name", Upgrade_0_0_12_PlanName, | ||
| "error", err) | ||
| return vm, err | ||
| } | ||
|
|
||
| // Update all suppliers' RevShare | ||
okdas marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| err = updateSuppliersRevShare(ctx) | ||
| if err != nil { | ||
| logger.Error("Failed to update suppliers RevShare", | ||
| "upgrade_plan_name", Upgrade_0_0_12_PlanName, | ||
| "error", err) | ||
| return vm, err | ||
| } | ||
|
|
||
| logger.Info("Running module migrations", "upgrade_plan_name", Upgrade_0_0_12_PlanName) | ||
|
There was a problem hiding this comment. Can we add a log like this before every section / update? |
||
| vm, err = mm.RunMigrations(ctx, configurator, vm) | ||
| if err != nil { | ||
| logger.Error("Failed to run migrations", | ||
| "upgrade_plan_name", Upgrade_0_0_12_PlanName, | ||
| "error", err) | ||
| return vm, err | ||
| } | ||
|
|
||
| logger.Info("Successfully completed upgrade handler", "upgrade_plan_name", Upgrade_0_0_12_PlanName) | ||
| return vm, nil | ||
| } | ||
| }, | ||
| // No changes to the KVStore in this upgrade. | ||
| StoreUpgrades: storetypes.StoreUpgrades{}, | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't there a certain .md you're supposed to update (for documentation purposes) when we do an upgrade?
https://dev.poktroll.com/protocol/upgrades/upgrade_list
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At the moment of release, we don't know when the upgrade will happen. My thinking, this should be a separate PR. But! We can prepare some changes in advance - that's a good idea.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's do it!
Wdyt of creating another PR (updating the docs that's a draft), and leave a TODO here.
var allUpgrades = []upgrades.Upgrade{ // v0.0.4 was the first upgrade we implemented and tested on network that is no longer used. // upgrades.Upgrade_0_0_4, // v0.0.10 was the first upgrade we implemented on Alpha TestNet. // upgrades.Upgrade_0_0_10, // v0.0.11 was the Alpha TestNet exclusive upgrade to bring it on par with Beta TestNet. // upgrades.Upgrade_0_0_11, // TODO(@okdas, #XX): Update the documentation once a block height has been selected. // Ref: https://dev.poktroll.com/protocol/upgrades/upgrade_list // v0.0.12 - the first upgrade going live on both Alpha and Beta TestNets. upgrades.Upgrade_0_0_12,There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nvm: I saw your changes in
.mdAFTER writing the message above. Leaving it as a reference.Wdyt of adding the output of the CLI transaction to say "Please update docusaurus/docs/protocol/upgrades/upgrade_list.md` now that you've submitted the plan?
You know how they say "Go to where your users are", I'm trying to "Go where the dev's eyes are".
They're looking at the CLI when something says
!!!!!!!!!:)