JoshOrndorff / recipes

A Hands-On Cookbook for Aspiring Blockchain Chefs
GNU General Public License v3.0
378 stars 186 forks source link

Map-set pallet: map keys to () instead of bool #300

Closed JoshOrndorff closed 4 years ago

JoshOrndorff commented 4 years ago

As of paritytech/substrate#6364 Substrate will allow empty storage values such as (). This means we no longer have to map to bool as a workaround.

herou commented 4 years ago

Hi @JoshOrndorff . I see that this issue is still open and I see that you offer mentorship for this. What exactly needs to be done here?

JoshOrndorff commented 4 years ago

Hi @herou, I'm glad you're interested.

This issue is to make an update to the map-set recipe. It's write-up is at https://substrate.dev/recipes/3-entrees/storage-api/map-set.html and it's code is https://github.com/substrate-developer-hub/recipes/tree/master/pallets/map-set.

When this recipe was first written it was not recommended to use () as the type being mapped to, so I used bool instead, as a hack. Now it is possible to map to () so we should. The primary change needed is on this line:

- Members get(fn members): map hasher(blake2_128_concat) T::AccountId => bool;
+ Members get(fn members): map hasher(blake2_128_concat) T::AccountId => ();

Then you would have to update the other code that uses this storage item, update the comments, and finally, update the write-up.

Hopefully that's enough to get you started. Make that change I suggested, and see if you can get the pallet to compile. Report back if you need more guidance, and thanks again for the help :)

herou commented 4 years ago

@JoshOrndorff I would suggest closing this ticket.

JoshOrndorff commented 4 years ago

Yes, thank you for solving this one in #335 @herou

herou commented 4 years ago

@JoshOrndorff It is a pleasure. Actually I am looking for more tickets to close but just a few of them can be mentored by you :/