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

[Tokenomics] Harden settlement (part 1: TLM isolation) #889

Open
wants to merge 22 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
656994e
wip: refactor:
bryanchriswhite Oct 22, 2024
aab77f5
chore: configurable tlm processors and dao reward address global vari…
bryanchriswhite Oct 23, 2024
f1339ef
refactor: rename session testkeeper WithSharedModuleParams helper
bryanchriswhite Oct 23, 2024
bec435f
refactor: TokenomicsModuleKeepers helpers to support injecting TLM pr…
bryanchriswhite Oct 23, 2024
2f33a71
refactor: eliminate non-deterministic iteration from claim settlement
bryanchriswhite Oct 23, 2024
c39c096
test: ALL TLMs are commutative
bryanchriswhite Oct 23, 2024
86e28cb
refactor: val & cons addr helpers
bryanchriswhite Oct 24, 2024
24ceb12
chore: don't schedule transfers for 0 amounts
bryanchriswhite Oct 24, 2024
a111ecc
test: improve permute test
bryanchriswhite Oct 24, 2024
c04a919
wip: chore: add comments
bryanchriswhite Oct 24, 2024
29a75b2
test: split up tlm processor test suite
bryanchriswhite Oct 24, 2024
618da66
fix: test typo
bryanchriswhite Oct 24, 2024
760ac25
fix: linter errors
bryanchriswhite Oct 25, 2024
53b668d
chore: resolve logging and godoc TODOS
bryanchriswhite Oct 25, 2024
d279222
refactor: rename TokenLogicModuleProcessor to TokenLogicModule & Toke…
bryanchriswhite Oct 25, 2024
6b96a1e
chore: resolve outstanding TODOs
bryanchriswhite Oct 25, 2024
67777cc
chore: self-review improvements & fix linter errors
bryanchriswhite Oct 25, 2024
bcc4f77
chore: self-review improvements
bryanchriswhite Oct 25, 2024
aecfe41
wip: fix: failing tests
bryanchriswhite Oct 25, 2024
cbb81c4
fix: failing test and simplify
bryanchriswhite Oct 25, 2024
3187418
Merge remote-tracking branch 'pokt/main' into issues/881/refactor/set…
bryanchriswhite Nov 4, 2024
6a02d5e
chore: add godoc comments
bryanchriswhite Nov 4, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .tool-versions
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,4 @@
# and `asdf current` to switch to the versions of dependencies listed below
golang 1.23.0
go 1.23.0
python 3.11.10
2 changes: 1 addition & 1 deletion api/poktroll/application/types.pulsar.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion api/poktroll/tokenomics/event.pulsar.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 7 additions & 0 deletions config.yml
Original file line number Diff line number Diff line change
@@ -1,6 +1,13 @@
version: 1
build:
main: cmd/poktrolld
ldflags:
# The dao reward address SHOULD match that of the "pnf" below (i.e. `make poktrolld_addr ACC_NAME=pnf`).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this a SHOULD and not a MUST?

# This is the address that will receive the dao/foundation rewards during claim settlement (global mint TLM).
# DEV_NOTE: This is a temporary solution which allows us to configure the dao reward address between networks
# (e.g. localnet, testnet, etc.)
# TODO_TECHDEBT: Make this a tokenomics module parameter.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is pretty important and I'd rather do it sooner rather than later.

Would prefer that we add a PR (before this) and avoid this TECHDEBT altogether. Wouldn't that be easier/faster/cleaner?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bryanchriswhite

  1. How long would you say it takes you to add a new param right now?
  2. Do you think there's a way (a need?) to streamline/automate [1] in some way?

[1] https://dev.poktroll.com/develop/developer_guide/adding_params?_highlight=para

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use TODO_TECHDEBT only when it's like a post mainnet and doesn't have a timeline sort of thing.

- "-X github.com/pokt-network/poktroll/x/tokenomics/types.DaoRewardAddress=pokt1eeeksh2tvkh7wzmfrljnhw4wrhs55lcuvmekkw"
accounts:
- name: faucet
mnemonic: "baby advance work soap slow exclude blur humble lucky rough teach wide chuckle captain rack laundry butter main very cannon donate armor dress follow"
Expand Down
15 changes: 15 additions & 0 deletions pkg/pokterrors/split.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package pokterrors

import "errors"

// Split returns a slice of errors which were joined with errors.Join.
// The returned errs slice will also include elements for any nested wrapped
// errors contained by the joined errors.
func Split(joinedErrs error) (errs []error) {
if joinedErrs != nil {
for err := errors.Unwrap(joinedErrs); err != nil; err = errors.Unwrap(err) {
errs = append(errs, err)
}
}
return errs
}
6 changes: 3 additions & 3 deletions tests/integration/application/min_stake_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import (
sessiontypes "github.com/pokt-network/poktroll/x/session/types"
sharedtypes "github.com/pokt-network/poktroll/x/shared/types"
suppliertypes "github.com/pokt-network/poktroll/x/supplier/types"
tokenomicskeeper "github.com/pokt-network/poktroll/x/tokenomics/keeper"
tlm "github.com/pokt-network/poktroll/x/tokenomics/token_logic_module"
)

type applicationMinStakeTestSuite struct {
Expand Down Expand Up @@ -235,7 +235,7 @@ func (s *applicationMinStakeTestSuite) getExpectedApp(claim *prooftypes.Claim) *
expectedBurnCoin, err := claim.GetClaimeduPOKT(sharedParams, relayMiningDifficulty)
require.NoError(s.T(), err)

globalInflationAmt, _ := tokenomicskeeper.CalculateGlobalPerClaimMintInflationFromSettlementAmount(expectedBurnCoin)
globalInflationAmt, _ := tlm.CalculateGlobalPerClaimMintInflationFromSettlementAmount(expectedBurnCoin)
expectedEndStake := s.appStake.Sub(expectedBurnCoin).Sub(globalInflationAmt)
return &apptypes.Application{
Address: s.appBech32,
Expand Down Expand Up @@ -307,7 +307,7 @@ func (s *applicationMinStakeTestSuite) assertAppStakeIsReturnedToBalance() {

expectedAppBurn := int64(s.numRelays * s.numComputeUnitsPerRelay * sharedtypes.DefaultComputeUnitsToTokensMultiplier)
expectedAppBurnCoin := cosmostypes.NewInt64Coin(volatile.DenomuPOKT, expectedAppBurn)
globalInflationCoin, _ := tokenomicskeeper.CalculateGlobalPerClaimMintInflationFromSettlementAmount(expectedAppBurnCoin)
globalInflationCoin, _ := tlm.CalculateGlobalPerClaimMintInflationFromSettlementAmount(expectedAppBurnCoin)
expectedAppBalance := s.appStake.Sub(expectedAppBurnCoin).Sub(globalInflationCoin)

appBalance := s.getAppBalance()
Expand Down
4 changes: 2 additions & 2 deletions tests/integration/tokenomics/relay_mining_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -147,8 +147,8 @@ func TestComputeNewDifficultyHash_RewardsReflectWorkCompleted(t *testing.T) {
// Calling SettlePendingClaims calls ProcessTokenLogicModules behind the scenes
settledResult, expiredResult, err := keepers.Keeper.SettlePendingClaims(sdkCtx)
require.NoError(t, err)
require.Equal(t, 1, int(settledResult.NumClaims))
require.Equal(t, 0, int(expiredResult.NumClaims))
require.Equal(t, 1, int(settledResult.GetNumClaims()))
require.Equal(t, 0, int(expiredResult.GetNumClaims()))

// Update the relay mining difficulty
_, err = keepers.Keeper.UpdateRelayMiningDifficulty(sdkCtx, map[string]uint64{service.Id: claimNumRelays})
Expand Down
197 changes: 197 additions & 0 deletions tests/integration/tokenomics/token_logic_modules/commutative_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,197 @@
package token_logic_modules

import (
"bytes"
"fmt"
"strings"
"testing"

"cosmossdk.io/log"
"github.com/cosmos/cosmos-sdk/types"
banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"
"github.com/stretchr/testify/require"

"github.com/pokt-network/poktroll/app/volatile"
"github.com/pokt-network/poktroll/testutil/keeper"
"github.com/pokt-network/poktroll/testutil/sample"
prooftypes "github.com/pokt-network/poktroll/x/proof/types"
sharedtypes "github.com/pokt-network/poktroll/x/shared/types"
tlm "github.com/pokt-network/poktroll/x/tokenomics/token_logic_module"
)

// zerouPOKT is a coin with the uPOKT denom and zero amount, intended for use in test assertions.
var zerouPOKT = types.NewInt64Coin(volatile.DenomuPOKT, 0)

// TestTLMProcessorTestSuite asserts that the network state that results from running
// each permutation of the default TLM processors is identical (demonstrating
// commutativity). It does this in the following steps:
//
// 1. Construct a TokenomicsModuleKeepers instance for each TLM processor permutation.
// 2. Create valid claims (which require no proofs).
// 3. Advance the block height to the settlement height and settle the claims.
// 5. Assert that the settlement states of all TLM order permutations match.
Comment on lines +27 to +32
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// commutativity). It does this in the following steps:
//
// 1. Construct a TokenomicsModuleKeepers instance for each TLM processor permutation.
// 2. Create valid claims (which require no proofs).
// 3. Advance the block height to the settlement height and settle the claims.
// 5. Assert that the settlement states of all TLM order permutations match.
// commutativity).
// It does this in the following steps:
// 1. Construct a TokenomicsModuleKeepers instance for each TLM processor permutation.
// 2. Create valid claims (which require no proofs).
// 3. Advance the block height to the settlement height and settle the claims.
// 4. Assert that the settlement states of all TLM order permutations match.

func (s *tlmProcessorTestSuite) TestTLMProcessorsAreCommutative() {
// Generate all permutations of TLM processor ordering.
tokenLogicModules := tlm.NewDefaultTokenLogicModules(s.foundationBech32)
tlmOrderPermutations := permute(tokenLogicModules)

numTLMOrderPermutations := factorial(len(tokenLogicModules))
require.Equal(s.T(), numTLMOrderPermutations, len(tlmOrderPermutations))

for i, tlmPermutation := range tlmOrderPermutations {
var tlmIds []string
for _, tokenLogicModule := range tlmPermutation {
tlmIds = append(tlmIds, tokenLogicModule.GetId().String())
}

// The test description is a unique identifier for each permutation.
// E.g.: "permutaiton_1_of_2__TLMRelayBurnEqualsMint_TLMGlobalMint"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thoughts on something like this?

Suggested change
// E.g.: "permutaiton_1_of_2__TLMRelayBurnEqualsMint_TLMGlobalMint"
// E.g.: "permutaiton_1_of_2:TLMRelayBurnEqualsMint_TLMGlobalMint"

testDesc := fmt.Sprintf(
"permutaiton_%d_of_%d__%s",
i+1, numTLMOrderPermutations,
strings.Join(tlmIds, "_"),
)

s.T().Run(testDesc, func(t *testing.T) {
s.setupKeepers(t, keeper.WithTLMProcessors(tlmPermutation))

// Assert that no pre-existing claims are present.
numExistingClaims := len(s.keepers.GetAllClaims(s.ctx))
require.Equal(t, 0, numExistingClaims)

s.createClaims(&s.keepers, 1000)
settledResults, expiredResults := s.settleClaims(t)

// Set the expected state based on the effects of the first iteration;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// Set the expected state based on the effects of the first iteration;
// First iteration only.
// Set the expected state based on the effects of the first iteration.

// this decouples the assertions from any specific tlm effects.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thoughts on actually having a function to assert on a results to make sure we don't assert on (for example) 0=0 everywhere?

if i == 0 {
s.setExpectedSettlementState(t, settledResults, expiredResults)
t.SkipNow()
}

s.assertExpectedSettlementState(t, settledResults, expiredResults)
})
}
}

// setupKeepers initializes a new instance of TokenomicsModuleKeepers and context
// with the given options, and creates the suite's service, application, and supplier
// from SetupTest(). It also sets the block height to 1 and the proposer address to
// the proposer address from SetupTest().
func (s *tlmProcessorTestSuite) setupKeepers(t *testing.T, opts ...keeper.TokenomicsModuleKeepersOptFn) {
defaultOpts := []keeper.TokenomicsModuleKeepersOptFn{
keeper.WithService(*s.service),
keeper.WithApplication(*s.app),
keeper.WithSupplier(*s.supplier),
keeper.WithModuleParams(map[string]types.Msg{
// TODO_TECHDEBT: Set tokenomics mint allocation params to maximize coverage, once available.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO_BETA?


// Set the proof params such that proofs are NEVER required.
prooftypes.ModuleName: s.getProofParams(),
// Set the CUTTM to simplify calculating settlement amount expectstions.
sharedtypes.ModuleName: s.getSharedParams(),
}),
}

s.keepers, s.ctx = keeper.NewTokenomicsModuleKeepers(
t, log.NewNopLogger(),
append(defaultOpts, opts...)...,
)

// Increment the block height to 1; valid session height and set the proposer address.
s.ctx = types.UnwrapSDKContext(s.ctx).
WithBlockHeight(1).
WithProposer(s.proposerConsAddr)
}

// setExpectedSettlementState sets the expected settlement state on the suite based
// on the current network state and the given settledResults and expiredResults.
func (s *tlmProcessorTestSuite) setExpectedSettlementState(
t *testing.T,
settledResults,
expiredResults tlm.PendingSettlementResults,
) {
t.Helper()

s.expectedSettledResults = settledResults
s.expectedExpiredResults = expiredResults
s.expectedSettlementState = s.getSettlementState(t)
}

// getSettlementState returns a settlement state based on the current network state.
func (s *tlmProcessorTestSuite) getSettlementState(t *testing.T) *settlementState {
t.Helper()

app, isAppFound := s.keepers.GetApplication(s.ctx, s.app.GetAddress())
require.True(t, isAppFound)

proposerBech32 := sample.AccAddressFromConsBech32(s.proposerConsAddr.String())

return &settlementState{
appStake: app.GetStake(),
supplierOwnerBalance: s.getBalance(t, s.supplier.GetOwnerAddress()),
proposerBalance: s.getBalance(t, proposerBech32),
foundationBalance: s.getBalance(t, s.foundationBech32),
sourceOwnerBalance: s.getBalance(t, s.sourceOwnerBech32),
}
}

// getBalance returns the current balance of the given bech32 address.
func (s *tlmProcessorTestSuite) getBalance(t *testing.T, bech32 string) *types.Coin {
t.Helper()

res, err := s.keepers.Balance(s.ctx, &banktypes.QueryBalanceRequest{
Address: bech32,
Denom: volatile.DenomuPOKT,
})
require.NoError(t, err)
require.NotNil(t, res)

return res.GetBalance()
}

// assertExpectedSettlementState asserts that the current network state matches the
// expected settlement state, and that actualSettledResults and actualExpiredResults
// match their corresponding expectations.
func (s *tlmProcessorTestSuite) assertExpectedSettlementState(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any way to update the test such that we moe forward one (or two) more sessions and confirm the state/balances are unchanged (since there are no new claims).

This is likely a different test, but just want to sanity checks the session start/end edge case + lingering claims.

t *testing.T,
actualSettledResults,
actualExpiredResults tlm.PendingSettlementResults,
) {
require.Equal(t, len(s.expectedSettledResults), len(actualSettledResults))
require.Equal(t, len(s.expectedExpiredResults), len(actualExpiredResults))

for _, expectedSettledResult := range s.expectedSettledResults {
// Find the corresponding actual settled result by matching on claim root hash.
foundActualResult := new(tlm.PendingSettlementResult)
for _, actualSettledResult := range actualSettledResults {
if bytes.Equal(expectedSettledResult.Claim.GetRootHash(), actualSettledResult.Claim.GetRootHash()) {
foundActualResult = actualSettledResult
break
}
}
// Assert that the corresponding actual settled result was found.
require.NotNil(t, foundActualResult)

// Assert that all mint, burn, and transfer operations match the expected settled result.
// Ordering of operations for a given type are not expected to be preserved between TLM
// processor permutations.
require.ElementsMatch(t, expectedSettledResult.Mints, foundActualResult.Mints)
require.ElementsMatch(t, expectedSettledResult.Burns, foundActualResult.Burns)
require.ElementsMatch(t, expectedSettledResult.ModToModTransfers, foundActualResult.ModToModTransfers)
require.ElementsMatch(t, expectedSettledResult.ModToAcctTransfers, foundActualResult.ModToAcctTransfers)

actualSettlementState := s.getSettlementState(t)

// Assert that app stake and rewardee balances are non-zero.
require.NotEqual(t, &zerouPOKT, actualSettlementState.appStake)
require.NotEqual(t, &zerouPOKT, actualSettlementState.appBalance)
require.NotEqual(t, &zerouPOKT, actualSettlementState.supplierOwnerBalance)
require.NotEqual(t, &zerouPOKT, actualSettlementState.proposerBalance)
require.NotEqual(t, &zerouPOKT, actualSettlementState.foundationBalance)
require.NotEqual(t, &zerouPOKT, actualSettlementState.sourceOwnerBalance)

// Assert that the expected and actual settlement states match.
require.EqualValues(t, s.expectedSettlementState, actualSettlementState)
}
}
98 changes: 98 additions & 0 deletions tests/integration/tokenomics/token_logic_modules/permute_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
package token_logic_modules

import (
"strings"
"testing"

"github.com/stretchr/testify/require"
)

// TestPermute checks the correctness of the permute function such that it is
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#PUC what permute permutates

// "safe" to use in tests (i.e. it does what it's supposed to).
func TestPermute(t *testing.T) {
input := []string{"1", "2", "3", "4"}
expected := map[string][]string{
"1234": {"1", "2", "3", "4"},
"1243": {"1", "2", "4", "3"},
"1342": {"1", "3", "4", "2"},
"1324": {"1", "3", "2", "4"},
"1423": {"1", "4", "2", "3"},
"1432": {"1", "4", "3", "2"},
"2341": {"2", "3", "4", "1"},
"2314": {"2", "3", "1", "4"},
"2413": {"2", "4", "1", "3"},
"2431": {"2", "4", "3", "1"},
"2143": {"2", "1", "4", "3"},
"2134": {"2", "1", "3", "4"},
"3412": {"3", "4", "1", "2"},
"3421": {"3", "4", "2", "1"},
"3124": {"3", "1", "2", "4"},
"3142": {"3", "1", "4", "2"},
"3241": {"3", "2", "4", "1"},
"3214": {"3", "2", "1", "4"},
"4123": {"4", "1", "2", "3"},
"4132": {"4", "1", "3", "2"},
"4231": {"4", "2", "3", "1"},
"4213": {"4", "2", "1", "3"},
"4312": {"4", "3", "1", "2"},
"4321": {"4", "3", "2", "1"},
}

actual := permute(input)
require.Equal(t, factorial(len(input)), len(actual))

// Assert that each actual result matches exactly one expected permutation.
for _, actualPermutation := range expected {
actualKey := strings.Join(actualPermutation, "")
expectedPermutation, isExpectedPermutation := expected[actualKey]
require.True(t, isExpectedPermutation)
require.Equal(t, expectedPermutation, actualPermutation)

// Remove observed expected permutation to identify any
// missing permutations after the loop.
delete(expected, actualKey)
}
// Assert that all expected permutations were observed (and deleted).
require.Len(t, expected, 0)
}

// permute generates all possible permutations of the input slice 'items'.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#PUC what w use this for in tets

func permute[T any](items []T) [][]T {
var permutations [][]T
// Create a copy to avoid modifying the original slice.
itemsCopy := make([]T, len(items))
copy(itemsCopy, items)
// Start the recursive permutation generation with swap index 0.
recursivePermute(itemsCopy, &permutations, 0)
return permutations
}

// recursivePermute recursively generates permutations by swapping elements.
func recursivePermute[T any](items []T, permutations *[][]T, swapIdx int) {
if swapIdx == len(items) {
// Append a copy of the current permutation to the result.
permutation := make([]T, len(items))
copy(permutation, items)
*permutations = append(*permutations, permutation)
return
}
for i := swapIdx; i < len(items); i++ {
// Swap the current element with the element at the swap index.
items[swapIdx], items[i] = items[i], items[swapIdx]
// Recurse with the next swap index.
recursivePermute[T](items, permutations, swapIdx+1)
// Swap back to restore the original state (backtrack).
items[swapIdx], items[i] = items[i], items[swapIdx]
}
}

func factorial(n int) int {
if n < 0 {
return 0 // Handle negative input as an invalid case
}
result := 1
for i := 2; i <= n; i++ {
result *= i
}
return result
}
Loading
Loading