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

Migrate substrate pallets to use pallet! #7882

Closed
44 tasks done
gui1117 opened this issue Jan 12, 2021 · 17 comments
Closed
44 tasks done

Migrate substrate pallets to use pallet! #7882

gui1117 opened this issue Jan 12, 2021 · 17 comments

Comments

@gui1117
Copy link
Contributor

gui1117 commented Jan 12, 2021

Here is the list of pallet and their status, if you want to migrate one you can put your name after it to avoid duplicated work.
some help guidelines to do migration: https://crates.parity.io/frame_support/attr.pallet.html#upgrade-guidelines
some help guidelines to review a migration: https://crates.parity.io/frame_support/attr.pallet.html#checking-upgrade-guidelines

@kianenigma
Copy link
Contributor

pallet-executive

Is this a mistake?

pallet-elections

I wouldn't migrate this one until an external team or someone asks for it tbh. It is not really maintained.

emostov added a commit that referenced this issue Mar 16, 2021
Part of #7882.

Converts the `Proxy` pallet to the new pallet attribute macro introduced in #6877.

[Upgrade guidelines used](https://substrate.dev/rustdocs/v3.0.0/frame_support/attr.pallet.html#upgrade-guidelines).

## ⚠️ Breaking Change  ⚠️ 

From [checking upgrade guidelines](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`: use of this pallet in `construct_runtime!` needs careful updating of the name in order to not 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 `Assets` 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 `Assets` pallet.

### Notes

There are some changes to the docs in metadata for the constants. The docs in the metadata for constants are now more complete.
gui1117 pushed a commit that referenced this issue Mar 17, 2021
* Migrate pallet-proxy to pallet attribute macro

Part of #7882.

Converts the `Proxy` pallet to the new pallet attribute macro introduced in #6877.

[Upgrade guidelines used](https://substrate.dev/rustdocs/v3.0.0/frame_support/attr.pallet.html#upgrade-guidelines).

## ⚠️ Breaking Change  ⚠️ 

From [checking upgrade guidelines](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`: use of this pallet in `construct_runtime!` needs careful updating of the name in order to not 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 `Assets` 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 `Assets` pallet.

### Notes

There are some changes to the docs in metadata for the constants. The docs in the metadata for constants are now more complete.
@shaunxw
Copy link
Contributor

shaunxw commented Mar 17, 2021

* [ ]  pallet-babe @shamb0

* [ ]  pallet-utility @shamb0

@thiolliere Some mistakes here, it should be:

@shawntabrizi
Copy link
Member

Here is an issue for migrating the Polkadot pallets: paritytech/polkadot#2882

hirschenberger pushed a commit to hirschenberger/substrate that referenced this issue Apr 14, 2021
* Migrate pallet-proxy to pallet attribute macro

Part of paritytech#7882.

Converts the `Proxy` pallet to the new pallet attribute macro introduced in paritytech#6877.

[Upgrade guidelines used](https://substrate.dev/rustdocs/v3.0.0/frame_support/attr.pallet.html#upgrade-guidelines).

## ⚠️ Breaking Change  ⚠️ 

From [checking upgrade guidelines](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`: use of this pallet in `construct_runtime!` needs careful updating of the name in order to not 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 `Assets` 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 `Assets` pallet.

### Notes

There are some changes to the docs in metadata for the constants. The docs in the metadata for constants are now more complete.
@shaunxw
Copy link
Contributor

shaunxw commented Jun 10, 2021

These pallets migration are merged and can be ticked now:

  • pallet-democracy
  • pallet-multisig
  • pallet-scheduler

@koushiro
Copy link
Contributor

koushiro commented Jun 10, 2021

I plan to continue migrating pallet-collective and pallet-membership to pallet attribute macro, is anyone working on it?

BTW, it seems that pallet-transaction-payment has not yet migrated to the new pallet attribute macro.

[x] pallet-transaction-payment @kianenigma #8044

@gui1117
Copy link
Contributor Author

gui1117 commented Jun 10, 2021

I put your name, nobody is working on it AFAIK

@apopiak
Copy link
Contributor

apopiak commented Jun 11, 2021

Just a note that we want to mark the extrinsics as pub (instead of pub(crate) from now on (see this PR: #9078).

@koushiro
Copy link
Contributor

koushiro commented Jun 11, 2021

@shawntabrizi I found a issue in the process of migrating pallet-membership to new pallet attribute macro (#9080). The storage prefix and pallet name in the metadata, generated by the new pallet macro, are consistent, but for the pallets that use the instance and the old macro, Instance+PalletName will be used as the storage prefix.
I saw issue #8391, but I want to know if there is a specific macro syntax to solve this problem.

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": [
        "PhantomData<(T::AccountId,<T as Config<I>>::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",
177c177
<         "sp_std::marker::PhantomData<(AccountId, Event)>"
---
>         "PhantomData<(T::AccountId,<T as Config<I>>::Event)>"

@gui1117
Copy link
Contributor Author

gui1117 commented Jun 11, 2021

@shawntabrizi I found a issue in the process of migrating pallet-membership to new pallet attribute macro (#9080). The storage prefix and pallet name in the metadata, generated by the new pallet macro, are consistent, but for the pallets that use the instance and the old macro, Instance+PalletName will be used as the storage prefix.
I saw issue #8391, but I want to know if there is a specific macro syntax to solve this problem.

the solution is to migrate the storages, in order to use their new pallet prefix.
The issue #8391 could help, but I don't think it is the correct way to solve it. Using the pallet name for pallet prefix is more future proof I think.

@koushiro
Copy link
Contributor

koushiro commented Jun 11, 2021

@thiolliere Is it possible to cause storage conflicts? e.g. Council and TechnicalCommitte use the same pallet(pallet-collective), but different instances.
Oops, I was wrong, this should not cause storage conflicts

@OLUWAMUYIWA
Copy link

What more is there to contribute to here, people?

@OLUWAMUYIWA
Copy link

I plan to migrate pallet-society. Is anyone else working on it?

@gui1117
Copy link
Contributor Author

gui1117 commented Jun 28, 2021

I plan to migrate pallet-society. Is anyone else working on it?

good, I'm not aware of anybody working on it yet

@OLUWAMUYIWA
Copy link

Okay then. I have picked it

@gui1117
Copy link
Contributor Author

gui1117 commented Nov 17, 2021

finally finished for substrate pallets, thanks for all contributions

@gui1117 gui1117 closed this as completed Nov 17, 2021
@shawntabrizi
Copy link
Member

Big thank you to everyone for this!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants