Last review of #690

pkg/crypto/protocol/difficulty.go

@@ -1,27 +1,44 @@
 package protocol
 
 import (
+	"bytes"
 	"encoding/hex"
 	"math/big"
 )
 
 var (
-	// Difficulty1HashBz is the chosen "highest" (easiest) target hash, which
-	// corresponds to the lowest possible difficulty. It effectively normalizes
-	// the difficulty number (which is returned by GetDifficultyFromHash) by defining
-	// the hash which corresponds to difficulty 1.
+	// BaseRelayDifficultyHashBz is the chosen "highest" (easiest) target hash, which
+	// corresponds to the lowest possible difficulty.
+	//
+	// It effectively normalizes the difficulty number (which is returned by GetDifficultyFromHash)
+	// by defining the hash which corresponds to the base difficulty.
+	//
+	// When this is the difficulty of a particular service, all relays are reward / volume applicable.
+	//
+	// Bitcoin uses a similar concept, where the target hash is defined as the hash:
 	// - https://bitcoin.stackexchange.com/questions/107976/bitcoin-difficulty-why-leading-0s
 	// - https://bitcoin.stackexchange.com/questions/121920/is-it-always-possible-to-find-a-number-whose-hash-starts-with-a-certain-number-o
-	Difficulty1HashBz, _ = hex.DecodeString("ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff")
+	BaseRelayDifficultyHashBz, _ = hex.DecodeString("ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff")
 )
 
 // GetDifficultyFromHash returns the "difficulty" of the given hash, with respect
-// to the "highest" (easiest) target hash, Difficulty1Hash.
+// to the "highest" (easiest) target hash, BaseRelayDifficultyHash.
 // The resultant value is not used for any business logic but is simplify there to have a human-readable version of the hash.
 func GetDifficultyFromHash(hashBz [RelayHasherSize]byte) int64 {
-	difficulty1HashInt := new(big.Int).SetBytes(Difficulty1HashBz)
+	baseRelayDifficultyHashInt := new(big.Int).SetBytes(BaseRelayDifficultyHashBz)
 	hashInt := new(big.Int).SetBytes(hashBz[:])
 
 	// difficulty is the ratio of the highest target hash to the given hash.
-	return new(big.Int).Div(difficulty1HashInt, hashInt).Int64()
+	return new(big.Int).Div(baseRelayDifficultyHashInt, hashInt).Int64()
+}
+
+// IsRelayVolumeApplicable returns true if the relay IS reward / volume applicable.
+// A relay is reward / volume applicable IFF its hash is less than the target hash.
+//   - relayHash is the hash of the relay to be checked.
+//   - targetHash is the hash of the relay difficulty target for a particular service.
+//
+// TODO_MAINNET: Devise a test that tries to attack the network and ensure that
+// there is sufficient telemetry.
+func IsRelayVolumeApplicable(relayHash, targetHash []byte) bool {
+	return bytes.Compare(relayHash, targetHash) == -1 // True if relayHash < targetHash
 }

pkg/crypto/protocol/difficulty_test.go

@@ -32,7 +32,7 @@ func TestGetDifficultyFromHash(t *testing.T) {
 		{
 			desc:               "Highest difficulty",
 			hashHex:            "0000000000000000000000000000000000000000000000000000000000000001",
-			expectedDifficulty: new(big.Int).SetBytes(Difficulty1HashBz).Int64(),
+			expectedDifficulty: new(big.Int).SetBytes(BaseRelayDifficultyHashBz).Int64(),
 		},
 	}
 
@@ -52,3 +52,55 @@ func TestGetDifficultyFromHash(t *testing.T) {
 		})
 	}
 }
+
+func TestIsRelayVolumeApplicable(t *testing.T) {
+	tests := []struct {
+		desc                     string
+		relayHashHex             string
+		targetHashHex            string
+		expectedVolumeApplicable bool
+	}{
+		{
+			desc:                     "Applicable: relayHash << targetHash",
+			relayHashHex:             "000fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff",
+			targetHashHex:            "00ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff",
+			expectedVolumeApplicable: true,
+		},
+		{
+			desc:                     "Applicable: relayHash < targetHash",
+			relayHashHex:             "00efffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff",
+			targetHashHex:            "00ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff",
+			expectedVolumeApplicable: true,
+		},
+		{
+			desc:                     "Not Applicable: relayHash = targetHash",
+			relayHashHex:             "00ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff",
+			targetHashHex:            "00ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff",
+			expectedVolumeApplicable: false,
+		},
+		{
+			desc:                     "Not applicable: relayHash > targetHash",
+			relayHashHex:             "0effffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff",
+			targetHashHex:            "00ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff",
+			expectedVolumeApplicable: false,
+		},
+		{
+			desc:                     "Not applicable: relayHash >> targetHash",
+			relayHashHex:             "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff",
+			targetHashHex:            "00ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff",
+			expectedVolumeApplicable: false,
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.desc, func(t *testing.T) {
+			relayHash, err := hex.DecodeString(test.relayHashHex)
+			require.NoError(t, err)
+
+			targetHash, err := hex.DecodeString(test.targetHashHex)
+			require.NoError(t, err)
+
+			require.Equal(t, test.expectedVolumeApplicable, IsRelayVolumeApplicable(relayHash, targetHash))
+		})
+	}
+}

pkg/crypto/protocol/hash.go

@@ -1,10 +1,11 @@
 package protocol
 
-// GetHashFromBytes returns the hash of the relay (full, request or response) bytes.
+// GetRelayHashFromBytes returns the hash of the relay (full, request or response) bytes.
 // It is used as helper in the case that the relay is already marshaled and
 // centralizes the hasher used.
-func GetHashFromBytes(relayBz []byte) (hash [RelayHasherSize]byte) {
+func GetRelayHashFromBytes(relayBz []byte) (hash [RelayHasherSize]byte) {
 	hasher := NewRelayHasher()
+
 	// NB: Intentionally ignoring the error, following sha256.Sum256 implementation.
 	_, _ = hasher.Write(relayBz)
 	hashBz := hasher.Sum(nil)

pkg/relayer/miner/gen/gen_fixtures.go

@@ -32,6 +32,7 @@ const (
 	defaultOutPath              = "relay_fixtures_test.go"
 )
 
+// 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

pkg/relayer/miner/miner.go

@@ -1,7 +1,6 @@
 package miner
 
 import (
-	"bytes"
 	"context"
 
 	"cosmossdk.io/depinject"
@@ -30,6 +29,7 @@ type miner struct {
 	//
 	// TODO_MAINNET(#543): 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_FOLLOWUP(@olshansk, #690): This needs to be maintained (and updated) on a per service level.
 	relayDifficultyTargetHash []byte
 }
 
@@ -109,23 +109,22 @@ func (mnr *miner) mapMineRelay(
 	_ context.Context,
 	relay *servicetypes.Relay,
 ) (_ either.Either[*relayer.MinedRelay], skip bool) {
-	// TODO_TECHDEBT(@red-0ne, #446): Centralize the configuration for the SMT spec.
-	// TODO_TECHDEBT(@red-0ne): marshal using canonical codec.
 	relayBz, err := relay.Marshal()
 	if err != nil {
 		return either.Error[*relayer.MinedRelay](err), false
 	}
-	relayHash := protocol.GetHashFromBytes(relayBz)
+	relayHashArr := protocol.GetRelayHashFromBytes(relayBz)
+	relayHash := relayHashArr[:]
 
 	// The relay IS NOT volume / reward applicable
-	if bytes.Compare(relayHash[:], mnr.relayDifficultyTargetHash) == 1 {
+	if !protocol.IsRelayVolumeApplicable(relayHash, mnr.relayDifficultyTargetHash) {
 		return either.Success[*relayer.MinedRelay](nil), true
 	}
 
 	// The relay IS volume / reward applicable
 	return either.Success(&relayer.MinedRelay{
 		Relay: *relay,
 		Bytes: relayBz,
-		Hash:  relayHash[:],
+		Hash:  relayHash,
 	}), false
 }

pkg/relayer/miner/miner_test.go

@@ -23,7 +23,7 @@ import (
 	servicetypes "github.com/pokt-network/poktroll/x/service/types"
 )
 
-var testTargetHash, _ = hex.DecodeString("0000ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff")
+var testRelayMiningTargetHash, _ = hex.DecodeString("0000ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff")
 
 // TestMiner_MinedRelays constructs an observable of mined relays, through which
 // it pipes pre-mined relay fixtures. It asserts that the observable only emits
@@ -43,7 +43,7 @@ func TestMiner_MinedRelays(t *testing.T) {
 
 	proofQueryClientMock := testqueryclients.NewTestProofQueryClient(t)
 	deps := depinject.Supply(proofQueryClientMock)
-	mnr, err := miner.NewMiner(deps, miner.WithRelayDifficultyTargetHash(testTargetHash))
+	mnr, err := miner.NewMiner(deps, miner.WithRelayDifficultyTargetHash(testRelayMiningTargetHash))
 	require.NoError(t, err)
 
 	minedRelays := mnr.MinedRelays(ctx, mockRelaysObs)
@@ -134,8 +134,7 @@ func unmarshalHexMinedRelay(
 	err = relay.Unmarshal(relayBz)
 	require.NoError(t, err)
 
-	// TODO_TECHDEBT(@red-0ne, #446): Centralize the configuration for the SMT spec.
-	relayHashArr := protocol.GetHashFromBytes(relayBz)
+	relayHashArr := protocol.GetRelayHashFromBytes(relayBz)
 	relayHash := relayHashArr[:]
 
 	return &relayer.MinedRelay{

proto/poktroll/proof/params.proto

@@ -12,6 +12,7 @@ message Params {
   option (amino.name) = "poktroll/x/proof/Params";
   option (gogoproto.equal) = true;
 
+  // TODO_FOLLOWUP(@olshansk, #690): Either delete this or change it to be named "minimum"
   // relay_difficulty_target_hash is the maximum value a relay hash must be less than to be volume/reward applicable.
   bytes relay_difficulty_target_hash = 1 [(gogoproto.jsontag) = "relay_difficulty_target_hash"];
 

testutil/testrelayer/relays.go

@@ -57,7 +57,7 @@ func NewUnsignedMinedRelay(
 	relayBz, err := relay.Marshal()
 	require.NoError(t, err)
 
-	relayHashArr := protocol.GetHashFromBytes(relayBz)
+	relayHashArr := protocol.GetRelayHashFromBytes(relayBz)
 	relayHash := relayHashArr[:]
 
 	return &relayer.MinedRelay{
@@ -111,7 +111,7 @@ func NewSignedMinedRelay(
 	relayBz, err := relay.Marshal()
 	require.NoError(t, err)
 
-	relayHashArr := protocol.GetHashFromBytes(relayBz)
+	relayHashArr := protocol.GetRelayHashFromBytes(relayBz)
 	relayHash := relayHashArr[:]
 
 	return &relayer.MinedRelay{

x/proof/keeper/msg_server_submit_proof.go

@@ -204,6 +204,7 @@ func (k msgServer) SubmitProof(
 	logger.Debug("successfully verified relay response signature")
 
 	// Get the proof module's governance parameters.
+	// TODO_FOLLOWUP(@olshansk, #690): Get the difficulty associated with the service
 	params := k.GetParams(ctx)
 
 	// Verify the relay difficulty is above the minimum required to earn rewards.
@@ -447,7 +448,8 @@ func verifyClosestProof(
 // TODO_TECHDEBT: Factor out the relay mining difficulty validation into a shared
 // function that can be used by both the proof and the miner packages.
 func validateRelayDifficulty(relayBz []byte, targetHash []byte) error {
-	relayHash := protocol.GetHashFromBytes(relayBz)
+	relayHashArr := protocol.GetRelayHashFromBytes(relayBz)
+	relayHash := relayHashArr[:]
 
 	if len(targetHash) != protocol.RelayHasherSize {
 		return types.ErrProofInvalidRelay.Wrapf(
@@ -458,17 +460,15 @@ func validateRelayDifficulty(relayBz []byte, targetHash []byte) error {
 		)
 	}
 
-	var targetHashArr [protocol.RelayHasherSize]byte
-	copy(targetHashArr[:], targetHash)
+	if !protocol.IsRelayVolumeApplicable(relayHash, targetHash) {
+		var targetHashArr [protocol.RelayHasherSize]byte
+		copy(targetHashArr[:], targetHash)
 
-	// TODO_MAINNET: Devise a test that tries to attack the network and ensure that there
-	// is sufficient telemetry.
-	// NB: If relayHash > targetHash, then the difficulty is less than the target difficulty.
-	if bytes.Compare(relayHash[:], targetHash[:]) == 1 {
-		relayDifficulty := protocol.GetDifficultyFromHash(relayHash)
+		relayDifficulty := protocol.GetDifficultyFromHash(relayHashArr)
 		targetDifficulty := protocol.GetDifficultyFromHash(targetHashArr)
+
 		return types.ErrProofInvalidRelay.Wrapf(
-			"relay difficulty %d is less than the target difficulty %d",
+			"the difficulty relay being proven is (%d), and is smaller than the target difficulty (%d) for service %s",
 			relayDifficulty,
 			targetDifficulty,
 		)

x/proof/keeper/msg_server_submit_proof_test.go

@@ -48,7 +48,7 @@ var (
 	//  - the relay difficulty target hash to the easiest difficulty so that these tests don't need to mine for valid relays.
 	//  - the proof request probability to 1 so that all test sessions require a proof.
 	testProofParams = prooftypes.Params{
-		RelayDifficultyTargetHash: protocol.Difficulty1HashBz,
+		RelayDifficultyTargetHash: protocol.BaseRelayDifficultyHashBz,
 		ProofRequestProbability:   1,
 	}
 )

x/proof/types/params.go

@@ -16,19 +16,22 @@ var (
 	_ client.ProofParams  = (*Params)(nil)
 	_ paramtypes.ParamSet = (*Params)(nil)
 
-	KeyMinRelayDifficultyBits                   = []byte("MinRelayDifficultyBits")
-	ParamRelayDifficultyTargetHash              = "relay_difficulty_target_hash"
-	DefaultRelayDifficultyTargetHashHex         = "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff" // all relays are payable
-	DefaultRelayDifficultyTargetHash, _         = hex.DecodeString(DefaultRelayDifficultyTargetHashHex) // TODO_MAINNET(#142, #401): Determine the default value.
-	KeyProofRequestProbability                  = []byte("ProofRequestProbability")
-	ParamProofRequestProbability                = "proof_request_probability"
-	DefaultProofRequestProbability      float32 = 0.25 // See: https://github.com/pokt-network/pocket-core/blob/staging/docs/proposals/probabilistic_proofs.md
-	KeyProofRequirementThreshold                = []byte("ProofRequirementThreshold")
-	ParamProofRequirementThreshold              = "proof_requirement_threshold"
-	DefaultProofRequirementThreshold    uint64  = 20 // See: https://github.com/pokt-network/pocket-core/blob/staging/docs/proposals/probabilistic_proofs.md
-	KeyProofMissingPenalty                      = []byte("ProofMissingPenalty")
-	ParamProofMissingPenalty                    = "proof_missing_penalty"
-	DefaultProofMissingPenalty                  = cosmostypes.NewCoin(volatile.DenomuPOKT, math.NewInt(320)) // See: https://github.com/pokt-network/pocket-core/blob/staging/docs/proposals/probabilistic_proofs.md
+	// TODO_FOLLOWUP(@olshansk, #690): Delete this parameter.
+	KeyRelayDifficultyTargetHash        = []byte("RelayDifficultyTargetHash")
+	ParamRelayDifficultyTargetHash      = "relay_difficulty_target_hash"
+	DefaultRelayDifficultyTargetHash, _ = hex.DecodeString("ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff") // all relays are payable
+
+	KeyProofRequestProbability             = []byte("ProofRequestProbability")
+	ParamProofRequestProbability           = "proof_request_probability"
+	DefaultProofRequestProbability float32 = 0.25 // See: https://github.com/pokt-network/pocket-core/blob/staging/docs/proposals/probabilistic_proofs.md
+
+	KeyProofRequirementThreshold            = []byte("ProofRequirementThreshold")
+	ParamProofRequirementThreshold          = "proof_requirement_threshold"
+	DefaultProofRequirementThreshold uint64 = 20 // See: https://github.com/pokt-network/pocket-core/blob/staging/docs/proposals/probabilistic_proofs.md
+
+	KeyProofMissingPenalty     = []byte("ProofMissingPenalty")
+	ParamProofMissingPenalty   = "proof_missing_penalty"
+	DefaultProofMissingPenalty = cosmostypes.NewCoin(volatile.DenomuPOKT, math.NewInt(320)) // See: https://github.com/pokt-network/pocket-core/blob/staging/docs/proposals/probabilistic_proofs.md
 )
 
 // ParamKeyTable the param key table for launch module
@@ -65,7 +68,7 @@ func DefaultParams() Params {
 func (p *Params) ParamSetPairs() paramtypes.ParamSetPairs {
 	return paramtypes.ParamSetPairs{
 		paramtypes.NewParamSetPair(
-			KeyMinRelayDifficultyBits,
+			KeyRelayDifficultyTargetHash,
 			&p.RelayDifficultyTargetHash,
 			ValidateRelayDifficultyTargetHash,
 		),