From 2307ef77a32e7544d873927e98489f8d660baab6 Mon Sep 17 00:00:00 2001 From: Bryan White Date: Wed, 10 Jul 2024 22:50:01 +0200 Subject: [PATCH 1/9] fix: window size calculations --- x/shared/session.go | 37 ++----------------------------------- x/shared/session_test.go | 4 ++-- 2 files changed, 4 insertions(+), 37 deletions(-) diff --git a/x/shared/session.go b/x/shared/session.go index 2bcc1b45f..6fd21d7e2 100644 --- a/x/shared/session.go +++ b/x/shared/session.go @@ -5,11 +5,6 @@ import ( sharedtypes "github.com/pokt-network/poktroll/x/shared/types" ) -const ( - minimumClaimWindowSizeBlocks = 1 - minimumProofWindowSizeBlocks = 1 -) - // TODO_DOCUMENT(@bryanchriswhite): Move this into the documentation: https://github.com/pokt-network/poktroll/pull/571#discussion_r1630923625 // GetSessionStartHeight returns the block height at which the session containing @@ -123,26 +118,12 @@ func GetEarliestSupplierClaimCommitHeight( // window open block hash and the supplier address. randomNumber := poktrand.SeededInt63(claimWindowOpenBlockHash, []byte(supplierAddr)) - distributionWindowSizeBlocks := GetClaimWindowSizeBlocks(sharedParams) + distributionWindowSizeBlocks := sharedParams.GetClaimWindowCloseOffsetBlocks() randCreateClaimHeightOffset := randomNumber % int64(distributionWindowSizeBlocks) return claimWindowOpenHeight + randCreateClaimHeightOffset } -// GetClaimWindowSizeBlocks returns the number of blocks between the opening and closing -// of the claim window, given the passed sharedParams. -func GetClaimWindowSizeBlocks(sharedParams *sharedtypes.Params) uint64 { - windowSizeBlocks := sharedParams.ClaimWindowCloseOffsetBlocks - - sharedParams.ClaimWindowOpenOffsetBlocks - - minimumClaimWindowSizeBlocks - - if windowSizeBlocks < 1 { - return 1 - } - - return windowSizeBlocks -} - // GetEarliestSupplierProofCommitHeight returns the earliest block height at which a proof // for the session that includes queryHeight can be committed for a given supplier // and the passed sharedParams. @@ -158,22 +139,8 @@ func GetEarliestSupplierProofCommitHeight( // window open block hash and the supplier address. randomNumber := poktrand.SeededInt63(proofWindowOpenBlockHash, []byte(supplierAddr)) - distributionWindowSizeBlocks := GetProofWindowSizeBlocks(sharedParams) + distributionWindowSizeBlocks := sharedParams.GetProofWindowCloseOffsetBlocks() randCreateProofHeightOffset := randomNumber % int64(distributionWindowSizeBlocks) return proofWindowOpenHeight + randCreateProofHeightOffset } - -// GetProofWindowSizeBlocks returns the number of blocks between the opening and closing -// of the proof window, given the passed sharedParams. -func GetProofWindowSizeBlocks(sharedParams *sharedtypes.Params) uint64 { - windowSizeBlocks := sharedParams.ProofWindowCloseOffsetBlocks - - sharedParams.ProofWindowOpenOffsetBlocks - - minimumProofWindowSizeBlocks - - if windowSizeBlocks < 1 { - return 1 - } - - return windowSizeBlocks -} diff --git a/x/shared/session_test.go b/x/shared/session_test.go index 31b19e62e..b11d89d60 100644 --- a/x/shared/session_test.go +++ b/x/shared/session_test.go @@ -217,10 +217,10 @@ func TestClaimProofWindows(t *testing.T) { require.GreaterOrEqual(t, earliestProofCommitHeight, claimWindowCloseHeight) require.Greater(t, proofWindowCloseHeight, earliestProofCommitHeight) - claimWindowSizeBlocks := GetClaimWindowSizeBlocks(&test.sharedParams) + claimWindowSizeBlocks := test.sharedParams.GetClaimWindowCloseOffsetBlocks() require.Greater(t, claimWindowSizeBlocks, uint64(0)) - proofWindowSizeBlocks := GetProofWindowSizeBlocks(&test.sharedParams) + proofWindowSizeBlocks := test.sharedParams.GetProofWindowCloseOffsetBlocks() require.Greater(t, proofWindowSizeBlocks, uint64(0)) wg.Done() From 392504e6b26ed615cc056144c0627ec60d1466bf Mon Sep 17 00:00:00 2001 From: Bryan White Date: Wed, 10 Jul 2024 22:50:10 +0200 Subject: [PATCH 2/9] chore: add session keeper as dep of tokenomics keeper --- testutil/integration/app.go | 1 + testutil/keeper/tokenomics.go | 5 +++++ x/tokenomics/keeper/keeper.go | 3 +++ x/tokenomics/module/module.go | 2 ++ x/tokenomics/types/expected_keepers.go | 7 +++++++ 5 files changed, 18 insertions(+) diff --git a/testutil/integration/app.go b/testutil/integration/app.go index 98197836a..619a57820 100644 --- a/testutil/integration/app.go +++ b/testutil/integration/app.go @@ -393,6 +393,7 @@ func NewCompleteIntegrationApp(t *testing.T) *App { applicationKeeper, proofKeeper, sharedKeeper, + sessionKeeper, ) tokenomicsModule := tokenomics.NewAppModule( cdc, diff --git a/testutil/keeper/tokenomics.go b/testutil/keeper/tokenomics.go index f54bc13c1..3eccaf55d 100644 --- a/testutil/keeper/tokenomics.go +++ b/testutil/keeper/tokenomics.go @@ -155,6 +155,9 @@ func TokenomicsKeeperWithActorAddrs(t testing.TB) ( mockSharedKeeper := mocks.NewMockSharedKeeper(ctrl) mockSharedKeeper.EXPECT().GetProofWindowCloseHeight(gomock.Any(), gomock.Any()).AnyTimes() + // Mock the session keeper + mockSessionKeeper := mocks.NewMockSessionKeeper(ctrl) + k := tokenomicskeeper.NewKeeper( cdc, runtime.NewKVStoreService(storeKey), @@ -165,6 +168,7 @@ func TokenomicsKeeperWithActorAddrs(t testing.TB) ( mockApplicationKeeper, mockProofKeeper, mockSharedKeeper, + mockSessionKeeper, ) sdkCtx := sdk.NewContext(stateStore, cmtproto.Header{}, false, log.NewNopLogger()) @@ -331,6 +335,7 @@ func NewTokenomicsModuleKeepers( appKeeper, proofKeeper, sharedKeeper, + sessionKeeper, ) require.NoError(t, tokenomicsKeeper.SetParams(ctx, tokenomicstypes.DefaultParams())) diff --git a/x/tokenomics/keeper/keeper.go b/x/tokenomics/keeper/keeper.go index 7e6c13b87..1a838f8be 100644 --- a/x/tokenomics/keeper/keeper.go +++ b/x/tokenomics/keeper/keeper.go @@ -26,6 +26,7 @@ type Keeper struct { applicationKeeper types.ApplicationKeeper proofKeeper types.ProofKeeper sharedKeeper types.SharedKeeper + sessionKeeper types.SessionKeeper } func NewKeeper( @@ -39,6 +40,7 @@ func NewKeeper( applicationKeeper types.ApplicationKeeper, proofKeeper types.ProofKeeper, sharedKeeper types.SharedKeeper, + sessionKeeper types.SessionKeeper, ) Keeper { if _, err := sdk.AccAddressFromBech32(authority); err != nil { panic(fmt.Sprintf("invalid authority address: %s", authority)) @@ -55,6 +57,7 @@ func NewKeeper( applicationKeeper: applicationKeeper, proofKeeper: proofKeeper, sharedKeeper: sharedKeeper, + sessionKeeper: sessionKeeper, } } diff --git a/x/tokenomics/module/module.go b/x/tokenomics/module/module.go index f4c2b388e..2e857d5fd 100644 --- a/x/tokenomics/module/module.go +++ b/x/tokenomics/module/module.go @@ -182,6 +182,7 @@ type ModuleInputs struct { ApplicationKeeper types.ApplicationKeeper ProofKeeper types.ProofKeeper SharedKeeper types.SharedKeeper + SessionKeeper types.SessionKeeper } type ModuleOutputs struct { @@ -207,6 +208,7 @@ func ProvideModule(in ModuleInputs) ModuleOutputs { in.ApplicationKeeper, in.ProofKeeper, in.SharedKeeper, + in.SessionKeeper, ) m := NewAppModule( in.Cdc, diff --git a/x/tokenomics/types/expected_keepers.go b/x/tokenomics/types/expected_keepers.go index fb82fa7d5..89df7105d 100644 --- a/x/tokenomics/types/expected_keepers.go +++ b/x/tokenomics/types/expected_keepers.go @@ -9,6 +9,7 @@ import ( apptypes "github.com/pokt-network/poktroll/x/application/types" prooftypes "github.com/pokt-network/poktroll/x/proof/types" + sessiontypes "github.com/pokt-network/poktroll/x/session/types" sharedtypes "github.com/pokt-network/poktroll/x/shared/types" ) @@ -53,3 +54,9 @@ type SharedKeeper interface { GetProofWindowCloseHeight(ctx context.Context, queryHeight int64) int64 } + +type SessionKeeper interface { + GetSession(context.Context, *sessiontypes.QueryGetSessionRequest) (*sessiontypes.QueryGetSessionResponse, error) + GetBlockHash(ctx context.Context, height int64) []byte + StoreBlockHash(ctx context.Context) +} From e636363caf099f4e3266511110ac046c5acfe9de Mon Sep 17 00:00:00 2001 From: Bryan White Date: Wed, 10 Jul 2024 22:51:47 +0200 Subject: [PATCH 3/9] chore: add sharedQuerier to tokenomics keeper --- x/tokenomics/keeper/keeper.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/x/tokenomics/keeper/keeper.go b/x/tokenomics/keeper/keeper.go index 1a838f8be..8952a9e0a 100644 --- a/x/tokenomics/keeper/keeper.go +++ b/x/tokenomics/keeper/keeper.go @@ -8,6 +8,8 @@ import ( "github.com/cosmos/cosmos-sdk/codec" sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/pokt-network/poktroll/pkg/client" + prooftypes "github.com/pokt-network/poktroll/x/proof/types" "github.com/pokt-network/poktroll/x/tokenomics/types" ) @@ -27,6 +29,8 @@ type Keeper struct { proofKeeper types.ProofKeeper sharedKeeper types.SharedKeeper sessionKeeper types.SessionKeeper + + sharedQuerier client.SharedQueryClient } func NewKeeper( @@ -46,6 +50,8 @@ func NewKeeper( panic(fmt.Sprintf("invalid authority address: %s", authority)) } + sharedQuerier := prooftypes.NewSharedKeeperQueryClient(sharedKeeper, sessionKeeper) + return Keeper{ cdc: cdc, storeService: storeService, @@ -58,6 +64,8 @@ func NewKeeper( proofKeeper: proofKeeper, sharedKeeper: sharedKeeper, sessionKeeper: sessionKeeper, + + sharedQuerier: sharedQuerier, } } From 5f477b5d94742339c8a04bb9f903902e540c9c40 Mon Sep 17 00:00:00 2001 From: Bryan White Date: Wed, 10 Jul 2024 22:56:04 +0200 Subject: [PATCH 4/9] refactor: querying expiring claims --- x/tokenomics/keeper/settle_pending_claims.go | 56 +++++++++++++++----- x/tokenomics/types/expected_keepers.go | 4 +- 2 files changed, 47 insertions(+), 13 deletions(-) diff --git a/x/tokenomics/keeper/settle_pending_claims.go b/x/tokenomics/keeper/settle_pending_claims.go index 74f0103de..b90faf9c0 100644 --- a/x/tokenomics/keeper/settle_pending_claims.go +++ b/x/tokenomics/keeper/settle_pending_claims.go @@ -4,6 +4,7 @@ import ( "fmt" sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/cosmos/cosmos-sdk/types/query" poktrand "github.com/pokt-network/poktroll/pkg/crypto/rand" "github.com/pokt-network/poktroll/telemetry" @@ -30,7 +31,10 @@ func (k Keeper) SettlePendingClaims(ctx sdk.Context) ( // TODO_BLOCKER(@Olshansk): Optimize this by indexing expiringClaims appropriately // and only retrieving the expiringClaims that need to be settled rather than all // of them and iterating through them one by one. - expiringClaims := k.getExpiringClaims(ctx) + expiringClaims, err := k.getExpiringClaims(ctx) + if err != nil { + return 0, 0, relaysPerServiceMap, computeUnitsPerServiceMap, err + } blockHeight := ctx.BlockHeight() @@ -160,25 +164,53 @@ func (k Keeper) SettlePendingClaims(ctx sdk.Context) ( // This is the height at which the proof window closes. // If the proof window closes and a proof IS NOT required -> settle the claim. // If the proof window closes and a proof IS required -> only settle it if a proof is available. -func (k Keeper) getExpiringClaims(ctx sdk.Context) (expiringClaims []prooftypes.Claim) { +func (k Keeper) getExpiringClaims(ctx sdk.Context) (expiringClaims []prooftypes.Claim, err error) { blockHeight := ctx.BlockHeight() - // TODO_TECHDEBT: Optimize this by indexing claims appropriately - // and only retrieving the claims that need to be settled rather than all - // of them and iterating through them one by one. - claims := k.proofKeeper.GetAllClaims(ctx) + // NB: This error can be safely ignored as on-chain SharedQueryClient implementation cannot return an error. + sharedParams, _ := k.sharedQuerier.GetParams(ctx) + //currentSessionEndHeight := shared.GetSessionStartHeight(sharedParams, blockHeight) + //previousSessionEndHeight := shared.GetSessionEndHeight(sharedParams, currentSessionEndHeight-1) + claimWindowSizeBlocks := sharedParams.GetClaimWindowCloseOffsetBlocks() + proofWindowSizeBlocks := sharedParams.GetProofWindowCloseOffsetBlocks() + + previousSessionEndHeight := blockHeight - + int64(sharedParams.GetGracePeriodEndOffsetBlocks()+ + claimWindowSizeBlocks+ + proofWindowSizeBlocks+1) + + allClaims := k.proofKeeper.GetAllClaims(ctx) + _ = allClaims + + for { + claimsRes, err := k.proofKeeper.AllClaims(ctx, &prooftypes.QueryAllClaimsRequest{ + Pagination: &query.PageRequest{ + Offset: uint64(len(expiringClaims)), + //Limit: 0, + //CountTotal: false, + }, + Filter: &prooftypes.QueryAllClaimsRequest_SessionEndHeight{ + SessionEndHeight: uint64(previousSessionEndHeight), + }, + }) + if err != nil { + return nil, err + } - // Loop over all claims we need to check for expiration - for _, claim := range claims { - claimSessionStartHeight := claim.GetSessionHeader().GetSessionStartBlockHeight() - expirationHeight := k.sharedKeeper.GetProofWindowCloseHeight(ctx, claimSessionStartHeight) - if blockHeight >= expirationHeight { + for _, claim := range claimsRes.GetClaims() { expiringClaims = append(expiringClaims, claim) } + + // Continue if there are more claims to fetch. + if claimsRes.Pagination.GetNextKey() != nil { + continue + } + + break } // Return the actually expiring claims - return expiringClaims + return expiringClaims, nil } // proofRequirementForClaim checks if a proof is required for a claim. diff --git a/x/tokenomics/types/expected_keepers.go b/x/tokenomics/types/expected_keepers.go index 89df7105d..371ddcff5 100644 --- a/x/tokenomics/types/expected_keepers.go +++ b/x/tokenomics/types/expected_keepers.go @@ -1,4 +1,4 @@ -//go:generate mockgen -destination ../../../testutil/tokenomics/mocks/expected_keepers_mock.go -package mocks . AccountKeeper,BankKeeper,ApplicationKeeper,ProofKeeper,SharedKeeper +//go:generate mockgen -destination ../../../testutil/tokenomics/mocks/expected_keepers_mock.go -package mocks . AccountKeeper,BankKeeper,ApplicationKeeper,ProofKeeper,SharedKeeper,SessionKeeper package types @@ -40,6 +40,8 @@ type ProofKeeper interface { GetProof(ctx context.Context, sessionId, supplierAddr string) (proof prooftypes.Proof, isProofFound bool) RemoveProof(ctx context.Context, sessionId, supplierAddr string) + AllClaims(ctx context.Context, req *prooftypes.QueryAllClaimsRequest) (*prooftypes.QueryAllClaimsResponse, error) + // Only used for testing & simulation UpsertClaim(ctx context.Context, claim prooftypes.Claim) UpsertProof(ctx context.Context, claim prooftypes.Proof) From 47b84bd2977da5783d51037bb3d443f4ad94731f Mon Sep 17 00:00:00 2001 From: Bryan White Date: Wed, 10 Jul 2024 23:03:49 +0200 Subject: [PATCH 5/9] chore: self-review improvements --- x/tokenomics/keeper/settle_pending_claims.go | 7 ------- 1 file changed, 7 deletions(-) diff --git a/x/tokenomics/keeper/settle_pending_claims.go b/x/tokenomics/keeper/settle_pending_claims.go index b90faf9c0..3a2a269fb 100644 --- a/x/tokenomics/keeper/settle_pending_claims.go +++ b/x/tokenomics/keeper/settle_pending_claims.go @@ -28,9 +28,6 @@ func (k Keeper) SettlePendingClaims(ctx sdk.Context) ( ) { logger := k.Logger().With("method", "SettlePendingClaims") - // TODO_BLOCKER(@Olshansk): Optimize this by indexing expiringClaims appropriately - // and only retrieving the expiringClaims that need to be settled rather than all - // of them and iterating through them one by one. expiringClaims, err := k.getExpiringClaims(ctx) if err != nil { return 0, 0, relaysPerServiceMap, computeUnitsPerServiceMap, err @@ -169,8 +166,6 @@ func (k Keeper) getExpiringClaims(ctx sdk.Context) (expiringClaims []prooftypes. // NB: This error can be safely ignored as on-chain SharedQueryClient implementation cannot return an error. sharedParams, _ := k.sharedQuerier.GetParams(ctx) - //currentSessionEndHeight := shared.GetSessionStartHeight(sharedParams, blockHeight) - //previousSessionEndHeight := shared.GetSessionEndHeight(sharedParams, currentSessionEndHeight-1) claimWindowSizeBlocks := sharedParams.GetClaimWindowCloseOffsetBlocks() proofWindowSizeBlocks := sharedParams.GetProofWindowCloseOffsetBlocks() @@ -186,8 +181,6 @@ func (k Keeper) getExpiringClaims(ctx sdk.Context) (expiringClaims []prooftypes. claimsRes, err := k.proofKeeper.AllClaims(ctx, &prooftypes.QueryAllClaimsRequest{ Pagination: &query.PageRequest{ Offset: uint64(len(expiringClaims)), - //Limit: 0, - //CountTotal: false, }, Filter: &prooftypes.QueryAllClaimsRequest_SessionEndHeight{ SessionEndHeight: uint64(previousSessionEndHeight), From a2802a5ea4ebd6fb9d765f26087d79fb278cbd49 Mon Sep 17 00:00:00 2001 From: Bryan White Date: Mon, 15 Jul 2024 13:07:57 +0200 Subject: [PATCH 6/9] chore: review feedback improvements --- x/tokenomics/keeper/settle_pending_claims.go | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/x/tokenomics/keeper/settle_pending_claims.go b/x/tokenomics/keeper/settle_pending_claims.go index 3a2a269fb..de35be1b7 100644 --- a/x/tokenomics/keeper/settle_pending_claims.go +++ b/x/tokenomics/keeper/settle_pending_claims.go @@ -166,24 +166,24 @@ func (k Keeper) getExpiringClaims(ctx sdk.Context) (expiringClaims []prooftypes. // NB: This error can be safely ignored as on-chain SharedQueryClient implementation cannot return an error. sharedParams, _ := k.sharedQuerier.GetParams(ctx) - claimWindowSizeBlocks := sharedParams.GetClaimWindowCloseOffsetBlocks() - proofWindowSizeBlocks := sharedParams.GetProofWindowCloseOffsetBlocks() + claimWindowSizeBlocks := sharedParams.GetClaimWindowOpenOffsetBlocks() + sharedParams.GetClaimWindowCloseOffsetBlocks() + proofWindowSizeBlocks := sharedParams.GetProofWindowOpenOffsetBlocks() + sharedParams.GetProofWindowCloseOffsetBlocks() - previousSessionEndHeight := blockHeight - - int64(sharedParams.GetGracePeriodEndOffsetBlocks()+ - claimWindowSizeBlocks+ + expiringSessionEndHeight := blockHeight - + int64(claimWindowSizeBlocks+ proofWindowSizeBlocks+1) allClaims := k.proofKeeper.GetAllClaims(ctx) _ = allClaims + var nextKey []byte for { claimsRes, err := k.proofKeeper.AllClaims(ctx, &prooftypes.QueryAllClaimsRequest{ Pagination: &query.PageRequest{ - Offset: uint64(len(expiringClaims)), + Key: nextKey, }, Filter: &prooftypes.QueryAllClaimsRequest_SessionEndHeight{ - SessionEndHeight: uint64(previousSessionEndHeight), + SessionEndHeight: uint64(expiringSessionEndHeight), }, }) if err != nil { @@ -195,7 +195,8 @@ func (k Keeper) getExpiringClaims(ctx sdk.Context) (expiringClaims []prooftypes. } // Continue if there are more claims to fetch. - if claimsRes.Pagination.GetNextKey() != nil { + nextKey = claimsRes.Pagination.GetNextKey() + if nextKey != nil { continue } From f488e0e0ce1a424a009cdf521af2cb96fdc39464 Mon Sep 17 00:00:00 2001 From: Bryan White Date: Mon, 15 Jul 2024 13:16:47 +0200 Subject: [PATCH 7/9] chore: add comment --- x/tokenomics/keeper/settle_pending_claims.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/x/tokenomics/keeper/settle_pending_claims.go b/x/tokenomics/keeper/settle_pending_claims.go index 579bb5cf9..92996fc5e 100644 --- a/x/tokenomics/keeper/settle_pending_claims.go +++ b/x/tokenomics/keeper/settle_pending_claims.go @@ -177,6 +177,8 @@ func (k Keeper) getExpiringClaims(ctx sdk.Context) (expiringClaims []prooftypes. claimWindowSizeBlocks := sharedParams.GetClaimWindowOpenOffsetBlocks() + sharedParams.GetClaimWindowCloseOffsetBlocks() proofWindowSizeBlocks := sharedParams.GetProofWindowOpenOffsetBlocks() + sharedParams.GetProofWindowCloseOffsetBlocks() + // expiringSessionEndHeight is the session end height of the session whose proof + // window has most recently closed. expiringSessionEndHeight := blockHeight - int64(claimWindowSizeBlocks+ proofWindowSizeBlocks+1) From 60bd91b02ae3b9380fc4f1f707395f4c87c03f46 Mon Sep 17 00:00:00 2001 From: Bryan White Date: Mon, 15 Jul 2024 13:21:09 +0200 Subject: [PATCH 8/9] fix: post-merge --- x/tokenomics/keeper/settle_pending_claims.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/tokenomics/keeper/settle_pending_claims.go b/x/tokenomics/keeper/settle_pending_claims.go index 92996fc5e..35b71f5a3 100644 --- a/x/tokenomics/keeper/settle_pending_claims.go +++ b/x/tokenomics/keeper/settle_pending_claims.go @@ -29,7 +29,7 @@ func (k Keeper) SettlePendingClaims(ctx sdk.Context) ( expiringClaims, err := k.getExpiringClaims(ctx) if err != nil { - return 0, 0, relaysPerServiceMap, computeUnitsPerServiceMap, err + return settledResult, expiredResult, err } blockHeight := ctx.BlockHeight() From 98284faca4c28097c5da259d807bc50f31d2bfb3 Mon Sep 17 00:00:00 2001 From: Bryan White Date: Mon, 15 Jul 2024 20:01:22 +0200 Subject: [PATCH 9/9] Empty commit