pokt-network / poktroll

The official Shannon upgrade implementation of the Pocket Network Protocol implemented using the Cosmos SDK
MIT License
15 stars 8 forks source link

[Tokenomics] Settlement safety #881

Open bryanchriswhite opened 3 days ago

bryanchriswhite commented 3 days ago

Objective

Improve saftey in claim settlement code.

Origin Document

Discussion with @red-0ne based on recent observations: Claim Settlement Safety - notion doc.

type TLMMintBurn struct {
    TLMName string
    DestinationAddr string
    Coin *cosmostypes.Coin
}

type TLMTransfer struct {
    TLMName string
    SourceAddr: string
    DestinationAddr: string 
    Coin *cosmostypes.Coin
}

type TLMProcessingResult { // Supersedes PendingClaimResult
    Claims: []*sharedtypes.Claim
    Mints: []TLMMintBurn
    Burns: []TLMMintBurn
    Transfers: []TLMTransfer
}

func (tlmPR) GetNumComputeUnits() int64
func (tlmPR) GetNumRelays() int64
func (tlmPR) GetNumClaims() int64
func (tlmPR) GetApplicationAddrs() []string
func (tlmPR) GetSupplierAddrs() []string
func (tlmPR) GetSessionEndHeight() int64
func (tlmPR) GetServices() []string // use for determinstic loops
func (tlmPR) RelaysPerServiceMap() map[string]uint64 // SHOULD NEVER be used for loops

func (tlmPR) Append(TLMProcessingResult) TLMProcessingResult
var tokenLogicModuleProcessors = []TokenLogicModule{...}

type TokenLogicModule struct {
    Name string
    Process func(
        context.Context,
        *sharedtypes.Service,
        *sessiontypes.SessionHeader,
        *apptypes.Application,
        *sharedtypes.Supplier,
        cosmostypes.Coin, // This is the "actualSettlementCoin" rather than just the "claimCoin" because of how settlement functions; see ensureClaimAmountLimits for details.
        *servicetypes.RelayMiningDifficulty,
    ) (TLMProcessingResult, error)
}

Goals

Deliverables

Non-goals / Non-deliverables

General deliverables


Creator: @bryanchriswhite Co-Owners: @red-0ne

Olshansk commented 1 day ago

@bryanchriswhite Leaving feedback on the notion doc here.

Using a map REQUIRES that all TLM functions are commutative. While it is probably a great requirement or goal, it doesn’t mean that we need to make a design choice which makes it a hard constraint, thereby introducing a chain halt pathway.

@bryanchriswhite I want us to keep this requirement.

Isolate TLM logic from keepers

👍 if possible.

Mutate all state deterministically, after TLM processing/calculations

👍 if possible

Ensure a single unexpected claim error doesn’t halt claim processing

Makes sense.

TLMName string

Can you make this an iota (enum)

type TLMProcessingResult { // Supersedes PendingClaimResult
    Claims: []*sharedtypes.Claim
    Mints: []TLMMintBurn
    Burns: []TLMMintBurn
    Transfers: []TLMTransfer
}

I'm not a fan of parallel arrays and would rather do something like this:

// Supersedes PendingClaimResult
type TLMProcessingResult struct { 
    Claim *sharedtypes.Claim
    Mints []TLMMintBurn
    Burns []TLMMintBurn
    Transfers []TLMTransfer
}

type TLMProcessingResults struct {
    TLMResults []*TLMProcessingResult
}

Other asks, ensure:

  1. Sufficient error/warning/info logging + events along the way
  2. Ensure (1) for each individual result
  3. Would love to be able to do a simple SQL query of (2) when the time is right