Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proposal to add a second AccountId argument to the check-membership pallet function #416

Open
ltfschoen opened this issue Mar 10, 2021 · 0 comments

Comments

@ltfschoen
Copy link

In the check-membership recipe, it shows how loose coupling is recommended over tight coupling.

The loose coupling example code is brilliant, and I was able to plug that into my projects codebase so I could have a membership.

But when I went to try out the code using the UI, I found that out of the box it didn't suit my use-case, and I had to make a minor tweak. And I think that my use-case would likely be more common that the current boilerplate.

So at the moment, when i tried it with polkadot.js.org/apps, the only accounts that you can add/remove to the membership are for the accounts that you have imported or created in polkadot.js.org/apps, since you can only add/remove the signer.

But I wanted to be able to add other accounts to the membership (for example from my address book, or even other accounts that weren't even mine, and I think that is a more likely scenario for most use cases.

So my proposal is just to add a second T::AccountId argument to both the add_member and remove_member functions, so the code would be like:

        #[weight = 10_000]
        pub fn add_member(
            origin,
            new_member: T::AccountId,
        ) -> DispatchResult {
            let _sender = ensure_signed(origin)?;
            // let new_member = ensure_signed(origin)?;

I'd be happy to create a PR if this is considered an improvement and a PR would be welcomed. It would require both vec-set and map-set pallets to be updated, along with their tests.
Or it could be created and labelled as an opportunity for a new Substrate developer to contribute.

Here is where i made the changes in my custom pallet where i used vec-set from Substrate recipes, and added that extra argument DataHighway-DHX/node@a6dff2a

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

No branches or pull requests

1 participant