diff --git a/design/sales flow charts/renewals_cleanup.jpg b/design/sales flow charts/renewals_cleanup.jpg
deleted file mode 100644
index a266b2a..0000000
Binary files a/design/sales flow charts/renewals_cleanup.jpg and /dev/null differ
diff --git a/design/sales2.md b/design/sales2.md
index d3ff71f..4b3d453 100644
--- a/design/sales2.md
+++ b/design/sales2.md
@@ -2,44 +2,246 @@
The sales module is responsible for selling a node's available storage in the
[marketplace](./marketplace.md). In order to do so, it needs to create an
-Availability for the storage provider (SP) to establish under which
-conditions it is willing to enter into a sale.
+`Availability` for the storage provider (SP) to establish conditions under which
+it is willing to enter into a sale. This is done in the `SalesStorage` module.
+
+```mermaid
+---
+config:
+ theme: redux
+ look: neo
+---
+flowchart TB
+ Sales --"updates"--> SalesStorage
+ SalesStorage --"queries"--> Sales
+
+ SalesStorage --"Availability +
SaleOrder
state"--> MetadataStore
+ SalesStorage --"dedicated quota"--> SalesRepo
+
+ %% storageDelete --> salesRepoDelete
+ %% salesActiveCleanup --> storageDelete
+ %% salesPassiveCleanup --> storageDelete
+
+ %% storageStore --> salesRepoStore
+ %% salesDownload --> storageCreateSalesObj
+
+ %% salesRepoStore --> metaRefCountCrud
+ %% salesRepoDelete --> metaRefCountCrud
+
+ %% storageCreateSalesObj --> metaCreateSalesObj
+
+ %% storageArchiveSalesObj --> metaArchiveSalesObj
+
+
+
-```ascii
-------------------------------------------------------------------
-| |
-| Sales |
-| |
-| ^ | |
-| | | updates ------------------ |
-| | --------------> | | |
-| | | SalesStorage | |
-| ------------------- | | |
-| queries ------------------ |
-| ^ ^ |
-| | | |
-| | | Availability + SaleOrder |
-| dedicated quota | | state |
-| v v |
-| ---------------- ----------------- |
-| | SalesRepo | | MetadataStore | |
-| ---------------- ----------------- |
-------------------------------------------------------------------
```
+
+Full diagram
+
+```mermaid
+---
+config:
+ theme: redux
+ look: neo
+ layout: elk
+---
+flowchart TB
+ subgraph sales["Sales"]
+ salesLoad["Load"]
+ salesProcessSlot["Process slot"]
+ salesActiveCleanup["Active cleanup"]
+ salesPassiveCleanup["Corrective cleanup"]
+ salesDownload["Download"]
+ salesLoad --> salesPassiveCleanup
+ end
+ subgraph storage["Sales storage"]
+ storageDelete["Delete dataset"]
+ storageStore["Store dataset"]
+ storageCreateSalesObj["Create SalesObject"]
+ storageArchiveSalesObj["Archive SalesObject"]
+ storageCreateSalesObj --> storageStore
+ storageDelete --> storageArchiveSalesObj
+ end
+
+subgraph metaData["MetadataStore"]
+ metaCreateSalesObj["Create SalesObject"]
+ metaArchiveSalesObj["Archive SalesObject"]
+ metaRefCountCrud["Ref count CRUD"]
+ end
+ subgraph fsm["Sales state machine"]
+ preparing["Preparing"]
+ reserving["Reserving"]
+ download["Download"]
+ initialProving["Gen proof"]
+ filling["Filling"]
+ filled["Filled"]
+ proving["Proving"]
+ payout["Payout"]
+ finished["Finished"]
+ errored["Errored"]
+ cancelled["Cancelled"]
+ ignored["Ignored"]
+ failed["Failed"]
+ preparing --> reserving
+ preparing --> ignored
+ preparing --> errored
+ reserving --> download
+ reserving --> ignored
+ reserving --> errored
+ download --> initialProving
+ download --> errored
+ initialProving --> filling
+ initialProving --> errored
+ filling --> filled
+ filling --> ignored
+ filling --> errored
+ filled --> proving
+ filled --> errored
+ proving --> payout
+ proving --> errored
+ payout --> finished
+ payout --> errored
+ finished --> errored
+ failed --> errored
+ end
+ subgraph contracts["Marketplace contracts"]
+ contractsFreeSlot["Free slot"]
+ end
+ subgraph market["Market abstraction"]
+ marketFreeSlot["Free slot"]
+ end
+ subgraph salesRepo["SalesRepo"]
+ salesRepoStore["Store dataset"]
+ salesRepoDelete["Delete dataset"]
+ end
+ subgraph salesAgent["SalesAgent"]
+ agentCleanUp["Active cleanup"]
+ agentDownload["Download"]
+ end
+
+
+ storageDelete --> salesRepoDelete
+ salesActiveCleanup --> storageDelete
+ salesPassiveCleanup --> storageDelete
+
+ storageStore --> salesRepoStore
+ download --> agentDownload
+ agentCleanUp --> salesActiveCleanup
+ agentDownload --> salesDownload
+ salesDownload --> storageCreateSalesObj
+
+ salesRepoStore --> metaRefCountCrud
+ salesRepoDelete --> metaRefCountCrud
+
+ storageCreateSalesObj --> metaCreateSalesObj
+
+ storageArchiveSalesObj --> metaArchiveSalesObj
+
+ salesProcessSlot --> salesAgent
+ %% salesAgent <--> fsm
+
+ cancelled --> agentCleanUp
+ failed --> agentCleanUp
+ finished --> agentCleanUp
+ errored --> agentCleanUp
+
+ marketFreeSlot --> contractsFreeSlot
+
+ payout --> marketFreeSlot
+ failed --> marketFreeSlot
+ cancelled --> marketFreeSlot
+
+```
+
+## `SalesStorage` module
+
The `SalesStorage` module manages the SP's availability and snapshots of past
-and present sales or `SalesOrders`, both of which are persisted in the `MetadataStore`. SPs can add
-and update their availability, which is managed through the `SalesStorage`
-module. As a `SalesOrder` traverses the sales state machine, it is created and
-updated1 through the `SalesStorage` module. Queries for availability
-and `SalesOrders` will also occur in the `SalesStorage` module. Datasets that
-are downloaded and deleted as part of the sales process will be handled in the
-`SalesRepo` module.
+and present sales or `SalesOrders`, both of which are persisted in the
+`MetadataStore`. SPs can add and update their availability, which is managed
+through the `SalesStorage` module. As a `SalesOrder` traverses the sales state
+machine, it is created and updated[^updates_trackstate] through the
+`SalesStorage` module. Queries for availability and `SalesOrders` will also
+occur in the `SalesStorage` module. Datasets that are downloaded and deleted as
+part of the sales process will be managed in the `SalesRepo` module.
-1 Updates are only needed to support [tracking the latest state in
- the `SalesOrder`](#tracking-latest-state-machine-state).
+[^updates_trackstate]: Updates are only needed to support [tracking the latest
+ state in the `SalesOrder`](#tracking-latest-state-machine-state).
-## Query support
+### Availability
+
+The SP's availability determines which sales it is willing to attempt to enter
+into. In other words, it represents *future sales* that an SP is willing to take
+on[^designrules]. It consists of parameters that will be matched to incoming storage
+requests via the slot queue.
+
+[^designrules]: See [design rules](#design-rules) for a further explanation.
+
+| Property | Description |
+|----------------------------|---------------------------------------------------------------------------------|
+| `duration` | Maximum duration of a storage request the SP is willing to host new slots for. |
+| `minPricePerBytePerSecond` | Minimum price per byte per second that the SP is willing to host new slots for. |
+
+The availability of a SP consists of the maximum duration and the minimum price
+per byte per second to sell storage for.
+
+#### `Availability` lifecycle
+
+A user can add, update, or delete an `Availability` at any time. The
+`Availability` will be stored in the MetadataStore. Only one `Availability` can
+be created and once created, it will exist permanently in the MetadataStore
+until it is deleted. The properties of a created `Availability` can be updated
+at any time.
+
+Because availability(ies) represents *future* sales (and not active sales), and
+because fields of the matching `Availability` are persisted in a `SalesOrder`,
+availabilities are not tied to active sales and can be manipulated at any time.
+
+### `SalesOrder` object
+
+The `SalesOrder` object represents a slot that a SP attempted to, or eventually
+did host. `SalesOrders` are created only when there is an attempt to download
+the slot data, meaning there was a successful availability match and a
+successful slot reservation. The purpose of `SalesOrders` is to keep track of
+sales for dataset cleanup operations, and to provide historical information for
+the SP.
+
+Cleanup routines will be able to query `SalesOrders` and compare them to those
+that are filled on chain, to ensure that datasets that are no longer being
+hosted do not remain on disk.
+
+In addition, SPs will likely want to list slots that have been hosted in the
+past. After a `StorageRequest` is completed, it is removed from the contract's
+`mySlots` storage, with the `StorageRequest` information queryable only by
+random access with the `RequestId`. Therefore, at a minimum, the `RequestId` and
+slot index of the slot that was hosted would need to be persisted by the SP for
+the SP to keep track of slots that were hosted.
+
+| Property | Description |
+|-------------|------------------------------------------------------------------------------------------------------------------------------------------------------|
+| `requestId` | `RequestId` of the `StorageRequest`. Can be used to retrieve storage request details. |
+| `slotIndex` | Slot index of the slot being hosted. |
+| `treeCid` | CID of the manifest dataset, used for `SalesRepo` interaction. TODO: `manifestCid` may be sufficient. Depends on the final design of the `RepoStore` |
+
+#### `SalesOrder` lifecycle
+
+At the point a SP reaches the `SaleDownload` state, a `SalesOrder` is created
+and it will live permanently in the MetadataStore. `SalesOrder` objects cannot
+be deleted as they represent historical sales of the SP.
+
+When the `SalesOrder` object is first created, its key will be created in the
+`/active` namespace. After data for the `SalesOrder` has been deleted (if there
+is any) in a clean up procedure, the key will be moved from the `/active`
+namespace to the `/archive` namespace. These key namespace manipulations
+facilitate future lookups in active/corrective clean up operations.
+
+If there's support for [tracking the latest state in the
+`SalesOrder`](#tracking-latest-state-machine-state), `SalesOrder.state`
+will be modified as the sale progresses through each state of the Sales state
+machine.
+
+### Query support
The `SalesStorage` module will need to support querying the availability and sales
data so the caller can understand if a sale can be serviced and to support clean
@@ -51,16 +253,16 @@ up routines. The following queries will need to be supported:
unnecessary resource
consumption](#concurrent-workers-prevent-unnecessary-resource-consumption),
by additionally querying the slot size of `SalesOrders` that are in or past
- the Downloading state.
+ the Downloading state (`/active` `SalesOrders`).
2. Clean up routines will need to know the "active sales", or any `SalesOrders`
in the `/active` key namespace (those that have not been archived) through
the state machine or clean up routines.
-3. Servicing a new slot will require sufficient "total collateral", which is the
- remaining balance in the funding account. In the future, this can be
- optimised to [prevent unnecessary resource
+3. Servicing a new slot will require sufficient [total
+ collateral](#total-collateral), which is the remaining balance in the funding
+ account. In the future, this can be optimised to [prevent unnecessary
+ resource
consumption](#concurrent-workers-prevent-unnecessary-resource-consumption),
- by additionally querying the collateral of `SalesOrders` that are in or past
- the Downloading state.
+ by additionally querying the collateral of `/active` `SalesOrders`.
## `SalesRepo` module
@@ -96,48 +298,7 @@ direction TB
classDef focusClass fill:#c4fdff,stroke:#333,stroke-width:4px,color:black
```
-## Availability
-
-The SP's availability determines which sales it is willing to attempt to enter
-into. In other words, it represents *future sales* that an SP is willing to take
-on. It consists of parameters that will be matched to incoming storage
-requests via the slot queue.
-
-
-| Property | Description |
-|----------------------------|------------------------------------------------------------------------------------------------------------------------------------------------------|
-| `id` | ID of the Availability. Note: this is only needed if there is support for [multiple availabilities](#multiple-availabilities). |
-| `duration` | Maximum duration of a storage request the SP is willing to host new slots for. |
-| `minPricePerBytePerSecond` | Minimum price per byte per second that the SP is willing to host new slots for. |
-| `enabled` | If set to false, the availability will not accept new slots. Updates to this value will not impact any existing slots that are already being hosted. |
-| `until` | Specifies the latest timestamp after which the availability will no longer host any slots. If set to 0, there will be no restrictions. |
-
-The availability of a SP consists of the maximum duration and the minimum price
-per byte per second to sell storage for.
-
-### Funding account vs profit account
-
-SPs should control two accounts: a funding account, and a profits account. The
-funds in the funding account represent the total collateral that a SP is willing
-to risk in all of its sales combined. This account will need to have some funds
-in it before slots can be hosted, assuming the storage request requires
-collateral. If a SP has been partially or wholly slashed in one of their sales,
-they may wish to top up this account to ensure there is sufficient collateral
-for future sales.
-
-The profits account is the account for which proceeds from sales are paid into.
-To minimise risk, this account should be stored in cold storage.
-
-It is recommended that the profit account is a separate account from the funding
-account so that profits are not placed at risk by being used as collateral. If a
-SP specifies the same account for funding and profits, and the SP is (partially
-or wholly) slashed, future collateral deposits may use their profits from
-previous sales.
-
-Note: having a separate profit account relies on the ability of the Vault
-contract to support multiple accounts.
-
-### Total collateral
+## Total collateral
The concept of "total collateral" means the total collateral the SP is willing
to risk at any one point in time. In other words, it is willing to risk "total
@@ -149,87 +310,134 @@ slots.
From the marketplace perspective, slots cannot be filled if there is an
insufficient balance in the funding account.
-### `Availability` lifecycle
+### Funding account vs profit account
-A user can add, update, or delete an `Availability` at any time. The
-`Availability` will be stored in the MetadataStore. Only one `Availability` can
-be created and once created, it will exist permanently in the MetadataStore
-until it is deleted. The properties of a created `Availability` can be updated
-at any time.
+SPs should control two accounts to safely host slots: a funding account, and a
+profits account.
-Because availability(ies) represents *future* sales (and not active sales), and
-because fields of the matching `Availability` are persisted in a `SalesOrder`,
-availabilities are not tied to active sales and can be manipulated at any time.
+The funds in the funding account represent the total collateral
+that a SP is willing to risk in all of its sales combined. This account will
+need to have some funds in it before slots can be hosted, assuming the storage
+request requires collateral. If a SP has been partially or wholly slashed in one
+of their sales, they may wish to top up this account to ensure there is
+sufficient collateral for future sales.
-## `SalesOrder` object
+The profits account is the account for which proceeds from sales are paid into.
+To minimise risk, this account should be stored in cold storage.
-The `SalesOrder` object represents a snapshot of the sale parameters at the time
-a slot is processed in the slot queue. It can be thought of as the market
-conditions at the time of sale. It includes fields of the storage request, slot,
-and the matching availability fields.
+While a SP could technically specify the same address for both accounts, it is
+recommended that the profit account is a separate account from the funding
+account so that profits are not placed at risk by being used as collateral. If a
+SP specifies the same account for funding and profits, and the SP is (partially
+or wholly) slashed, future collateral deposits may use their profits from
+previous sales.
-| Property | Description |
-|----------------------------|---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
-| `requestId` | RequestId of the StorageRequest. Can be used to retrieve storage request details. |
-| `slotIndex` | lot index of the slot being processed. |
-| `duration` | `duration` from the matched Availability. |
-| `minPricePerBytePerSecond` | `minPricePerByte` from the matched Availabilty. |
-| `state` | Latest state in the sales state machine that was reached. Note: this is only needed when there support for [tracking the latest state in the `SalesOrder`](#tracking-latest-state-machine-state). |
-
-### `SalesOrder` lifecycle
-
-At the point a SP reaches the `SaleDownload` state, a `SalesOrder` is created
-and it will live permanently in the MetadataStore. `SalesOrder` objects cannot
-be deleted as they represent historical sales of the SP.
-
-When the `SalesOrder` object is first created, its key will be created in the
-`/active` namespace. After data for the `SalesOrder` has been deleted (if there
-is any) in a clean up procedure, the key will be moved from the `/active`
-namespace to the `/archive` namespace. These key namespace manipulations
-facilitate future lookups in active/passive clean up operations.
-
-If there's support for [tracking the latest state in the
-`SalesOrder`](#tracking-latest-state-machine-state), `SalesOrder.state`
-will be modified as the sale progresses through each state of the Sales state
-machine.
+Note: having a separate profit account relies on the ability of the Vault
+contract to support multiple accounts.
## Cleanup routines
-The responsibility of the cleanup routine is to ensure that any data associated
-with a Sale is deleted from the `SalesRepo`. Once the data has been deleted, the
-`SalesOrder` will reflect that it has been cleaned up by being archived.
+The responsibility of the cleanup routine is to ensure that any data that is no
+longer part of an active sales is deleted from the `SalesRepo`. Once the data
+has been deleted, the `SalesOrder` will reflect that it has been cleaned up by
+being archived.
There are two types of cleanup routines that a SP node will take part in: active
-and passive. Active cleanup routines are run as part of a state in the Sales
-state machine. Passive cleanup routines are continuously run at a specified time
-interval. Both perform a similar task, however the active cleanups operate on a
-single `SalesOrder`, while passive cleanups operate over a set of `SalesOrders`
-and have additional conditions for cleanup.
+and corrective. Active cleanup routines are run as part of a final state in the
+Sales state machine. Corrective cleanup routines are continuously run at a
+specified time interval with the goal of cleaning up any datasets that may not
+have been cleaned up by active cleanup due to a node restart. Both perform a
+similar task, however the active cleanups operate on a single `SalesOrder`,
+while corrective cleanups operate over a set of `SalesOrders` and have additional
+conditions for cleanup.
+
+```mermaid
+---
+config:
+ theme: redux
+---
+flowchart TB
+ subgraph Sales
+ Load[Load]
+ ProcessSlot[Process Slot]
+ ActiveCleanup[Active cleanup]
+ PassiveCleanup[Corrective cleanup]
+ Load --> PassiveCleanup
+ end
+ FSM[Sales state machine]
+ ProcessSlot --> SalesAgent
+ SalesAgent <--> FSM
+ SalesAgent --> ActiveCleanup
+```
### Active cleanup
The active cleanup routine is typically run as part of the a final state in the
-Sales state machine, ie `SaleFinished`. In this routine, active sales will be
+Sales state machine, eg `SaleFinished`. In this routine, active sales will be
retrieved from the Marketplace contract via `mySlots`. If the slot id associated
with the sale is not in the set of active sales, any data associated with the
-slot will be deleted. Then, the `SalesOrder` will be archived, by moving its key
-to the `/archive` namespace.
+slot will be deleted. Finally, the `SalesOrder` will be archived, by moving its
+key to the `/archive` namespace.
-### Passive cleanup
+```mermaid
+---
+config:
+ theme: redux
+---
+flowchart TB
+ CleanUp(["Active CleanUp"]) -- Current SaleOrder -->
-At regular time intervals, active sales will be retrieved from the Marketplace
+ Delete["Delete dataset"] -->
+ Archive["Archive SalesOrder"] -->
+ Done
+
+ CleanUp:::start
+ classDef start fill:#000000, color:#FFFFFF
+```
+
+Note that in the case of [renewals](#renewals-prevent-dataset-deletion) or in
+any case that the same dataset as the one being deleted is simultaneously being
+downloaded or processed, the dataset ref count is enough to prevent deletion of
+the dataset.
+
+### Corrective cleanup
+
+Node shutdowns can sometimes come in the middle of a non-atomic operation such
+as persisting a `SalesOrder` and downloading a dataset. In this case, corrective
+cleanup is needed to ensure that datasets that not being actively hosted are
+removed from the node.
+
+On node startup, active sales will be retrieved from the Marketplace
contract via `mySlots`. Then, all `SalesOrders` in the `/active` namespace will
-be queried. Any `SalesOrders` with a slot id not in the set of active sales and
-with a `StorageRequest` state that is "completed" (failed, cancelled, finished),
-will have the data associated with the slot deleted, if there is any. Then, the
+be queried. Any `SalesOrders` with a slot id not in the set of active sales
+(`mySlots`) will have the data associated with the slot deleted, if there is
+any. Any `SalesOrders` associated with `StorageRequests` that are in the `New`
+or `Fulfilled` state should be ignored in this process, otherwise datasets of
+sales that are in the process of being processed may be impacted. Finally, the
`SalesOrder` will be archived by moving its key to the `/archive` namespace.
-`SalesOrders` with a `StorageRequest` state that is not yet completed should not
-have their data deleted, as the SP may be in the process of starting to host a
-slot, with the sale in an early state of the Sales state machine.
-### Node startup
+```mermaid
+---
+config:
+ theme: redux
+---
+flowchart TB
+ CleanUp(["Corrective CleanUp"]) --/active SalesOrders -->
+ %% TimeInterval[Every time interval] -- /active SalesOrders-->
+ QueryResults(("SalesOrders not
actively filled
on chain"))
-On node startup, the passive cleanup routine should be run.
+ CleanUp -- Active sales on chain -->
+ QueryResults --"SalesOrder"-->
+ IsActiveRequest{Is request active?} --"No"-->
+ Delete["Delete dataset"] -->
+ Archive["Archive SalesOrder"] -->
+ QueryResults
+
+ IsActiveRequest --"Yes"--> QueryResults
+
+ CleanUp:::start
+ classDef start fill:#000000, color:#FFFFFF
+```
## Sale flow
@@ -247,6 +455,26 @@ availability ID stored in the `SalesOrder` object.
Note that the total collateral across all availabilities that a SP is
willing to risk remains as the balance of funds in the funding account.
+Support for multiple availabilities will need to add new properties to the
+`Availability` object:
+
+| Property | Description |
+|-----------|----------------------------------------------------------------------------------------------------------------------------------|
+| `id` | ID of the Availability. |
+| `enabled` | If set to false, the SP will not use this Availability to host new slots. |
+| `until` | Only accept slots whose request ends before `until`. If set to 0, there will be no restrictions. Useful for planned maintenance. |
+
+The `id` property will be used to form the key for storage in the
+`MetadataStore`. This value will be used to uniquely identify the `Availability`
+for CRUD and REST API operations.
+
+The `enabled` property will allow an Availability to be disabled so that other,
+enabled Availabilities can still be used to match new sales.
+
+The `until` property matches Availabilities with requests that end before
+`until`. This is useful if there is upcoming planning maintenance, such as a
+disk swap.
+
### Concurrent workers support
Concurrent workers allow a SP to reserve, download, generate an initial proof
@@ -282,6 +510,12 @@ process, ie downloading, proof generating, or filling. To
facilitate this, `SalesOrder.state` would need to track the latest state of the
sale in the sales state machine.
+The following property would need to be added to the `SalesOrder` object:
+
+| Property | Description |
+|----------|-----------------------------------------------------------|
+| `state` | Latest state in the sales state machine that was reached. |
+
Tracking the latest state opens up the possibility for further optimisations,
see below.
@@ -291,36 +525,75 @@ Depends on: Tracking latest state machine state
Depends on: Concurrent workers
To prevent unnecessary reserving, downloading, and proof generation when there
-are concurrent workers, when checking to ensure there's enough collateral
-available, instead of only checking the funding account's current balance, also
-check collateral that will be used to fill slots in `SalesOrders` that are
-`/active`. Without this check, SPs may reserve, download, and generate a proof
-for a sale that would ultimately result in not having enough collateral. For
-example, if funding account balance is 100, and the SP is currently downloading
-two sales with 100 collateral each, then that would mean that the download that
-finishes last will ultimately be wasted as the SP would not have enough
-collateral to fill both slots.
+are concurrent workers, collateral and storage quota checks can be optimised.
+Instead of only checking the funding account's current balance for collateral,
+and only checking the remaining storage quota, also check collateral and slot
+size for sales that are downloading and proof generating. This can be done by
+querying `/active` `SalesOrders` that are not filled on chain (in `mySlots`).
+Without this check, SPs may reserve, download, and generate a proof for a sale
+that would ultimately result in not having enough collateral. For example, if
+funding account balance is 100, and the SP is currently downloading two sales
+with 100 collateral each, then that would mean that the download that finishes
+last will ultimately be wasted as the SP would not have enough collateral to
+fill both slots.
+
+To ensure the [design rules](#objects-must-not-perform-accounting) are adhered
+to, we should avoid using only `/active` `SalesOrders` to determine total
+collateral and slot size, as opposed to using only those not filled on chain (in
+`mySlots`). This is because there are many circumstances that may lead to
+incorrectly accounted `SalesOrders` and that would affect the SPs ability to
+fill slots. In the language of the design rules, `SalesOrders` state for filled
+slots is not the "source of truth" and therefore should not be relied upon.
+
+One caveat, however, is that slot matching must wait for cleanup routines to
+complete during startup. This is because on startup, there may be `/active`
+`SalesOrders` not in `/mySlots` that will get deleted/archived during startup
+cleanup and should not count towards total collateral or slot size.
+
+The following properties would need to be added to the `SalesOrder` object in
+order to prevent unnecessary resource consumption:
+
+| Property | Description |
+|--------------|-------------------------------------------------------------------------------------------|
+| `slotSize` | `slotSize` from the `StorageAsk`. |
+| `collateral` | Collateral consumed for the request, calculated using `collateralPerByte` and `slotSize`. |
### Renewals: prevent dataset deletion
During renewals, there could potentially be a new sale for the same dataset that
is already in an active sale. The `SlotId` (and `RequestId`) will differ,
-however the CID will be the same. Renewals should occur well before the initial
-sale finishes. However, if the new sale is close in time to the completion of
-the first sale, then as the dataset for the first sale is being cleaned up,
-it may delete the dataset that is needed by the new sale. The new sale may have
-been in the process of being downloaded, or having proofs generated.
+however the manifest CID and potentially the slot index will be the same,
+resulting in the same dataset being hosted. Renewals should occur well before
+the initial sale finishes. However, if the new sale is close in time to the
+completion of the first sale, then as the dataset for the first sale is being
+cleaned up, it may delete the dataset that is needed by the new sale. The new
+sale may have been in the process of being downloaded, or having proofs
+generated.
-This can be prevented by having an in-memory ref count of datasets. When a
+This can be prevented by having a persisted ref count of datasets. When a
dataset is stored, the ref count of the dataset (`hash(treeCid, slotIndex)`) is
-incremented. When the dataset is deleted, the ref count is decremented. Only
-when the ref count is 0 is the dataset actually deleted in the `RepoStore`. The
-ref count does not require persistence because on startup, hosted slots will not
-be deleted.
+incremented. TODO: `manifestCid` may be used instead depending on find
+`RepoStore` design. When the dataset is deleted, the ref count is decremented.
+Only when the ref count is 0 is the dataset actually deleted in the underlying
+`RepoStore`.
-Ref count handling can be managed in `SalesRepo` module. This module is
-responsible for interacting with the underlying `RepoStore`, and managing the
-internal ref count. It will expose functions for storing and deleting datasets.
+On startup, state machine states are restored for active slots, effectively
+skipping previous states that incremented the ref count. Therefore, the ref
+count must be persisted so that the ref count reflects the full and partial
+datasets on disk. To illustrate, let's use the case where the node hosted a slot
+and it went down in the process of renewing the same slot but had not filled it
+yet. In this case, the ref count for a dataset would be 2. Upon node restart,
+two things will happen: the corrective cleanup routine will try to delete the
+renewal dataset that was being processed and the filled slot would get restored
+to its previous point in the state machine, where it will attempt to delete the
+dataset when it's finished. If the ref count had not been persisted, it would be
+0, and the corrective cleanup would delete the dataset that is currently filled,
+which could cause the SP to be slashed.
+
+Ref count handling can be managed in `SalesRepo` module, and it can be persisted
+in the `MetadataStore`. This module is responsible for interacting with the
+underlying `RepoStore`, and managing the internal ref count. It will expose
+functions for storing and deleting datasets.
Note that any calls to ref count should be locked, as they may be read and
updated concurrently.
@@ -357,14 +630,82 @@ direction TB
#### Alternative idea
-Preventing deletion of datasets that are being downloaded or proof generating
-can also be achieved by first checking if the slot id exists in `/mySlots` (at this stage
-the initial sale should no longer be in `/mySlots`). If it does not, then check
-if there are more than one `/active` (reached downloading) `SalesOrders` with
-the same `hash(treeCid, slotIndex)` that exist. If there are not, delete the
-dataset. Finally, archive the `SalesOrder`.
+Preventing deletion of datasets that are downloading or generating proofs can
+also be achieved by checking if there are more than one `/active` (reached
+downloading) `SalesOrders` with the same `hash(treeCid, slotIndex)` that exist.
+If there are not, delete the dataset. Finally, archive the `SalesOrder`.
-![Cleanup handling renewals]()
+```mermaid
+---
+config:
+ theme: redux
+---
+flowchart TB
+ CleanUp(["Clean Up"]) --"SalesOrder"-->
+ ExistsMultiple{"Exists more than
one /active SalesOrder
with slot id?"} -- "No" -->
+ Delete["Delete dataset (if
one exists)"] -->
+ Archive["Archive SalesOrder"]
+
+ ExistsMultiple -- "Yes" -->
+ DoNotDelete["Do not delete dataset"]
+
+ CleanUp:::start
+ classDef start fill:#000000, color:#FFFFFF
+```
## Purchasing
+## Design rules
+
+Based on past implementations of the sales and purchasing modules, a couple of
+rules have been created that the design in this document should not deviate
+from.
+
+### Objects MUST NOT perform accounting
+
+The first, and most important, rule is that there should never be any accounting
+operations where there is a "source of truth", particularly `Availabilities`.
+Accounting incorporates actions done in other modules of the Codex node (eg
+storage) or in the contracts (eg collateral), and then reflecting those changed
+values back into the `Availability`. Accounting is not a good idea for several
+reasons.
+
+Firstly, there are a large number of logic branches that are
+created where accounting updates need to occur, creating a significant
+amount of complexity in the codebase. This makes the code difficult to reason
+about and therefore difficult to ensure that all possible scenarios are covered.
+In other words, this creates many edge cases, associated bugs, and a larger
+testing burden. This is further exacerbated with concurrent workers.
+
+Secondly, accounting updates are not atomic with their underlying operation.
+This opens up the potential for unrecoverable exceptions or a `SIGTERM` after
+the underlying operation but before the accounting update, leaving the object,
+eg `Availability`, out of sync.
+
+Finally, values that would require accounting should instead be sourced from
+their underlying modules, as the "source of truth". For example, "available
+collateral" can be sourced from the balance of the funding wallet, and
+"available storage" can be sourced from the remaining quota of the `SalesRepo`.
+
+Examples of the "no accounting" rule:
+
+1. No slot size accounting
+2. No collateral accounting
+3. No reservations accounting (reservations were removed anyway due to a design
+ change in the RepoStore)
+
+An example of how this rule does not apply is with the `SalesRepo` module. The
+`SalesRepo` module stores a `refCount`, but only because that information does
+not exist in the underlying `RepoStore` as the "source of truth".
+
+### `Availabilities` MUST NOT represent past or active sales
+
+`Availabilities` MUST represent future sales only. A SP's availability defines
+the conditions of sales they are willing to enter into. After entering
+into a sale, a SP can update its availability, and therefore change the
+conditions to be met for future sales. If the `Availability` was linked to the
+past or future sales, updating the availability would lose information
+pertaining to those sales.
+
+In the design, this rule has been followed by copying information from the
+matched `Availability` into a `SalesOrder`.