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

[Service] refactor: TargetNumRelays var usage to param usage #943

Open
wants to merge 1 commit into
base: todo_beta/params/target_num_relays
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 4 additions & 1 deletion tests/integration/application/min_stake_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -255,11 +255,14 @@ func (s *applicationMinStakeTestSuite) getExpectedApp(claim *prooftypes.Claim) *
func (s *applicationMinStakeTestSuite) newRelayminingDifficulty() servicetypes.RelayMiningDifficulty {
s.T().Helper()

targetNumRelays := s.keepers.ServiceKeeper.GetParams(s.ctx).TargetNumRelays

return servicekeeper.NewDefaultRelayMiningDifficulty(
s.ctx,
cosmoslog.NewNopLogger(),
s.serviceId,
servicekeeper.TargetNumRelays,
targetNumRelays,
targetNumRelays,
)
}

Expand Down
23 changes: 11 additions & 12 deletions tests/integration/tokenomics/relay_mining_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import (
"github.com/pokt-network/poktroll/testutil/testrelayer"
apptypes "github.com/pokt-network/poktroll/x/application/types"
prooftypes "github.com/pokt-network/poktroll/x/proof/types"
servicekeeper "github.com/pokt-network/poktroll/x/service/keeper"
servicetypes "github.com/pokt-network/poktroll/x/service/types"
sessiontypes "github.com/pokt-network/poktroll/x/session/types"
sharedtypes "github.com/pokt-network/poktroll/x/shared/types"
Expand All @@ -30,15 +29,6 @@ const (
)

func TestComputeNewDifficultyHash_RewardsReflectWorkCompleted(t *testing.T) {
// Update the target number of relays to a value that suits the test.
// A too high number would make the difficulty stay at BaseRelayDifficultyHash
initialTargetRelays := servicekeeper.TargetNumRelays
servicekeeper.TargetNumRelays = 1000
t.Cleanup(func() {
// Reset the target number of relays to its initial value.
servicekeeper.TargetNumRelays = initialTargetRelays
})

// Prepare the test service.
service := sharedtypes.Service{
Id: "svc1",
Expand Down Expand Up @@ -89,10 +79,17 @@ func TestComputeNewDifficultyHash_RewardsReflectWorkCompleted(t *testing.T) {
sdkCtx := sdk.UnwrapSDKContext(ctx)
sdkCtx = sdkCtx.WithBlockHeight(1)

// Update the target number of relays to a value that suits the test.
// A too high number would make the difficulty stay at BaseRelayDifficultyHash
serviceParams := keepers.ServiceKeeper.GetParams(ctx)
serviceParams.TargetNumRelays = 1000
err := keepers.ServiceKeeper.SetParams(ctx, serviceParams)
require.NoError(t, err)

// Set the CUTTM to 1 to simplify the math
sharedParams := keepers.SharedKeeper.GetParams(sdkCtx)
sharedParams.ComputeUnitsToTokensMultiplier = uint64(1)
err := keepers.SharedKeeper.SetParams(sdkCtx, sharedParams)
err = keepers.SharedKeeper.SetParams(sdkCtx, sharedParams)
require.NoError(t, err)

// Update the relay mining difficulty so there's always a difficulty to retrieve
Expand Down Expand Up @@ -159,10 +156,12 @@ func TestComputeNewDifficultyHash_RewardsReflectWorkCompleted(t *testing.T) {
updatedRelayMiningDifficulty, ok := keepers.ServiceKeeper.GetRelayMiningDifficulty(sdkCtx, service.Id)
require.True(t, ok)

targetNumRelays := keepers.ServiceKeeper.GetParams(ctx).TargetNumRelays

// Compute the new difficulty hash based on the updated relay mining difficulty.
newDifficultyHash := protocol.ComputeNewDifficultyTargetHash(
protocol.BaseRelayDifficultyHashBz,
servicekeeper.TargetNumRelays,
targetNumRelays,
updatedRelayMiningDifficulty.NumRelaysEma,
)

Expand Down
9 changes: 8 additions & 1 deletion testutil/keeper/tokenomics.go
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,14 @@ func TokenomicsKeeperWithActorAddrs(t testing.TB) (
Return(sharedtypes.Service{}, false).
AnyTimes()

relayMiningDifficulty := servicekeeper.NewDefaultRelayMiningDifficulty(sdkCtx, log.NewNopLogger(), service.Id, servicekeeper.TargetNumRelays)
targetNumRelays := servicetypes.DefaultTargetNumRelays
relayMiningDifficulty := servicekeeper.NewDefaultRelayMiningDifficulty(
sdkCtx,
log.NewNopLogger(),
service.Id,
targetNumRelays,
targetNumRelays,
)
mockServiceKeeper.EXPECT().
GetRelayMiningDifficulty(gomock.Any(), gomock.Any()).
Return(relayMiningDifficulty, true).
Expand Down
10 changes: 8 additions & 2 deletions x/proof/keeper/msg_server_create_claim_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -174,8 +174,14 @@ func TestMsgServer_CreateClaim_Success(t *testing.T) {
claimCreatedEvents := testutilevents.FilterEvents[*prooftypes.EventClaimCreated](t, events)
require.Len(t, claimCreatedEvents, 1)

targetNumRelays := servicekeeper.TargetNumRelays
relayMiningDifficulty := servicekeeper.NewDefaultRelayMiningDifficulty(ctx, keepers.Logger(), service.Id, targetNumRelays)
targetNumRelays := keepers.ServiceKeeper.GetParams(ctx).TargetNumRelays
relayMiningDifficulty := servicekeeper.NewDefaultRelayMiningDifficulty(
ctx,
keepers.Logger(),
service.Id,
targetNumRelays,
targetNumRelays,
)

numEstimatedComputUnits, err := claim.GetNumEstimatedComputeUnits(relayMiningDifficulty)
require.NoError(t, err)
Expand Down
10 changes: 8 additions & 2 deletions x/proof/keeper/msg_server_submit_proof_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -235,8 +235,14 @@ func TestMsgServer_SubmitProof_Success(t *testing.T) {

proofSubmittedEvent := proofSubmittedEvents[0]

targetNumRelays := servicekeeper.TargetNumRelays
relayMiningDifficulty := servicekeeper.NewDefaultRelayMiningDifficulty(ctx, keepers.Logger(), service.Id, targetNumRelays)
targetNumRelays := keepers.ServiceKeeper.GetParams(ctx).TargetNumRelays
relayMiningDifficulty := servicekeeper.NewDefaultRelayMiningDifficulty(
ctx,
keepers.Logger(),
service.Id,
targetNumRelays,
targetNumRelays,
)

numEstimatedComputUnits, err := claim.GetNumEstimatedComputeUnits(relayMiningDifficulty)
require.NoError(t, err)
Expand Down
9 changes: 8 additions & 1 deletion x/proof/keeper/proof_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -782,7 +782,14 @@ func setRelayMiningDifficultyHash(
targetHash []byte,
logger log.Logger,
) {
relayMiningDifficulty := servicekeeper.NewDefaultRelayMiningDifficulty(ctx, logger, serviceId, servicekeeper.TargetNumRelays)
targetNumRelays := serviceKeeper.GetParams(ctx).TargetNumRelays
relayMiningDifficulty := servicekeeper.NewDefaultRelayMiningDifficulty(
ctx,
logger,
serviceId,
targetNumRelays,
targetNumRelays,
)
relayMiningDifficulty.TargetHash = targetHash
serviceKeeper.SetRelayMiningDifficulty(ctx, relayMiningDifficulty)
}
1 change: 1 addition & 0 deletions x/proof/types/expected_keepers.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,4 +63,5 @@ type ServiceKeeper interface {
// Only used for testing & simulation
SetService(ctx context.Context, service sharedtypes.Service)
SetRelayMiningDifficulty(ctx context.Context, relayMiningDifficulty servicetypes.RelayMiningDifficulty)
GetParams(ctx context.Context) servicetypes.Params
}
11 changes: 9 additions & 2 deletions x/service/keeper/relay_mining_difficulty.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,18 @@ func (k Keeper) GetRelayMiningDifficulty(

difficultyBz := store.Get(types.RelayMiningDifficultyKey(serviceId))
if difficultyBz == nil {
targetNumRelays := k.GetParams(ctx).TargetNumRelays
k.Logger().Warn(fmt.Sprintf(
"relayMiningDifficulty not found for service: %s, defaulting to base difficulty with protocol TargetNumRelays (%d)",
serviceId, TargetNumRelays,
serviceId, targetNumRelays,
))
difficulty = NewDefaultRelayMiningDifficulty(ctx, k.logger, serviceId, TargetNumRelays)
difficulty = NewDefaultRelayMiningDifficulty(
ctx,
k.logger,
serviceId,
targetNumRelays,
targetNumRelays,
)
return difficulty, false
}

Expand Down
23 changes: 11 additions & 12 deletions x/service/keeper/update_relay_mining_difficulty.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,6 @@ import (
)

var (
// TargetNumRelays is the target number of relays we want the network to mine for
// a specific service across all applications & suppliers per session.
// This number determines the total number of leafs to be created across in
// the off-chain SMTs, across all suppliers, for each service.
// It indirectly drives the off-chain resource requirements of the network
// in additional to playing a critical role in Relay Mining.
// TODO_MAINNET(#542): Make this a governance parameter and figure out the correct value.
TargetNumRelays = uint64(10e4)

// Exponential moving average (ema) smoothing factor, commonly known as alpha.
// Usually, alpha = 2 / (N+1), where N is the number of periods.
// Large alpha -> more weight on recent data; less smoothing and fast response.
Expand Down Expand Up @@ -53,12 +44,19 @@ func (k Keeper) UpdateRelayMiningDifficulty(
// Iterate over the relaysPerServiceMap deterministically by sorting the keys.
// This ensures that the order of the keys is consistent across different nodes.
// See comment: https://github.com/pokt-network/poktroll/pull/840#discussion_r1796663285
targetNumRelays := k.GetParams(ctx).TargetNumRelays
sortedRelayPerServiceMapKeys := getSortedMapKeys(relaysPerServiceMap)
for _, serviceId := range sortedRelayPerServiceMapKeys {
numRelays := relaysPerServiceMap[serviceId]
prevDifficulty, found := k.GetRelayMiningDifficulty(ctx, serviceId)
if !found {
prevDifficulty = NewDefaultRelayMiningDifficulty(ctx, logger, serviceId, numRelays)
prevDifficulty = NewDefaultRelayMiningDifficulty(
ctx,
logger,
serviceId,
numRelays,
Copy link
Contributor Author

@bryanchriswhite bryanchriswhite Nov 21, 2024

Choose a reason for hiding this comment

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

Not sure if we need to make this change:

https://discord.com/channels/824324475256438814/1138895490331705354/1307077093351362621

Suggested change
numRelays,
targetNumRelays,

(cc @red-0ne )

Copy link
Contributor Author

@bryanchriswhite bryanchriswhite Nov 21, 2024

Choose a reason for hiding this comment

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

This change causes some test failures:

=== RUN   TestComputeNewDifficultyHash_MonotonicallyIncreasingRelays
    update_relay_mining_difficulty_test.go:45: Relay Mining Increasing Difficult Debug. NumRelays: 1000, EMA: 90099, TargetHash: ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
    update_relay_mining_difficulty_test.go:34: 
        	Error Trace:	/home/bwhite/Projects/pokt/poktroll/x/service/keeper/update_relay_mining_difficulty_test.go:34
        	Error:      	"10000" is not greater than "90099"
        	Test:       	TestComputeNewDifficultyHash_MonotonicallyIncreasingRelays
--- FAIL: TestComputeNewDifficultyHash_MonotonicallyIncreasingRelays (0.00s)
=== RUN   TestUpdateRelayMiningDifficulty_Base
    update_relay_mining_difficulty_test.go:105: 
        	Error Trace:	/home/bwhite/Projects/pokt/poktroll/x/service/keeper/update_relay_mining_difficulty_test.go:105
        	Error:      	Not equal: 
        	            	expected: 0x3e8
        	            	actual  : 0x15ff3
        	Test:       	TestUpdateRelayMiningDifficulty_Base
--- FAIL: TestUpdateRelayMiningDifficulty_Base (0.00s)

Expected :0x3e8
Actual   :0x15ff3
=== RUN   TestUpdateRelayMiningDifficulty_FirstDifficulty/First_Difficulty_way_below_target
    update_relay_mining_difficulty_test.go:217: 
        	Error Trace:	/home/bwhite/Projects/pokt/poktroll/x/service/keeper/update_relay_mining_difficulty_test.go:217
        	Error:      	Not equal: 
        	            	expected: 0x64
        	            	actual  : 0x15f99
        	Test:       	TestUpdateRelayMiningDifficulty_FirstDifficulty/First_Difficulty_way_below_target
--- FAIL: TestUpdateRelayMiningDifficulty_FirstDifficulty/First_Difficulty_way_below_target (0.00s)

Expected :0x64
Actual   :0x15f99
=== RUN   TestUpdateRelayMiningDifficulty_FirstDifficulty/First_Difficulty_way_above_target
    update_relay_mining_difficulty_test.go:217: 
        	Error Trace:	/home/bwhite/Projects/pokt/poktroll/x/service/keeper/update_relay_mining_difficulty_test.go:217
        	Error:      	Not equal: 
        	            	expected: 0x5f5e100
        	            	actual  : 0x99f610
        	Test:       	TestUpdateRelayMiningDifficulty_FirstDifficulty/First_Difficulty_way_above_target
--- FAIL: TestUpdateRelayMiningDifficulty_FirstDifficulty/First_Difficulty_way_above_target (0.00s)

Expected :0x5f5e100
Actual   :0x99f610

targetNumRelays,
)
}

// TODO_MAINNET(@Olshansk): We could potentially compute the smoothing factor
Expand All @@ -83,7 +81,7 @@ func (k Keeper) UpdateRelayMiningDifficulty(
// We kept scaling down even though numRelaysEma was decreasing.
// To avoid continuing to increase the difficulty (i.e. scaling down), the
// relative starting difficulty has to be kept constant.
difficultyHash := protocol.ComputeNewDifficultyTargetHash(protocol.BaseRelayDifficultyHashBz, TargetNumRelays, newRelaysEma)
difficultyHash := protocol.ComputeNewDifficultyTargetHash(protocol.BaseRelayDifficultyHashBz, targetNumRelays, newRelaysEma)
newDifficulty := types.RelayMiningDifficulty{
ServiceId: serviceId,
BlockHeight: sdkCtx.BlockHeight(),
Expand Down Expand Up @@ -152,11 +150,12 @@ func NewDefaultRelayMiningDifficulty(
logger log.Logger,
serviceId string,
numRelays uint64,
targetNumRelays uint64,
) types.RelayMiningDifficulty {
logger = logger.With("helper", "NewDefaultRelayMiningDifficulty")

// Compute the target hash based on the number of relays seen for the first time.
newDifficultyHash := protocol.ComputeNewDifficultyTargetHash(protocol.BaseRelayDifficultyHashBz, TargetNumRelays, numRelays)
newDifficultyHash := protocol.ComputeNewDifficultyTargetHash(protocol.BaseRelayDifficultyHashBz, targetNumRelays, numRelays)

logger.Warn(types.ErrServiceMissingRelayMiningDifficulty.Wrapf(
"No previous relay mining difficulty found for service %s.\n"+
Expand Down
16 changes: 8 additions & 8 deletions x/service/keeper/update_relay_mining_difficulty_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import (
"github.com/pokt-network/poktroll/pkg/crypto/protocol"
testutilevents "github.com/pokt-network/poktroll/testutil/events"
keepertest "github.com/pokt-network/poktroll/testutil/keeper"
servicekeeper "github.com/pokt-network/poktroll/x/service/keeper"
servicetypes "github.com/pokt-network/poktroll/x/service/types"
)

Expand Down Expand Up @@ -150,7 +149,8 @@ func TestUpdateRelayMiningDifficulty_Base(t *testing.T) {
require.True(t, found)
require.Less(t, difficultySvc22.NumRelaysEma, difficultySvc21.NumRelaysEma)
// Since the relays EMA is lower than the target, the difficulty hash is all 1s
require.Less(t, difficultySvc22.NumRelaysEma, servicekeeper.TargetNumRelays)
targetNumRelays := keeper.GetParams(ctx).TargetNumRelays
require.Less(t, difficultySvc22.NumRelaysEma, targetNumRelays)
require.Equal(t, difficultySvc22.TargetHash, makeBytesFullOfOnes(32))

// svc3 is new so the relay ema is equal to the first value provided
Expand All @@ -172,29 +172,29 @@ func TestUpdateRelayMiningDifficulty_FirstDifficulty(t *testing.T) {
}{
{
desc: "First Difficulty way below target",
numRelays: servicekeeper.TargetNumRelays / 1e3,
numRelays: servicetypes.DefaultTargetNumRelays / 1e3,
expectedRelayMiningDifficulty: servicetypes.RelayMiningDifficulty{
ServiceId: "svc1",
BlockHeight: 1,
NumRelaysEma: servicekeeper.TargetNumRelays / 1e3,
NumRelaysEma: servicetypes.DefaultTargetNumRelays / 1e3,
TargetHash: defaultDifficulty(), // default difficulty without any leading 0 bits
},
}, {
desc: "First Difficulty equal to target",
numRelays: servicekeeper.TargetNumRelays,
numRelays: servicetypes.DefaultTargetNumRelays,
expectedRelayMiningDifficulty: servicetypes.RelayMiningDifficulty{
ServiceId: "svc1",
BlockHeight: 1,
NumRelaysEma: servicekeeper.TargetNumRelays,
NumRelaysEma: servicetypes.DefaultTargetNumRelays,
TargetHash: defaultDifficulty(), // default difficulty without any leading 0 bits
},
}, {
desc: "First Difficulty way above target",
numRelays: servicekeeper.TargetNumRelays * 1e3,
numRelays: servicetypes.DefaultTargetNumRelays * 1e3,
expectedRelayMiningDifficulty: servicetypes.RelayMiningDifficulty{
ServiceId: "svc1",
BlockHeight: 1,
NumRelaysEma: servicekeeper.TargetNumRelays * 1e3,
NumRelaysEma: servicetypes.DefaultTargetNumRelays * 1e3,
TargetHash: append(
[]byte{0b00000000}, // at least 8 leading 0 bits
makeBytesFullOfOnes(31)...,
Expand Down
9 changes: 8 additions & 1 deletion x/tokenomics/keeper/keeper_settle_pending_claims_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,14 @@ func (s *TestSuite) SetupTest() {
// Calculate the number of claimed compute units.
s.numClaimedComputeUnits = s.numRelays * service.ComputeUnitsPerRelay

s.relayMiningDifficulty = servicekeeper.NewDefaultRelayMiningDifficulty(sdkCtx, s.keepers.Logger(), testServiceId, servicekeeper.TargetNumRelays)
targetNumRelays := s.keepers.ServiceKeeper.GetParams(sdkCtx).TargetNumRelays
s.relayMiningDifficulty = servicekeeper.NewDefaultRelayMiningDifficulty(
sdkCtx,
s.keepers.Logger(),
testServiceId,
targetNumRelays,
targetNumRelays,
)

// Calculate the number of estimated compute units.
s.numEstimatedComputeUnits = getEstimatedComputeUnits(s.numClaimedComputeUnits, s.relayMiningDifficulty)
Expand Down
9 changes: 8 additions & 1 deletion x/tokenomics/keeper/settle_pending_claims.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,14 @@ func (k Keeper) SettlePendingClaims(ctx cosmostypes.Context) (
serviceId := claim.GetSessionHeader().GetServiceId()
relayMiningDifficulty, found := k.serviceKeeper.GetRelayMiningDifficulty(ctx, serviceId)
if !found {
relayMiningDifficulty = servicekeeper.NewDefaultRelayMiningDifficulty(ctx, logger, serviceId, servicekeeper.TargetNumRelays)
targetNumRelays := k.serviceKeeper.GetParams(ctx).TargetNumRelays
relayMiningDifficulty = servicekeeper.NewDefaultRelayMiningDifficulty(
ctx,
logger,
serviceId,
targetNumRelays,
targetNumRelays,
)
}
// numEstimatedComputeUnits is the probabilistic estimation of the off-chain
// work done by the relay miner in this session. It is derived from the claimed
Expand Down
9 changes: 8 additions & 1 deletion x/tokenomics/keeper/token_logic_modules.go
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,14 @@ func (k Keeper) ProcessTokenLogicModules(
// Retrieving the relay mining difficulty for service.
relayMiningDifficulty, found := k.serviceKeeper.GetRelayMiningDifficulty(ctx, service.Id)
if !found {
relayMiningDifficulty = servicekeeper.NewDefaultRelayMiningDifficulty(ctx, logger, service.Id, servicekeeper.TargetNumRelays)
targetNumRelays := k.serviceKeeper.GetParams(ctx).TargetNumRelays
relayMiningDifficulty = servicekeeper.NewDefaultRelayMiningDifficulty(
ctx,
logger,
service.Id,
targetNumRelays,
targetNumRelays,
)
}
sharedParams := k.sharedKeeper.GetParams(ctx)

Expand Down
2 changes: 2 additions & 0 deletions x/tokenomics/types/expected_keepers.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,4 +94,6 @@ type ServiceKeeper interface {
UpdateRelayMiningDifficulty(ctx context.Context, relaysPerServiceMap map[string]uint64) (map[string]servicetypes.RelayMiningDifficulty, error)
// Only used for testing & simulation
SetService(ctx context.Context, service sharedtypes.Service)
GetParams(ctx context.Context) servicetypes.Params
SetParams(ctx context.Context, params servicetypes.Params) error
}
Loading