[Mining] refactor: difficulty in terms of target hash (#690)

Change the difficulty from leading zero bits to to a target hash.

- #584

---

Co-authored-by: Daniel Olshansky <[email protected]>
Co-authored-by: Redouane Lakrache <[email protected]>

Makefile

@@ -420,7 +420,7 @@ test_verbose: check_go_version ## Run all go tests verbosely
 	go test -count=1 -v -race -tags test ./...
 
 .PHONY: test_all
-test_all: check_go_version ## Run all go tests showing detailed output only on failures
+test_all: warn_flaky_tests check_go_version ## Run all go tests showing detailed output only on failures
 	go test -count=1 -race -tags test ./...
 
 .PHONY: test_all_with_integration
@@ -503,17 +503,22 @@ go_develop_and_test: go_develop test_all ## Generate protos, mocks and run all t
 # TODO_DISCUSS_IN_THIS_COMMIT - SHOULD NEVER BE COMMITTED TO MASTER. It is a way for the reviewer of a PR to start / reply to a discussion.
 # TODO_IN_THIS_COMMIT         - SHOULD NEVER BE COMMITTED TO MASTER. It is a way to start the review process while non-critical changes are still in progress
 
+
+# Define shared variable for the exclude parameters
+EXCLUDE_GREP = --exclude-dir={.git,vendor,./docusaurus,.vscode,.idea} --exclude={Makefile,reviewdog.yml,*.pb.go,*.pulsar.go}
+
 .PHONY: todo_list
 todo_list: ## List all the TODOs in the project (excludes vendor and prototype directories)
-	grep --exclude-dir={.git,vendor,./docusaurus} -r TODO  .
+	grep -r $(EXCLUDE_GREP) TODO . | grep -v 'TODO()'
 
 .PHONY: todo_count
 todo_count: ## Print a count of all the TODOs in the project
-	grep --exclude-dir={.git,vendor,./docusaurus} -r TODO  . | wc -l
+	grep -r $(EXCLUDE_GREP) TODO . | grep -v 'TODO()' | wc -l
 
 .PHONY: todo_this_commit
 todo_this_commit: ## List all the TODOs needed to be done in this commit
-	grep -n --exclude-dir={.git,vendor,.vscode,.idea} --exclude={Makefile,reviewdog.yml} -r -e "TODO_IN_THIS_"
+	grep -r $(EXCLUDE_GREP) TODO_IN_THIS .| grep -v 'TODO()'
+
 
 ####################
 ###   Gateways   ###
@@ -809,6 +814,19 @@ warn_message_local_stress_test: ## Print a warning message when kicking off a lo
 	@echo "|                                                                                               |"
 	@echo "+-----------------------------------------------------------------------------------------------+"
 
+PHONY: warn_flaky_tests
+warn_flaky_tests: ## Print a warning message that some unit tests may be flaky
+	@echo "+-----------------------------------------------------------------------------------------------+"
+	@echo "|                                                                                               |"
+	@echo "|     IMPORTANT: READ ME IF YOUR TESTS FAIL!!!                                                  |"
+	@echo "|                                                                                               |"
+	@echo "|     1. Our unit / integration tests are far from perfect & some are flaky                     |"
+	@echo "|     2. If you ran 'make go_develop_and_test' and a failure occurred, try to run:              |"
+	@echo "|     	'make test_all' once or twice more                                                     |"
+	@echo "|     3. If the same error persists, isolate it with 'go test -v ./path/to/failing/module       |"
+	@echo "|                                                                                               |"
+	@echo "+-----------------------------------------------------------------------------------------------+"
+
 ##############
 ### Claims ###
 ##############

e2e/tests/parse_params_test.go

@@ -3,6 +3,7 @@
 package e2e
 
 import (
+	"encoding/hex"
 	"fmt"
 	"strconv"
 
@@ -132,8 +133,8 @@ func (s *suite) newProofMsgUpdateParams(params paramsMap) cosmostypes.Msg {
 
 	for paramName, paramValue := range params {
 		switch paramName {
-		case prooftypes.ParamMinRelayDifficultyBits:
-			msgUpdateParams.Params.MinRelayDifficultyBits = uint64(paramValue.value.(int64))
+		case prooftypes.ParamRelayDifficultyTargetHash:
+			msgUpdateParams.Params.RelayDifficultyTargetHash, _ = hex.DecodeString(string(paramValue.value.([]byte)))
 		case prooftypes.ParamProofRequestProbability:
 			msgUpdateParams.Params.ProofRequestProbability = paramValue.value.(float32)
 		case prooftypes.ParamProofRequirementThreshold:

e2e/tests/update_params.feature

@@ -37,11 +37,11 @@ Feature: Params Namespace
     And all "proof" module params are set to their default values
     And an authz grant from the "gov" "module" account to the "pnf" "user" account for the "/poktroll.proof.MsgUpdateParams" message exists
     When the "pnf" account sends an authz exec message to update all "proof" module params
-      | name                        | value | type  |
-      | min_relay_difficulty_bits   | 8     | int64 |
-      | proof_request_probability   | 0.1   | float |
-      | proof_requirement_threshold | 100   | int64 |
-      | proof_missing_penalty       | 500   | coin  |
+      | name                         | value                                                            | type  |
+      | relay_difficulty_target_hash | 00000000ffffffffffffffffffffffffffffffffffffffffffffffffffffffff | bytes |
+      | proof_request_probability    | 0.1                                                              | float |
+      | proof_requirement_threshold  | 100                                                              | int64 |
+      | proof_missing_penalty        | 500                                                              | coin  |
     Then all "proof" module params should be updated
 
   # NB: If you are reading this and the proof module has parameters
@@ -89,6 +89,6 @@ Feature: Params Namespace
     And all "proof" module params are set to their default values
     And an authz grant from the "gov" "module" account to the "pnf" "user" account for the "/poktroll.proof.MsgUpdateParams" message exists
     When the "unauthorized" account sends an authz exec message to update "proof" the module param
-      | name                       | value | type  |
-      | "min_relay_difficulty_bits | 666   | int64 |
+      | name                      | value | type  |
+      | proof_request_probability | 0.1   | float |
     Then the "proof" module param "min_relay_difficulty_bits" should be set to its default value

e2e/tests/update_params_test.go

@@ -3,6 +3,7 @@
 package e2e
 
 import (
+	"encoding/hex"
 	"encoding/json"
 	"fmt"
 	"reflect"
@@ -370,9 +371,9 @@ func (s *suite) assertExpectedModuleParamsUpdated(moduleName string) {
 		params := prooftypes.DefaultParams()
 		paramsMap := s.expectedModuleParams[moduleName]
 
-		minRelayDifficultyBits, ok := paramsMap[prooftypes.ParamMinRelayDifficultyBits]
+		relayDifficultyTargetHash, ok := paramsMap[prooftypes.ParamRelayDifficultyTargetHash]
 		if ok {
-			params.MinRelayDifficultyBits = uint64(minRelayDifficultyBits.value.(int64))
+			params.RelayDifficultyTargetHash, _ = hex.DecodeString(string(relayDifficultyTargetHash.value.([]byte)))
 		}
 
 		proofRequestProbability, ok := paramsMap[prooftypes.ParamProofRequestProbability]

pkg/client/events/query_client_test.go

@@ -366,7 +366,6 @@ func behavesLikeEitherObserver[V any](
 		require.NoError(t, err)
 		require.Equal(t, notificationsLimit, int(atomic.LoadInt32(&eventsCounter)))
 
-		// TODO_THIS_COMMIT: is this necessary?
 		time.Sleep(10 * time.Millisecond)
 
 		if onLimit != nil {

pkg/client/interface.go

@@ -335,7 +335,7 @@ 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 {
-	GetMinRelayDifficultyBits() uint64
+	GetRelayDifficultyTargetHash() []byte
 	GetProofRequestProbability() float32
 	GetProofRequirementThreshold() uint64
 	GetProofMissingPenalty() *cosmostypes.Coin

pkg/crypto/protocol/difficulty.go

@@ -1,20 +1,45 @@
 package protocol
 
 import (
-	"encoding/binary"
-	"math/bits"
+	"bytes"
+	"encoding/hex"
+	"math/big"
 )
 
-// CountHashDifficultyBits returns the number of leading zero bits in the given byte slice.
-// TODO_MAINNET: Consider generalizing difficulty to a target hash. See:
-// - 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
-// - https://github.com/pokt-network/poktroll/pull/656/files#r1666712528
-func CountHashDifficultyBits(bz [32]byte) int {
-	// Using BigEndian for contiguous bit/byte ordering such leading zeros
-	// accumulate across adjacent bytes.
-	// E.g.: []byte{0, 0b00111111, 0x00, 0x00} has 10 leading zero bits. If
-	// LittleEndian were applied instead, it would have 18 leading zeros because it would
-	// look like []byte{0, 0, 0b00111111, 0}.
-	return bits.LeadingZeros64(binary.BigEndian.Uint64(bz[:]))
+var (
+	// 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
+	BaseRelayDifficultyHashBz, _ = hex.DecodeString("ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff")
+)
+
+// GetDifficultyFromHash returns the "difficulty" of the given hash, with respect
+// 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 {
+	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.
+	// TODO_MAINNET: Can this cause an integer overflow?
+	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

@@ -1,51 +1,106 @@
-package protocol_test
+package protocol
 
 import (
-	"fmt"
+	"encoding/hex"
+	"math/big"
 	"testing"
 
 	"github.com/stretchr/testify/require"
-
-	"github.com/pokt-network/poktroll/pkg/crypto/protocol"
 )
 
-func TestCountDifficultyBits(t *testing.T) {
+func TestGetDifficultyFromHash(t *testing.T) {
 	tests := []struct {
-		bz         []byte
-		difficulty int
+		desc               string
+		hashHex            string
+		expectedDifficulty int64
 	}{
 		{
-			bz:         []byte{0b11111111},
-			difficulty: 0,
+			desc:               "Difficulty 1",
+			hashHex:            "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff",
+			expectedDifficulty: 1,
+		},
+		{
+			desc:               "Difficulty 2",
+			hashHex:            "7fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff",
+			expectedDifficulty: 2,
+		},
+		{
+			desc:               "Difficulty 4",
+			hashHex:            "3fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff",
+			expectedDifficulty: 4,
 		},
 		{
-			bz:         []byte{0b01111111},
-			difficulty: 1,
+			desc:               "Highest difficulty",
+			hashHex:            "0000000000000000000000000000000000000000000000000000000000000001",
+			expectedDifficulty: new(big.Int).SetBytes(BaseRelayDifficultyHashBz).Int64(),
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.desc, func(t *testing.T) {
+			hashBytes, err := hex.DecodeString(test.hashHex)
+			if err != nil {
+				t.Fatalf("failed to decode hash: %v", err)
+			}
+
+			var hashBz [RelayHasherSize]byte
+			copy(hashBz[:], hashBytes)
+
+			difficulty := GetDifficultyFromHash(hashBz)
+			t.Logf("test: %s, difficulty: %d", test.desc, difficulty)
+			require.Equal(t, test.expectedDifficulty, difficulty)
+		})
+	}
+}
+
+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,
 		},
 		{
-			bz:         []byte{0, 255},
-			difficulty: 8,
+			desc:                     "Applicable: relayHash < targetHash",
+			relayHashHex:             "00efffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff",
+			targetHashHex:            "00ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff",
+			expectedVolumeApplicable: true,
 		},
 		{
-			bz:         []byte{0, 0b01111111},
-			difficulty: 9,
+			desc:                     "Not Applicable: relayHash = targetHash",
+			relayHashHex:             "00ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff",
+			targetHashHex:            "00ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff",
+			expectedVolumeApplicable: false,
 		},
 		{
-			bz:         []byte{0, 0b00111111},
-			difficulty: 10,
+			desc:                     "Not applicable: relayHash > targetHash",
+			relayHashHex:             "0effffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff",
+			targetHashHex:            "00ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff",
+			expectedVolumeApplicable: false,
 		},
 		{
-			bz:         []byte{0, 0, 255},
-			difficulty: 16,
+			desc:                     "Not applicable: relayHash >> targetHash",
+			relayHashHex:             "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff",
+			targetHashHex:            "00ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff",
+			expectedVolumeApplicable: false,
 		},
 	}
 
 	for _, test := range tests {
-		t.Run(fmt.Sprintf("difficulty_%d_zero_bits", test.difficulty), func(t *testing.T) {
-			var bz [32]byte
-			copy(bz[:], test.bz)
-			actualDifficulty := protocol.CountHashDifficultyBits(bz)
-			require.Equal(t, test.difficulty, actualDifficulty)
+		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

@@ -0,0 +1,15 @@
+package protocol
+
+// 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 GetRelayHashFromBytes(relayBz []byte) (hash [RelayHasherSize]byte) {
+	hasher := NewRelayHasher()
+
+	// NB: Intentionally ignoring the error, following sha256.Sum256 implementation.
+	_, _ = hasher.Write(relayBz)
+	hashBz := hasher.Sum(nil)
+	copy(hash[:], hashBz)
+
+	return hash
+}

pkg/crypto/protocol/hasher.go

@@ -3,10 +3,13 @@ package protocol
 import "crypto/sha256"
 
 const (
-	TrieHasherSize = sha256.Size
-	TrieRootSize   = TrieHasherSize + trieRootMetadataSize
-	// TODO_CONSIDERATION: Export this from the SMT package.
-	trieRootMetadataSize = 16
+	RelayHasherSize      = sha256.Size
+	TrieHasherSize       = sha256.Size
+	TrieRootSize         = TrieHasherSize + trieRootMetadataSize
+	trieRootMetadataSize = 16 // TODO_CONSIDERATION: Export this from the SMT package.
 )
 
-var NewTrieHasher = sha256.New
+var (
+	NewRelayHasher = sha256.New
+	NewTrieHasher  = sha256.New
+)

pkg/crypto/rand/integer.go

@@ -17,6 +17,16 @@ import (
 func SeededInt63(seedParts ...[]byte) int64 {
 	seedHashInputBz := bytes.Join(append([][]byte{}, seedParts...), nil)
 	seedHash := crypto.Sha256(seedHashInputBz)
+
+	// TODO_MAINNET: To support other language implementations of the protocol, the
+	// pseudo-random number generator used here should be language-agnostic (i.e. not
+	// golang specific).
+	//
+	// Additionally, there is a precision loss here when converting the hash to an int64.
+	// Since the math/rand.Source interface only supports int64 seeds, we are forced to
+	// truncate the hash to 64 bits. This is not ideal, as it reduces the entropy of the
+	// seed. We should consider using a different random number generator that supports
+	// byte array seeds.
 	seed, _ := binary.Varint(seedHash)
 
 	return rand.NewSource(seed).Int63()

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
@@ -152,7 +153,6 @@ func genRandomizedMinedRelayFixtures(
 				Res: nil,
 			}
 
-			// TODO_TECHDEBT(@red-0ne): use canonical codec.
 			relayBz, err := relay.Marshal()
 			if err != nil {
 				errCh <- err