Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Migrate pallet-membership to the new pallet attribute macro #9080

Merged
25 commits merged into from
Sep 7, 2021
Merged

Migrate pallet-membership to the new pallet attribute macro #9080

25 commits merged into from
Sep 7, 2021

Conversation

koushiro
Copy link
Contributor

@koushiro koushiro commented Jun 11, 2021

Part of #7882

Migrate the pallet-membership to the new pallet attribute macro

metadata generated by old macro
{
  "name": "TechnicalMembership",
  "storage": {
    "prefix": "Instance1Membership",
    "entries": [
      {
        "name": "Members",
        "modifier": "Default",
        "ty": {
          "Plain": "Vec<T::AccountId>"
        },
        "default": [
          0
        ],
        "documentation": [
          " The current membership, stored as an ordered Vec."
        ]
      },
      {
        "name": "Prime",
        "modifier": "Optional",
        "ty": {
          "Plain": "T::AccountId"
        },
        "default": [
          0
        ],
        "documentation": [
          " The current prime member, if one exists."
        ]
      }
    ]
  },
  "calls": [
    {
      "name": "add_member",
      "arguments": [
        {
          "name": "who",
          "ty": "T::AccountId"
        }
      ],
      "documentation": [
        " Add a member `who` to the set.",
        "",
        " May only be called from `T::AddOrigin`."
      ]
    },
    {
      "name": "remove_member",
      "arguments": [
        {
          "name": "who",
          "ty": "T::AccountId"
        }
      ],
      "documentation": [
        " Remove a member `who` from the set.",
        "",
        " May only be called from `T::RemoveOrigin`."
      ]
    },
    {
      "name": "swap_member",
      "arguments": [
        {
          "name": "remove",
          "ty": "T::AccountId"
        },
        {
          "name": "add",
          "ty": "T::AccountId"
        }
      ],
      "documentation": [
        " Swap out one member `remove` for another `add`.",
        "",
        " May only be called from `T::SwapOrigin`.",
        "",
        " Prime membership is *not* passed from `remove` to `add`, if extant."
      ]
    },
    {
      "name": "reset_members",
      "arguments": [
        {
          "name": "members",
          "ty": "Vec<T::AccountId>"
        }
      ],
      "documentation": [
        " Change the membership to a new set, disregarding the existing membership. Be nice and",
        " pass `members` pre-sorted.",
        "",
        " May only be called from `T::ResetOrigin`."
      ]
    },
    {
      "name": "change_key",
      "arguments": [
        {
          "name": "new",
          "ty": "T::AccountId"
        }
      ],
      "documentation": [
        " Swap out the sending member for some other key `new`.",
        "",
        " May only be called from `Signed` origin of a current member.",
        "",
        " Prime membership is passed from the origin account to `new`, if extant."
      ]
    },
    {
      "name": "set_prime",
      "arguments": [
        {
          "name": "who",
          "ty": "T::AccountId"
        }
      ],
      "documentation": [
        " Set the prime member. Must be a current member.",
        "",
        " May only be called from `T::PrimeOrigin`."
      ]
    },
    {
      "name": "clear_prime",
      "arguments": [],
      "documentation": [
        " Remove the prime member if it exists.",
        "",
        " May only be called from `T::PrimeOrigin`."
      ]
    }
  ],
  "event": [
    {
      "name": "MemberAdded",
      "arguments": [],
      "documentation": [
        " The given member was added; see the transaction for who."
      ]
    },
    {
      "name": "MemberRemoved",
      "arguments": [],
      "documentation": [
        " The given member was removed; see the transaction for who."
      ]
    },
    {
      "name": "MembersSwapped",
      "arguments": [],
      "documentation": [
        " Two members were swapped; see the transaction for who."
      ]
    },
    {
      "name": "MembersReset",
      "arguments": [],
      "documentation": [
        " The membership was reset; see the transaction for who the new set is."
      ]
    },
    {
      "name": "KeyChanged",
      "arguments": [],
      "documentation": [
        " One of the members' keys changed."
      ]
    },
    {
      "name": "Dummy",
      "arguments": [
        "sp_std::marker::PhantomData<(AccountId, Event)>"
      ],
      "documentation": [
        " Phantom member, never used."
      ]
    }
  ],
  "constants": [],
  "errors": [
    {
      "name": "AlreadyMember",
      "documentation": [
        " Already a member."
      ]
    },
    {
      "name": "NotMember",
      "documentation": [
        " Not a member."
      ]
    }
  ],
  "index": 15
}
metadata generated by new macro
{
  "name": "TechnicalMembership",
  "storage": {
    "prefix": "TechnicalMembership",
    "entries": [
      {
        "name": "Members",
        "modifier": "Default",
        "ty": {
          "Plain": "Vec<T::AccountId>"
        },
        "default": [
          0
        ],
        "documentation": [
          " The current membership, stored as an ordered Vec."
        ]
      },
      {
        "name": "Prime",
        "modifier": "Optional",
        "ty": {
          "Plain": "T::AccountId"
        },
        "default": [
          0
        ],
        "documentation": [
          " The current prime member, if one exists."
        ]
      }
    ]
  },
  "calls": [
    {
      "name": "add_member",
      "arguments": [
        {
          "name": "who",
          "ty": "T::AccountId"
        }
      ],
      "documentation": [
        " Add a member `who` to the set.",
        "",
        " May only be called from `T::AddOrigin`."
      ]
    },
    {
      "name": "remove_member",
      "arguments": [
        {
          "name": "who",
          "ty": "T::AccountId"
        }
      ],
      "documentation": [
        " Remove a member `who` from the set.",
        "",
        " May only be called from `T::RemoveOrigin`."
      ]
    },
    {
      "name": "swap_member",
      "arguments": [
        {
          "name": "remove",
          "ty": "T::AccountId"
        },
        {
          "name": "add",
          "ty": "T::AccountId"
        }
      ],
      "documentation": [
        " Swap out one member `remove` for another `add`.",
        "",
        " May only be called from `T::SwapOrigin`.",
        "",
        " Prime membership is *not* passed from `remove` to `add`, if extant."
      ]
    },
    {
      "name": "reset_members",
      "arguments": [
        {
          "name": "members",
          "ty": "Vec<T::AccountId>"
        }
      ],
      "documentation": [
        " Change the membership to a new set, disregarding the existing membership. Be nice and",
        " pass `members` pre-sorted.",
        "",
        " May only be called from `T::ResetOrigin`."
      ]
    },
    {
      "name": "change_key",
      "arguments": [
        {
          "name": "new",
          "ty": "T::AccountId"
        }
      ],
      "documentation": [
        " Swap out the sending member for some other key `new`.",
        "",
        " May only be called from `Signed` origin of a current member.",
        "",
        " Prime membership is passed from the origin account to `new`, if extant."
      ]
    },
    {
      "name": "set_prime",
      "arguments": [
        {
          "name": "who",
          "ty": "T::AccountId"
        }
      ],
      "documentation": [
        " Set the prime member. Must be a current member.",
        "",
        " May only be called from `T::PrimeOrigin`."
      ]
    },
    {
      "name": "clear_prime",
      "arguments": [],
      "documentation": [
        " Remove the prime member if it exists.",
        "",
        " May only be called from `T::PrimeOrigin`."
      ]
    }
  ],
  "event": [
    {
      "name": "MemberAdded",
      "arguments": [],
      "documentation": [
        " The given member was added; see the transaction for who."
      ]
    },
    {
      "name": "MemberRemoved",
      "arguments": [],
      "documentation": [
        " The given member was removed; see the transaction for who."
      ]
    },
    {
      "name": "MembersSwapped",
      "arguments": [],
      "documentation": [
        " Two members were swapped; see the transaction for who."
      ]
    },
    {
      "name": "MembersReset",
      "arguments": [],
      "documentation": [
        " The membership was reset; see the transaction for who the new set is."
      ]
    },
    {
      "name": "KeyChanged",
      "arguments": [],
      "documentation": [
        " One of the members' keys changed."
      ]
    },
    {
      "name": "Dummy",
      "arguments": [
        "sp_std::marker::PhantomData<(AccountId, Event)>"
      ],
      "documentation": [
        " Phantom member, never used."
      ]
    }
  ],
  "constants": [],
  "errors": [
    {
      "name": "AlreadyMember",
      "documentation": [
        " Already a member."
      ]
    },
    {
      "name": "NotMember",
      "documentation": [
        " Not a member."
      ]
    }
  ],
  "index": 15
}

diff:

4c4
<     "prefix": "Instance1Membership",
---
>     "prefix": "TechnicalMembership",

⚠️ Breaking Change ⚠️

From https://crates.parity.io/frame_support/attr.pallet.html#checking-upgrade-guidelines

storages now use PalletInfo for module_prefix instead of the one given to decl_storage: Thus any use of this pallet in construct_runtime! should be careful to update name in order not to break storage or to upgrade storage (moreover for instantiable pallet). If pallet is published, make sure to warn about this breaking change.

So users of the pallet must be careful about the name they used in construct_runtime!. Hence the runtime-migration label, which might not be needed depending on the configuration of the pallet.

polkadot and kusama use TechnicalMembership as pallet name (but the current module_prefix is Instance1Membership), thus need to migrate the storages.


polkadot companion: paritytech/polkadot#3263

@koushiro koushiro marked this pull request as ready for review June 16, 2021 07:43
Signed-off-by: koushiro <[email protected]>
Signed-off-by: koushiro <[email protected]>
@gui1117 gui1117 added A0-please_review Pull request needs code review. B3-apinoteworthy C1-low PR touches the given topic and has a low impact on builders. labels Jun 16, 2021
@koushiro
Copy link
Contributor Author

@KiChjang PTAL

@KiChjang
Copy link
Contributor

bot merge

@ghost
Copy link

ghost commented Aug 20, 2021

Trying merge.

@gui1117
Copy link
Contributor

gui1117 commented Aug 20, 2021

merging master should probably fix CI, but we need to test one time with try-runtime the companion before the merge IMHO, I'll do once I have proper connection. Or we should try the try-runtime bot

@KiChjang
Copy link
Contributor

bot merge cancel

@ghost
Copy link

ghost commented Aug 20, 2021

Merge cancelled.

@gui1117
Copy link
Contributor

gui1117 commented Aug 23, 2021

there was a new release, let's wait for PR to cleanup and get the new version and empty migration, kusama to get the runtime upgrade, then we can update this PR and do test with try-runtime on kusama

@gui1117
Copy link
Contributor

gui1117 commented Aug 27, 2021

I tested with try-runtime on kusama with command polkadot try-runtime --chain kusama-dev --block-at 45a70b74a6215bf682f96ca87e3728efef0f4dad4587a1859e8c13eed3b5ac46 on-runtime-upgrade live -m TechnicalMembership -m Instance1Membership and it worked (with the small fix 01b5ecd)

In the companion I deleted previous migration of 9090 and added new one. maybe this can be reviewed here paritytech/polkadot#3731
But otherwise it is ready to be merged

@gui1117
Copy link
Contributor

gui1117 commented Sep 7, 2021

bot merge

@ghost
Copy link

ghost commented Sep 7, 2021

Waiting for commit status.

This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review. C1-low PR touches the given topic and has a low impact on builders. D5-nicetohaveaudit ⚠️ PR contains trivial changes to logic that should be properly reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants