From a604ee549b964654524e4ba9c744b9337ff73076 Mon Sep 17 00:00:00 2001 From: Arash Deshmeh Date: Tue, 13 Aug 2024 10:06:41 -0400 Subject: [PATCH 01/36] Check the number of claimed compute units against the service --- testutil/keeper/proof.go | 7 + testutil/keeper/tokenomics.go | 1 + x/proof/keeper/keeper.go | 3 + x/proof/keeper/msg_server_create_claim.go | 48 ++++-- .../keeper/msg_server_create_claim_test.go | 148 +++++++++++++++++- x/proof/keeper/service.go | 27 ++++ x/proof/module/module.go | 2 + x/proof/types/errors.go | 2 + x/proof/types/expected_keepers.go | 9 +- 9 files changed, 226 insertions(+), 21 deletions(-) create mode 100644 x/proof/keeper/service.go diff --git a/testutil/keeper/proof.go b/testutil/keeper/proof.go index 80049be35..1a322a45d 100644 --- a/testutil/keeper/proof.go +++ b/testutil/keeper/proof.go @@ -61,6 +61,7 @@ type ProofModuleKeepers struct { prooftypes.ApplicationKeeper prooftypes.AccountKeeper prooftypes.SharedKeeper + prooftypes.ServiceKeeper Codec *codec.ProtoCodec } @@ -89,6 +90,7 @@ func ProofKeeper(t testing.TB) (keeper.Keeper, context.Context) { mockAppKeeper := mocks.NewMockApplicationKeeper(ctrl) mockAccountKeeper := mocks.NewMockAccountKeeper(ctrl) mockSharedKeeper := mocks.NewMockSharedKeeper(ctrl) + mockServiceKeeper := mocks.NewMockServiceKeeper(ctrl) k := keeper.NewKeeper( cdc, @@ -99,6 +101,7 @@ func ProofKeeper(t testing.TB) (keeper.Keeper, context.Context) { mockAppKeeper, mockAccountKeeper, mockSharedKeeper, + mockServiceKeeper, ) ctx := sdk.NewContext(stateStore, cmtproto.Header{}, false, log.NewNopLogger()) @@ -228,6 +231,7 @@ func NewProofModuleKeepers(t testing.TB, opts ...ProofKeepersOpt) (_ *ProofModul appKeeper, accountKeeper, sharedKeeper, + serviceKeeper, ) require.NoError(t, proofKeeper.SetParams(ctx, types.DefaultParams())) @@ -238,6 +242,7 @@ func NewProofModuleKeepers(t testing.TB, opts ...ProofKeepersOpt) (_ *ProofModul ApplicationKeeper: &appKeeper, AccountKeeper: &accountKeeper, SharedKeeper: &sharedKeeper, + ServiceKeeper: &serviceKeeper, Codec: cdc, } @@ -274,6 +279,8 @@ func (keepers *ProofModuleKeepers) AddServiceActors( {Service: service}, }, }) + + keepers.SetService(ctx, *service) } // GetSessionHeader is a helper to retrieve the session header diff --git a/testutil/keeper/tokenomics.go b/testutil/keeper/tokenomics.go index a86f20c27..8d922891d 100644 --- a/testutil/keeper/tokenomics.go +++ b/testutil/keeper/tokenomics.go @@ -394,6 +394,7 @@ func NewTokenomicsModuleKeepers( appKeeper, accountKeeper, sharedKeeper, + serviceKeeper, ) require.NoError(t, proofKeeper.SetParams(sdkCtx, prooftypes.DefaultParams())) diff --git a/x/proof/keeper/keeper.go b/x/proof/keeper/keeper.go index e7bbc9475..9e9400828 100644 --- a/x/proof/keeper/keeper.go +++ b/x/proof/keeper/keeper.go @@ -32,6 +32,7 @@ type ( applicationKeeper types.ApplicationKeeper accountKeeper types.AccountKeeper sharedKeeper types.SharedKeeper + serviceKeeper types.ServiceKeeper ringClient crypto.RingClient accountQuerier client.AccountQueryClient @@ -49,6 +50,7 @@ func NewKeeper( applicationKeeper types.ApplicationKeeper, accountKeeper types.AccountKeeper, sharedKeeper types.SharedKeeper, + serviceKeeper types.ServiceKeeper, ) Keeper { if _, err := sdk.AccAddressFromBech32(authority); err != nil { panic(fmt.Sprintf("invalid authority address: %s", authority)) @@ -91,6 +93,7 @@ func NewKeeper( applicationKeeper: applicationKeeper, accountKeeper: accountKeeper, sharedKeeper: sharedKeeper, + serviceKeeper: serviceKeeper, ringClient: ringKeeperClient, accountQuerier: accountQuerier, diff --git a/x/proof/keeper/msg_server_create_claim.go b/x/proof/keeper/msg_server_create_claim.go index eccf4a324..7f8f5de6a 100644 --- a/x/proof/keeper/msg_server_create_claim.go +++ b/x/proof/keeper/msg_server_create_claim.go @@ -2,6 +2,7 @@ package keeper import ( "context" + "fmt" cosmostypes "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/gogoproto/proto" @@ -19,10 +20,10 @@ func (k msgServer) CreateClaim( ) (_ *types.MsgCreateClaimResponse, err error) { // Declare claim to reference in telemetry. var ( - claim types.Claim - isExistingClaim bool - numRelays uint64 - numComputeUnits uint64 + claim types.Claim + isExistingClaim bool + numRelays uint64 + numClaimComputeUnits uint64 ) // Defer telemetry calls so that they reference the final values the relevant variables. @@ -31,7 +32,7 @@ func (k msgServer) CreateClaim( if !isExistingClaim { telemetry.ClaimCounter(types.ClaimProofStage_CLAIMED, 1, err) telemetry.ClaimRelaysCounter(types.ClaimProofStage_CLAIMED, numRelays, err) - telemetry.ClaimComputeUnitsCounter(types.ClaimProofStage_CLAIMED, numComputeUnits, err) + telemetry.ClaimComputeUnitsCounter(types.ClaimProofStage_CLAIMED, numClaimComputeUnits, err) } }() @@ -77,17 +78,36 @@ func (k msgServer) CreateClaim( if err != nil { return nil, status.Error(codes.Internal, types.ErrProofInvalidClaimRootHash.Wrap(err.Error()).Error()) } - numComputeUnits, err = claim.GetNumComputeUnits() + + numClaimComputeUnits, err = claim.GetNumComputeUnits() if err != nil { return nil, status.Error(codes.Internal, types.ErrProofInvalidClaimRootHash.Wrap(err.Error()).Error()) } - _, isExistingClaim = k.Keeper.GetClaim(ctx, claim.GetSessionHeader().GetSessionId(), claim.GetSupplierAddress()) - // TODO_UPNEXT(#705): Check (and test) that numClaimComputeUnits is equal - // to num_relays * the_compute_units_per_relay for this_service. - // Add a comment that for now, we expect it to be the case because every - // relay for a specific service is wroth the same, but may change in the - // future. + // For now we expect the following equation to always hold: + // numClaimComputeUnits = numRelays * service.computeUnitsPerRelay + // This is because for any specific service, every relay is worth the same. + // However, this may change in the future. + serviceCupr, err := k.getServiceCupr(ctx, claim.SessionHeader.Service.Id) + if err != nil { + return nil, status.Error(codes.Internal, types.ErrProofServiceNotFound.Wrap(err.Error()).Error()) + } + + if numClaimComputeUnits != numRelays*serviceCupr { + return nil, status.Error( + codes.InvalidArgument, + types.ErrProofComputeUnitsMismatch.Wrap( + fmt.Sprintf("claim compute units: %d is not equal to number of relays %d * compute units per relay %d for service %s", + numClaimComputeUnits, + numRelays, + serviceCupr, + claim.SessionHeader.Service.Id, + ), + ).Error(), + ) + } + + _, isExistingClaim = k.Keeper.GetClaim(ctx, claim.GetSessionHeader().GetSessionId(), claim.GetSupplierAddress()) // Upsert the claim k.Keeper.UpsertClaim(ctx, claim) @@ -101,7 +121,7 @@ func (k msgServer) CreateClaim( &types.EventClaimUpdated{ Claim: &claim, NumRelays: numRelays, - NumComputeUnits: numComputeUnits, + NumComputeUnits: numClaimComputeUnits, }, ) case false: @@ -109,7 +129,7 @@ func (k msgServer) CreateClaim( &types.EventClaimCreated{ Claim: &claim, NumRelays: numRelays, - NumComputeUnits: numComputeUnits, + NumComputeUnits: numClaimComputeUnits, }, ) } diff --git a/x/proof/keeper/msg_server_create_claim_test.go b/x/proof/keeper/msg_server_create_claim_test.go index fb3f7f4bd..e88010005 100644 --- a/x/proof/keeper/msg_server_create_claim_test.go +++ b/x/proof/keeper/msg_server_create_claim_test.go @@ -26,9 +26,17 @@ const ( expectedNumRelays = 10 computeUnitsPerRelay = 1 expectedNumComputeUnits = expectedNumRelays * computeUnitsPerRelay + + nonDefaultComputeUnitsPerRelay = 9999 + expectedNonDefaultNumComputeUnits = expectedNumRelays * nonDefaultComputeUnitsPerRelay ) -var defaultMerkleRoot = testproof.SmstRootWithSumAndCount(expectedNumComputeUnits, expectedNumRelays) +var ( + defaultMerkleRoot = testproof.SmstRootWithSumAndCount(expectedNumComputeUnits, expectedNumRelays) + + // Merkle root for Smst of a claim for the service with non-default custom units per relay + customCuprMerkleRoot = testproof.SmstRootWithSumAndCount(expectedNonDefaultNumComputeUnits, expectedNumRelays) +) func TestMsgServer_CreateClaim_Success(t *testing.T) { var claimWindowOpenBlockHash []byte @@ -40,6 +48,10 @@ func TestMsgServer_CreateClaim_Success(t *testing.T) { sharedParams *sharedtypes.Params, queryHeight int64, ) int64 + merkleRoot smt.MerkleSumRoot + // The Compute Units Per Relay for the service used in the test. + serviceComputeUnitsPerRelay uint64 + expectedNumComputeUnits uint64 }{ { desc: "claim message height equals supplier's earliest claim commit height", @@ -51,10 +63,23 @@ func TestMsgServer_CreateClaim_Success(t *testing.T) { supplierAddr, ) }, + merkleRoot: defaultMerkleRoot, + serviceComputeUnitsPerRelay: computeUnitsPerRelay, + expectedNumComputeUnits: expectedNumComputeUnits, + }, + { + desc: "claim message height equals claim window close height", + getClaimMsgHeight: shared.GetClaimWindowCloseHeight, + merkleRoot: defaultMerkleRoot, + serviceComputeUnitsPerRelay: computeUnitsPerRelay, + expectedNumComputeUnits: expectedNumComputeUnits, }, { - desc: "claim message height equals claim window close height", - getClaimMsgHeight: shared.GetClaimWindowCloseHeight, + desc: "claim message for service with >1 compute units per relay", + getClaimMsgHeight: shared.GetClaimWindowCloseHeight, + merkleRoot: customCuprMerkleRoot, + serviceComputeUnitsPerRelay: nonDefaultComputeUnitsPerRelay, + expectedNumComputeUnits: expectedNonDefaultNumComputeUnits, }, } @@ -75,7 +100,7 @@ func TestMsgServer_CreateClaim_Success(t *testing.T) { service := &sharedtypes.Service{ Id: testServiceId, - ComputeUnitsPerRelay: computeUnitsPerRelay, + ComputeUnitsPerRelay: test.serviceComputeUnitsPerRelay, OwnerAddress: sample.AccAddress(), } appAddr := sample.AccAddress() @@ -94,6 +119,8 @@ func TestMsgServer_CreateClaim_Success(t *testing.T) { }, }) + keepers.SetService(ctx, *service) + sessionRes, err := keepers.GetSession( ctx, &sessiontypes.QueryGetSessionRequest{ @@ -119,7 +146,7 @@ func TestMsgServer_CreateClaim_Success(t *testing.T) { supplierAddr, appAddr, service, - defaultMerkleRoot, + test.merkleRoot, ) createClaimRes, err := srv.CreateClaim(ctx, claimMsg) require.NoError(t, err) @@ -153,7 +180,7 @@ func TestMsgServer_CreateClaim_Success(t *testing.T) { require.Truef(t, ok, "unexpected event type %T", event) require.EqualValues(t, &claim, claimCreatedEvent.GetClaim()) - require.Equal(t, uint64(expectedNumComputeUnits), claimCreatedEvent.GetNumComputeUnits()) + require.Equal(t, uint64(test.expectedNumComputeUnits), claimCreatedEvent.GetNumComputeUnits()) require.Equal(t, uint64(expectedNumRelays), claimCreatedEvent.GetNumRelays()) }) } @@ -370,6 +397,7 @@ func TestMsgServer_CreateClaim_Error(t *testing.T) { }, ) require.NoError(t, err) + require.NoError(t, err) require.NotNil(t, sessionRes) require.Equal(t, appAddr, sessionRes.GetSession().GetApplication().GetAddress()) @@ -508,6 +536,114 @@ func TestMsgServer_CreateClaim_Error(t *testing.T) { } } +func TestMsgServer_CreateClaim_Error_ComputeUnitsMismatch(t *testing.T) { + // Set block height to 1 so there is a valid session on-chain. + blockHeightOpt := keepertest.WithBlockHeight(1) + keepers, ctx := keepertest.NewProofModuleKeepers(t, blockHeightOpt) + sdkCtx := cosmostypes.UnwrapSDKContext(ctx) + srv := keeper.NewMsgServerImpl(*keepers.Keeper) + + // The base session start height used for testing + sessionStartHeight := int64(1) + // service is the only service for which a session should exist. + // this service has a value of greater than 1 for the compute units per relay. + service := &sharedtypes.Service{ + Id: testServiceId, + ComputeUnitsPerRelay: nonDefaultComputeUnitsPerRelay, + OwnerAddress: sample.AccAddress(), + } + // supplierAddr is staked for "svc1" such that it is expected to be in the session. + supplierAddr := sample.AccAddress() + + // appAddr is staked for "svc1" such that it is expected to be in the session. + appAddr := sample.AccAddress() + + supplierKeeper := keepers.SupplierKeeper + appKeeper := keepers.ApplicationKeeper + + // Add a supplier that is expected to be in the session. + supplierKeeper.SetSupplier(ctx, sharedtypes.Supplier{ + Address: supplierAddr, + Services: []*sharedtypes.SupplierServiceConfig{ + {Service: service}, + }, + }) + + // Add an application that is expected to be in the session. + appKeeper.SetApplication(ctx, apptypes.Application{ + Address: appAddr, + ServiceConfigs: []*sharedtypes.ApplicationServiceConfig{ + {Service: service}, + }, + }) + + // Add the service that is expected to be on-chain. + keepers.SetService(ctx, *service) + + // Query for the session which contains the expected app and supplier pair. + sessionRes, err := keepers.SessionKeeper.GetSession( + ctx, + &sessiontypes.QueryGetSessionRequest{ + ApplicationAddress: appAddr, + Service: service, + BlockHeight: 1, + }, + ) + require.NoError(t, err) + require.NotNil(t, sessionRes) + require.Equal(t, appAddr, sessionRes.GetSession().GetApplication().GetAddress()) + + sessionResSuppliers := sessionRes.GetSession().GetSuppliers() + require.NotEmpty(t, sessionResSuppliers) + require.Equal(t, supplierAddr, sessionResSuppliers[0].GetAddress()) + + // Increment the block height to the test claim height. + sessionHeader := sessionRes.GetSession().GetHeader() + sharedParams := keepers.SharedKeeper.GetParams(ctx) + testClaimHeight := shared.GetClaimWindowCloseHeight(&sharedParams, sessionHeader.GetSessionEndBlockHeight()) + sdkCtx = sdkCtx.WithBlockHeight(testClaimHeight) + ctx = sdkCtx + + testClaimMsg := newTestClaimMsg(t, + sessionStartHeight, + sessionRes.GetSession().GetSessionId(), + supplierAddr, + appAddr, + service, + defaultMerkleRoot, + ) + + // use the test claim message to create a claim object to get the number of relays and compute units in the claim. + testClaim := types.Claim{RootHash: testClaimMsg.GetRootHash()} + testClaimNumComputeUnits, err := testClaim.GetNumComputeUnits() + require.NoError(t, err) + testClaimNumRelays, err := testClaim.GetNumRelays() + require.NoError(t, err) + + createClaimRes, err := srv.CreateClaim(ctx, testClaimMsg) + + require.ErrorContains(t, + err, + status.Error( + codes.InvalidArgument, + types.ErrProofComputeUnitsMismatch.Wrapf( + "claim compute units: %d is not equal to number of relays %d * compute units per relay %d for service %s", + testClaimNumComputeUnits, + testClaimNumRelays, + nonDefaultComputeUnitsPerRelay, + sessionHeader.Service.Id, + ).Error(), + ).Error(), + ) + + require.Nil(t, createClaimRes) + + // Assert that no events were emitted. + sdkCtx = cosmostypes.UnwrapSDKContext(ctx) + events := sdkCtx.EventManager().Events() + require.Equal(t, 0, len(events)) +} + func newTestClaimMsg( t *testing.T, sessionStartHeight int64, diff --git a/x/proof/keeper/service.go b/x/proof/keeper/service.go new file mode 100644 index 000000000..4ab969424 --- /dev/null +++ b/x/proof/keeper/service.go @@ -0,0 +1,27 @@ +package keeper + +import ( + "context" + "fmt" +) + +// getServiceCupr ensures that a service with the ServiceID of the given service +// exists. +// It returns the compute units per relay for the service with the given id. +func (k Keeper) getServiceCupr( + ctx context.Context, + serviceId string, +) (uint64, error) { + logger := k.Logger().With("method", "getServiceCupr") + + service, found := k.serviceKeeper.GetService(ctx, serviceId) + if !found { + return 0, fmt.Errorf("service %s not found", serviceId) + } + + logger. + With("service_id", serviceId). + Debug("got service for proof") + + return service.ComputeUnitsPerRelay, nil +} diff --git a/x/proof/module/module.go b/x/proof/module/module.go index a96341571..2c35e02ed 100644 --- a/x/proof/module/module.go +++ b/x/proof/module/module.go @@ -176,6 +176,7 @@ type ModuleInputs struct { ApplicationKeeper types.ApplicationKeeper AccountKeeper types.AccountKeeper SharedKeeper types.SharedKeeper + ServiceKeeper types.ServiceKeeper } type ModuleOutputs struct { @@ -200,6 +201,7 @@ func ProvideModule(in ModuleInputs) ModuleOutputs { in.ApplicationKeeper, in.AccountKeeper, in.SharedKeeper, + in.ServiceKeeper, ) m := NewAppModule( in.Cdc, diff --git a/x/proof/types/errors.go b/x/proof/types/errors.go index 7e9c64867..5b8e54e80 100644 --- a/x/proof/types/errors.go +++ b/x/proof/types/errors.go @@ -33,4 +33,6 @@ var ( ErrProofProofOutsideOfWindow = sdkerrors.Register(ModuleName, 1122, "proof attempted outside of the session's proof window") ErrProofSupplierMismatch = sdkerrors.Register(ModuleName, 1123, "supplier address does not match the claim or proof") ErrProofAccNotFound = sdkerrors.Register(ModuleName, 1124, "account not found") + ErrProofServiceNotFound = sdkerrors.Register(ModuleName, 1125, "service not found") + ErrProofComputeUnitsMismatch = sdkerrors.Register(ModuleName, 1126, "mismatch: claim compute units != number of relays * service compute units per relay") ) diff --git a/x/proof/types/expected_keepers.go b/x/proof/types/expected_keepers.go index f7eb8dd8f..f29cded45 100644 --- a/x/proof/types/expected_keepers.go +++ b/x/proof/types/expected_keepers.go @@ -1,4 +1,4 @@ -//go:generate mockgen -destination=../../../testutil/proof/mocks/expected_keepers_mock.go -package=mocks . BankKeeper,SessionKeeper,ApplicationKeeper,AccountKeeper,SharedKeeper +//go:generate mockgen -destination=../../../testutil/proof/mocks/expected_keepers_mock.go -package=mocks . BankKeeper,SessionKeeper,ApplicationKeeper,AccountKeeper,SharedKeeper,ServiceKeeper package types @@ -53,3 +53,10 @@ type ApplicationKeeper interface { type SharedKeeper interface { GetParams(ctx context.Context) sharedtypes.Params } + +// ServiceKeeper defines the expected interface for the Service module. +type ServiceKeeper interface { + GetService(ctx context.Context, serviceID string) (sharedtypes.Service, bool) + // Only used for testing & simulation + SetService(ctx context.Context, service sharedtypes.Service) +} From faf83af6d5f261492785059879f455c3de40f0cd Mon Sep 17 00:00:00 2001 From: Arash Deshmeh Date: Mon, 19 Aug 2024 09:26:54 -0400 Subject: [PATCH 02/36] RelayMiner uses service ComputeUnitsPerRelay when adding relays to SMT --- pkg/client/interface.go | 20 +++++- pkg/client/query/errors.go | 1 + pkg/client/query/servicequerier.go | 62 +++++++++++++++++++ pkg/deps/config/suppliers.go | 33 ++++++++++ pkg/relayer/cmd/cmd.go | 3 +- pkg/relayer/miner/options.go | 10 --- pkg/relayer/session/service.go | 31 ++++++++++ pkg/relayer/session/session.go | 21 +++++-- pkg/relayer/session/session_test.go | 7 ++- .../testqueryclients/servicequerier.go | 60 ++++++++++++++++++ 10 files changed, 231 insertions(+), 17 deletions(-) create mode 100644 pkg/client/query/servicequerier.go delete mode 100644 pkg/relayer/miner/options.go create mode 100644 pkg/relayer/session/service.go create mode 100644 testutil/testclient/testqueryclients/servicequerier.go diff --git a/pkg/client/interface.go b/pkg/client/interface.go index a5cec3ae8..4688671bc 100644 --- a/pkg/client/interface.go +++ b/pkg/client/interface.go @@ -9,6 +9,8 @@ //go:generate mockgen -destination=../../testutil/mockclient/session_query_client_mock.go -package=mockclient . SessionQueryClient //go:generate mockgen -destination=../../testutil/mockclient/shared_query_client_mock.go -package=mockclient . SharedQueryClient //go:generate mockgen -destination=../../testutil/mockclient/proof_query_client_mock.go -package=mockclient . ProofQueryClient +//go:generate mockgen -destination=../../testutil/mockclient/tokenomics_query_client_mock.go -package=mockclient . TokenomicsQueryClient +//go:generate mockgen -destination=../../testutil/mockclient/service_query_client_mock.go -package=mockclient . ServiceQueryClient //go:generate mockgen -destination=../../testutil/mockclient/cosmos_tx_builder_mock.go -package=mockclient github.com/cosmos/cosmos-sdk/client TxBuilder //go:generate mockgen -destination=../../testutil/mockclient/cosmos_keyring_mock.go -package=mockclient github.com/cosmos/cosmos-sdk/crypto/keyring Keyring //go:generate mockgen -destination=../../testutil/mockclient/cosmos_client_mock.go -package=mockclient github.com/cosmos/cosmos-sdk/client AccountRetriever @@ -335,7 +337,6 @@ type BlockQueryClient interface { // protobuf message. Since the generated go types don't include interface types, this // is necessary to prevent dependency cycles. type ProofParams interface { - GetRelayDifficultyTargetHash() []byte GetProofRequestProbability() float32 GetProofRequirementThreshold() uint64 GetProofMissingPenalty() *cosmostypes.Coin @@ -347,3 +348,20 @@ type ProofQueryClient interface { // GetParams queries the chain for the current shared module parameters. GetParams(ctx context.Context) (ProofParams, error) } + +// TokenomicsRelayMiningDifficulty is a go interface type which corresponding to the poktroll.tokenomics.RelayMiningDifficulty +// protobuf message. This is necessary to prevent dependency cycles. +type TokenomicsRelayMiningDifficulty interface { + GetTargetHash() []byte +} + +type TokenomicsQueryClient interface { + GetServiceRelayDifficultyTargetHash(ctx context.Context, serviceId string) (TokenomicsRelayMiningDifficulty, error) +} + +// ServiceQueryClient defines an interface that enables the querying of the +// on-chain service information +type ServiceQueryClient interface { + // GetService queries the chain for the details of the service provided + GetService(ctx context.Context, serviceId string) (sharedtypes.Service, error) +} diff --git a/pkg/client/query/errors.go b/pkg/client/query/errors.go index 487b79734..19b4893bf 100644 --- a/pkg/client/query/errors.go +++ b/pkg/client/query/errors.go @@ -9,4 +9,5 @@ var ( ErrQueryRetrieveSession = sdkerrors.Register(codespace, 3, "error while trying to retrieve a session") ErrQueryPubKeyNotFound = sdkerrors.Register(codespace, 4, "account pub key not found") ErrQuerySessionParams = sdkerrors.Register(codespace, 5, "unable to query session params") + ErrQueryRetrieveService = sdkerrors.Register(codespace, 6, "error while trying to retrieve a service") ) diff --git a/pkg/client/query/servicequerier.go b/pkg/client/query/servicequerier.go new file mode 100644 index 000000000..9b5cdfc76 --- /dev/null +++ b/pkg/client/query/servicequerier.go @@ -0,0 +1,62 @@ +package query + +import ( + "context" + + "cosmossdk.io/depinject" + "github.com/cosmos/gogoproto/grpc" + + "github.com/pokt-network/poktroll/pkg/client" + servicetypes "github.com/pokt-network/poktroll/x/service/types" + sharedtypes "github.com/pokt-network/poktroll/x/shared/types" +) + +var _ client.ServiceQueryClient = (*serviceQuerier)(nil) + +// serviceQuerier is a wrapper around the servicetypes.QueryClient that enables the +// querying of on-chain service information through a single exposed method +// which returns a sharedtypes.Service struct +type serviceQuerier struct { + clientConn grpc.ClientConn + serviceQuerier servicetypes.QueryClient +} + +// NewServiceQuerier returns a new instance of a client.ServiceQueryClient by +// injecting the dependecies provided by the depinject.Config. +// +// Required dependencies: +// - clientCtx (grpc.ClientConn) +func NewServiceQuerier(deps depinject.Config) (client.ServiceQueryClient, error) { + servq := &serviceQuerier{} + + if err := depinject.Inject( + deps, + &servq.clientConn, + ); err != nil { + return nil, err + } + + servq.serviceQuerier = servicetypes.NewQueryClient(servq.clientConn) + + return servq, nil +} + +// GetService returns a sharedtypes.Service struct for a given serviceId. +// It implements the ServiceQueryClient#GetService function. +func (servq *serviceQuerier) GetService( + ctx context.Context, + serviceId string, +) (sharedtypes.Service, error) { + req := &servicetypes.QueryGetServiceRequest{ + Id: serviceId, + } + + res, err := servq.serviceQuerier.Service(ctx, req) + if err != nil { + return sharedtypes.Service{}, ErrQueryRetrieveService.Wrapf( + "serviceId: %s; error: [%v]", + serviceId, err, + ) + } + return res.Service, nil +} diff --git a/pkg/deps/config/suppliers.go b/pkg/deps/config/suppliers.go index b00b7f54e..680d98171 100644 --- a/pkg/deps/config/suppliers.go +++ b/pkg/deps/config/suppliers.go @@ -464,6 +464,39 @@ func NewSupplyProofQueryClientFn() SupplierFn { } } +// NewSupplyTokenomicsQueryClientFn returns a function which constructs a +// TokenomicsQueryClient instance and returns a new depinject.Config which +// is supplied with the given deps and the new TokenomicsQueryClient. +func NewSupplyTokenomicsQueryClientFn() SupplierFn { + return func( + _ context.Context, + deps depinject.Config, + _ *cobra.Command, + ) (depinject.Config, error) { + tokenomicsQuerier, err := query.NewTokenomicsQuerier(deps) + if err != nil { + return nil, err + } + + return depinject.Configs(deps, depinject.Supply(tokenomicsQuerier)), nil + } +} + +func NewSupplyServiceQueryClientFn() SupplierFn { + return func( + _ context.Context, + deps depinject.Config, + _ *cobra.Command, + ) (depinject.Config, error) { + serviceQuerier, err := query.NewServiceQuerier(deps) + if err != nil { + return nil, err + } + + return depinject.Configs(deps, depinject.Supply(serviceQuerier)), nil + } +} + // newSupplyTxClientFn returns a new depinject.Config which is supplied with // the given deps and the new TxClient. func newSupplyTxClientsFn(ctx context.Context, deps depinject.Config, signingKeyName string) (depinject.Config, error) { diff --git a/pkg/relayer/cmd/cmd.go b/pkg/relayer/cmd/cmd.go index 1f1fdd185..d5aff21ac 100644 --- a/pkg/relayer/cmd/cmd.go +++ b/pkg/relayer/cmd/cmd.go @@ -194,12 +194,13 @@ func setupRelayerDependencies( config.NewSupplyTxClientContextFn(queryNodeGRPCUrl, txNodeRPCUrl), // leaf config.NewSupplyDelegationClientFn(), // leaf config.NewSupplySharedQueryClientFn(), // leaf - config.NewSupplyProofQueryClientFn(), + config.NewSupplyTokenomicsQueryClientFn(), supplyMiner, config.NewSupplyAccountQuerierFn(), config.NewSupplyApplicationQuerierFn(), config.NewSupplySupplierQuerierFn(), config.NewSupplySessionQuerierFn(), + config.NewSupplyServiceQueryClientFn(), config.NewSupplyRingCacheFn(), supplyTxFactory, supplyTxContext, diff --git a/pkg/relayer/miner/options.go b/pkg/relayer/miner/options.go deleted file mode 100644 index ebee97c40..000000000 --- a/pkg/relayer/miner/options.go +++ /dev/null @@ -1,10 +0,0 @@ -package miner - -import "github.com/pokt-network/poktroll/pkg/relayer" - -// WithRelayDifficultyTargetHash sets the relayDifficultyTargetHash of the miner. -func WithRelayDifficultyTargetHash(targetHash []byte) relayer.MinerOption { - return func(mnr relayer.Miner) { - mnr.(*miner).relayDifficultyTargetHash = targetHash - } -} diff --git a/pkg/relayer/session/service.go b/pkg/relayer/session/service.go new file mode 100644 index 000000000..ee6b50857 --- /dev/null +++ b/pkg/relayer/session/service.go @@ -0,0 +1,31 @@ +package session + +import ( + "context" + "fmt" + + "github.com/pokt-network/poktroll/x/service/types" +) + +// getServiceComputeUnitsPerRelay returns the compute units per relay for the service specified in +// the relay request's metadata. The session manager's service query client is used to fetch the onchain +// service. +func (rs *relayerSessionsManager) getServiceComputeUnitsPerRelay( + ctx context.Context, + relayRequestMetadata *types.RelayRequestMetadata, +) (uint64, error) { + sessionHeader := relayRequestMetadata.SessionHeader + if sessionHeader.Service == nil { + return 0, fmt.Errorf("getServiceComputeUnitsPerRelay: received nil service") + } + + service, err := rs.serviceQueryClient.GetService(ctx, sessionHeader.Service.Id) + if err != nil { + return 0, fmt.Errorf("getServiceComputeUnitsPerRelay: could not get on-chain service %s: %w", + sessionHeader.Service.Id, + err, + ) + } + + return service.ComputeUnitsPerRelay, nil +} diff --git a/pkg/relayer/session/session.go b/pkg/relayer/session/session.go index 961e772b7..756249f0c 100644 --- a/pkg/relayer/session/session.go +++ b/pkg/relayer/session/session.go @@ -54,6 +54,11 @@ type relayerSessionsManager struct { // sharedQueryClient is used to query shared module parameters. sharedQueryClient client.SharedQueryClient + + // serviceQueryClient is used to query for a service with a given ID. + // This is used to get the ComputeUnitsPerRelay, which is used as the weight of a mined relay + // when adding a mined relay to a session's tree. + serviceQueryClient client.ServiceQueryClient } // NewRelayerSessions creates a new relayerSessions. @@ -81,6 +86,7 @@ func NewRelayerSessions( &rs.blockClient, &rs.supplierClients, &rs.sharedQueryClient, + &rs.serviceQueryClient, ); err != nil { return nil, err } @@ -400,7 +406,7 @@ func (rs *relayerSessionsManager) waitForBlock(ctx context.Context, targetHeight // to the session tree. If it encounters an error, it returns the error. Otherwise, // it skips output (only outputs errors). func (rs *relayerSessionsManager) mapAddMinedRelayToSessionTree( - _ context.Context, + ctx context.Context, relay *relayer.MinedRelay, ) (_ error, skip bool) { // ensure the session tree exists for this relay @@ -419,9 +425,16 @@ func (rs *relayerSessionsManager) mapAddMinedRelayToSessionTree( With("application", smst.GetSessionHeader().GetApplicationAddress()). With("supplier_address", smst.GetSupplierAddress().String()) - // TODO_BETA(#705): Make sure to update the weight of each relay to the value - // associated with `relayDifficultyTargetHash` in the `miner/miner.go`. - if err := smst.Update(relay.Hash, relay.Bytes, 1); err != nil { + serviceComputeUnitsPerRelay, err := rs.getServiceComputeUnitsPerRelay(ctx, &relayMetadata) + if err != nil { + // TODO_IMPROVE: log additional info? + rs.logger.Error().Err(err).Msg("failed to get service compute units per relay") + return err, false + } + + // The weight of each relay is specified by the corresponding service's ComputeUnitsPerRelay field. + // This is independent of the relay difficulty target hash for each service, which is supplied by the tokenomics module. + if err := smst.Update(relay.Hash, relay.Bytes, serviceComputeUnitsPerRelay); err != nil { // TODO_IMPROVE: log additional info? logger.Error().Err(err).Msg("failed to update smt") return err, false diff --git a/pkg/relayer/session/session_test.go b/pkg/relayer/session/session_test.go index d412c2117..599ab6c0e 100644 --- a/pkg/relayer/session/session_test.go +++ b/pkg/relayer/session/session_test.go @@ -32,6 +32,9 @@ import ( // TODO_TEST: Add a test case which simulates a cold-started relayminer with unclaimed relays. +// TODO_INCOMPLETE: Add a test case which verifies that the service's compute units per relay is used as +// the weight of a relay when updating a session's SMT. + func TestRelayerSessionsManager_Start(t *testing.T) { // TODO_TECHDEBT(#446): Centralize the configuration for the SMT spec. var ( @@ -84,7 +87,9 @@ func TestRelayerSessionsManager_Start(t *testing.T) { sharedQueryClientMock := testqueryclients.NewTestSharedQueryClient(t) - deps := depinject.Supply(blockClient, blockQueryClientMock, supplierClientMap, sharedQueryClientMock) + serviceQueryClientMock := testqueryclients.NewTestServiceQueryClient(t) + + deps := depinject.Supply(blockClient, blockQueryClientMock, supplierClientMap, sharedQueryClientMock, serviceQueryClientMock) storesDirectoryOpt := testrelayer.WithTempStoresDirectory(t) // Create a new relayer sessions manager. diff --git a/testutil/testclient/testqueryclients/servicequerier.go b/testutil/testclient/testqueryclients/servicequerier.go new file mode 100644 index 000000000..6afecf7b2 --- /dev/null +++ b/testutil/testclient/testqueryclients/servicequerier.go @@ -0,0 +1,60 @@ +package testqueryclients + +import ( + "context" + "fmt" + "testing" + + "github.com/golang/mock/gomock" + + "github.com/pokt-network/poktroll/testutil/mockclient" + sharedtypes "github.com/pokt-network/poktroll/x/shared/types" +) + +// services is a map of: serviceId -> Service +var services map[string]sharedtypes.Service + +func init() { + services = make(map[string]sharedtypes.Service) +} + +// NewTestSessionQueryClient creates a mock of the SessionQueryClient +// which allows the caller to call GetSession any times and will return +// the session matching the app address, serviceID and the blockHeight passed. +func NewTestServiceQueryClient( + t *testing.T, +) *mockclient.MockServiceQueryClient { + ctrl := gomock.NewController(t) + + serviceQuerier := mockclient.NewMockServiceQueryClient(ctrl) + serviceQuerier.EXPECT().GetService(gomock.Any(), gomock.Any()). + DoAndReturn(func( + _ context.Context, + serviceId string, + ) (*sharedtypes.Service, error) { + service, ok := services[serviceId] + if !ok { + return nil, fmt.Errorf("error while trying to retrieve a service") + } + + return &service, nil + }). + AnyTimes() + + return serviceQuerier +} + +// AddToExistingServices adds the given service to the services map to mock it "existing" +// on chain, it will also remove the services from the map when the test is cleaned up. +func AddToExistingServices( + t *testing.T, + service sharedtypes.Service, +) { + t.Helper() + + services[service.Id] = service + + t.Cleanup(func() { + delete(services, service.Id) + }) +} From 3debe2c06e370d6e5723d5647d698a0a88fcc0bc Mon Sep 17 00:00:00 2001 From: Arash Deshmeh Date: Mon, 19 Aug 2024 09:43:10 -0400 Subject: [PATCH 03/36] RelayMiner uses the tokenomic modules difficulty target for a service to mine relays --- pkg/client/query/tokenomicsquerier.go | 55 +++++++++++ pkg/relayer/miner/gen/gen_fixtures.go | 97 ++++++++++++------- pkg/relayer/miner/gen/template.go | 4 +- pkg/relayer/miner/miner.go | 75 ++++++++------ pkg/relayer/miner/miner_test.go | 13 ++- pkg/relayer/miner/relay_fixtures_test.go | 24 ++--- .../testqueryclients/tokenomicsquerier.go | 67 +++++++++++++ 7 files changed, 251 insertions(+), 84 deletions(-) create mode 100644 pkg/client/query/tokenomicsquerier.go create mode 100644 testutil/testclient/testqueryclients/tokenomicsquerier.go diff --git a/pkg/client/query/tokenomicsquerier.go b/pkg/client/query/tokenomicsquerier.go new file mode 100644 index 000000000..52b4da453 --- /dev/null +++ b/pkg/client/query/tokenomicsquerier.go @@ -0,0 +1,55 @@ +package query + +import ( + "context" + + "cosmossdk.io/depinject" + "github.com/cosmos/gogoproto/grpc" + + "github.com/pokt-network/poktroll/pkg/client" + tokenomicstypes "github.com/pokt-network/poktroll/x/tokenomics/types" +) + +// tokenomicsQuerier is a wrapper around the tokenomicstypes.QueryClient that enables the +// querying of on-chain tokenomics module data. +type tokenomicsQuerier struct { + clientConn grpc.ClientConn + tokenomicsQuerier tokenomicstypes.QueryClient +} + +// NewTokenomicsQuerier returns a new instance of a client.TokenomicsQueryClient by +// injecting the dependecies provided by the depinject.Config. +// +// Required dependencies: +// - grpc.ClientConn +func NewTokenomicsQuerier(deps depinject.Config) (client.TokenomicsQueryClient, error) { + querier := &tokenomicsQuerier{} + + if err := depinject.Inject( + deps, + &querier.clientConn, + ); err != nil { + return nil, err + } + + querier.tokenomicsQuerier = tokenomicstypes.NewQueryClient(querier.clientConn) + + return querier, nil +} + +// GetServiceRelayDifficultyTargetHash queries the onchain data for +// the relay mining difficulty associated with the given service. +func (tq *tokenomicsQuerier) GetServiceRelayDifficultyTargetHash( + ctx context.Context, + serviceId string, +) (client.TokenomicsRelayMiningDifficulty, error) { + req := &tokenomicstypes.QueryGetRelayMiningDifficultyRequest{ + ServiceId: serviceId, + } + + res, err := tq.tokenomicsQuerier.RelayMiningDifficulty(ctx, req) + if err != nil { + return nil, err + } + return &res.RelayMiningDifficulty, nil +} diff --git a/pkg/relayer/miner/gen/gen_fixtures.go b/pkg/relayer/miner/gen/gen_fixtures.go index 9c6e7fbe5..1b35150b4 100644 --- a/pkg/relayer/miner/gen/gen_fixtures.go +++ b/pkg/relayer/miner/gen/gen_fixtures.go @@ -8,9 +8,9 @@ import ( "bytes" "context" "crypto/rand" + "encoding/hex" "flag" "fmt" - "hash" "log" "os" "strings" @@ -21,25 +21,35 @@ import ( "github.com/pokt-network/poktroll/pkg/observable" "github.com/pokt-network/poktroll/pkg/observable/channel" "github.com/pokt-network/poktroll/pkg/relayer" - "github.com/pokt-network/poktroll/pkg/relayer/miner" + "github.com/pokt-network/poktroll/testutil/sample" 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" ) const ( - defaultDifficultyBits = 16 - defaultFixtureLimitPerGroup = 5 - defaultRandLength = 16 - defaultOutPath = "relay_fixtures_test.go" + // defaultDifficultyThresholdHashStr is the default difficulty hash in string formet, used to separate the generated relays into minable and unminable categories. + // This hash should match the service-specific difficulty target hash used by the relay miner tests in pkg/relayer/miner/miner_test.go. + defaultDifficultyThresholdHashStr = "0000ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff" + defaultFixtureLimitPerGroup = 5 + defaultRandLength = 16 + defaultOutPath = "relay_fixtures_test.go" + + // svcId is the ID of the service referenced in each relay request's metadata. + // This is passed to the tokenomics client to get the service's difficulty target hash. + // It should match the testSvcId const in pkg/relayer/miner/miner_test.go + testSvcId = "svc1" ) // TODO_FOLLOWUP(@olshansk, #690): Do a global anycase grep for "DifficultyBits" and update/remove things appropriately. var ( - // flagDifficultyBitsThreshold is the number of leading zero bits that a - // randomized, serialized relay must have to be included in the - // `marshaledMinableRelaysHex` slice which is generated. It is also used as - // the maximum difficulty allowed for relays to be included in the - // `marshaledUnminableRelaysHex` slice. - flagDifficultyBitsThreshold int + + // flagDifficultyThresholdHashStr is the difficulty threshold hash, in string format, that a + // randomized, serialized relay must be greater than to be included in the + // `marshaledMinableRelaysHex` slice which is generated. + // It is also used as the maximum difficulty threshold hash allowed for relays to be + // included in the `marshaledUnminableRelaysHex` slice. + flagDifficultyThresholdHashStr string // flagFixtureLimitPerGroup is the number of randomized, serialized relays that will be // generated for each of `marshaledMinableRelaysHex` and @@ -56,7 +66,7 @@ type marshalable interface { } func init() { - flag.IntVar(&flagDifficultyBitsThreshold, "difficulty-bits-threshold", defaultDifficultyBits, "the number of leading zero bits that a randomized, serialized relay must have to be included in the `marshaledMinableRelaysHex` slice which is generated. It is also used as the maximum difficulty allowed for relays to be included in the `marshaledUnminableRelaysHex` slice.") + flag.StringVar(&flagDifficultyThresholdHashStr, "difficulty-threshold-hash-str", defaultDifficultyThresholdHashStr, "the difficulty threshold hash, in string format, that a randomized, serialized relay be greater than or equal to when hashed, to be included in the `marshaledMinableRelaysHex` slice which is generated. It is also used as the maximum difficulty threshold hash for relays to be included in the `marshaledUnminableRelaysHex` slice.") flag.IntVar(&flagFixtureLimitPerGroup, "fixture-limit-per-group", defaultFixtureLimitPerGroup, "the number of randomized, serialized relays that will be generated for each of `marshaledMinableRelaysHex` and `marshaledUnminableRelaysHex`.") flag.StringVar(&flagOut, "out", defaultOutPath, "the path to the generated file.") } @@ -84,7 +94,8 @@ func main() { randRelaysObs, errCh := genRandomizedMinedRelayFixtures( ctx, defaultRandLength, - miner.DefaultRelayHasher, + // TODO_TECHDEBT: need to find out how the removal of miner.DefaultRelayerHasher in commit 608edc31b39c6ff9a285af46d1c1037cd36a7252 did not + // trigger any test failures while it was still referenced here (in the line that was in this comment's position before being deleted). ) exitOnError(errCh) @@ -100,7 +111,7 @@ func main() { if err := relayFixturesTemplate.Execute( outputBuffer, map[string]any{ - "difficultyBitsThreshold": flagDifficultyBitsThreshold, + "difficultyThresholdHashStr": flagDifficultyThresholdHashStr, "MarshaledMinableRelaysHex": marshaledMinableRelaysHex, "MarshaledUnminableRelaysHex": marshaledUnminableRelaysHex, }, @@ -121,7 +132,6 @@ func main() { func genRandomizedMinedRelayFixtures( ctx context.Context, randLength int, - newHasher func() hash.Hash, ) (observable.Observable[*relayer.MinedRelay], <-chan error) { var ( errCh = make(chan error, 1) @@ -145,7 +155,17 @@ func genRandomizedMinedRelayFixtures( // Populate a relay with the minimally sufficient randomized data. relay := servicetypes.Relay{ Req: &servicetypes.RelayRequest{ - Meta: &servicetypes.RelayRequestMetadata{ + Meta: servicetypes.RelayRequestMetadata{ + SessionHeader: &sessiontypes.SessionHeader{ + ApplicationAddress: sample.AccAddress(), + Service: &sharedtypes.Service{ + Id: testSvcId, + }, + SessionId: "session_id", + SessionStartBlockHeight: 1, + SessionEndBlockHeight: 2, + }, + Signature: randBz, }, Payload: nil, @@ -160,11 +180,8 @@ func genRandomizedMinedRelayFixtures( } // Hash relay bytes - relayHash, err := hashBytes(newHasher, relayBz) - if err != nil { - errCh <- err - return - } + relayHashArr := protocol.GetRelayHashFromBytes(relayBz) + relayHash := relayHashArr[:] randBzPublishCh <- &relayer.MinedRelay{ Relay: relay, @@ -177,16 +194,6 @@ func genRandomizedMinedRelayFixtures( return randBzObs, errCh } -// hashBytes hashes the given bytes using the given hasher. -func hashBytes(newHasher func() hash.Hash, relayBz []byte) ([]byte, error) { - hasher := newHasher() - if _, err := hasher.Write(relayBz); err != nil { - return nil, err - } - - return hasher.Sum(nil), nil -} - // exitOnError exits the program if an error is received on the given error // channel. func exitOnError(errCh <-chan error) { @@ -197,18 +204,36 @@ func exitOnError(errCh <-chan error) { }() } +// getDifficultyThresholdHash returns the difficulty threshold hash set by the +// flagDifficultyThresholdHashStr flag in string formet, or the default value if +// the flag has not been set. +func getDifficultyThresholdHash() [protocol.RelayHasherSize]byte { + difficultyThresholdHash, _ := hex.DecodeString(flagDifficultyThresholdHashStr) + var thresholdHashArr [protocol.RelayHasherSize]byte + copy(thresholdHashArr[:], difficultyThresholdHash) + return thresholdHashArr +} + // difficultyGTE returns true if the given hash has a difficulty greater than or -// equal to flagDifficultyBitsThreshold. +// equal to that represented by flagDifficultyThresholdHashStr. func difficultyGTE(hash []byte) bool { - return protocol.CountDifficultyBits(hash) >= flagDifficultyBitsThreshold + var hashArr [protocol.RelayHasherSize]byte + copy(hashArr[:], hash) + + return protocol.GetDifficultyFromHash(hashArr) >= protocol.GetDifficultyFromHash(getDifficultyThresholdHash()) } // difficultyLT returns true if the given hash has a difficulty less than -// flagDifficultyBitsThreshold. +// that represented by flagDifficultyThresholdHashStr. func difficultyLT(hash []byte) bool { - return protocol.CountDifficultyBits(hash) < flagDifficultyBitsThreshold + var hashArr [protocol.RelayHasherSize]byte + copy(hashArr[:], hash) + + return protocol.GetDifficultyFromHash(hashArr) < protocol.GetDifficultyFromHash(getDifficultyThresholdHash()) } +// TODO_IMPROVE: this function could be simplified to a great extent to make it easier to understand the +// test cases. // getMarshaledRelayFmtLines performs two map operations followed by a collect. // The first map filters mined relays from the given observable, skipping when // shouldAccept is false. This map, and as a result, all downstream observables diff --git a/pkg/relayer/miner/gen/template.go b/pkg/relayer/miner/gen/template.go index b6803a09a..dd88fdddd 100644 --- a/pkg/relayer/miner/gen/template.go +++ b/pkg/relayer/miner/gen/template.go @@ -18,7 +18,7 @@ package miner_test var ( // marshaledMinableRelaysHex are the hex encoded strings of serialized - // relayer.MinedRelays which have been pre-mined to difficulty {{.difficultyBitsThreshold}} by + // relayer.MinedRelays which have been pre-mined to difficulty {{.difficultyThresholdHashStr}} by // populating the signature with random bytes. It is intended for use // in tests. marshaledMinableRelaysHex = []string{ @@ -27,7 +27,7 @@ var ( // marshaledUnminableRelaysHex are the hex encoded strings of serialized // relayer.MinedRelays which have been pre-mined to **exclude** relays with - // difficulty {{.difficultyBitsThreshold}} (or greater). Like marshaledMinableRelaysHex, this is done + // difficulty {{.difficultyThresholdHashStr}} (or greater). Like marshaledMinableRelaysHex, this is done // by populating the signature with random bytes. It is intended for use in // tests. marshaledUnminableRelaysHex = []string{ diff --git a/pkg/relayer/miner/miner.go b/pkg/relayer/miner/miner.go index 96dcffa50..b55d24226 100644 --- a/pkg/relayer/miner/miner.go +++ b/pkg/relayer/miner/miner.go @@ -2,6 +2,8 @@ package miner import ( "context" + "errors" + "fmt" "cosmossdk.io/depinject" @@ -22,16 +24,9 @@ var _ relayer.Miner = (*miner)(nil) // difficulty of each, finally publishing those with sufficient difficulty to // minedRelayObs as they are applicable for relay volume. type miner struct { - // proofQueryClient is used to query for the minimum relay difficulty. - proofQueryClient client.ProofQueryClient - + // tokenomicsQueryClient is used to query for the relay difficulty target hash of a service. // relay_difficulty is the target hash which a relay hash must be less than to be volume/reward applicable. - // - // TODO_BETA(#705): This is populated by querying the corresponding on-chain parameter during construction. - // If this parameter is updated on-chain the relayminer will need to be restarted to query the new value. - // TODO_BETA(#705): This needs to be maintained (and updated) on a per service level. - // Make sure to update the `smst.Update` call in `relayer/session` alongside it. - relayDifficultyTargetHash []byte + tokenomicsQueryClient client.TokenomicsQueryClient } // NewMiner creates a new miner from the given dependencies and options. It @@ -48,7 +43,7 @@ func NewMiner( ) (*miner, error) { mnr := &miner{} - if err := depinject.Inject(deps, &mnr.proofQueryClient); err != nil { + if err := depinject.Inject(deps, &mnr.tokenomicsQueryClient); err != nil { return nil, err } @@ -56,10 +51,6 @@ func NewMiner( opt(mnr) } - if err := mnr.setDefaults(); err != nil { - return nil, err - } - return mnr, nil } @@ -86,28 +77,13 @@ func (mnr *miner) MinedRelays( return filter.EitherSuccess(ctx, eitherMinedRelaysObs) } -// setDefaults ensures that the miner has been configured with a hasherConstructor and uses -// the default hasherConstructor if not. -func (mnr *miner) setDefaults() error { - ctx := context.TODO() - params, err := mnr.proofQueryClient.GetParams(ctx) - if err != nil { - return err - } - - if len(mnr.relayDifficultyTargetHash) == 0 { - mnr.relayDifficultyTargetHash = params.GetRelayDifficultyTargetHash() - } - return nil -} - // mapMineRelay is intended to be used as a MapFn. -// 1. It hashes the relay and compares its difficult to the minimum threshold. +// 1. It hashes the relay and compares its difficulty to the minimum threshold. // 2. If the relay difficulty is sufficient -> return an Either[MineRelay Value] // 3. If an error is encountered -> return an Either[error] // 4. Otherwise, skip the relay. func (mnr *miner) mapMineRelay( - _ context.Context, + ctx context.Context, relay *servicetypes.Relay, ) (_ either.Either[*relayer.MinedRelay], skip bool) { relayBz, err := relay.Marshal() @@ -117,8 +93,13 @@ func (mnr *miner) mapMineRelay( relayHashArr := protocol.GetRelayHashFromBytes(relayBz) relayHash := relayHashArr[:] + relayDifficultyTargetHash, err := mnr.getServiceRelayDifficultyTargetHash(ctx, relay.Req) + if err != nil { + return either.Error[*relayer.MinedRelay](err), false + } + // The relay IS NOT volume / reward applicable - if !protocol.IsRelayVolumeApplicable(relayHash, mnr.relayDifficultyTargetHash) { + if !protocol.IsRelayVolumeApplicable(relayHash, relayDifficultyTargetHash) { return either.Success[*relayer.MinedRelay](nil), true } @@ -129,3 +110,33 @@ func (mnr *miner) mapMineRelay( Hash: relayHash, }), false } + +// getServiceRelayDifficultyTargetHash returns the relay difficulty target hash for the service referenced by the relay. +// If the service does not have a relay difficulty target hash defined, the default difficulty target hash is returned. +func (mnr *miner) getServiceRelayDifficultyTargetHash(ctx context.Context, req *servicetypes.RelayRequest) ([]byte, error) { + if req == nil { + return nil, errors.New("relay request is nil") + } + + sessionHeader := req.Meta.SessionHeader + if sessionHeader == nil { + return nil, errors.New("relay metadata has nil session header") + } + + if err := sessionHeader.ValidateBasic(); err != nil { + return nil, fmt.Errorf("invalid session header: %w", err) + } + + serviceRelayDifficulty, err := mnr.tokenomicsQueryClient.GetServiceRelayDifficultyTargetHash(ctx, sessionHeader.Service.Id) + if err != nil { + // TODO_DISCUSS_IN_THIS_COMMIT: do we need to distinguish between errors? There could potentially be a scenario in the future where + // the query client fails for some reason other than "service has no relay difficulty set", which is currently the only + // error scenario. The line below could then lead to masking the error and returning a default value for a service that + // has a relay difficulty target hash set. + // Solving this needs updating the tokenomics keeper to use an additional boolean in the output to distinguish errors + // from the case where the service has no relay difficulty target hash set. + return protocol.BaseRelayDifficultyHashBz, nil + } + + return serviceRelayDifficulty.GetTargetHash(), nil +} diff --git a/pkg/relayer/miner/miner_test.go b/pkg/relayer/miner/miner_test.go index f4e6dea7d..01a67c37c 100644 --- a/pkg/relayer/miner/miner_test.go +++ b/pkg/relayer/miner/miner_test.go @@ -23,6 +23,11 @@ import ( servicetypes "github.com/pokt-network/poktroll/x/service/types" ) +// testSvcId is the ID of the service used in the relays. It is used to initialize the tokenomics module that +// is used by the relay miner to fetch the relay difficulty target hash for the service corresponding to the relay requests. +// The fixtures generated by pkg/relayer/miner/gen/gen_fixtures.go use a const with the same name and value. +const testSvcId = "svc1" + var testRelayMiningTargetHash, _ = hex.DecodeString("0000ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff") // TestMiner_MinedRelays constructs an observable of mined relays, through which @@ -42,8 +47,12 @@ func TestMiner_MinedRelays(t *testing.T) { ) proofQueryClientMock := testqueryclients.NewTestProofQueryClient(t) - deps := depinject.Supply(proofQueryClientMock) - mnr, err := miner.NewMiner(deps, miner.WithRelayDifficultyTargetHash(testRelayMiningTargetHash)) + + testqueryclients.SetServiceRelayDifficultyTargetHash(t, testSvcId, testRelayMiningTargetHash) + tokenomicsQueryClientMock := testqueryclients.NewTestTokenomicsQueryClient(t) + + deps := depinject.Supply(proofQueryClientMock, tokenomicsQueryClientMock) + mnr, err := miner.NewMiner(deps) require.NoError(t, err) minedRelays := mnr.MinedRelays(ctx, mockRelaysObs) diff --git a/pkg/relayer/miner/relay_fixtures_test.go b/pkg/relayer/miner/relay_fixtures_test.go index c32dd4509..fc0ceb27b 100644 --- a/pkg/relayer/miner/relay_fixtures_test.go +++ b/pkg/relayer/miner/relay_fixtures_test.go @@ -7,27 +7,27 @@ package miner_test var ( // marshaledMinableRelaysHex are the hex encoded strings of serialized - // relayer.MinedRelays which have been pre-mined to difficulty 16 by + // relayer.MinedRelays which have been pre-mined to difficulty 0000ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff by // populating the signature with random bytes. It is intended for use // in tests. marshaledMinableRelaysHex = []string{ - "0a140a121210bffa0b5c3f03c71f93d7611af54ba80b", - "0a140a1212106655cb3dccaada7837b094962d65426d", - "0a140a121210b19bcf19e96cb58bb37bae3a1769fc08", - "0a140a121210146014db743850694ce211403e91c451", - "0a140a12121016da74aea084a4d40aebf61d876809d4", + "0a5d0a5b0a470a2d636f736d6f7331736b68616e616d7a6666767478686a77737a796437636d6d703361776174706339786777733012060a04737663311a0a73657373696f6e5f696420012802121052cc1367527c76d24f584276c1fdc8fa", + "0a5d0a5b0a470a2d636f736d6f73316e73616537636a396161786b70726e326b796132346b6634787830346d39363265686434326d12060a04737663311a0a73657373696f6e5f696420012802121079a61fcd9a27848c5735122c855087e2", + "0a5d0a5b0a470a2d636f736d6f733134716a73303478616a337a737a643971773478747363336b61366a3933387a6479763076757712060a04737663311a0a73657373696f6e5f696420012802121012d86bfa37fc43ac6db7584f671f7d7d", + "0a5d0a5b0a470a2d636f736d6f73316836717a38766e716632347a6b7437723774727a7a6873736c6466776c376d6574667636726112060a04737663311a0a73657373696f6e5f6964200128021210f9bf6d38c0f24a4b44e840970aceef36", + "0a5d0a5b0a470a2d636f736d6f7331387863636e77367a327272613864736a6b3364646371366a656a376676306b3332797478386812060a04737663311a0a73657373696f6e5f6964200128021210d1f5c2dde31f2470e8a95fc298bafb09", } // marshaledUnminableRelaysHex are the hex encoded strings of serialized // relayer.MinedRelays which have been pre-mined to **exclude** relays with - // difficulty 16 (or greater). Like marshaledMinableRelaysHex, this is done + // difficulty 0000ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff (or greater). Like marshaledMinableRelaysHex, this is done // by populating the signature with random bytes. It is intended for use in // tests. marshaledUnminableRelaysHex = []string{ - "0a140a12121070b059e32e4149bcded39d8eb0186fce", - "0a140a121210f5bb373e3d6cabe04675083ac4ffc36e", - "0a140a1212104ae2b0539ecd82ec56bf23b276d543c1", - "0a140a121210d4238199f9e52347b46d060c477b464e", - "0a140a1212103118a0689c4eb883e384d12378bc61e2", + "0a5d0a5b0a470a2d636f736d6f7331656e65616e637976336e6a6536673071647077397965337a6b7337726b7468776175326c683412060a04737663311a0a73657373696f6e5f696420012802121024d44328a07eb3c365ad54fdf0757f1b", + "0a5d0a5b0a470a2d636f736d6f7331796d6d6a777a3370736533303976737165666c38636c776870303436767239746b6c78636e6112060a04737663311a0a73657373696f6e5f69642001280212109f7dc58b68b5c9b723bd07cbf6379ded", + "0a5d0a5b0a470a2d636f736d6f73317a64347a666d337338617774776e6764756164786a787778677876633233743974616a37673512060a04737663311a0a73657373696f6e5f696420012802121083b3fd95164a12ceb98be02df65257fa", + "0a5d0a5b0a470a2d636f736d6f73316663646571397a79707130336b6a7a7768777a7435756c733271677464656668636863326c6612060a04737663311a0a73657373696f6e5f69642001280212107e415cb2a7b18d74427742da94ca74f9", + "0a5d0a5b0a470a2d636f736d6f733178736b7937777279307964736b75376577617230716a6b6b763671653864716575637464766812060a04737663311a0a73657373696f6e5f69642001280212107dcb1de0e616be6e619eacb42b026c5d", } ) diff --git a/testutil/testclient/testqueryclients/tokenomicsquerier.go b/testutil/testclient/testqueryclients/tokenomicsquerier.go new file mode 100644 index 000000000..6efb8e06b --- /dev/null +++ b/testutil/testclient/testqueryclients/tokenomicsquerier.go @@ -0,0 +1,67 @@ +package testqueryclients + +import ( + "context" + "fmt" + "testing" + + "github.com/golang/mock/gomock" + + "github.com/pokt-network/poktroll/testutil/mockclient" + tokenomicstypes "github.com/pokt-network/poktroll/x/tokenomics/types" +) + +// relayDifficultyTargets is a map of: serviceId -> RelayMiningDifficulty +// It is updated by the SetServiceRelayDifficultyTargetHash, and read by +// the mock tokenomics query client to get a specific service's relay difficulty +// target hash. +var relayDifficultyTargets map[string]*tokenomicstypes.RelayMiningDifficulty + +func init() { + relayDifficultyTargets = make(map[string]*tokenomicstypes.RelayMiningDifficulty) +} + +// NewTestTokenomicsQueryClient creates a mock of the TokenomicsQueryClient +// which allows the caller to call GetSession any times and will return +// the session matching the app address, serviceID and the blockHeight passed. +func NewTestTokenomicsQueryClient( + t *testing.T, +) *mockclient.MockTokenomicsQueryClient { + ctrl := gomock.NewController(t) + + tokenomicsQuerier := mockclient.NewMockTokenomicsQueryClient(ctrl) + tokenomicsQuerier.EXPECT().GetServiceRelayDifficultyTargetHash(gomock.Any(), gomock.Any()). + DoAndReturn(func( + _ context.Context, + serviceId string, + ) (*tokenomicstypes.RelayMiningDifficulty, error) { + relayDifficulty, ok := relayDifficultyTargets[serviceId] + if !ok { + return nil, fmt.Errorf("error while trying to retrieve the relay mining difficulty for service %s", serviceId) + } + + return relayDifficulty, nil + }). + AnyTimes() + + return tokenomicsQuerier +} + +// AddServiceRelayDifficultyTargetHash sets the relay difficulty target hash +// for the given service to mock it "existing" on chain. +// It will also remove the service relay difficulty target hashes from the map when the test is cleaned up. +func SetServiceRelayDifficultyTargetHash(t *testing.T, + serviceId string, + relayDifficultyTargetHash []byte, +) { + t.Helper() + + relayDifficultyTargets[serviceId] = &tokenomicstypes.RelayMiningDifficulty{ + ServiceId: serviceId, + TargetHash: relayDifficultyTargetHash, + } + + t.Cleanup(func() { + delete(relayDifficultyTargets, serviceId) + }) +} From 7240a6812368ba703ed64f6d7205991d7a2696f8 Mon Sep 17 00:00:00 2001 From: Arash Deshmeh Date: Mon, 19 Aug 2024 11:45:22 -0400 Subject: [PATCH 04/36] Change to use supplier OperatorAddress field name --- testutil/integration/app.go | 8 +++++--- x/proof/keeper/msg_server_create_claim_test.go | 4 ++-- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/testutil/integration/app.go b/testutil/integration/app.go index ccd56beba..ea09eea22 100644 --- a/testutil/integration/app.go +++ b/testutil/integration/app.go @@ -394,6 +394,7 @@ func NewCompleteIntegrationApp(t *testing.T) *App { applicationKeeper, accountKeeper, sharedKeeper, + serviceKeeper, ) proofModule := proof.NewAppModule( cdc, @@ -509,9 +510,10 @@ func NewCompleteIntegrationApp(t *testing.T) *App { // Prepare a new default service defaultService := sharedtypes.Service{ - Id: "svc1", - Name: "svcName1", - OwnerAddress: sample.AccAddress(), + Id: "svc1", + Name: "svcName1", + OwnerAddress: sample.AccAddress(), + ComputeUnitsPerRelay: 1, } serviceKeeper.SetService(integrationApp.sdkCtx, defaultService) integrationApp.DefaultService = &defaultService diff --git a/x/proof/keeper/msg_server_create_claim_test.go b/x/proof/keeper/msg_server_create_claim_test.go index 519d02144..b1cbd7a84 100644 --- a/x/proof/keeper/msg_server_create_claim_test.go +++ b/x/proof/keeper/msg_server_create_claim_test.go @@ -563,7 +563,7 @@ func TestMsgServer_CreateClaim_Error_ComputeUnitsMismatch(t *testing.T) { // Add a supplier that is expected to be in the session. supplierKeeper.SetSupplier(ctx, sharedtypes.Supplier{ - Address: supplierAddr, + OperatorAddress: supplierAddr, Services: []*sharedtypes.SupplierServiceConfig{ {Service: service}, }, @@ -595,7 +595,7 @@ func TestMsgServer_CreateClaim_Error_ComputeUnitsMismatch(t *testing.T) { sessionResSuppliers := sessionRes.GetSession().GetSuppliers() require.NotEmpty(t, sessionResSuppliers) - require.Equal(t, supplierAddr, sessionResSuppliers[0].GetAddress()) + require.Equal(t, supplierAddr, sessionResSuppliers[0].GetOperatorAddress()) // Increment the block height to the test claim height. sessionHeader := sessionRes.GetSession().GetHeader() From f04df4efa12a7a8987d6458aff1e0c99c18b70ff Mon Sep 17 00:00:00 2001 From: Arash <23505281+adshmh@users.noreply.github.com> Date: Thu, 22 Aug 2024 11:32:26 -0400 Subject: [PATCH 05/36] Update pkg/relayer/miner/gen/gen_fixtures.go Co-authored-by: Bryan White --- pkg/relayer/miner/gen/gen_fixtures.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/pkg/relayer/miner/gen/gen_fixtures.go b/pkg/relayer/miner/gen/gen_fixtures.go index 1b35150b4..503a77c05 100644 --- a/pkg/relayer/miner/gen/gen_fixtures.go +++ b/pkg/relayer/miner/gen/gen_fixtures.go @@ -217,10 +217,7 @@ func getDifficultyThresholdHash() [protocol.RelayHasherSize]byte { // difficultyGTE returns true if the given hash has a difficulty greater than or // equal to that represented by flagDifficultyThresholdHashStr. func difficultyGTE(hash []byte) bool { - var hashArr [protocol.RelayHasherSize]byte - copy(hashArr[:], hash) - - return protocol.GetDifficultyFromHash(hashArr) >= protocol.GetDifficultyFromHash(getDifficultyThresholdHash()) + return protocol.IsRelayVolumeApplicable(hash, getDifficultyThresholdHash()) } // difficultyLT returns true if the given hash has a difficulty less than From 66a371b939e409ecd24f21d0d48c3b72263bc4ac Mon Sep 17 00:00:00 2001 From: Arash <23505281+adshmh@users.noreply.github.com> Date: Thu, 22 Aug 2024 11:32:39 -0400 Subject: [PATCH 06/36] Update pkg/relayer/miner/gen/gen_fixtures.go Co-authored-by: Bryan White --- pkg/relayer/miner/gen/gen_fixtures.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/pkg/relayer/miner/gen/gen_fixtures.go b/pkg/relayer/miner/gen/gen_fixtures.go index 503a77c05..a81ea3213 100644 --- a/pkg/relayer/miner/gen/gen_fixtures.go +++ b/pkg/relayer/miner/gen/gen_fixtures.go @@ -223,10 +223,7 @@ func difficultyGTE(hash []byte) bool { // difficultyLT returns true if the given hash has a difficulty less than // that represented by flagDifficultyThresholdHashStr. func difficultyLT(hash []byte) bool { - var hashArr [protocol.RelayHasherSize]byte - copy(hashArr[:], hash) - - return protocol.GetDifficultyFromHash(hashArr) < protocol.GetDifficultyFromHash(getDifficultyThresholdHash()) + return !protocol.IsRelayVolumeApplicable(hash, getDifficultyThresholdHash()) } // TODO_IMPROVE: this function could be simplified to a great extent to make it easier to understand the From 8376f0924daf616aa63b243601144ea0254cc61f Mon Sep 17 00:00:00 2001 From: Arash <23505281+adshmh@users.noreply.github.com> Date: Thu, 22 Aug 2024 11:33:18 -0400 Subject: [PATCH 07/36] Update pkg/relayer/miner/miner.go Co-authored-by: Bryan White --- pkg/relayer/miner/miner.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/relayer/miner/miner.go b/pkg/relayer/miner/miner.go index b55d24226..b02c558e8 100644 --- a/pkg/relayer/miner/miner.go +++ b/pkg/relayer/miner/miner.go @@ -118,7 +118,7 @@ func (mnr *miner) getServiceRelayDifficultyTargetHash(ctx context.Context, req * return nil, errors.New("relay request is nil") } - sessionHeader := req.Meta.SessionHeader + sessionHeader := req.GetMeta().GetSessionHeader() if sessionHeader == nil { return nil, errors.New("relay metadata has nil session header") } From ec6d613597588a098fc6f2851a7418c9d3b2a543 Mon Sep 17 00:00:00 2001 From: Arash <23505281+adshmh@users.noreply.github.com> Date: Thu, 22 Aug 2024 11:33:57 -0400 Subject: [PATCH 08/36] Update pkg/relayer/miner/gen/template.go Co-authored-by: Bryan White --- pkg/relayer/miner/gen/template.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/relayer/miner/gen/template.go b/pkg/relayer/miner/gen/template.go index dd88fdddd..cf7a4fdad 100644 --- a/pkg/relayer/miner/gen/template.go +++ b/pkg/relayer/miner/gen/template.go @@ -18,7 +18,7 @@ package miner_test var ( // marshaledMinableRelaysHex are the hex encoded strings of serialized - // relayer.MinedRelays which have been pre-mined to difficulty {{.difficultyThresholdHashStr}} by + // relayer.MinedRelays which have been pre-mined to difficult {{.difficultyThresholdHashStr}} (or greater) by // populating the signature with random bytes. It is intended for use // in tests. marshaledMinableRelaysHex = []string{ From 7ed02d7d553f3a34ee5989e9f1f4f9398e3c1eee Mon Sep 17 00:00:00 2001 From: Arash <23505281+adshmh@users.noreply.github.com> Date: Thu, 22 Aug 2024 11:34:59 -0400 Subject: [PATCH 09/36] Apply suggestions from code review Co-authored-by: Bryan White --- pkg/relayer/session/service.go | 6 +++--- testutil/testclient/testqueryclients/servicequerier.go | 1 + testutil/testclient/testqueryclients/tokenomicsquerier.go | 1 + 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/pkg/relayer/session/service.go b/pkg/relayer/session/service.go index ee6b50857..ff0f4ff0e 100644 --- a/pkg/relayer/session/service.go +++ b/pkg/relayer/session/service.go @@ -14,8 +14,8 @@ func (rs *relayerSessionsManager) getServiceComputeUnitsPerRelay( ctx context.Context, relayRequestMetadata *types.RelayRequestMetadata, ) (uint64, error) { - sessionHeader := relayRequestMetadata.SessionHeader - if sessionHeader.Service == nil { + sessionHeader := relayRequestMetadata.GetSessionHeader() + if sessionHeader.GetService() == nil { return 0, fmt.Errorf("getServiceComputeUnitsPerRelay: received nil service") } @@ -27,5 +27,5 @@ func (rs *relayerSessionsManager) getServiceComputeUnitsPerRelay( ) } - return service.ComputeUnitsPerRelay, nil + return service.GetComputeUnitsPerRelay(), nil } diff --git a/testutil/testclient/testqueryclients/servicequerier.go b/testutil/testclient/testqueryclients/servicequerier.go index 6afecf7b2..e89bc3346 100644 --- a/testutil/testclient/testqueryclients/servicequerier.go +++ b/testutil/testclient/testqueryclients/servicequerier.go @@ -24,6 +24,7 @@ func init() { func NewTestServiceQueryClient( t *testing.T, ) *mockclient.MockServiceQueryClient { + t.Helper() ctrl := gomock.NewController(t) serviceQuerier := mockclient.NewMockServiceQueryClient(ctrl) diff --git a/testutil/testclient/testqueryclients/tokenomicsquerier.go b/testutil/testclient/testqueryclients/tokenomicsquerier.go index 6efb8e06b..b7807376f 100644 --- a/testutil/testclient/testqueryclients/tokenomicsquerier.go +++ b/testutil/testclient/testqueryclients/tokenomicsquerier.go @@ -27,6 +27,7 @@ func init() { func NewTestTokenomicsQueryClient( t *testing.T, ) *mockclient.MockTokenomicsQueryClient { + t.Helper() ctrl := gomock.NewController(t) tokenomicsQuerier := mockclient.NewMockTokenomicsQueryClient(ctrl) From 14600219516e4cf67345d5f0a9d3ce9edc11bd18 Mon Sep 17 00:00:00 2001 From: Arash <23505281+adshmh@users.noreply.github.com> Date: Thu, 22 Aug 2024 11:36:31 -0400 Subject: [PATCH 10/36] Apply suggestions from code review Co-authored-by: Bryan White --- .../testclient/testqueryclients/tokenomicsquerier.go | 2 +- x/proof/keeper/msg_server_create_claim.go | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/testutil/testclient/testqueryclients/tokenomicsquerier.go b/testutil/testclient/testqueryclients/tokenomicsquerier.go index b7807376f..f090e9218 100644 --- a/testutil/testclient/testqueryclients/tokenomicsquerier.go +++ b/testutil/testclient/testqueryclients/tokenomicsquerier.go @@ -38,7 +38,7 @@ func NewTestTokenomicsQueryClient( ) (*tokenomicstypes.RelayMiningDifficulty, error) { relayDifficulty, ok := relayDifficultyTargets[serviceId] if !ok { - return nil, fmt.Errorf("error while trying to retrieve the relay mining difficulty for service %s", serviceId) + return nil, tokenomicstypes.ErrTokenomicsMissingRelayMiningDifficulty.Wrapf("retrieving the relay mining difficulty for service %s", serviceId) } return relayDifficulty, nil diff --git a/x/proof/keeper/msg_server_create_claim.go b/x/proof/keeper/msg_server_create_claim.go index 6b7f17cf6..7762e69ad 100644 --- a/x/proof/keeper/msg_server_create_claim.go +++ b/x/proof/keeper/msg_server_create_claim.go @@ -81,19 +81,19 @@ func (k msgServer) CreateClaim( numClaimComputeUnits, err = claim.GetNumComputeUnits() if err != nil { - return nil, status.Error(codes.Internal, types.ErrProofInvalidClaimRootHash.Wrap(err.Error()).Error()) + return nil, status.Error(codes.Internal, types.ErrProofInvalidClaimRootHash.Wrapf("%v", err).Error()) } - // For now we expect the following equation to always hold: + // For now, we expect the following equation to always hold: // numClaimComputeUnits = numRelays * service.computeUnitsPerRelay // This is because for any specific service, every relay is worth the same. // However, this may change in the future. - serviceCupr, err := k.getServiceCupr(ctx, claim.SessionHeader.Service.Id) + serviceComputeUnitsPerRelay, err := k.getServiceCupr(ctx, claim.SessionHeader.Service.Id) if err != nil { - return nil, status.Error(codes.Internal, types.ErrProofServiceNotFound.Wrap(err.Error()).Error()) + return nil, status.Error(codes.NotFound, types.ErrProofServiceNotFound.Wrapf("%v", err).Error()) } - if numClaimComputeUnits != numRelays*serviceCupr { + if numClaimComputeUnits != numRelays*serviceComputeUnitsPerRelay { return nil, status.Error( codes.InvalidArgument, types.ErrProofComputeUnitsMismatch.Wrap( From ad4b8281e4f6acfce5b4232dbef10ffb8ae0760f Mon Sep 17 00:00:00 2001 From: Arash <23505281+adshmh@users.noreply.github.com> Date: Thu, 22 Aug 2024 11:38:23 -0400 Subject: [PATCH 11/36] Apply suggestions from code review Co-authored-by: Bryan White --- x/proof/keeper/service.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x/proof/keeper/service.go b/x/proof/keeper/service.go index 4ab969424..1fb59c187 100644 --- a/x/proof/keeper/service.go +++ b/x/proof/keeper/service.go @@ -5,10 +5,10 @@ import ( "fmt" ) -// getServiceCupr ensures that a service with the ServiceID of the given service +// getServiceComputeUnitsPerRelay is used to ensure that a service with the ServiceID exists. // exists. // It returns the compute units per relay for the service with the given id. -func (k Keeper) getServiceCupr( +func (k Keeper) getServiceComputeUnitsPerRelay( ctx context.Context, serviceId string, ) (uint64, error) { From 0c8e38d0135ecb1ba7ba4cb5da742369e510b11c Mon Sep 17 00:00:00 2001 From: Arash <23505281+adshmh@users.noreply.github.com> Date: Thu, 22 Aug 2024 11:38:38 -0400 Subject: [PATCH 12/36] Update testutil/testclient/testqueryclients/servicequerier.go Co-authored-by: Bryan White --- testutil/testclient/testqueryclients/servicequerier.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/testutil/testclient/testqueryclients/servicequerier.go b/testutil/testclient/testqueryclients/servicequerier.go index e89bc3346..467326f65 100644 --- a/testutil/testclient/testqueryclients/servicequerier.go +++ b/testutil/testclient/testqueryclients/servicequerier.go @@ -35,7 +35,7 @@ func NewTestServiceQueryClient( ) (*sharedtypes.Service, error) { service, ok := services[serviceId] if !ok { - return nil, fmt.Errorf("error while trying to retrieve a service") + return nil, prooftypes.ErrProofServiceNotFound.Wrapf("service %s not found", serviceId) } return &service, nil From 20fe4e4cdd238bf0bd4256ed3e943000fbe049bd Mon Sep 17 00:00:00 2001 From: Arash <23505281+adshmh@users.noreply.github.com> Date: Thu, 22 Aug 2024 11:38:50 -0400 Subject: [PATCH 13/36] Update x/proof/keeper/msg_server_create_claim.go Co-authored-by: Bryan White --- x/proof/keeper/msg_server_create_claim.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/proof/keeper/msg_server_create_claim.go b/x/proof/keeper/msg_server_create_claim.go index 7762e69ad..b93751d94 100644 --- a/x/proof/keeper/msg_server_create_claim.go +++ b/x/proof/keeper/msg_server_create_claim.go @@ -100,7 +100,7 @@ func (k msgServer) CreateClaim( fmt.Sprintf("claim compute units: %d is not equal to number of relays %d * compute units per relay %d for service %s", numClaimComputeUnits, numRelays, - serviceCupr, + serviceComputeUnitsPerRelay, claim.SessionHeader.Service.Id, ), ).Error(), From ea07f5057d20d41903f0359abac1f9752674711d Mon Sep 17 00:00:00 2001 From: Arash <23505281+adshmh@users.noreply.github.com> Date: Thu, 22 Aug 2024 11:39:01 -0400 Subject: [PATCH 14/36] Update x/proof/keeper/msg_server_create_claim_test.go Co-authored-by: Bryan White --- x/proof/keeper/msg_server_create_claim_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/proof/keeper/msg_server_create_claim_test.go b/x/proof/keeper/msg_server_create_claim_test.go index b1cbd7a84..351e73911 100644 --- a/x/proof/keeper/msg_server_create_claim_test.go +++ b/x/proof/keeper/msg_server_create_claim_test.go @@ -34,7 +34,7 @@ const ( var ( defaultMerkleRoot = testproof.SmstRootWithSumAndCount(expectedNumComputeUnits, expectedNumRelays) - // Merkle root for Smst of a claim for the service with non-default custom units per relay + // Merkle root for Smst of a claim for the service with non-default compute units per relay customCuprMerkleRoot = testproof.SmstRootWithSumAndCount(expectedNonDefaultNumComputeUnits, expectedNumRelays) ) From 3edb218b9c54edc63a84d712e89f7bbf922cd595 Mon Sep 17 00:00:00 2001 From: Arash <23505281+adshmh@users.noreply.github.com> Date: Thu, 22 Aug 2024 11:39:23 -0400 Subject: [PATCH 15/36] Update x/proof/keeper/service.go Co-authored-by: Bryan White --- x/proof/keeper/service.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/proof/keeper/service.go b/x/proof/keeper/service.go index 1fb59c187..710d8c332 100644 --- a/x/proof/keeper/service.go +++ b/x/proof/keeper/service.go @@ -16,7 +16,7 @@ func (k Keeper) getServiceComputeUnitsPerRelay( service, found := k.serviceKeeper.GetService(ctx, serviceId) if !found { - return 0, fmt.Errorf("service %s not found", serviceId) + return 0, prooftypes.ErrProofServiceNotFound.Wrapf("service %s not found", serviceId) } logger. From 3a94474bee0444eb20f2ca58c3f8cc26d08f7e88 Mon Sep 17 00:00:00 2001 From: Arash Deshmeh Date: Mon, 26 Aug 2024 08:54:02 -0400 Subject: [PATCH 16/36] address review comments on relay fixture generation --- Makefile | 2 +- pkg/relayer/miner/gen/gen_fixtures.go | 22 +++++++++---------- pkg/relayer/miner/gen/template.go | 8 ++++--- pkg/relayer/miner/relay_fixtures_test.go | 28 +++++++++++++----------- 4 files changed, 31 insertions(+), 29 deletions(-) diff --git a/Makefile b/Makefile index b12cca3ea..316e086d6 100644 --- a/Makefile +++ b/Makefile @@ -472,7 +472,7 @@ go_testgen_accounts: ## Generate test accounts for usage in test environments go generate ./testutil/testkeyring/keyring.go .PHONY: go_develop -go_develop: check_ignite_version proto_regen go_mockgen ## Generate protos and mocks +go_develop: check_ignite_version proto_regen go_mockgen go_testgen_fixtures ## Generate protos and mocks .PHONY: go_develop_and_test go_develop_and_test: go_develop test_all ## Generate protos, mocks and run all tests diff --git a/pkg/relayer/miner/gen/gen_fixtures.go b/pkg/relayer/miner/gen/gen_fixtures.go index a81ea3213..7ea406a0a 100644 --- a/pkg/relayer/miner/gen/gen_fixtures.go +++ b/pkg/relayer/miner/gen/gen_fixtures.go @@ -35,10 +35,10 @@ const ( defaultRandLength = 16 defaultOutPath = "relay_fixtures_test.go" - // svcId is the ID of the service referenced in each relay request's metadata. + // defaultSvcID is the ID of the service referenced in each relay request's metadata. // This is passed to the tokenomics client to get the service's difficulty target hash. // It should match the testSvcId const in pkg/relayer/miner/miner_test.go - testSvcId = "svc1" + defaultSvcID = "svc1" ) // TODO_FOLLOWUP(@olshansk, #690): Do a global anycase grep for "DifficultyBits" and update/remove things appropriately. @@ -58,6 +58,9 @@ var ( // flagOut is the path to the generated file. flagOut string + + // flagSvcID is the service ID referenced in generated relay fixtures. + flagSvcID string ) // TODO_TECHDEBT: remove once marshaling using canonical codec. @@ -69,6 +72,7 @@ func init() { flag.StringVar(&flagDifficultyThresholdHashStr, "difficulty-threshold-hash-str", defaultDifficultyThresholdHashStr, "the difficulty threshold hash, in string format, that a randomized, serialized relay be greater than or equal to when hashed, to be included in the `marshaledMinableRelaysHex` slice which is generated. It is also used as the maximum difficulty threshold hash for relays to be included in the `marshaledUnminableRelaysHex` slice.") flag.IntVar(&flagFixtureLimitPerGroup, "fixture-limit-per-group", defaultFixtureLimitPerGroup, "the number of randomized, serialized relays that will be generated for each of `marshaledMinableRelaysHex` and `marshaledUnminableRelaysHex`.") flag.StringVar(&flagOut, "out", defaultOutPath, "the path to the generated file.") + flag.StringVar(&flagSvcID, "service-id", defaultSvcID, "the service ID referenced in generated relays.") } // This is utility for generating relay fixtures for testing. It is not intended @@ -94,8 +98,6 @@ func main() { randRelaysObs, errCh := genRandomizedMinedRelayFixtures( ctx, defaultRandLength, - // TODO_TECHDEBT: need to find out how the removal of miner.DefaultRelayerHasher in commit 608edc31b39c6ff9a285af46d1c1037cd36a7252 did not - // trigger any test failures while it was still referenced here (in the line that was in this comment's position before being deleted). ) exitOnError(errCh) @@ -159,7 +161,7 @@ func genRandomizedMinedRelayFixtures( SessionHeader: &sessiontypes.SessionHeader{ ApplicationAddress: sample.AccAddress(), Service: &sharedtypes.Service{ - Id: testSvcId, + Id: flagSvcID, }, SessionId: "session_id", SessionStartBlockHeight: 1, @@ -179,14 +181,12 @@ func genRandomizedMinedRelayFixtures( return } - // Hash relay bytes relayHashArr := protocol.GetRelayHashFromBytes(relayBz) - relayHash := relayHashArr[:] randBzPublishCh <- &relayer.MinedRelay{ Relay: relay, Bytes: relayBz, - Hash: relayHash, + Hash: relayHashArr[:], } } }() @@ -207,11 +207,9 @@ func exitOnError(errCh <-chan error) { // getDifficultyThresholdHash returns the difficulty threshold hash set by the // flagDifficultyThresholdHashStr flag in string formet, or the default value if // the flag has not been set. -func getDifficultyThresholdHash() [protocol.RelayHasherSize]byte { +func getDifficultyThresholdHash() []byte { difficultyThresholdHash, _ := hex.DecodeString(flagDifficultyThresholdHashStr) - var thresholdHashArr [protocol.RelayHasherSize]byte - copy(thresholdHashArr[:], difficultyThresholdHash) - return thresholdHashArr + return difficultyThresholdHash } // difficultyGTE returns true if the given hash has a difficulty greater than or diff --git a/pkg/relayer/miner/gen/template.go b/pkg/relayer/miner/gen/template.go index cf7a4fdad..64d598fe6 100644 --- a/pkg/relayer/miner/gen/template.go +++ b/pkg/relayer/miner/gen/template.go @@ -18,9 +18,11 @@ package miner_test var ( // marshaledMinableRelaysHex are the hex encoded strings of serialized - // relayer.MinedRelays which have been pre-mined to difficult {{.difficultyThresholdHashStr}} (or greater) by - // populating the signature with random bytes. It is intended for use - // in tests. + // relayer.MinedRelays which have been pre-mined to difficulty {{.difficultyThresholdHashStr}} (or greater) by + // randomizing the application address and populating the signature with random bytes. + // The ID, starting block height, and ending block height of the session referenced in the relays. + // are hard-coded to "session_id", 1, and 2, respectively. + // The resulting slice of minable relays is intended for use in tests. marshaledMinableRelaysHex = []string{ {{.MarshaledMinableRelaysHex}} } diff --git a/pkg/relayer/miner/relay_fixtures_test.go b/pkg/relayer/miner/relay_fixtures_test.go index fc0ceb27b..cf081f14c 100644 --- a/pkg/relayer/miner/relay_fixtures_test.go +++ b/pkg/relayer/miner/relay_fixtures_test.go @@ -7,15 +7,17 @@ package miner_test var ( // marshaledMinableRelaysHex are the hex encoded strings of serialized - // relayer.MinedRelays which have been pre-mined to difficulty 0000ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff by - // populating the signature with random bytes. It is intended for use - // in tests. + // relayer.MinedRelays which have been pre-mined to difficulty 0000ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff (or greater) by + // randomizing the application address and populating the signature with random bytes. + // The ID, starting block height, and ending block height of the session referenced in the relays. + // are hard-coded to "session_id", 1, and 2, respectively. + // The resulting slice of minable relays is intended for use in tests. marshaledMinableRelaysHex = []string{ - "0a5d0a5b0a470a2d636f736d6f7331736b68616e616d7a6666767478686a77737a796437636d6d703361776174706339786777733012060a04737663311a0a73657373696f6e5f696420012802121052cc1367527c76d24f584276c1fdc8fa", - "0a5d0a5b0a470a2d636f736d6f73316e73616537636a396161786b70726e326b796132346b6634787830346d39363265686434326d12060a04737663311a0a73657373696f6e5f696420012802121079a61fcd9a27848c5735122c855087e2", - "0a5d0a5b0a470a2d636f736d6f733134716a73303478616a337a737a643971773478747363336b61366a3933387a6479763076757712060a04737663311a0a73657373696f6e5f696420012802121012d86bfa37fc43ac6db7584f671f7d7d", - "0a5d0a5b0a470a2d636f736d6f73316836717a38766e716632347a6b7437723774727a7a6873736c6466776c376d6574667636726112060a04737663311a0a73657373696f6e5f6964200128021210f9bf6d38c0f24a4b44e840970aceef36", - "0a5d0a5b0a470a2d636f736d6f7331387863636e77367a327272613864736a6b3364646371366a656a376676306b3332797478386812060a04737663311a0a73657373696f6e5f6964200128021210d1f5c2dde31f2470e8a95fc298bafb09", + "0a5d0a5b0a470a2d636f736d6f7331763036716566367575343873346e6a7a6877683339793871653632707264753966733067337112060a04737663311a0a73657373696f6e5f69642001280212100983b27f62ffc0bceaf89dd5994ffa86", + "0a5d0a5b0a470a2d636f736d6f7331307930746634763539333733787076647a7638387932347a636b76737a713768683035706e3212060a04737663311a0a73657373696f6e5f69642001280212104eb6d1204a75a51893be017b968a9c7c", + "0a5d0a5b0a470a2d636f736d6f7331786566773036743473617330756c36726d6c306e637234736d676d6d323237786d32356a736512060a04737663311a0a73657373696f6e5f69642001280212107045bd106ea71338bf3e32eb530f7257", + "0a5d0a5b0a470a2d636f736d6f73316864336d63776573706a6a6a32376d336a7271346e703663386b656a633634716a79336c736e12060a04737663311a0a73657373696f6e5f69642001280212109ea55c51c6e59c5df61c4f99671e9da2", + "0a5d0a5b0a470a2d636f736d6f73317379386e74616b7579756861666e333373716d787372636335377a7172777238636a6635376c12060a04737663311a0a73657373696f6e5f69642001280212109d64b4c4060d5a18890cc665ef1a325a", } // marshaledUnminableRelaysHex are the hex encoded strings of serialized @@ -24,10 +26,10 @@ var ( // by populating the signature with random bytes. It is intended for use in // tests. marshaledUnminableRelaysHex = []string{ - "0a5d0a5b0a470a2d636f736d6f7331656e65616e637976336e6a6536673071647077397965337a6b7337726b7468776175326c683412060a04737663311a0a73657373696f6e5f696420012802121024d44328a07eb3c365ad54fdf0757f1b", - "0a5d0a5b0a470a2d636f736d6f7331796d6d6a777a3370736533303976737165666c38636c776870303436767239746b6c78636e6112060a04737663311a0a73657373696f6e5f69642001280212109f7dc58b68b5c9b723bd07cbf6379ded", - "0a5d0a5b0a470a2d636f736d6f73317a64347a666d337338617774776e6764756164786a787778677876633233743974616a37673512060a04737663311a0a73657373696f6e5f696420012802121083b3fd95164a12ceb98be02df65257fa", - "0a5d0a5b0a470a2d636f736d6f73316663646571397a79707130336b6a7a7768777a7435756c733271677464656668636863326c6612060a04737663311a0a73657373696f6e5f69642001280212107e415cb2a7b18d74427742da94ca74f9", - "0a5d0a5b0a470a2d636f736d6f733178736b7937777279307964736b75376577617230716a6b6b763671653864716575637464766812060a04737663311a0a73657373696f6e5f69642001280212107dcb1de0e616be6e619eacb42b026c5d", + "0a5d0a5b0a470a2d636f736d6f733135716a73396c383061757864617574733632646339747a6135646878777670687274306a747112060a04737663311a0a73657373696f6e5f696420012802121092fe03bcf79cf4c1bc5c73be8807bf57", + "0a5d0a5b0a470a2d636f736d6f7331747567617567703770616c7073347265736a773672686e73306b6c65363370676b67306e347912060a04737663311a0a73657373696f6e5f69642001280212107df68afdf890296a8d0b5ab2897d42c2", + "0a5d0a5b0a470a2d636f736d6f733176706a6a61757033686b686637637237706b7471647965726e6c35736a70673837706d76646a12060a04737663311a0a73657373696f6e5f6964200128021210eed9901b61bb430c28ba5d3c53facaf0", + "0a5d0a5b0a470a2d636f736d6f7331656b3935353239336d783637636c347664687038686e366c3872766d33346168767a3839706b12060a04737663311a0a73657373696f6e5f69642001280212108ef39c28c052336a5f9bc5fc07f3ef16", + "0a5d0a5b0a470a2d636f736d6f73313971326b707067373371767073636a736b306e6734757a70673076783672666d617739706b6712060a04737663311a0a73657373696f6e5f6964200128021210b14f04972472e130c61405ada3ff3f84", } ) From dbfe60453dd6812168d4934b7a668202eb977957 Mon Sep 17 00:00:00 2001 From: Arash Deshmeh Date: Mon, 26 Aug 2024 08:59:35 -0400 Subject: [PATCH 17/36] address review comments --- pkg/relayer/session/session_test.go | 8 +++++++- x/proof/keeper/msg_server_create_claim.go | 2 +- x/proof/keeper/msg_server_create_claim_test.go | 4 ++-- x/proof/keeper/service.go | 2 +- 4 files changed, 11 insertions(+), 5 deletions(-) diff --git a/pkg/relayer/session/session_test.go b/pkg/relayer/session/session_test.go index 8624895eb..c04d29fd9 100644 --- a/pkg/relayer/session/session_test.go +++ b/pkg/relayer/session/session_test.go @@ -89,7 +89,13 @@ func TestRelayerSessionsManager_Start(t *testing.T) { serviceQueryClientMock := testqueryclients.NewTestServiceQueryClient(t) - deps := depinject.Supply(blockClient, blockQueryClientMock, supplierClientMap, sharedQueryClientMock, serviceQueryClientMock) + deps := depinject.Supply( + blockClient, + blockQueryClientMock, + supplierClientMap, + sharedQueryClientMock, + serviceQueryClientMock, + ) storesDirectoryOpt := testrelayer.WithTempStoresDirectory(t) // Create a new relayer sessions manager. diff --git a/x/proof/keeper/msg_server_create_claim.go b/x/proof/keeper/msg_server_create_claim.go index b93751d94..2c0b92abd 100644 --- a/x/proof/keeper/msg_server_create_claim.go +++ b/x/proof/keeper/msg_server_create_claim.go @@ -88,7 +88,7 @@ func (k msgServer) CreateClaim( // numClaimComputeUnits = numRelays * service.computeUnitsPerRelay // This is because for any specific service, every relay is worth the same. // However, this may change in the future. - serviceComputeUnitsPerRelay, err := k.getServiceCupr(ctx, claim.SessionHeader.Service.Id) + serviceComputeUnitsPerRelay, err := k.getServiceComputeUnitsPerRelay(ctx, claim.SessionHeader.Service.Id) if err != nil { return nil, status.Error(codes.NotFound, types.ErrProofServiceNotFound.Wrapf("%v", err).Error()) } diff --git a/x/proof/keeper/msg_server_create_claim_test.go b/x/proof/keeper/msg_server_create_claim_test.go index 351e73911..8c33993c8 100644 --- a/x/proof/keeper/msg_server_create_claim_test.go +++ b/x/proof/keeper/msg_server_create_claim_test.go @@ -35,7 +35,7 @@ var ( defaultMerkleRoot = testproof.SmstRootWithSumAndCount(expectedNumComputeUnits, expectedNumRelays) // Merkle root for Smst of a claim for the service with non-default compute units per relay - customCuprMerkleRoot = testproof.SmstRootWithSumAndCount(expectedNonDefaultNumComputeUnits, expectedNumRelays) + customComputeUnitsPerRelayMerkleRoot = testproof.SmstRootWithSumAndCount(expectedNonDefaultNumComputeUnits, expectedNumRelays) ) func TestMsgServer_CreateClaim_Success(t *testing.T) { @@ -77,7 +77,7 @@ func TestMsgServer_CreateClaim_Success(t *testing.T) { { desc: "claim message for service with >1 compute units per relay", getClaimMsgHeight: shared.GetClaimWindowCloseHeight, - merkleRoot: customCuprMerkleRoot, + merkleRoot: customComputeUnitsPerRelayMerkleRoot, serviceComputeUnitsPerRelay: nonDefaultComputeUnitsPerRelay, expectedNumComputeUnits: expectedNonDefaultNumComputeUnits, }, diff --git a/x/proof/keeper/service.go b/x/proof/keeper/service.go index 710d8c332..2ffe310f6 100644 --- a/x/proof/keeper/service.go +++ b/x/proof/keeper/service.go @@ -12,7 +12,7 @@ func (k Keeper) getServiceComputeUnitsPerRelay( ctx context.Context, serviceId string, ) (uint64, error) { - logger := k.Logger().With("method", "getServiceCupr") + logger := k.Logger().With("method", "getServiceComputeUnitsPerRelay") service, found := k.serviceKeeper.GetService(ctx, serviceId) if !found { From 8de0752c26b9dd6a7ab30038c04236c37740c473 Mon Sep 17 00:00:00 2001 From: Arash Deshmeh Date: Mon, 26 Aug 2024 09:17:56 -0400 Subject: [PATCH 18/36] address review comment on sentinel errors --- pkg/relayer/miner/miner.go | 3 ++- pkg/relayer/session/errors.go | 2 ++ pkg/relayer/session/service.go | 6 +++--- x/proof/keeper/service.go | 5 +++-- 4 files changed, 10 insertions(+), 6 deletions(-) diff --git a/pkg/relayer/miner/miner.go b/pkg/relayer/miner/miner.go index b02c558e8..70b38aa68 100644 --- a/pkg/relayer/miner/miner.go +++ b/pkg/relayer/miner/miner.go @@ -118,7 +118,8 @@ func (mnr *miner) getServiceRelayDifficultyTargetHash(ctx context.Context, req * return nil, errors.New("relay request is nil") } - sessionHeader := req.GetMeta().GetSessionHeader() + meta := req.GetMeta() + sessionHeader := meta.GetSessionHeader() if sessionHeader == nil { return nil, errors.New("relay metadata has nil session header") } diff --git a/pkg/relayer/session/errors.go b/pkg/relayer/session/errors.go index bdd711d16..7acd69a57 100644 --- a/pkg/relayer/session/errors.go +++ b/pkg/relayer/session/errors.go @@ -12,4 +12,6 @@ var ( ErrSessionTreeAlreadyMarkedAsClaimed = sdkerrors.Register(codespace, 6, "session tree already marked as claimed") ErrSessionSupplierClientNotFound = sdkerrors.Register(codespace, 7, "supplier client not found") ErrSessionUpdatingTree = sdkerrors.Register(codespace, 8, "error updating session SMST") + ErrSessionRelayMetaHasNoServiceID = sdkerrors.Register(codespace, 9, "service ID not specified in relay metadata") + ErrSessionRelayMetaHasInvalidServiceID = sdkerrors.Register(codespace, 10, "service specified in relay metadata not found") ) diff --git a/pkg/relayer/session/service.go b/pkg/relayer/session/service.go index ff0f4ff0e..f1f29fbb4 100644 --- a/pkg/relayer/session/service.go +++ b/pkg/relayer/session/service.go @@ -2,7 +2,6 @@ package session import ( "context" - "fmt" "github.com/pokt-network/poktroll/x/service/types" ) @@ -16,12 +15,13 @@ func (rs *relayerSessionsManager) getServiceComputeUnitsPerRelay( ) (uint64, error) { sessionHeader := relayRequestMetadata.GetSessionHeader() if sessionHeader.GetService() == nil { - return 0, fmt.Errorf("getServiceComputeUnitsPerRelay: received nil service") + return 0, ErrSessionRelayMetaHasNoServiceID } service, err := rs.serviceQueryClient.GetService(ctx, sessionHeader.Service.Id) if err != nil { - return 0, fmt.Errorf("getServiceComputeUnitsPerRelay: could not get on-chain service %s: %w", + return 0, ErrSessionRelayMetaHasInvalidServiceID.Wrapf( + "getServiceComputeUnitsPerRelay: could not get on-chain service %s: %w", sessionHeader.Service.Id, err, ) diff --git a/x/proof/keeper/service.go b/x/proof/keeper/service.go index 2ffe310f6..1a8242037 100644 --- a/x/proof/keeper/service.go +++ b/x/proof/keeper/service.go @@ -2,7 +2,8 @@ package keeper import ( "context" - "fmt" + + "github.com/pokt-network/poktroll/x/proof/types" ) // getServiceComputeUnitsPerRelay is used to ensure that a service with the ServiceID exists. @@ -16,7 +17,7 @@ func (k Keeper) getServiceComputeUnitsPerRelay( service, found := k.serviceKeeper.GetService(ctx, serviceId) if !found { - return 0, prooftypes.ErrProofServiceNotFound.Wrapf("service %s not found", serviceId) + return 0, types.ErrProofServiceNotFound.Wrapf("service %s not found", serviceId) } logger. From 6e195a957147fcb6a42896d9bcd5666a51a33d38 Mon Sep 17 00:00:00 2001 From: Arash Deshmeh Date: Mon, 26 Aug 2024 09:32:51 -0400 Subject: [PATCH 19/36] address review comment on GetServiceRelayDifficultyTargetHash --- pkg/relayer/miner/miner.go | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/pkg/relayer/miner/miner.go b/pkg/relayer/miner/miner.go index 70b38aa68..3bf3bc260 100644 --- a/pkg/relayer/miner/miner.go +++ b/pkg/relayer/miner/miner.go @@ -130,13 +130,7 @@ func (mnr *miner) getServiceRelayDifficultyTargetHash(ctx context.Context, req * serviceRelayDifficulty, err := mnr.tokenomicsQueryClient.GetServiceRelayDifficultyTargetHash(ctx, sessionHeader.Service.Id) if err != nil { - // TODO_DISCUSS_IN_THIS_COMMIT: do we need to distinguish between errors? There could potentially be a scenario in the future where - // the query client fails for some reason other than "service has no relay difficulty set", which is currently the only - // error scenario. The line below could then lead to masking the error and returning a default value for a service that - // has a relay difficulty target hash set. - // Solving this needs updating the tokenomics keeper to use an additional boolean in the output to distinguish errors - // from the case where the service has no relay difficulty target hash set. - return protocol.BaseRelayDifficultyHashBz, nil + return nil, err } return serviceRelayDifficulty.GetTargetHash(), nil From 7bdcf1567ca249539b6305d91e8883312d5367c7 Mon Sep 17 00:00:00 2001 From: Arash <23505281+adshmh@users.noreply.github.com> Date: Tue, 27 Aug 2024 13:52:48 -0400 Subject: [PATCH 20/36] Apply suggestions from code review Co-authored-by: Bryan White --- pkg/relayer/miner/gen/gen_fixtures.go | 16 +++++++--------- pkg/relayer/miner/gen/template.go | 4 ++-- .../testqueryclients/servicequerier.go | 8 ++------ .../testqueryclients/tokenomicsquerier.go | 7 +------ 4 files changed, 12 insertions(+), 23 deletions(-) diff --git a/pkg/relayer/miner/gen/gen_fixtures.go b/pkg/relayer/miner/gen/gen_fixtures.go index 7ea406a0a..ee4068bdd 100644 --- a/pkg/relayer/miner/gen/gen_fixtures.go +++ b/pkg/relayer/miner/gen/gen_fixtures.go @@ -28,7 +28,7 @@ import ( ) const ( - // defaultDifficultyThresholdHashStr is the default difficulty hash in string formet, used to separate the generated relays into minable and unminable categories. + // defaultDifficultyThresholdHashStr is the default difficulty hash as a hex string, used to separate the generated relays into minable and unminable categories. // This hash should match the service-specific difficulty target hash used by the relay miner tests in pkg/relayer/miner/miner_test.go. defaultDifficultyThresholdHashStr = "0000ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff" defaultFixtureLimitPerGroup = 5 @@ -43,12 +43,11 @@ const ( // TODO_FOLLOWUP(@olshansk, #690): Do a global anycase grep for "DifficultyBits" and update/remove things appropriately. var ( - - // flagDifficultyThresholdHashStr is the difficulty threshold hash, in string format, that a + // flagDifficultyThresholdHashStr is the difficulty threshold hash, as a hex string, that a // randomized, serialized relay must be greater than to be included in the - // `marshaledMinableRelaysHex` slice which is generated. - // It is also used as the maximum difficulty threshold hash allowed for relays to be - // included in the `marshaledUnminableRelaysHex` slice. + // generated `marshaledMinableRelaysHex` slice. + // It is also used as the maximum difficulty threshold hash, below which, relays + // included in the generated `marshaledUnminableRelaysHex` slice. flagDifficultyThresholdHashStr string // flagFixtureLimitPerGroup is the number of randomized, serialized relays that will be @@ -204,9 +203,8 @@ func exitOnError(errCh <-chan error) { }() } -// getDifficultyThresholdHash returns the difficulty threshold hash set by the -// flagDifficultyThresholdHashStr flag in string formet, or the default value if -// the flag has not been set. +// getDifficultyThresholdHash returns the difficulty threshold hash represented by the +// flagDifficultyThresholdHashStr flag, or the default value if the flag has not been set. func getDifficultyThresholdHash() []byte { difficultyThresholdHash, _ := hex.DecodeString(flagDifficultyThresholdHashStr) return difficultyThresholdHash diff --git a/pkg/relayer/miner/gen/template.go b/pkg/relayer/miner/gen/template.go index 64d598fe6..52611e696 100644 --- a/pkg/relayer/miner/gen/template.go +++ b/pkg/relayer/miner/gen/template.go @@ -20,8 +20,8 @@ var ( // marshaledMinableRelaysHex are the hex encoded strings of serialized // relayer.MinedRelays which have been pre-mined to difficulty {{.difficultyThresholdHashStr}} (or greater) by // randomizing the application address and populating the signature with random bytes. - // The ID, starting block height, and ending block height of the session referenced in the relays. - // are hard-coded to "session_id", 1, and 2, respectively. + // The ID, starting block height, and ending block height of the session referenced + // in the relays are hard-coded to "session_id", 1, and 2, respectively. // The resulting slice of minable relays is intended for use in tests. marshaledMinableRelaysHex = []string{ {{.MarshaledMinableRelaysHex}} diff --git a/testutil/testclient/testqueryclients/servicequerier.go b/testutil/testclient/testqueryclients/servicequerier.go index 467326f65..7c3579d2f 100644 --- a/testutil/testclient/testqueryclients/servicequerier.go +++ b/testutil/testclient/testqueryclients/servicequerier.go @@ -2,21 +2,17 @@ package testqueryclients import ( "context" - "fmt" "testing" "github.com/golang/mock/gomock" "github.com/pokt-network/poktroll/testutil/mockclient" + prooftypes "github.com/pokt-network/poktroll/x/proof/types" sharedtypes "github.com/pokt-network/poktroll/x/shared/types" ) // services is a map of: serviceId -> Service -var services map[string]sharedtypes.Service - -func init() { - services = make(map[string]sharedtypes.Service) -} +var services = make(map[string]sharedtypes.Service) // NewTestSessionQueryClient creates a mock of the SessionQueryClient // which allows the caller to call GetSession any times and will return diff --git a/testutil/testclient/testqueryclients/tokenomicsquerier.go b/testutil/testclient/testqueryclients/tokenomicsquerier.go index f090e9218..82117116f 100644 --- a/testutil/testclient/testqueryclients/tokenomicsquerier.go +++ b/testutil/testclient/testqueryclients/tokenomicsquerier.go @@ -2,7 +2,6 @@ package testqueryclients import ( "context" - "fmt" "testing" "github.com/golang/mock/gomock" @@ -15,11 +14,7 @@ import ( // It is updated by the SetServiceRelayDifficultyTargetHash, and read by // the mock tokenomics query client to get a specific service's relay difficulty // target hash. -var relayDifficultyTargets map[string]*tokenomicstypes.RelayMiningDifficulty - -func init() { - relayDifficultyTargets = make(map[string]*tokenomicstypes.RelayMiningDifficulty) -} +var relayDifficultyTargets = make(map[string]*tokenomicstypes.RelayMiningDifficulty) // NewTestTokenomicsQueryClient creates a mock of the TokenomicsQueryClient // which allows the caller to call GetSession any times and will return From d0b66fac99b1a17f919b5f0da80701b572bd3d2e Mon Sep 17 00:00:00 2001 From: Arash Deshmeh Date: Wed, 28 Aug 2024 08:37:09 -0400 Subject: [PATCH 21/36] address review comment: re-generate test fixtures --- pkg/relayer/miner/relay_fixtures_test.go | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/pkg/relayer/miner/relay_fixtures_test.go b/pkg/relayer/miner/relay_fixtures_test.go index cf081f14c..ec4af0be0 100644 --- a/pkg/relayer/miner/relay_fixtures_test.go +++ b/pkg/relayer/miner/relay_fixtures_test.go @@ -13,11 +13,11 @@ var ( // are hard-coded to "session_id", 1, and 2, respectively. // The resulting slice of minable relays is intended for use in tests. marshaledMinableRelaysHex = []string{ - "0a5d0a5b0a470a2d636f736d6f7331763036716566367575343873346e6a7a6877683339793871653632707264753966733067337112060a04737663311a0a73657373696f6e5f69642001280212100983b27f62ffc0bceaf89dd5994ffa86", - "0a5d0a5b0a470a2d636f736d6f7331307930746634763539333733787076647a7638387932347a636b76737a713768683035706e3212060a04737663311a0a73657373696f6e5f69642001280212104eb6d1204a75a51893be017b968a9c7c", - "0a5d0a5b0a470a2d636f736d6f7331786566773036743473617330756c36726d6c306e637234736d676d6d323237786d32356a736512060a04737663311a0a73657373696f6e5f69642001280212107045bd106ea71338bf3e32eb530f7257", - "0a5d0a5b0a470a2d636f736d6f73316864336d63776573706a6a6a32376d336a7271346e703663386b656a633634716a79336c736e12060a04737663311a0a73657373696f6e5f69642001280212109ea55c51c6e59c5df61c4f99671e9da2", - "0a5d0a5b0a470a2d636f736d6f73317379386e74616b7579756861666e333373716d787372636335377a7172777238636a6635376c12060a04737663311a0a73657373696f6e5f69642001280212109d64b4c4060d5a18890cc665ef1a325a", + "0a5d0a5b0a470a2d636f736d6f73317a387336676d6335306e6a38306e71367138356e66376c3663723477636a766b61643779386812060a04737663311a0a73657373696f6e5f6964200128021210db053f9e0abf922e6cddc648678d0a88", + "0a5d0a5b0a470a2d636f736d6f7331646d7139646e3077777677307a3872673930706e737532786835676c3963676c6c77786d6d3812060a04737663311a0a73657373696f6e5f6964200128021210a2e27cad60751773b2468483a7aabee4", + "0a5d0a5b0a470a2d636f736d6f733130746772796737336438763979707a68763779616e636174346d6b6473686c347078397a613312060a04737663311a0a73657373696f6e5f696420012802121078522b759b6ed8c0d2891d21dcaf4d74", + "0a5d0a5b0a470a2d636f736d6f7331637968727463337a65656e7a63616872306c396a37646d3865687a6b30776e75616b3574347912060a04737663311a0a73657373696f6e5f6964200128021210139f82fd26920aa23dea91dda0ee5954", + "0a5d0a5b0a470a2d636f736d6f73317a64666874346d793264783779736336343638656565336a7a7165636865793733756d77337a12060a04737663311a0a73657373696f6e5f69642001280212102f525aa98228d86ec92eb090dda16418", } // marshaledUnminableRelaysHex are the hex encoded strings of serialized @@ -26,10 +26,10 @@ var ( // by populating the signature with random bytes. It is intended for use in // tests. marshaledUnminableRelaysHex = []string{ - "0a5d0a5b0a470a2d636f736d6f733135716a73396c383061757864617574733632646339747a6135646878777670687274306a747112060a04737663311a0a73657373696f6e5f696420012802121092fe03bcf79cf4c1bc5c73be8807bf57", - "0a5d0a5b0a470a2d636f736d6f7331747567617567703770616c7073347265736a773672686e73306b6c65363370676b67306e347912060a04737663311a0a73657373696f6e5f69642001280212107df68afdf890296a8d0b5ab2897d42c2", - "0a5d0a5b0a470a2d636f736d6f733176706a6a61757033686b686637637237706b7471647965726e6c35736a70673837706d76646a12060a04737663311a0a73657373696f6e5f6964200128021210eed9901b61bb430c28ba5d3c53facaf0", - "0a5d0a5b0a470a2d636f736d6f7331656b3935353239336d783637636c347664687038686e366c3872766d33346168767a3839706b12060a04737663311a0a73657373696f6e5f69642001280212108ef39c28c052336a5f9bc5fc07f3ef16", - "0a5d0a5b0a470a2d636f736d6f73313971326b707067373371767073636a736b306e6734757a70673076783672666d617739706b6712060a04737663311a0a73657373696f6e5f6964200128021210b14f04972472e130c61405ada3ff3f84", + "0a5d0a5b0a470a2d636f736d6f733173726775717a6e3366376435753933366d39356c6d7577757a636b7461706b726d743477783712060a04737663311a0a73657373696f6e5f696420012802121025329160fdc25be9d2f1747c1ffb4e78", + "0a5d0a5b0a470a2d636f736d6f73317433306d796d3033736463386e766479793936776b78346e6d7672713565666432786836797212060a04737663311a0a73657373696f6e5f696420012802121060f8483a9c8ec7e94d16afda952dccf9", + "0a5d0a5b0a470a2d636f736d6f733163686e30677972667a346664666a7a7573676d356376636868726371373770327767716d376312060a04737663311a0a73657373696f6e5f69642001280212100674c9bd3b43f1d7520e6e33a62430ac", + "0a5d0a5b0a470a2d636f736d6f7331656573687932386e63376d6b736573713366636d6a77356e333761333234677267743466393712060a04737663311a0a73657373696f6e5f6964200128021210d4d45eb1ba69be25ccb217f5bdc0ddf5", + "0a5d0a5b0a470a2d636f736d6f733137366a683337726c6c75376e73726d736c7072327332763476723532383075326e323338306c12060a04737663311a0a73657373696f6e5f6964200128021210ad8281326ac760bc40e3c71d48b2bf51", } ) From b5bfcffee5b3815e5c0a86bed9aad30c8f743f72 Mon Sep 17 00:00:00 2001 From: Arash Deshmeh Date: Wed, 28 Aug 2024 08:40:51 -0400 Subject: [PATCH 22/36] Add a compile check target for miner fixtures generator --- Makefile | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index 316e086d6..b8cff71c3 100644 --- a/Makefile +++ b/Makefile @@ -429,7 +429,7 @@ test_verbose: check_go_version ## Run all go tests verbosely # It is not compatible with `-race`, which is why it's omitted here. # See ref for more details: https://github.com/golang/go/issues/54482#issuecomment-1251124908 .PHONY: test_all -test_all: warn_flaky_tests check_go_version ## Run all go tests showing detailed output only on failures +test_all: warn_flaky_tests check_go_version test_gen_fixtures ## Run all go tests showing detailed output only on failures go test -count=1 -buildmode=pie -tags test ./... .PHONY: test_all_with_integration @@ -451,6 +451,14 @@ test_integration: check_go_version ## Run only the in-memory integration "unit" itest: check_go_version ## Run tests iteratively (see usage for more) ./tools/scripts/itest.sh $(filter-out $@,$(MAKECMDGOALS)) +# Verify there are no compile errors in pkg/relayer/miner/gen directory. +# This is done by overriding the build tags through passing a `*.go` argument to `go test`. +# .go files in that directory contain a `go:build ignore` directive to avoid introducing +# unintentional churn in the randomly generated fixtures. +.PHONY: test_gen_fixtures +test_gen_fixtures: check_go_version ## Run all go tests verbosely + go test -count=1 -v -race ./pkg/relayer/miner/gen/*.go + .PHONY: go_mockgen go_mockgen: ## Use `mockgen` to generate mocks used for testing purposes of all the modules. find . -name "*_mock.go" | xargs --no-run-if-empty rm @@ -472,7 +480,7 @@ go_testgen_accounts: ## Generate test accounts for usage in test environments go generate ./testutil/testkeyring/keyring.go .PHONY: go_develop -go_develop: check_ignite_version proto_regen go_mockgen go_testgen_fixtures ## Generate protos and mocks +go_develop: check_ignite_version proto_regen go_mockgen ## Generate protos and mocks .PHONY: go_develop_and_test go_develop_and_test: go_develop test_all ## Generate protos, mocks and run all tests From c220a8249d5ff36e336dcaf5a1889fe909628ff8 Mon Sep 17 00:00:00 2001 From: Arash Deshmeh Date: Wed, 28 Aug 2024 09:01:54 -0400 Subject: [PATCH 23/36] Fix linter warning --- pkg/relayer/session/service.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/relayer/session/service.go b/pkg/relayer/session/service.go index f1f29fbb4..6ecd1d0e1 100644 --- a/pkg/relayer/session/service.go +++ b/pkg/relayer/session/service.go @@ -21,7 +21,7 @@ func (rs *relayerSessionsManager) getServiceComputeUnitsPerRelay( service, err := rs.serviceQueryClient.GetService(ctx, sessionHeader.Service.Id) if err != nil { return 0, ErrSessionRelayMetaHasInvalidServiceID.Wrapf( - "getServiceComputeUnitsPerRelay: could not get on-chain service %s: %w", + "getServiceComputeUnitsPerRelay: could not get on-chain service %s: %v", sessionHeader.Service.Id, err, ) From 3e56c16e1c0a22fda5030c9fc58ae4cb7bea1273 Mon Sep 17 00:00:00 2001 From: Arash <23505281+adshmh@users.noreply.github.com> Date: Thu, 29 Aug 2024 08:28:52 -0400 Subject: [PATCH 24/36] Update pkg/relayer/session/session.go Co-authored-by: Daniel Olshansky --- pkg/relayer/session/session.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/relayer/session/session.go b/pkg/relayer/session/session.go index 7988092a5..bb63746ca 100644 --- a/pkg/relayer/session/session.go +++ b/pkg/relayer/session/session.go @@ -427,7 +427,6 @@ func (rs *relayerSessionsManager) mapAddMinedRelayToSessionTree( serviceComputeUnitsPerRelay, err := rs.getServiceComputeUnitsPerRelay(ctx, &relayMetadata) if err != nil { - // TODO_IMPROVE: log additional info? rs.logger.Error().Err(err).Msg("failed to get service compute units per relay") return err, false } From d0e39842fab66a0ae41025d228e294f145bbc34b Mon Sep 17 00:00:00 2001 From: Arash <23505281+adshmh@users.noreply.github.com> Date: Thu, 29 Aug 2024 08:29:47 -0400 Subject: [PATCH 25/36] Update pkg/deps/config/suppliers.go Co-authored-by: Daniel Olshansky --- pkg/deps/config/suppliers.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pkg/deps/config/suppliers.go b/pkg/deps/config/suppliers.go index 0a11084b8..494dfa453 100644 --- a/pkg/deps/config/suppliers.go +++ b/pkg/deps/config/suppliers.go @@ -484,6 +484,9 @@ func NewSupplyTokenomicsQueryClientFn() SupplierFn { } } +// NewSupplyServiceQueryClientFn returns a function which constructs a +// NewSupplyServiceQueryClient instance and returns a new depinject.Config which +// is supplied with the given deps and the new ServiceQueryClient. func NewSupplyServiceQueryClientFn() SupplierFn { return func( _ context.Context, From f6b931b411a7d7b2eff444cebf1c52a6d64aa6b3 Mon Sep 17 00:00:00 2001 From: Arash <23505281+adshmh@users.noreply.github.com> Date: Thu, 29 Aug 2024 08:30:52 -0400 Subject: [PATCH 26/36] Update pkg/relayer/session/session_test.go Co-authored-by: Daniel Olshansky --- pkg/relayer/session/session_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/relayer/session/session_test.go b/pkg/relayer/session/session_test.go index c04d29fd9..d9bbe56ba 100644 --- a/pkg/relayer/session/session_test.go +++ b/pkg/relayer/session/session_test.go @@ -32,7 +32,7 @@ import ( // TODO_TEST: Add a test case which simulates a cold-started relayminer with unclaimed relays. -// TODO_INCOMPLETE: Add a test case which verifies that the service's compute units per relay is used as +// TODO_BETA(@red-0ne): Add a test case which verifies that the service's compute units per relay is used as // the weight of a relay when updating a session's SMT. func TestRelayerSessionsManager_Start(t *testing.T) { From 5cda533293ed8018023d57ef6bfa41bbfebadfd1 Mon Sep 17 00:00:00 2001 From: Arash <23505281+adshmh@users.noreply.github.com> Date: Thu, 29 Aug 2024 08:31:37 -0400 Subject: [PATCH 27/36] Update x/proof/keeper/msg_server_create_claim.go Co-authored-by: Daniel Olshansky --- x/proof/keeper/msg_server_create_claim.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/proof/keeper/msg_server_create_claim.go b/x/proof/keeper/msg_server_create_claim.go index 2c0b92abd..00e9d272b 100644 --- a/x/proof/keeper/msg_server_create_claim.go +++ b/x/proof/keeper/msg_server_create_claim.go @@ -84,7 +84,7 @@ func (k msgServer) CreateClaim( return nil, status.Error(codes.Internal, types.ErrProofInvalidClaimRootHash.Wrapf("%v", err).Error()) } - // For now, we expect the following equation to always hold: + // DEV_NOTE: For now, we expect the following equation to always hold: // numClaimComputeUnits = numRelays * service.computeUnitsPerRelay // This is because for any specific service, every relay is worth the same. // However, this may change in the future. From 5fc003b610be2db9e4b3f64fb075a2d84cc5e7fa Mon Sep 17 00:00:00 2001 From: Arash <23505281+adshmh@users.noreply.github.com> Date: Tue, 3 Sep 2024 10:13:17 -0400 Subject: [PATCH 28/36] Update x/proof/keeper/msg_server_create_claim_test.go Co-authored-by: Daniel Olshansky --- .../keeper/msg_server_create_claim_test.go | 20 +++++++++---------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/x/proof/keeper/msg_server_create_claim_test.go b/x/proof/keeper/msg_server_create_claim_test.go index 8c33993c8..f8376e9b2 100644 --- a/x/proof/keeper/msg_server_create_claim_test.go +++ b/x/proof/keeper/msg_server_create_claim_test.go @@ -545,6 +545,7 @@ func TestMsgServer_CreateClaim_Error_ComputeUnitsMismatch(t *testing.T) { // The base session start height used for testing sessionStartHeight := int64(1) + // service is the only service for which a session should exist. // this service has a value of greater than 1 for the compute units per relay. service := &sharedtypes.Service{ @@ -552,16 +553,13 @@ func TestMsgServer_CreateClaim_Error_ComputeUnitsMismatch(t *testing.T) { ComputeUnitsPerRelay: nonDefaultComputeUnitsPerRelay, OwnerAddress: sample.AccAddress(), } - // supplierAddr is staked for "svc1" such that it is expected to be in the session. - supplierAddr := sample.AccAddress() - - // appAddr is staked for "svc1" such that it is expected to be in the session. - appAddr := sample.AccAddress() - - supplierKeeper := keepers.SupplierKeeper - appKeeper := keepers.ApplicationKeeper + // Add the service that is expected to be on-chain. + keepers.SetService(ctx, *service) // Add a supplier that is expected to be in the session. + // supplierAddr is staked for "svc1" such that it is expected to be in the session. + supplierKeeper := keepers.SupplierKeeper + supplierAddr := sample.AccAddress() supplierKeeper.SetSupplier(ctx, sharedtypes.Supplier{ OperatorAddress: supplierAddr, Services: []*sharedtypes.SupplierServiceConfig{ @@ -570,6 +568,9 @@ func TestMsgServer_CreateClaim_Error_ComputeUnitsMismatch(t *testing.T) { }) // Add an application that is expected to be in the session. + // appAddr is staked for "svc1" such that it is expected to be in the session. + appKeeper := keepers.ApplicationKeeper + appAddr := sample.AccAddress() appKeeper.SetApplication(ctx, apptypes.Application{ Address: appAddr, ServiceConfigs: []*sharedtypes.ApplicationServiceConfig{ @@ -577,9 +578,6 @@ func TestMsgServer_CreateClaim_Error_ComputeUnitsMismatch(t *testing.T) { }, }) - // Add the service that is expected to be on-chain. - keepers.SetService(ctx, *service) - // Query for the session which contains the expected app and supplier pair. sessionRes, err := keepers.SessionKeeper.GetSession( ctx, From 35f43a9f3f2a6b2dd4a766a527be2db47fa9cec1 Mon Sep 17 00:00:00 2001 From: Arash <23505281+adshmh@users.noreply.github.com> Date: Tue, 3 Sep 2024 10:14:05 -0400 Subject: [PATCH 29/36] Update x/proof/keeper/msg_server_create_claim_test.go Co-authored-by: Daniel Olshansky --- x/proof/keeper/msg_server_create_claim_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/x/proof/keeper/msg_server_create_claim_test.go b/x/proof/keeper/msg_server_create_claim_test.go index f8376e9b2..69772d46e 100644 --- a/x/proof/keeper/msg_server_create_claim_test.go +++ b/x/proof/keeper/msg_server_create_claim_test.go @@ -602,6 +602,7 @@ func TestMsgServer_CreateClaim_Error_ComputeUnitsMismatch(t *testing.T) { sdkCtx = sdkCtx.WithBlockHeight(testClaimHeight) ctx = sdkCtx + // Prepare a message to submit a claim while also creating a new test claim testClaimMsg := newTestClaimMsg(t, sessionStartHeight, sessionRes.GetSession().GetSessionId(), From 2eea7034c2f0e453bd68081b19b88ba38eb3a475 Mon Sep 17 00:00:00 2001 From: Arash <23505281+adshmh@users.noreply.github.com> Date: Tue, 3 Sep 2024 10:20:03 -0400 Subject: [PATCH 30/36] Update x/proof/keeper/msg_server_create_claim_test.go Co-authored-by: Daniel Olshansky --- x/proof/keeper/msg_server_create_claim_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/x/proof/keeper/msg_server_create_claim_test.go b/x/proof/keeper/msg_server_create_claim_test.go index 69772d46e..e7ca9dba8 100644 --- a/x/proof/keeper/msg_server_create_claim_test.go +++ b/x/proof/keeper/msg_server_create_claim_test.go @@ -619,8 +619,9 @@ func TestMsgServer_CreateClaim_Error_ComputeUnitsMismatch(t *testing.T) { testClaimNumRelays, err := testClaim.GetNumRelays() require.NoError(t, err) + // Ensure that submitting the claim fails because the number of compute units + // claimed does not match the expected amount as a function of (relay, service_CUPR) createClaimRes, err := srv.CreateClaim(ctx, testClaimMsg) - require.ErrorContains(t, err, status.Error( From 5c76ec4a7f73a2c25bd6ff5d94d1133467e45d0b Mon Sep 17 00:00:00 2001 From: Arash <23505281+adshmh@users.noreply.github.com> Date: Tue, 3 Sep 2024 10:20:32 -0400 Subject: [PATCH 31/36] Update x/proof/keeper/msg_server_create_claim.go Co-authored-by: Daniel Olshansky --- x/proof/keeper/msg_server_create_claim.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/x/proof/keeper/msg_server_create_claim.go b/x/proof/keeper/msg_server_create_claim.go index 00e9d272b..c4c0ba2bc 100644 --- a/x/proof/keeper/msg_server_create_claim.go +++ b/x/proof/keeper/msg_server_create_claim.go @@ -93,7 +93,8 @@ func (k msgServer) CreateClaim( return nil, status.Error(codes.NotFound, types.ErrProofServiceNotFound.Wrapf("%v", err).Error()) } - if numClaimComputeUnits != numRelays*serviceComputeUnitsPerRelay { + numExpectedComputeUnitsToClaim := numRelays*serviceComputeUnitsPerRelay + if numClaimComputeUnits != numExpectedComputeUnitsToClaim { return nil, status.Error( codes.InvalidArgument, types.ErrProofComputeUnitsMismatch.Wrap( From 6cdb0c6128e12479c81509caa78576c7e95e4162 Mon Sep 17 00:00:00 2001 From: Arash Deshmeh Date: Tue, 3 Sep 2024 17:29:47 -0400 Subject: [PATCH 32/36] address review comment on testqueryclients --- testutil/testclient/testqueryclients/servicequerier.go | 2 ++ testutil/testclient/testqueryclients/tokenomicsquerier.go | 3 +++ 2 files changed, 5 insertions(+) diff --git a/testutil/testclient/testqueryclients/servicequerier.go b/testutil/testclient/testqueryclients/servicequerier.go index 7c3579d2f..394e5e75e 100644 --- a/testutil/testclient/testqueryclients/servicequerier.go +++ b/testutil/testclient/testqueryclients/servicequerier.go @@ -11,6 +11,8 @@ import ( sharedtypes "github.com/pokt-network/poktroll/x/shared/types" ) +// TODO_TECHDEBT: refactor the methods using this variable to avoid having a global scope +// for the map across unit tests run under the same testing.T instance. // services is a map of: serviceId -> Service var services = make(map[string]sharedtypes.Service) diff --git a/testutil/testclient/testqueryclients/tokenomicsquerier.go b/testutil/testclient/testqueryclients/tokenomicsquerier.go index 82117116f..dc8c2e72e 100644 --- a/testutil/testclient/testqueryclients/tokenomicsquerier.go +++ b/testutil/testclient/testqueryclients/tokenomicsquerier.go @@ -10,6 +10,9 @@ import ( tokenomicstypes "github.com/pokt-network/poktroll/x/tokenomics/types" ) +// TODO_TECHDEBT: refactor the methods using this variable to avoid having a global scope +// for the map across unit tests run under the same testing.T instance. +// Ditto for other similar package-level variables in this package. // relayDifficultyTargets is a map of: serviceId -> RelayMiningDifficulty // It is updated by the SetServiceRelayDifficultyTargetHash, and read by // the mock tokenomics query client to get a specific service's relay difficulty From e8af1635539501dc0ae77c9db5e05bf057aaf75b Mon Sep 17 00:00:00 2001 From: Arash Deshmeh Date: Tue, 3 Sep 2024 17:36:07 -0400 Subject: [PATCH 33/36] Empty commit From ec98f2d9908c9a6955161055b64bada5d0be0d6a Mon Sep 17 00:00:00 2001 From: Arash Deshmeh Date: Tue, 3 Sep 2024 18:07:32 -0400 Subject: [PATCH 34/36] fix linter warning --- x/proof/keeper/msg_server_create_claim_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/proof/keeper/msg_server_create_claim_test.go b/x/proof/keeper/msg_server_create_claim_test.go index e7ca9dba8..f08f64a70 100644 --- a/x/proof/keeper/msg_server_create_claim_test.go +++ b/x/proof/keeper/msg_server_create_claim_test.go @@ -545,7 +545,7 @@ func TestMsgServer_CreateClaim_Error_ComputeUnitsMismatch(t *testing.T) { // The base session start height used for testing sessionStartHeight := int64(1) - + // service is the only service for which a session should exist. // this service has a value of greater than 1 for the compute units per relay. service := &sharedtypes.Service{ From 28d0c09263240829017e4a4bd26123a19936ed8a Mon Sep 17 00:00:00 2001 From: Arash Deshmeh Date: Tue, 3 Sep 2024 18:30:15 -0400 Subject: [PATCH 35/36] fix linter warning --- x/proof/keeper/msg_server_create_claim.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/proof/keeper/msg_server_create_claim.go b/x/proof/keeper/msg_server_create_claim.go index c4c0ba2bc..44a1a508a 100644 --- a/x/proof/keeper/msg_server_create_claim.go +++ b/x/proof/keeper/msg_server_create_claim.go @@ -93,7 +93,7 @@ func (k msgServer) CreateClaim( return nil, status.Error(codes.NotFound, types.ErrProofServiceNotFound.Wrapf("%v", err).Error()) } - numExpectedComputeUnitsToClaim := numRelays*serviceComputeUnitsPerRelay + numExpectedComputeUnitsToClaim := numRelays * serviceComputeUnitsPerRelay if numClaimComputeUnits != numExpectedComputeUnitsToClaim { return nil, status.Error( codes.InvalidArgument, From 9ccb738d87b783fde63093d2e6f21857779289c7 Mon Sep 17 00:00:00 2001 From: Arash Deshmeh Date: Wed, 4 Sep 2024 19:01:48 -0400 Subject: [PATCH 36/36] Use the default relay difficulty if none is found for the service --- pkg/relayer/miner/miner.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/pkg/relayer/miner/miner.go b/pkg/relayer/miner/miner.go index 3bf3bc260..bfa6a8dac 100644 --- a/pkg/relayer/miner/miner.go +++ b/pkg/relayer/miner/miner.go @@ -15,6 +15,7 @@ import ( "github.com/pokt-network/poktroll/pkg/observable/filter" "github.com/pokt-network/poktroll/pkg/observable/logging" "github.com/pokt-network/poktroll/pkg/relayer" + prooftypes "github.com/pokt-network/poktroll/x/proof/types" servicetypes "github.com/pokt-network/poktroll/x/service/types" ) @@ -130,7 +131,9 @@ func (mnr *miner) getServiceRelayDifficultyTargetHash(ctx context.Context, req * serviceRelayDifficulty, err := mnr.tokenomicsQueryClient.GetServiceRelayDifficultyTargetHash(ctx, sessionHeader.Service.Id) if err != nil { - return nil, err + // TODO_IMPROVE: log the error and a message saying the default relay difficulty target hash + // is being used. + return prooftypes.DefaultRelayDifficultyTargetHash, nil } return serviceRelayDifficulty.GetTargetHash(), nil