Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Design Proposition For Performance Issue #3

Open
anilhelvaci opened this issue Mar 21, 2024 · 2 comments
Open

Design Proposition For Performance Issue #3

anilhelvaci opened this issue Mar 21, 2024 · 2 comments

Comments

@anilhelvaci
Copy link
Collaborator

anilhelvaci commented Mar 21, 2024

Design Proposition For KREAd Performance Issue

Context

See Agoric/agoric-sdk#8862

Problems targeted

  • Assets live on zoe vat
  • Assets live in open seats
    • Payments living in open seats are stored in a single purse and Zoe struggles to add/remove new payments as the purse size grows when the amount is a copyBag
  • AmountMath operations with copyBag causes too much computation

Alternative Inventory

The current KREAd implementation uses multiple ZCFSeats to store equipped items and a duplicate of user's character
to ensure users can interact with the contract under Zoe's protection. What we propose has two main improvements to this implementation;

  • Wash Trades
    Get rid of the duplicate character. Based on this comment from Chris Hibbert, it is explained that we can indeed execute "wash trades", wish means that the user can give and request the same asset when building an offer. Although It is important to notice that the proposal will need a different keyword for the want and give Character amount.

  • Dedicated purse over open ZCFSeat
    To remove the dependency on the inventorySeat, we intend to use the Items issuer to make an empty Purse for each Character that will hold the equipped Items. This purse will be included in the Character record, replacing the inventory seat attribute. This way we can deposit and withdraw the required Items to assure the normal flow of KREAd without the performance load of the previous design.

    • Market Place
      When a user wish to sell a Character or an Item, the asset will be escrowed in Zoe, which contributes for the performance issue described above. To address this issue, one additional change we intend to implement is to create a dedicated Purse for every sellRecord, that will hold the asset until the sell is executed or the user decides to cancel it. This Purse will be recorded in the contract state, by including it in the MarketEntryGuard, along with the seller Seat.
  • Replace ITEMS issuer
    To complement the change above, we wish to replace the ZCFMint created for Item into an IssuerKit. The reason behind this decision is based on the Vats where the Purses will be living in. When using the makeZCFMint method, the Purses created from the issuer returned, will live in vat-zoe, while with the makeIssuerKit method, the Purses created will be stored in vat-kread. We expect that this will reduce the load on Zoe and improve its performance.

    • If using the purses living in vat-zoe does not create a problem for Zoe, we can consider skipping this feature.

Implementation Approaches

Both approaches below takes the idea above and implements them with different considerations.

  • Inventory Vat Approach
    Deploys a new Zoe contract that handles the inventory logic described above to relieve vat-kread from all copyBag computations(add, subtract).
  • In vat-kread Approach
    Implements alternative inventory approach inside the kreadKit (or a dedicated virtual object, TBD) to avoid vat-to-vat communication overhead.

Migration

There are users who already minted characters and items with the existing way. We must implement a mechanism to migrate those users to the latest version. This means;

  • We must let the user know their brand for KREAdITEM will change
    • If we end up not changing the ITEMS issuer, this is omitted
  • We must burn the character living in inventorySeat
  • We must mint items using new KREAdITEM with the exact amounts that are equipped to the character
    • If we end up not changing the ITEMS issuer, this is omitted
  • Send those newly minted items to the purse created for user's character
  • Burn old items
  • Exit inventorySeat

We should probably perform this migration as old users try to interact with the application after the upgrade instead of trying to move all characters and items at once(for the sake of Zoe). So we end two main ways to do this;

  1. Require a dedicated migration offer from the user if their character DOES NOT have a registered purse
    • Pros
      • By isolating migration process into a new offer, contract code complexity will probably be reduced
      • The usual interactions(equip, unequip, ..etc) will not take longer time
      • (If ITEMS issuer changes) We can structure the offer in the frontend in a way that users include their unequipped items in their migration offers so that we have a plan for how to get rid of assets that still use old ITEMS issuer.
    • Cons
      • Users will have to pay an additional transaction fee for one time
  2. Perform migration as a middleware operation once an action request from an old user is received(equip, unequip, ..etc).
    • Pros
      • Better UX
      • Less transaction fee for the user
    • Cons
      • More complexity in contract code
      • Harder to track block time for the usual operations(equip, unequip, ..etc) that include migration(we'll have manually look for in the usual operation includes any migration or not).

How to migrate characters/items that are on sale?

This question has its own heading simply because it's a pretty big one and we don't have a high level of confidence in the possible directions we're considering to take.

The KREAd market place has currently 524 items and 35 characters in sale. All sales are managed with an open ZCFSeat. There are two types of item sales, (1) First sale, means items minted by KREAd and being sold directly by the protocol, (2) Secondary sale, means users are selling the items they own. All character sales are done by the users, meaning they are similar to secondary item sales. From Zoe's point of view, it makes no difference whether the sale is secondary or not because all payments escrowed in various number of ZCFSeats are managed by the same purse. However during the migration, we might have to deal with first and secondary sales differently. The reason being, secondary sales actually have a user expecting a payout when first sales send the payout to some protocol account. Which means, in order not to break customer experience we must take careful approach in this. This problem has both a product and technical aspect to it. The product side is more related to the secondary sales whereas technical side is going to care about the first sales. This is simply because the majority of 524 items on sale are first sales. Therefore, most of the load is coming from there. The ideas we've considered so far for the first item sales;

  • Given that all items for the first sale are minted into a zcf seat called internalSellSeat;
    • Have a repeater defined from the timer service and on every interval withdraw a number of items
    • Since the performance issue is a problem of O(numberOfItemsInPurse), we can increase the number of items withdrawn as the time moves on because numberOfItemsInPurse will decrease
    • Partition the withdrawn items into a number of purses where O(numberOfItemsInPurse) is not a problem
    • We can also consider minting the items on the go instead of first minting them and then selling them, which's what I'm personally leaning towards

Other Open Questions

  • How to handle bought but unequipped items that users are holding in their own purses? (In the case we change ITEMS issuer)
@anilhelvaci
Copy link
Collaborator Author

anilhelvaci commented Mar 26, 2024

Inventory Vat Approach

prepared by: @anilhelvaci

As stated in the problems targeted section, we are trying to relieve zoe from having to deal with KreadCharacter and KreadItem purses.

Assumptions

  • The purse created from issuer.makeEmptyPurse() where issuer is coming from a ZCFMint (as is the case with KREAd) is going to be living in vat-zoe which is still going to be responsible for the add and subtract operations for copyBag items. This is still an expensive approach and we want to move it to a vat related to KREAd. So we need a separate the vat that holds the references to the purses.

  • Moving the issuerKit to a vat other than vat-zoe will put the execution costs on KREAd and not on zoe. This is a requirement mentioned in perf investigation: KREAd, ERTP, NFT Purses, Zoe Agoric/agoric-sdk#8862 and in the quote:

    ...and our charge-for-execution plans need a way to assign the costs to v63-kread
    
  • Zoe allows wash trades as long as we use different keywords for give and want part of the proposal. As referenced in Drop Zoe's prohibition on "Wash Sales" Agoric/agoric-sdk#8859

Solution

This approach presents a solution with the current features;

  • We create a new smart contract called KreadInventory
    • This contract creates a new issuer kit called KreadItem using ERTP's makeIssuerKit method
    • Keeps a table where characters (or some unique property in the character) is the key and value is a purse created using kreadItemIssuer.makeEmptyPurse().
    • The items will be first minted and then deposited into these purses during a new character mint.
      • Minted character will be sent to the user directly and only one payment will be minted per character
      • Since equipped items will be sent to the inventory purse (in vat-inventory) there will be no reason for keeping an inventorySeat.
    • When there's a request to vat-kread(original KREAd contract) whether it is mint, equip, unequip or swap, KREAd contract makes another offer to this vat-inventory to fulfill user's offer.
    • Communication between vat-kread and vat-inventory will happen via offerTo method of zoeHelpers API

Diagram for operation mint with this solution

sequenceDiagram
    box Blue User
    participant Usw as SmartWallet
    end
    participant Zoe
    box Black KREAd
    participant Contract
    participant State
    end
    participant Zoe1 as Zoe
    box Green KREAdInventory
    participant Isw as KREAdInventoryContract
    end

    Isw ->> Isw: create KreadItem issuer kit
    Usw->>Zoe: mint offerSpec
    Zoe->>Contract: Proposal - give Price
    Zoe-->>Usw: UserSeat
    Contract->>Contract: create Character amount
    rect rgb(125, 0, 0)
    critical create Inventory Account
    Contract->>+Zoe1: registerCharacter offerSpec
    Zoe1-->>Contract: { userSeat, deposited }
    Zoe1 ->> Isw: registerCharacterHandler
    Isw ->> Isw: create purse for equipped items
    Isw ->> Isw: update character -> purse table
    Isw-->>Zoe1: mintItemsIntoPurseInvitation
    Zoe1-->>- Contract: offerResult<mintItemsIntoPurseInvitation>
    Contract ->> Contract: userSeat.getOfferResult()
    end
    end
    Contract->>Contract: mint Character
    rect rgb(79, 29, 2)
    critical create Inventory Account
    Contract->>+Zoe1: mintItemsIntoPurse offerSpec
    Zoe1-->>Contract: { userSeat, deposited }
    Zoe1 ->> Isw: mintItemsIntoPurseHandler
    Isw ->> Isw: find items purse for character
    Isw ->> Isw: mint defaultItems into the purse
    Isw-->>Zoe1: "Items successfully minted"
    Zoe1-->>- Contract: offerResult< "Items successfully minted">
    Contract ->> Contract: userSeat.getOfferResult()
    end
    end
    Contract->>Zoe: reallocate Character
    Contract->>Contract: exit user zcfSeat
    Zoe-->>Usw: Character payout
    Contract->>Contract: build new Character object (including the Inventory SW)
    Contract->>State: Add the new Character to Entries storage
Loading

Diagram for operation equip with this solution

sequenceDiagram
    box Blue User
    participant Usw as SmartWallet
    end
    participant Zoe
    box Black KREAd
    participant Contract
    participant State
    end
    participant Zoe1 as Zoe
    box Green KREAdInventory
    participant Isw as KREAdInventoryContract
    end

    Usw->>Zoe: equip offerSpec
    Zoe->>Contract: Proposal - { give CharacterIn + Item } { want CharacterOut }
    Zoe-->>Usw: UserSeat
    rect rgb(125, 0, 0)
    critical create Inventory Account
    Contract->>+Zoe1: equipItem offerSpec
    Zoe1-->>Contract: { userSeat, deposited }
    Zoe1 ->> Isw: equipItemHandler
    Isw ->> Isw: find items purse for character
    Isw ->> Isw: add new Item to the purse
    Isw-->>Zoe1: "Item successfully added"
    Zoe1-->>- Contract: offerResult<"Item successfully added">
    Contract ->> Contract: userSeat.getOfferResult()
    end
    end
    Contract -->> Zoe: "Item successfully added"
    Zoe-->>Usw: offerResult<"Item successfully added">
    Contract->>Contract: update storage with character and new items
Loading

Migration

There are users who already minted characters and items with the existing way. We must implement a mechanism to migrate those users to the latest version. This means;

  • We must let the user know their brand for KREAdITEM will change
  • We must burn the character living in inventorySeat
  • We must mint items using new KREAdITEM with the exact amounts that are equipped to the character
  • Send those newly minted items to vat-inventory by registering user's character and creating an item purse for it
  • Burn old items
  • Exit inventorySeat

We should probably perform this migration as old users try to interact with the application after the upgrade instead of trying to move all characters and items at once(for the sake of Zoe). So we end two main ways to do this;

  1. Require a new migration offer from the user if their character IS NOT registered in vat-inventory
    • Pros
      • By isolating migration process into a new offer contract code complexity will probably be reduced
      • The usual interactions(equip, unequip, ..etc) will not take longer time
    • Cons
      • Users will have to pay an additional transaction fee for one time
  2. Perform migration as a middleware operation once an action request from an old user is received(equip, unequip, ..etc).
    • Pros
      • Better UX
      • Less transaction fee for the user
    • Cons
      • More complexity in contract code
      • Harder to track block time for the usual operations(equip, unequip, ..etc) that include migration(we'll have manually look for in the usual operation includes any migration or not).

Open Questions

  • How to handle bought but unequipped items that users are holding in their own purses?
  • How to migrate characters/items that are on sale?

Considerations

  • What's the cost of vat-to-vat communication that will take place between vat-kread and vat-inventory?
    • Does SwingSet perform any serialization/unserialization when we try to communicate with another vat?
      • If yes, are better of turning vat-inventory to a virtual object that lives under vat-kread when overall KREAd performance accounted?

@anilhelvaci
Copy link
Collaborator Author

In vat-kread Approach

Prepared by: @Jorge-Lopes

Since the purpose of the next diagrams are to display the changes we intend to do to the original design, we will omit some details of the normal flow of the KREAd features that are not relevant.

Mint Character

sequenceDiagram
    box Blue User
    participant Usw as SmartWallet
    end
    participant Zoe
    box Green KREAd
    participant kreadKit
    participant State
    end

    kreadKit ->> kreadKit: itemKit = makeIssuerKit()
    Usw->>Zoe: mint offerSpec
    Zoe->>kreadKit: Proposal { give Price }
    Zoe-->>Usw: UserSeat
    kreadKit->>kreadKit: characterMint.mintPayment(baseCharacter)
    kreadKit->>kreadKit: itemMint.mintPayment(baseItems)
    create participant Purse
    kreadKit->>Purse: itemsIssuer.makeEmptyPurse()
    Purse-->>kreadKit: provide inventoryPurse
    create participant Seat
    kreadKit->>Seat: zcf.makeEmptySeatKit()
    Seat-->>kreadKit: provide userSeat and zcfSeat
    kreadKit->>Zoe: transfer Character
    kreadKit->>Seat: transfer Items
    kreadKit->>Zoe: exit userSeat
    kreadKit->>Seat: exit inventorySeat
    kreadKit->>Seat: inventorySeat.getPayouts()
    destroy Seat
    Seat-->>kreadKit: return Items payment
    kreadKit->>Purse: deposit Items into Purse
    Zoe-->>Usw: Character payout
    kreadKit->>State: remove baseCharacter from character.base
    note over kreadKit: the new Character has the attribute Items purse
    kreadKit->>State: add the new Character to character.entries
    kreadKit->>State: remove baseItems from items.base
    kreadKit->>State: add Purse to items.entries
Loading

Equip Item

On this diagram, we can observe an inventorySeat being created, the reason for that is to ensure the offer safety required for this process. More specifically, we need to ensure that the user is providing the respective Item payment that was specified in the proposal.

sequenceDiagram
    box Blue User
    participant Usw as SmartWallet
    end
    participant Zoe
    box Green KREAd
    participant kreadKit
    participant State
    end

    Usw->>Zoe: equip offerSpec
    Zoe->>kreadKit: Proposal {give CharacterIn + Item } {want CharacterOut}
    Zoe-->>Usw: UserSeat
    kreadKit->>kreadKit: validate Character
    kreadKit->>State: get characterRecord from character.entries
    State-->>kreadKit: return characterRecord
    kreadKit->>State: get respective Purse from items.entries
    create participant Purse
    State->>Purse: get Purse
    Purse-->>kreadKit: provide inventoryPurse
    create participant Seat
    kreadKit->>Seat: zcf.makeEmptySeatKit()
    Seat-->>kreadKit: provide userSeat and zcfSeat
    kreadKit->>Zoe: transfer Character
    kreadKit->>Seat: transfer Items
    kreadKit->>Zoe: exit userSeat
    kreadKit->>Seat: exit inventorySeat
    kreadKit->>Seat: inventorySeat.getPayouts()
    destroy Seat
    Seat-->>kreadKit: return Items payment
    kreadKit->>Purse: deposit Items into Purse
    Zoe-->>Usw: Character payout

Loading

Unequip Item

sequenceDiagram
    box Blue User
    participant Usw as SmartWallet
    end
    participant Zoe
    box Green KREAd
    participant kreadKit
    participant State
    end

    Usw->>Zoe: unequip offerSpec
    Zoe->>kreadKit: Proposal {give CharacterIn} {want CharacterOut + Item}
    Zoe-->>Usw: UserSeat
    kreadKit->>kreadKit: validate Character
    kreadKit->>State: get characterRecord from character.entries
    State-->>kreadKit: return characterRecord
    create participant Purse
    kreadKit->>Purse: get respective Purse
    Purse-->>kreadKit: provide inventoryPurse
    kreadKit->>Purse: Purse.withdraw(Item)
    Purse-->>kreadKit: return Item payment
    kreadKit->>Zoe: transfer Character + Item
    kreadKit->>Zoe: exit userSeat
    Zoe-->>Usw: Character + Item payout
Loading

Sell Character

sequenceDiagram
    box Blue User
    participant Usw as SmartWallet
    end
    participant Zoe
    box Green KREAd
    participant kreadKit
    participant State
    end

    Usw->>Zoe: sellCharacter offerSpec
    Zoe->>kreadKit: Proposal {give Character} {want Price}
    Zoe-->>Usw: UserSeat
    kreadKit->>kreadKit: validate Character
    create participant Purse
    kreadKit->>Purse: itemsIssuer.makeEmptyPurse()
    Note right of kreadKit: Why purse from item?
    Purse-->>kreadKit: provide characterMarketPurse
    create participant Seat
    kreadKit->>Seat: zcf.makeEmptySeatKit()
    Seat-->>kreadKit: provide userSeat and zcfSeat
    kreadKit->>Seat: transfer Character
    kreadKit->>Seat: exit inventorySeat
    kreadKit->>Seat: inventorySeat.getPayouts()
    destroy Seat
    Seat-->>kreadKit: return payment
    kreadKit->>Purse: deposit Character into Purse
    note over kreadKit: the marketEntryGuard will include the Purse
    kreadKit->>State: add new sell entry to market.characterEntries
Loading

Buy Character

sequenceDiagram
    box Blue User
    participant Usw as SmartWallet
    end
    participant Zoe
    box Green KREAd
    participant kreadKit
    participant State
    end

    Usw->>Zoe: buyCharacter offerSpec
    Zoe->>kreadKit: Proposal {give Price} {want Character}
    Zoe-->>Usw: UserSeat
    kreadKit->>kreadKit: validate Character
    kreadKit->>State: get sell record from market.characterEntries
    State-->>kreadKit: return sell record
    create participant Purse
    kreadKit->>Purse: get respective Purse
    Purse-->>kreadKit: provide characterMarketPurse
    kreadKit->>Purse: purse.withdraw(Character)
    Purse-->>kreadKit: return Character payment
    create participant Seat
    kreadKit->>Seat: get respective seller seat
    Seat-->>kreadKit: provide userSeat
    kreadKit->>Seat: transfer Price
    kreadKit->>Zoe: transfer Character
    destroy Seat
    kreadKit->>Seat: exit sellerSeat
    kreadKit->>Zoe: exit userSeat
    kreadKit->>State: remove sellRecord to market.characterEntries
Loading

Contract state

The major difference in the contact state is the inventory Purse that was included into the CharacterEntryGuard

classDiagram

baggage *-- contractState
contractState *-- charactersState
contractState *-- itemsState
contractState *-- marketState
charactersState *-- characters
charactersState *-- baseCharacters
itemsState *-- items
itemsState *-- baseItems
marketState *-- characterMarket
marketState *-- itemMarket
marketState *-- marketMetrics

baseCharacters o-- BaseCharacterGuard
characters o-- CharacterEntryGuard
items o-- ItemRecorderGuard
baseItems o-- ItemGuard
characterMarket o-- MarketEntryGuard
itemMarket o-- MarketEntryGuard
marketMetrics o-- MarketMetricsGuard
CharacterEntryGuard o-- HistoryGuard
ItemRecorderGuard o-- HistoryGuard
CharacterEntryGuard o-- CharacterGuard
ItemRecorderGuard o-- ItemGuard
MarketEntryGuard o-- CharacterGuard
MarketEntryGuard o-- ItemGuard

class characters {
    +String keyShape
    +CharacterEntryGuard valueShape
}

class baseCharacters {
    +Number keyShape
    +BaseCharacterGuard valueShape
}

class items {
    +Number keyShape
    +ItemRecorderGuard valueShape
}

class baseItems {
    +String keyShape
    +List~ItemGuard~ valueShape
}

class characterMarket {
    +String keyShape
    +MarketEntryGuard valueShape
}

class itemMarket {
    +Number keyShape
    +MarketEntryGuard valueShape
}

class marketMetrics {
    +String keyShape
    +MarketMetricsGuard valueShape
}

class BaseCharacterGuard {
    +String title
    +String description
    +String origin
    +Number level
    +String artistMetadata
    +String image
    +String characterTraits
}

class CharacterEntryGuard {
    +String name
    +CharacterGuard character
    +Purse inventory
    +RecorderKit inventoryKit
    +List~HistoryGuard~ history
}

class ItemRecorderGuard {
    +Number id
    +ItemGuard item
    +List~HistoryGuard~ history
}

class MarketEntryGuard {
    +String or Number id
    +Seat seat
    +Purse purse
    +RecorderKit recorderKit
    +Amount askingPrice
    +Amount royalty
    +Amount platformFee
    +CharacterGuard or ItemGuard asset
    +bool isFirstSale
}

class MarketMetricsGuard {
  +Number amountSold
  +Number collectionSize
  +Number averageLevel
  +Number marketplaceAverageLevel
  +Number latestSalePrice
  +Number putForSaleCount
}

class HistoryGuard {
    +String type
    +Any data
    +Timestamp timestamp
}

class CharacterGuard {
    +String title
    +String description
    +String origin
    +Number level
    +String artistMetadata
    +String image
    +String characterTraits
    +String name
    +Number keyId
    +Number id
    +timestamp date
}

class ItemGuard {
    +String name
    +String category
    +String description
    +bool functional
    +String origin
    +String image
    +String thumbnail
    +Number rarity
    +Number level
    +Number filtering
    +Number weight
    +Number sense
    +Number reserves
    +Number durability
    +String colors
    +List~String~ artistMetadata
}
Loading

@anilhelvaci anilhelvaci changed the title [WIP] Design Proposition For Performance Issue Design Proposition For Performance Issue Mar 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant