[Proof] Prevent proof submission when not required (#822)

## Summary

Prevent the `SubmitProof` keeper from upserting a proof if the `Proof`
is not required.

## Issue

`Proof`s are among the most block space-consuming primitives in
Poktroll.

In some cases, a `Proof` submission is unnecessary, but the current
`SubmitProof` keeper still allows non-required proofs to be submitted.

The `SubmitProof` keeper has enough information to determine whether the
`Proof` corresponding to a claim is necessary. The protocol could
leverage this information to avoid saving non-required `Proof`s
on-chain, thereby optimizing block space usage.

## Type of change

Select one or more from the following:

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

## Testing

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

## Sanity Checklist

- [x] I have tested my changes using the available tooling
- [x] I have commented my code
- [x] I have performed a self-review of my own code; both comments &
source code
- [ ] I create and reference any new tickets, if applicable
- [ ] I have left TODOs throughout the codebase, if applicable

x/proof/keeper/msg_server_submit_proof.go

@@ -15,6 +15,7 @@ import (
 
 	"github.com/pokt-network/poktroll/telemetry"
 	"github.com/pokt-network/poktroll/x/proof/types"
+	"github.com/pokt-network/poktroll/x/shared"
 	sharedtypes "github.com/pokt-network/poktroll/x/shared/types"
 )
 
@@ -103,6 +104,16 @@ func (k msgServer) SubmitProof(
 		return nil, status.Error(codes.Internal, types.ErrProofClaimNotFound.Wrap(err.Error()).Error())
 	}
 
+	// Check if a proof is required for the claim.
+	proofRequirement, err := k.ProofRequirementForClaim(ctx, claim)
+	if err != nil {
+		return nil, status.Error(codes.Internal, err.Error())
+	}
+	if proofRequirement == types.ProofRequirementReason_NOT_REQUIRED {
+		logger.Warn("trying to submit a proof for a claim that does not require one")
+		return nil, status.Error(codes.FailedPrecondition, types.ErrProofNotRequired.Error())
+	}
+
 	// Get metadata for the event we want to emit
 	numRelays, err = claim.GetNumRelays()
 	if err != nil {
@@ -191,3 +202,112 @@ func (k Keeper) deductProofSubmissionFee(ctx context.Context, supplierOperatorAd
 
 	return nil
 }
+
+// ProofRequirementForClaim checks if a proof is required for a claim.
+// If it is not, the claim will be settled without a proof.
+// If it is, the claim will only be settled if a valid proof is available.
+// TODO_BLOCKER(@olshansk, #419): Document safety assumptions of the probabilistic proofs mechanism.
+func (k Keeper) ProofRequirementForClaim(ctx context.Context, claim *types.Claim) (_ types.ProofRequirementReason, err error) {
+	logger := k.logger.With("method", "proofRequirementForClaim")
+
+	var requirementReason = types.ProofRequirementReason_NOT_REQUIRED
+
+	// Defer telemetry calls so that they reference the final values the relevant variables.
+	defer func() {
+		telemetry.ProofRequirementCounter(requirementReason, err)
+	}()
+
+	// NB: Assumption that claim is non-nil and has a valid root sum because it
+	// is retrieved from the store and validated, on-chain, at time of creation.
+	var numClaimComputeUnits uint64
+	numClaimComputeUnits, err = claim.GetNumComputeUnits()
+	if err != nil {
+		return requirementReason, err
+	}
+
+	proofParams := k.GetParams(ctx)
+
+	// Require a proof if the claim's compute units meets or exceeds the threshold.
+	//
+	// TODO_BLOCKER(@Olshansk, #419): This is just VERY BASIC placeholder logic to have something
+	// in place while we implement proper probabilistic proofs. If you're reading it,
+	// do not overthink it and look at the documents linked in #419.
+	//
+	// TODO_IMPROVE(@bryanchriswhite, @red-0ne): It might make sense to include
+	// whether there was a proof submission error downstream from here. This would
+	// require a more comprehensive metrics API.
+	if numClaimComputeUnits >= proofParams.GetProofRequirementThreshold() {
+		requirementReason = types.ProofRequirementReason_THRESHOLD
+
+		logger.Info(fmt.Sprintf(
+			"claim requires proof due to compute units (%d) exceeding threshold (%d)",
+			numClaimComputeUnits,
+			proofParams.GetProofRequirementThreshold(),
+		))
+		return requirementReason, nil
+	}
+
+	// Hash of block when proof submission is allowed.
+	earliestProofCommitBlockHash, err := k.getEarliestSupplierProofCommitBlockHash(ctx, claim)
+	if err != nil {
+		return requirementReason, err
+	}
+
+	// The probability that a proof is required.
+	proofRequirementSampleValue, err := claim.GetProofRequirementSampleValue(earliestProofCommitBlockHash)
+	if err != nil {
+		return requirementReason, err
+	}
+
+	// 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 proofRequirementSampleValue <= proofParams.GetProofRequestProbability() {
+		requirementReason = types.ProofRequirementReason_PROBABILISTIC
+
+		logger.Info(fmt.Sprintf(
+			"claim requires proof due to random sample (%.2f) being less than or equal to probability (%.2f)",
+			proofRequirementSampleValue,
+			proofParams.GetProofRequestProbability(),
+		))
+		return requirementReason, nil
+	}
+
+	logger.Info(fmt.Sprintf(
+		"claim does not require proof due to compute units (%d) being less than the threshold (%d) and random sample (%.2f) being greater than probability (%.2f)",
+		numClaimComputeUnits,
+		proofParams.GetProofRequirementThreshold(),
+		proofRequirementSampleValue,
+		proofParams.GetProofRequestProbability(),
+	))
+	return requirementReason, nil
+}
+
+// getEarliestSupplierProofCommitBlockHash returns the block hash of the earliest
+// block at which a claim may have its proof committed.
+func (k Keeper) getEarliestSupplierProofCommitBlockHash(
+	ctx context.Context,
+	claim *types.Claim,
+) (blockHash []byte, err error) {
+	sharedParams, err := k.sharedQuerier.GetParams(ctx)
+	if err != nil {
+		return nil, err
+	}
+
+	sessionEndHeight := claim.GetSessionHeader().GetSessionEndBlockHeight()
+	supplierOperatorAddress := claim.GetSupplierOperatorAddress()
+
+	proofWindowOpenHeight := shared.GetProofWindowOpenHeight(sharedParams, sessionEndHeight)
+	proofWindowOpenBlockHash := k.sessionKeeper.GetBlockHash(ctx, proofWindowOpenHeight)
+
+	// TODO_TECHDEBT(@red-0ne): Update the method header of this function to accept (sharedParams, Claim, BlockHash).
+	// After doing so, please review all calling sites and simplify them accordingly.
+	earliestSupplierProofCommitHeight := shared.GetEarliestSupplierProofCommitHeight(
+		sharedParams,
+		sessionEndHeight,
+		proofWindowOpenBlockHash,
+		supplierOperatorAddress,
+	)
+
+	return k.sessionKeeper.GetBlockHash(ctx, earliestSupplierProofCommitHeight), nil
+}

x/proof/keeper/msg_server_submit_proof_test.go

@@ -692,6 +692,143 @@ func TestMsgServer_SubmitProof_Error(t *testing.T) {
 	}
 }
 
+func TestMsgServer_SubmitProof_FailSubmittingNonRequiredProof(t *testing.T) {
+	opts := []keepertest.ProofKeepersOpt{
+		// Set block hash so we can have a deterministic expected on-chain proof requested by the protocol.
+		keepertest.WithBlockHash(blockHeaderHash),
+		// Set block height to 1 so there is a valid session on-chain.
+		keepertest.WithBlockHeight(1),
+	}
+	keepers, ctx := keepertest.NewProofModuleKeepers(t, opts...)
+	sharedParams := keepers.SharedKeeper.GetParams(ctx)
+
+	// Set proof keeper params to disable relay mining but never require a proof.
+	proofParams := keepers.Keeper.GetParams(ctx)
+	proofParams.ProofRequestProbability = 0
+	proofParams.RelayDifficultyTargetHash = testProofParams.RelayDifficultyTargetHash
+	err := keepers.Keeper.SetParams(ctx, proofParams)
+	require.NoError(t, err)
+
+	// Construct a keyring to hold the keypairs for the accounts used in the test.
+	keyRing := keyring.NewInMemory(keepers.Codec)
+
+	// Create a pre-generated account iterator to create accounts for the test.
+	preGeneratedAccts := testkeyring.PreGeneratedAccounts()
+
+	// Create accounts in the account keeper with corresponding keys in the
+	// keyring for the application and supplier.
+	supplierOperatorAddr := testkeyring.CreateOnChainAccount(
+		ctx, t,
+		supplierOperatorUid,
+		keyRing,
+		keepers,
+		preGeneratedAccts,
+	).String()
+	appAddr := testkeyring.CreateOnChainAccount(
+		ctx, t,
+		"app",
+		keyRing,
+		keepers,
+		preGeneratedAccts,
+	).String()
+
+	fundSupplierOperatorAccount(t, ctx, keepers, supplierOperatorAddr)
+
+	service := &sharedtypes.Service{
+		Id:                   testServiceId,
+		ComputeUnitsPerRelay: computeUnitsPerRelay,
+		OwnerAddress:         sample.AccAddress(),
+	}
+
+	// Add a supplier and application pair that are expected to be in the session.
+	keepers.AddServiceActors(ctx, t, service, supplierOperatorAddr, appAddr)
+
+	// Get the session for the application/supplier pair which is expected
+	// to be claimed and for which a valid proof would be accepted.
+	// Given the setup above, it is guaranteed that the supplier created
+	// will be part of the session.
+	sessionHeader := keepers.GetSessionHeader(ctx, t, appAddr, service, 1)
+
+	// Construct a proof message server from the proof keeper.
+	srv := keeper.NewMsgServerImpl(*keepers.Keeper)
+
+	// Prepare a ring client to sign & validate relays.
+	ringClient, err := rings.NewRingClient(depinject.Supply(
+		polyzero.NewLogger(),
+		prooftypes.NewAppKeeperQueryClient(keepers.ApplicationKeeper),
+		prooftypes.NewAccountKeeperQueryClient(keepers.AccountKeeper),
+		prooftypes.NewSharedKeeperQueryClient(keepers.SharedKeeper, keepers.SessionKeeper),
+	))
+	require.NoError(t, err)
+
+	// Submit the corresponding proof.
+	numRelays := uint64(5)
+	sessionTree := testtree.NewFilledSessionTree(
+		ctx, t,
+		numRelays, service.ComputeUnitsPerRelay,
+		supplierOperatorUid, supplierOperatorAddr,
+		sessionHeader, sessionHeader, sessionHeader,
+		keyRing,
+		ringClient,
+	)
+
+	// Advance the block height to the test claim msg height.
+	claimMsgHeight := shared.GetEarliestSupplierClaimCommitHeight(
+		&sharedParams,
+		sessionHeader.GetSessionEndBlockHeight(),
+		blockHeaderHash,
+		supplierOperatorAddr,
+	)
+	ctx = keepertest.SetBlockHeight(ctx, claimMsgHeight)
+
+	// Create a valid claim.
+	createClaimAndStoreBlockHash(
+		ctx, t, 1,
+		supplierOperatorAddr,
+		appAddr,
+		service,
+		sessionTree,
+		sessionHeader,
+		srv,
+		keepers,
+	)
+
+	// Advance the block height to the proof path seed height.
+	earliestSupplierProofCommitHeight := shared.GetEarliestSupplierProofCommitHeight(
+		&sharedParams,
+		sessionHeader.GetSessionEndBlockHeight(),
+		blockHeaderHash,
+		supplierOperatorAddr,
+	)
+	ctx = keepertest.SetBlockHeight(ctx, earliestSupplierProofCommitHeight-1)
+
+	// Store proof path seed block hash in the session keeper so that it can
+	// look it up during proof validation.
+	keepers.StoreBlockHash(ctx)
+
+	// Compute expected proof path.
+	expectedMerkleProofPath = protocol.GetPathForProof(blockHeaderHash, sessionHeader.GetSessionId())
+
+	// Advance the block height to the test proof msg height.
+	proofMsgHeight := shared.GetEarliestSupplierProofCommitHeight(
+		&sharedParams,
+		sessionHeader.GetSessionEndBlockHeight(),
+		blockHeaderHash,
+		supplierOperatorAddr,
+	)
+	ctx = keepertest.SetBlockHeight(ctx, proofMsgHeight)
+
+	proofMsg := newTestProofMsg(t,
+		supplierOperatorAddr,
+		sessionHeader,
+		sessionTree,
+		expectedMerkleProofPath,
+	)
+	submitProofRes, err := srv.SubmitProof(ctx, proofMsg)
+	require.Nil(t, submitProofRes)
+	require.ErrorIs(t, err, status.Error(codes.FailedPrecondition, prooftypes.ErrProofNotRequired.Error()))
+}
+
 // newTestProofMsg creates a new submit proof message that can be submitted
 // to be validated and stored on-chain.
 func newTestProofMsg(

x/tokenomics/keeper/proof_requirement_test.go → x/proof/keeper/proof_requirement_test.go

@@ -4,26 +4,25 @@ import (
 	"sync/atomic"
 	"testing"
 
-	"cosmossdk.io/log"
 	cosmostypes "github.com/cosmos/cosmos-sdk/types"
 	"github.com/stretchr/testify/require"
 
 	poktrand "github.com/pokt-network/poktroll/pkg/crypto/rand"
 	"github.com/pokt-network/poktroll/testutil/keeper"
 	tetsproof "github.com/pokt-network/poktroll/testutil/proof"
 	"github.com/pokt-network/poktroll/testutil/sample"
-	prooftypes "github.com/pokt-network/poktroll/x/proof/types"
+	"github.com/pokt-network/poktroll/x/proof/types"
 )
 
 func TestKeeper_IsProofRequired(t *testing.T) {
 	// Set expectedCompute units to be below the proof requirement threshold to only
 	// exercise the probabilistic branch of the #isProofRequired() logic.
-	expectedComputeUnits := prooftypes.DefaultProofRequirementThreshold - 1
-	keepers, ctx := keeper.NewTokenomicsModuleKeepers(t, log.NewNopLogger())
+	expectedComputeUnits := types.DefaultProofRequirementThreshold - 1
+	keepers, ctx := keeper.NewProofModuleKeepers(t)
 	sdkCtx := cosmostypes.UnwrapSDKContext(ctx)
 
 	var (
-		probability = prooftypes.DefaultProofRequestProbability
+		probability = types.DefaultProofRequestProbability
 		// This was empirically determined to avoid false negatives in unit tests.
 		// As a maintainer of the codebase, you may need to adjust these.
 		tolerance  = 0.10
@@ -40,10 +39,10 @@ func TestKeeper_IsProofRequired(t *testing.T) {
 	for i := int64(0); i < sampleSize; i++ {
 		claim := tetsproof.ClaimWithRandomHash(t, sample.AccAddress(), sample.AccAddress(), expectedComputeUnits)
 
-		proofRequirementReason, err := keepers.Keeper.ProofRequirementForClaim(sdkCtx, &claim)
+		proofRequirementReason, err := keepers.ProofRequirementForClaim(sdkCtx, &claim)
 		require.NoError(t, err)
 
-		if proofRequirementReason != prooftypes.ProofRequirementReason_NOT_REQUIRED {
+		if proofRequirementReason != types.ProofRequirementReason_NOT_REQUIRED {
 			numTrueSamples.Add(1)
 		}
 	}

x/proof/types/errors.go

@@ -37,4 +37,5 @@ var (
 	ErrProofComputeUnitsMismatch      = sdkerrors.Register(ModuleName, 1126, "mismatch: claim compute units != number of relays * service compute units per relay")
 	ErrProofNotEnoughFunds            = sdkerrors.Register(ModuleName, 1127, "not enough funds to submit proof")
 	ErrProofFailedToDeductFee         = sdkerrors.Register(ModuleName, 1128, "failed to deduct proof submission fee")
+	ErrProofNotRequired               = sdkerrors.Register(ModuleName, 1129, "proof not required")
 )