Skip to content

Commit

Permalink
[Service] refactor: TargetNumRelays var usage to param usage (#943)
Browse files Browse the repository at this point in the history
## Summary

Replace usage of `servicekeeper.TargetNumRelays` with the new param and
delete it.

## Issue

- `TODO_BETA`

## Type of change

Select one or more from the following:

- [ ] New feature, functionality or library
- [ ] Consensus breaking; add the `consensus-breaking` label if so. See
#791 for details
- [ ] Bug fix
- [x] Code health or cleanup
- [ ] Documentation
- [ ] Other (specify)

## Testing

- [ ] **Documentation**: `make docusaurus_start`; only needed if you
make doc changes
- [x] **Unit Tests**: `make go_develop_and_test`
- [ ] **LocalNet E2E Tests**: `make test_e2e`
- [x] **DevNet E2E Tests**: Add the `devnet-test-e2e` label to the PR.

## Sanity Checklist

- [x] I have tested my changes using the available tooling
- [ ] I have commented my code
- [x] I have performed a self-review of my own code; both comments &
source code
- [ ] I create and reference any new tickets, if applicable
- [ ] I have left TODOs throughout the codebase, if applicable
  • Loading branch information
bryanchriswhite authored Nov 27, 2024
1 parent ce94775 commit 9ae1129
Show file tree
Hide file tree
Showing 14 changed files with 102 additions and 44 deletions.
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,
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
}

0 comments on commit 9ae1129

Please sign in to comment.