Multiwrap: using thirdweb-dev/contracts/feature/

[kumaryash90]

No description provided.

[nkrishang]

We don't want to have opinionated handling of token IDs in our sdk features. We leave the handling of tokenIds to the top level contract that will be inheriting this TokenBundle feature e.g. Multiwrap in this case.

The reason for the above:

• Folks have product level preferences of how token IDs should be handled. We shouldn't make the assumption for the end user that token Ids will be assigned serially.
• The TokenBundle contract should not have special / privileged status as the base contract that's going to specify how token IDs will work.

[nkrishang]

See the comments for token ID handling w.r.t nextTokenIdToMint.

Since we want to hand off token Id handling to the derived contract e.g. Multiwrap, a function like _getNextTokenId() should be exposed as a virtual function, so that the derived contract can override it and supply the right values to this function. What do you think about that?

One question is -- conceptually, does TokenBundle have anything to do with "token IDs"? Why can't _getNextTokenId be _getIdForNewBundle? That is, let the user choose whether the identifier of a bundle is a token Id or not.

Think of the potential use cases for TokenBundle. I don't think it's limited to contracts like Multiwrap that issue NFTs (i.e. token IDs) for a token bundle.

I can see TokenBundle being used in a simple escrow contract, or even in a DeFi contract, since the main utility that i see TokenBundle providing is a generic + compressed way of describing a list of tokens. This has a wide range of uses.

[kumaryash90]

Yes, makes sense. I like the virtual function idea.

I have added a bundleId variable and removed any tokenId association; leaving it to the top-level contract to map their tokenId with bundleId. Modified related functions accordingly, in both TempMultiwrap and TokenBundle contracts.

For now, I've added some sanity checks while creating/updating/deleting bundles. These can be removed if we assume that the top-level contract will take care of these.

I was wondering if it would be logical to bring any token transfer functions inside TokenBundle contract, since creation/deletion of a bundle would almost always require transfer of underlying tokens.

For now, I've kept the getNextTokenId() virtual function as a comment inside TokenBundle.