Added support to band, added formBand and disbandBand

[StErMi]

Hi there,

this is just a proposal to experiment with the concept of building blocks and how we could interact with core blocks to create new ones.

When I say "block" I mean band, artist, venue and so on.

Proposal

A band should be formed by 5 members of different types.

• Vocalist
• Lead Guitar
• Rhythm Guitar
• Bass
• Drums

You can form a band only when you own an artist of each type. After forming a band you can disband it.

Tests

I've created a /test/band-test.js where I'm testing only the "success" cases. I'll add more tests as soon as possible.

Code implementation

Types.sol

I've added a Role enum, changing the Musician role from string to Role type. I need a Role type to enforce check when you form a band. You can't (at least in this implementation) form a band of only singers. @Shpigford this is just a proposal, so feel free to suggest how a band should be formed, I only needed some type of hard limit for implementation/test

I've added 5 new uint tokenId to the Band type because I need to trace which artists are part of a Band

	uint256 vocalistId;
	uint256 leadGuitarId;
	uint256 rhythmGuitarId;
	uint256 bassId;
	uint256 drumsId;

I've updated the MusicianRenderer to properly render the new role type and I've updated the metadata-test.js to correctly pass the test.

I've added two methods to the main contract formBand and disbandBand. Maybe all this implementations could be exported inside an external contract that is inherited by RockburgNFT to not have everything inside of it.

formBand

When you form a Band you need to specify the name, bandType (like Blues, Jazz, Rock&Roll), and the tokenId of 5 artists that will perform in a band. Those artists must be owned by the sender and need to be from each of the required types.

If all the checks pass the contract is going totransfer the ownership of those artists to the Contract itself and mint a new Band associating the artistIds to the band members.
At this point the User is not the owner of the artists anymore, this is needed to prevent him to transfer them (sell) to others while he still owns the band. If he wants he can transfer the whole Band.

disbandBand

This is the inverse process of forming a band. I check if the user owners the band and if so I'm going to transfer all the artists back to his account (he could not be the original owner of the band, maybe he has sold that band to someone else).

After doing that we need to decide what to do with the band and I think that we have two options here @Shpigford @m1guelpf

• Option1: "soft delete". We transfer the band from the user to the Contract and we reset the ids of the artists. We could update some states like disbanded to true only for metadata/frontend display purposes. Maybe adding a formDate and disbandDate just for fun.
• Option2: "hard delete". We burn the band deleting the NFT token and removing it from the _bands mapping.

ERC721Holder

I needed to add it in order to implement the Implementation of the IERC721Receiver interface. @m1guelpf I'm not a super expert here, just used it for another project I was implementing ("Unloot the Loot bags").

This is needed to allow our own contract to accept transfers. Maybe we could skip this if we use the internal _transfer instead of the safeTransferFrom?

Why have I used _transfer instead of safeTransferFrom in disbandBand?

the safeTransferFrom implementation is checking that the msg.sender (the user) is the owner of the token or has the approval to transfer it. In this case, the from part of the safeTransferFrom is the contract itself. It seems that this function was not meant to be used internally when the NFT Contract is also the sender so I used the _transfer directly.
I don't know if this is the best implementation but I've already asked around about it so I'll wait for an answer on this topic.

Feedback?

What do you think?

[m1guelpf]

Anyone can call disbandBand right now, we should add an ownership check here

[StErMi]

@m1guelpf

• Updated the branch with typescript
• added ownership checks on disband
• added more checks on role/type during form a band
• added more tests to band-test.ts. They should cover most cases but I'm not a super expert in solidity security

Another thing I'm still not expert in code optimization. Coming from web2 I still don't have the "eye" to see possible optimizations.