Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SessionManager] Implement off-chain proof params usage #765

Merged
merged 16 commits into from
Sep 6, 2024
Merged
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 36 additions & 2 deletions e2e/tests/0_settlement.feature
Original file line number Diff line number Diff line change
@@ -7,7 +7,7 @@

Feature: Tokenomics Namespace

Scenario: Emissions equals burn when a claim is created and a valid proof is submitted and required
Scenario: Emissions equals burn when a claim is created and a valid proof is submitted and required via threshold
# Baseline
Given the user has the pocketd binary installed
# Network preparation
@@ -17,11 +17,45 @@ Feature: Tokenomics Namespace
And the "application" account for "app1" is staked
And the service "anvil" registered for application "app1" has a compute units per relay of "1"
# Start servicing
# Set proof_requirement_threshold to 9 < num_relays (10) * compute_units_per_relay (1)
red-0ne marked this conversation as resolved.
Show resolved Hide resolved
# to make sure a proof is required.
And the "proof" module parameters are set as follows
| name | value | type |
| relay_difficulty_target_hash | ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff | bytes |
| proof_request_probability | 0.25 | float |
| proof_requirement_threshold | 9 | int64 |
| proof_missing_penalty | 320 | coin |
When the supplier "supplier1" has serviced a session with "10" relays for service "anvil" for application "app1"
# Wait for the Claim & Proof lifecycle
And the user should wait for the "proof" module "CreateClaim" Message to be submitted
And the user should wait for the "proof" module "SubmitProof" Message to be submitted
And the user should wait for the "tokenomics" module "ClaimSettled" end block event to be broadcast
And the user should wait for the ClaimSettled event with "THRESHOLD" proof requirement to be broadcast
# Validate the results
Then the account balance of "supplier1" should be "420" uPOKT "more" than before
And the "application" stake of "app1" should be "420" uPOKT "less" than before

Scenario: Emissions equals burn when a claim is created but a proof is not required
# Baseline
Given the user has the pocketd binary installed
# Network preparation
And an account exists for "supplier1"
And the "supplier" account for "supplier1" is staked
And an account exists for "app1"
And the "application" account for "app1" is staked
And the service "anvil" registered for application "app1" has a compute units per relay of "1"
# Set proof_request_probability to 0 and proof_requirement_threshold to 100 to make sure a proof is not required.
And the "proof" module parameters are set as follows
| name | value | type |
| relay_difficulty_target_hash | ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff | bytes |
| proof_request_probability | 0 | float |
| proof_requirement_threshold | 100 | int64 |
| proof_missing_penalty | 320 | coin |
# Start servicing
When the supplier "supplier1" has serviced a session with "10" relays for service "anvil" for application "app1"
# Wait for the Claim & Proof lifecycle
And the user should wait for the "proof" module "CreateClaim" Message to be submitted
# No proof should be submitted, don't wait for one.
And the user should wait for the ClaimSettled event with "NOT_REQUIRED" proof requirement to be broadcast
# Validate the results
Then the account balance of "supplier1" should be "420" uPOKT "more" than before
And the "application" stake of "app1" should be "420" uPOKT "less" than before
2 changes: 1 addition & 1 deletion e2e/tests/init_test.go
Original file line number Diff line number Diff line change
@@ -331,7 +331,7 @@ func (s *suite) getConfigFileContent(
services:
- service_id: %s
endpoints:
- publicly_exposed_url: http://relayminer:8545
- publicly_exposed_url: http://relayminer1:8545
rpc_type: json_rpc`,
ownerAddress, operatorAddress, amount, serviceId)
default:
10 changes: 9 additions & 1 deletion e2e/tests/session.feature
Original file line number Diff line number Diff line change
@@ -2,7 +2,15 @@ Feature: Session Namespace

Scenario: Supplier completes claim/proof lifecycle for a valid session
Given the user has the pocketd binary installed
When the supplier "supplier1" has serviced a session with "5" relays for service "svc1" for application "app1"
# Set proof_requirement_threshold to 4 < num_relays (5) * compute_units_per_relay (1)
# to make sure a proof is required.
And the "proof" module parameters are set as follows
| name | value | type |
| relay_difficulty_target_hash | ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff | bytes |
| proof_request_probability | 0.25 | float |
| proof_requirement_threshold | 4 | int64 |
| proof_missing_penalty | 320 | coin |
When the supplier "supplier1" has serviced a session with "5" relays for service "anvil" for application "app1"
And the user should wait for the "proof" module "CreateClaim" Message to be submitted
And the user should wait for the "proof" module "ClaimCreated" tx event to be broadcast
Then the claim created by supplier "supplier1" for service "svc1" for application "app1" should be persisted on-chain
Copy link
Contributor

Choose a reason for hiding this comment

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

Am I understanding correctly that eliminating this step improved flakiness because the claim was being pruned (after settlement) too quickly for the test?

My main concern is that while the ClaimSettled event may be a valid signal that the claim was created ( subsequently settled, and pruned), the step removed here attempts to assert that:

  1. The claim is retrievable by the CLI (end-to-end-to-end test)
  2. That the retrieved query satisfies our expectations

Do you see a way we could maintain coverage around those assertions?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this is coming from the assertion we make about the claims count

	require.Greater(s, len(allClaimsRes.Claims), len(preExistingClaims), "number of claims must have increased")

Which in some circumstances (tests repeated to quickly) fails because the previous claim gets removed.

I'll put back the step even if it's at the cost of some flakiness.

18 changes: 16 additions & 2 deletions e2e/tests/session_steps_test.go
Original file line number Diff line number Diff line change
@@ -11,6 +11,7 @@ import (

abci "github.com/cometbft/cometbft/abci/types"
cosmostypes "github.com/cosmos/cosmos-sdk/types"
"github.com/regen-network/gocuke"
"github.com/stretchr/testify/require"

"github.com/pokt-network/poktroll/pkg/client/block"
@@ -65,11 +66,12 @@ func (s *suite) TheUserShouldWaitForTheModuleTxEventToBeBroadcast(module, eventT
s.waitForTxResultEvent(newEventTypeMatchFn(module, eventType))
}

func (s *suite) TheUserShouldWaitForTheModuleEndBlockEventToBeBroadcast(module, eventType string) {
func (s *suite) TheUserShouldWaitForTheClaimsettledEventWithProofRequirementToBeBroadcast(proofRequirement string) {
s.waitForNewBlockEvent(
combineEventMatchFns(
newEventTypeMatchFn(module, eventType),
newEventTypeMatchFn("tokenomics", "ClaimSettled"),
newEventModeMatchFn("EndBlock"),
newEventAttributeMatchFn("proof_requirement", fmt.Sprintf("%q", proofRequirement)),
),
)
}
@@ -167,6 +169,18 @@ func (s *suite) TheClaimCreatedBySupplierForServiceForApplicationShouldBeSuccess
s.waitForNewBlockEvent(isValidClaimSettledEvent)
}

func (suite *suite) TheModuleParametersAreSetAsFollows(moduleName string, params gocuke.DataTable) {
suite.AnAuthzGrantFromTheAccountToTheAccountForTheMessageExists(
"gov",
"module",
"pnf",
"user",
fmt.Sprintf("/poktroll.%s.MsgUpdateParams", moduleName),
)

suite.TheAccountSendsAnAuthzExecMessageToUpdateAllModuleParams("pnf", moduleName, params)
}

func (s *suite) sendRelaysForSession(
appName string,
supplierOperatorName string,
1 change: 1 addition & 0 deletions pkg/relayer/cmd/cmd.go
Original file line number Diff line number Diff line change
@@ -201,6 +201,7 @@ func setupRelayerDependencies(
config.NewSupplySupplierQuerierFn(),
config.NewSupplySessionQuerierFn(),
config.NewSupplyServiceQueryClientFn(),
config.NewSupplyProofQueryClientFn(),
config.NewSupplyRingCacheFn(),
supplyTxFactory,
supplyTxContext,
9 changes: 4 additions & 5 deletions pkg/relayer/session/claim.go
Original file line number Diff line number Diff line change
@@ -180,14 +180,13 @@ func (rs *relayerSessionsManager) newMapClaimSessionsFn(
return either.Success(sessionTrees), false
}

// Map key is the supplier operator address.
claimMsgs := make([]client.MsgCreateClaim, 0)
for _, sessionTree := range sessionTrees {
claimMsgs = append(claimMsgs, &prooftypes.MsgCreateClaim{
claimMsgs := make([]client.MsgCreateClaim, len(sessionTrees))
for idx, sessionTree := range sessionTrees {
claimMsgs[idx] = &prooftypes.MsgCreateClaim{
RootHash: sessionTree.GetClaimRoot(),
SessionHeader: sessionTree.GetSessionHeader(),
SupplierOperatorAddress: sessionTree.GetSupplierOperatorAddress().String(),
})
}
}

// Create claims for each supplier operator address in `sessionTrees`.
167 changes: 118 additions & 49 deletions pkg/relayer/session/proof.go
Original file line number Diff line number Diff line change
@@ -13,6 +13,7 @@ import (
"github.com/pokt-network/poktroll/pkg/observable/logging"
"github.com/pokt-network/poktroll/pkg/relayer"
"github.com/pokt-network/poktroll/x/proof/types"
prooftypes "github.com/pokt-network/poktroll/x/proof/types"
"github.com/pokt-network/poktroll/x/shared"
)

@@ -136,39 +137,19 @@ func (rs *relayerSessionsManager) waitForEarliestSubmitProofsHeightAndGeneratePr
logger = logger.With("earliest_supplier_proof_commit_height", earliestSupplierProofsCommitHeight)
logger.Info().Msg("waiting & blocking for proof path seed block height")

// proofWindowOpenHeight - 1 is the block that will have its hash used as the
// source of entropy for all the session trees in that batch, waiting for it to
// be received before proceeding.
// earliestSupplierProofsCommitHeight - 1 is the block that will have its hash
// used as the source of entropy for all the session trees in that batch,
// waiting for it to be received before proceeding.
proofPathSeedBlockHeight := earliestSupplierProofsCommitHeight - 1
proofPathSeedBlock := rs.waitForBlock(ctx, proofPathSeedBlockHeight)

logger = logger.With("proof_path_bock_hash", fmt.Sprintf("%x", proofPathSeedBlock.Hash()))
logger = logger.With("proof_path_seed_block", fmt.Sprintf("%x", proofPathSeedBlock.Hash()))
logger.Info().Msg("observed proof path seed block height")

// Generate proofs for all sessionTrees concurrently while waiting for the
// earliest submitProofsHeight (pseudorandom submission distribution) to be reached.
// Use a channel to block until all proofs for the sessionTrees have been generated.
proofsGeneratedCh := make(chan []relayer.SessionTree)
defer close(proofsGeneratedCh)
go rs.goProveClaims(
ctx,
sessionTrees,
proofPathSeedBlock,
proofsGeneratedCh,
failedSubmitProofsSessionsCh,
)

logger.Info().Msg("waiting & blocking for earliest supplier proof commit height")

// Wait for the earliestSupplierProofsCommitHeight to be reached before proceeding.
_ = rs.waitForBlock(ctx, earliestSupplierProofsCommitHeight)
successProofs, failedProofs := rs.proveClaims(ctx, sessionTrees, proofPathSeedBlock)
failedSubmitProofsSessionsCh <- failedProofs

logger.Info().Msg("observed earliest supplier proof commit height")

// Once the earliest submitProofsHeight has been reached, and all proofs have
// been generated, return the sessionTrees that have been successfully proven
// to be submitted on-chain.
return <-proofsGeneratedCh
return successProofs
}

// newMapProveSessionsFn returns a new MapFn that submits proofs on the given
@@ -187,13 +168,13 @@ func (rs *relayerSessionsManager) newMapProveSessionsFn(
}

// Map key is the supplier operator address.
proofMsgs := make([]client.MsgSubmitProof, 0)
for _, session := range sessionTrees {
proofMsgs = append(proofMsgs, &types.MsgSubmitProof{
proofMsgs := make([]client.MsgSubmitProof, len(sessionTrees))
for idx, session := range sessionTrees {
proofMsgs[idx] = &types.MsgSubmitProof{
Proof: session.GetProofBz(),
SessionHeader: session.GetSessionHeader(),
SupplierOperatorAddress: session.GetSupplierOperatorAddress().String(),
})
}
}

// Submit proofs for each supplier operator address in `sessionTrees`.
@@ -216,32 +197,45 @@ func (rs *relayerSessionsManager) newMapProveSessionsFn(
}
}

// goProveClaims generates the proofs corresponding to the given sessionTrees,
// proveClaims generates the proofs corresponding to the given sessionTrees,
// then sends the successful and failed proofs to their respective channels.
// This function MUST be run as a goroutine.
func (rs *relayerSessionsManager) goProveClaims(
func (rs *relayerSessionsManager) proveClaims(
ctx context.Context,
sessionTrees []relayer.SessionTree,
sessionPathBlock client.Block,
proofsGeneratedCh chan<- []relayer.SessionTree,
failSubmitProofsSessionsCh chan<- []relayer.SessionTree,
) {
// The hash of this block is used to determine which branch of the proof
// should be generated for.
proofPathSeedBlock client.Block,
red-0ne marked this conversation as resolved.
Show resolved Hide resolved
) (successProofs []relayer.SessionTree, failedProofs []relayer.SessionTree) {
logger := rs.logger.With("method", "goProveClaims")

// Separate the sessionTrees into those that failed to generate a proof
// and those that succeeded, then send them on their respective channels.
failedProofs := []relayer.SessionTree{}
successProofs := []relayer.SessionTree{}
// sessionTreesWithProofRequired will accumulate all the sessionTrees that
// will require a proof to be submitted.
sessionTreesWithProofRequired := make([]relayer.SessionTree, 0)
for _, sessionTree := range sessionTrees {
select {
case <-ctx.Done():
return
default:
isProofRequired, err := rs.isProofRequired(ctx, sessionTree, proofPathSeedBlock)

// If an error is encountered while determining if a proof is required,
// do not create the claim since the proof requirement is unknown.
// Creating a claim and not submitting a proof (if necessary) could lead to a stake burn!!
red-0ne marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
failedProofs = append(failedProofs, sessionTree)
rs.logger.Error().Err(err).Msg("failed to determine if proof is required, skipping claim creation")
continue
}

// If a proof is required, add the session to the list of sessions that require a proof.
if isProofRequired {
sessionTreesWithProofRequired = append(sessionTreesWithProofRequired, sessionTree)
}
}

// Separate the sessionTrees into those that failed to generate a proof
// and those that succeeded, before returning each of them.
for _, sessionTree := range sessionTreesWithProofRequired {
// Generate the proof path for the sessionTree using the previously committed
// sessionPathBlock hash.
path := protocol.GetPathForProof(
sessionPathBlock.Hash(),
proofPathSeedBlock.Hash(),
sessionTree.GetSessionHeader().GetSessionId(),
)

@@ -258,6 +252,81 @@ func (rs *relayerSessionsManager) goProveClaims(
successProofs = append(successProofs, sessionTree)
}

failSubmitProofsSessionsCh <- failedProofs
proofsGeneratedCh <- successProofs
return successProofs, failedProofs
}

// isProofRequired determines whether a proof is required for the given session's
// claim based on the current proof module governance parameters.
// TODO_TECHDEBT: Once the on/off-chain loggers are unified, move this logic to
Copy link
Member

Choose a reason for hiding this comment

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

Is it overkill to just pass a logger, unify it now, and add a TECHDEBT explaining why we pas sin a logger?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The on-chain logger cosmossdk.io/log and off-chain one github.com/pokt-network/poktroll/pkg/polylog seem to be incompatible.

I'm maybe missing something but we can't pass in a logger in both on/off-chain.

Copy link
Member

Choose a reason for hiding this comment

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

Did you confirm that this doesn't work?

I believe this is the problem polylog was meant to solve. cc @bryanchriswhite

Screenshot 2024-09-06 at 3 38 18 PM

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried, but had the following:
image

Will sync-up with @bryanchriswhite on this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated the comments to include investigation.

Copy link
Contributor

Choose a reason for hiding this comment

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

// a shared helper used by both off-chain and on-chain routines.
func (rs *relayerSessionsManager) isProofRequired(
ctx context.Context,
sessionTree relayer.SessionTree,
// The hash of this block is used to determine whether the proof is required
// w.r.t. the probabilistic features.
proofRequirementSeedBlock client.Block,
) (isProofRequired bool, err error) {
logger := rs.logger.With(
"session_id", sessionTree.GetSessionHeader().GetSessionId(),
"claim_root", fmt.Sprintf("%x", sessionTree.GetClaimRoot()),
"supplier_operator_address", sessionTree.GetSupplierOperatorAddress().String(),
)

// Create the claim object and use its methods to determine if a proof is required.
claim := claimFromSessionTree(sessionTree)

// Get the number of compute units accumulated through the given session.
numClaimComputeUnits, err := claim.GetNumComputeUnits()
Olshansk marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
return false, err
}

proofParams, err := rs.proofQueryClient.GetParams(ctx)
if err != nil {
return false, err
}

logger = logger.With(
"num_claim_compute_units", numClaimComputeUnits,
"proof_requirement_threshold", proofParams.GetProofRequirementThreshold(),
)

// Require a proof if the claim's compute units meets or exceeds the threshold.
// TODO_MAINNET: This should be proportional to the supplier's stake as well.
if numClaimComputeUnits >= proofParams.GetProofRequirementThreshold() {
red-0ne marked this conversation as resolved.
Show resolved Hide resolved
logger.Info().Msg("compute units is above threshold, claim requires proof")

return true, nil
}

proofRequirementCheckValue, err := claim.GetProofRequirementCheckValue(proofRequirementSeedBlock.Hash())
if err != nil {
return false, err
}

logger = logger.With(
"proof_requirement_check_value", proofRequirementCheckValue,
"proof_request_probability", proofParams.GetProofRequestProbability(),
)

// Require a proof probabilistically based on the proof_request_probability param.
// NB: A random value between 0 and 1 will be less than or equal to proof_request_probability
// with probability equal to the proof_request_probability.
if proofRequirementCheckValue <= proofParams.GetProofRequestProbability() {
logger.Info().Msg("claim hash seed is below proof request probability, claim requires proof")

return true, nil
}

logger.Info().Msg("claim does not require proof")
return false, nil
}

// claimFromSessionTree creates a Claim object from the given SessionTree.
func claimFromSessionTree(sessionTree relayer.SessionTree) prooftypes.Claim {
return prooftypes.Claim{
SupplierOperatorAddress: sessionTree.GetSupplierOperatorAddress().String(),
SessionHeader: sessionTree.GetSessionHeader(),
RootHash: sessionTree.GetClaimRoot(),
}
}
7 changes: 7 additions & 0 deletions pkg/relayer/session/session.go
Original file line number Diff line number Diff line change
@@ -59,13 +59,19 @@ type relayerSessionsManager struct {
// 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

// proofQueryClient is used to query for the proof requirement threshold and
// requirement probability governance parameters to determine whether a submitted
// claim requires a proof.
proofQueryClient client.ProofQueryClient
}

// NewRelayerSessions creates a new relayerSessions.
//
// Required dependencies:
// - client.BlockClient
// - client.SupplierClientMap
// - client.ProofQueryClient
//
// Available options:
// - WithStoresDirectory
@@ -87,6 +93,7 @@ func NewRelayerSessions(
&rs.supplierClients,
&rs.sharedQueryClient,
&rs.serviceQueryClient,
&rs.proofQueryClient,
); err != nil {
return nil, err
}
Loading