onflow / nft-storefront

A general-purpose Cadence contract for trading NFTs on Flow
The Unlicense
102 stars 53 forks source link

Cuts In Listing Events #34

Closed rheaplex closed 2 years ago

rheaplex commented 2 years ago

The ListingAvailable event does not currently contain the Cuts array.

This was an explicit design decision:

  1. The Cuts array may be of arbitrary length, which may have gas cost implications in future.
  2. The Cut struct contains a Capability to a Vault.Receiver, which the owner may not want broadcasting.
  3. If we replace the Capability with the Address of the vault, this exposes the address (which is undesirable) and the vault may be moved anyway.
  4. It is simple enough, and more accurate, to query this information with a script.

All of that said, the Storefront workflows that require third parties to scan for events that they then query the Listing for in order to determine if the Listing offers them a cut as payment for selling the token offered by the listing might benefit from having access to some cuts information at this point in Flow development.

I recommend using the Vault.uuid rather than the address if we do not wish to expose the address - this is used in other contracts so it is a pattern. This would require a 'report struct' that the uuid and amount are copied to in order to be placed into an event. The same is true of using addresses.

We could either add the Cuts array to the ListingAvailable, probably as an optional to allow for this information to be omitted if including it would be prohibitive or to reflect the current workflow that omits it.

Or we could optionally emit a specific ListingCuts event immediately after the ListingAvailable event. This would contain the Listing id and the Cuts report struct array:

pub struct CutDetails {
    pub let vaultId: UInt64
    pub let amount: UFix64

    init(vaultId: UInt64, amount: UFix64) {
        self.vaultId = vaultId
        self.amount = amount
    }
}

pub event ListingCuts {
    pub let listingId: UInt64
    pub let cuts: [CutDetails]
}

The Listing creation code then translates the Cuts array into an array of CutDetails and emits a ListingCuts event containing them.

Doing it this way does not affect current workflows that do not expect this information, and allows new workflows that start by checking Cuts.

This code does add gas cost and code complexity to Listing creation, and it may remain preferable to specify that offchain watchers should run a script to check the cuts for a Listing that they may be interested in - this allocates cost to those who benefit from expending it. But should it be desired to add Cut details to Storefront events (and check with Dete before deciding this!), this is a way to do it.

bluesign commented 2 years ago

I have mixed feelings about this but if it is ok I would like to mention a tangent here.

Is it somehow possible to document design decisions somehow?

I have some comments on StoreFront but I really don't want to go out of basic design decisions.

One simple example:

Instead of listing to 10 markets, I can list one, markets can do the middleman function.

From user perspective it is better to know I bought for X and selling from Y>X will make me profit.

But maybe this is already considered but we have no idea as developers.