Multiple collections per seller

[zajck]

Start of the discussion about #14

[zajck]

There are some implementation choices for this proposal.

Keeping the list of collection addresses.

Currently we have one collection per seller, so we have one mapping

// seller id => cloneAddres
mapping(uint256 => address) cloneAddress;

Obviously we need a different mapping for storing a list, most likely something like

// seller id => cloneAddresses
mapping(uint256 => address[]) cloneAddresses;

Now we have two options:

1. Keeping cloneAddress functional.
   It is set during the createSeller and is the default clone address. When additional collections are created by calling createNewCollection, we store those addresses in new cloneAddresses. Whenever we need to retrieve the clone address, logic depends on collectionIndex - if index is 0, get cloneAddress, otherwise get it from cloneAddresses.
   1. + existing data remain untouched, createSeller remains as is
   2. - a slightly more complex lookup logic
2. Deprecate cloneAddress.
   Whole list is stored in cloneAddresses. Old data needs to be moved in the first slot of the new variable.
   1. + simple lookups
   2. - need backfilling
   3. - we still need to keep a placeholder for cloneAddress in the protocol, otherwise storage gets corrupted

Voucher ownership.

BosonVoucher inherits OZ's OwnableUpgradeable, since some methods are restricted by onlyOwner. Voucher owner is seller's assistant and the only way to transfer ownership to someone else is to call updateSeller and optInToSellerUpdate on the protocol.
With multiple collections we also need to manage their ownership.

1. Ownership is managed on each voucher.
   When voucher is cloned, we initialize and set assistant as the owner. If assistant is changed, protocol loops over all collections and call transferOwnership on each of them
   1. + we can keep OZ's OwnableUpgradeable contract
   2. + day to day operations that are restricted with onlyOwner are more efficient
   3. - transferring ownership is less efficient. Potentially could run into a problem of block gas limit and we would need a way out of it, which would require additional protocol changes.
2. Ownership is managed on the protocol.
   This is beacon like pattern, where owner is not stored on the voucher itself, but we have a view function on the protocol that returns voucher contract owner. OZ's owner() function needs to be overriden to look owner up in the protocol instead of local storage.
   1. + transferring ownership is efficient and safe. However it does not emit OwnershipTransferred events for each voucher.
   2. - modifier onlyOwner is more costly since it needs to call the protocol. This affects methods preMint, burnPremintedVouchers, setContractURI, callExternalContract and setApprovalForAllToContract.
   3. - all methods that use owner() at some point are affected. This includes all initial transfers of preminted vouchers, reserveRange, burnPremintedVouchers, getSellerId, withdrawToProtocol

[anajuliabit]

IMO it makes more sense for the first index always to be the seller's "general" storefront, and subsequent indexes to be for custom brands, and we maintain compatibility which existing collections.

thinking about ownership looks safer for me if we always get the owner by calling protocol. but we need to evaluate how much the cost of methods would increase, as we still need to keep those functions under the max gas limit

[zajck]

Boson voucher name and symbol

So far each seller had 1 voucher contract with

• name: Boson Voucher (rNFT) {sellerId} (e.g. "Boson Voucher (rNFT) 15")
• symbol: BOSON_VOUCHER_RNFT_{sellerId} (e.g. "BOSON_VOUCHER_RNFT_15")

If seller has multiple voucher contracts we want to distinguish them, so we need a different naming.

One possible approach would be to add collectionIndex, for example:

• name: Boson Voucher (rNFT) {sellerId}_{collectionIndex} (e.g. "Boson Voucher (rNFT) 15_3")
• symbol: BOSON_VOUCHER_RNFT_{sellerId}_{collectionIndex} (e.g. "BOSON_VOUCHER_RNFT_15_3").

It does not reveal any information about the collection.

Alternatively, we could allow seller to add a custom part to the name

• name: Boson Voucher (rNFT) {sellerId}_{collectionIndex}: {externalId} (e.g. "Boson Voucher (rNFT) 15_3: Nike")

This would look nicer on explorers (etherscan for example). But someone malicious seller might want to deceive the buyers by setting some reputable brand in the name.

[anajuliabit]

I believe the seller should be able to choose the name, as it can involve marketing considerations.

[thecryptofruit]

Another option could be to have a neutral suffix, like a short hash, in order to (i) be neutral) and (ii) keep the names short (char vs int entropy).

[zajck]

@thecryptofruit can you write down exactly what you mean in a similar format that I used?

Something like:
name: Boson Voucher (rNFT) {hash}
symbol: BOSON_VOUCHER_RNFT_{hash}

Or would you keep the sellerId as part of the name/symbol?