[Mining] refactor: difficulty in terms of target hash

Makefile

@@ -420,7 +420,7 @@
 	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 @@
 # 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 @@
 	@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 occured, try to run:               |"
+	@echo "|     	'make test_all' once or twice more                                                     |"
+	@echo "|     3. If the same error persistes, isolate it with 'go test -v ./path/to/failing/module      |"
+	@echo "|                                                                                               |"
+	@echo "+-----------------------------------------------------------------------------------------------+"
+
 ##############
 ### Claims ###
 ##############

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/crypto/protocol/difficulty.go

@@ -1,27 +1,45 @@
 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()
+	// 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

@@ -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/crypto/protocol/hasher.go

@@ -3,14 +3,13 @@ package protocol
 import "crypto/sha256"
 
 const (
-	RelayHasherSize = sha256.Size
-	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 (
 	NewRelayHasher = sha256.New
-	NewTrieHasher = sha256.New
+	NewTrieHasher  = sha256.New
 )

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

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"];
 

tests/integration/tokenomics/relay_mining_difficulty_test.go

@@ -21,8 +21,6 @@ import (
 	tokenomicstypes "github.com/pokt-network/poktroll/x/tokenomics/types"
 )
 
-// TODO_UPNEXT(@Olshansk, #571): Implement these tests
-
 func init() {
 	cmd.InitSDKConfig()
 }

testutil/network/network.go

@@ -50,7 +50,7 @@ func New(t *testing.T, configs ...Config) *Network {
 		cfg = configs[0]
 	}
 	net, err := network.New(t, t.TempDir(), cfg)
-	require.NoError(t, err)
+	require.NoError(t, err, "TODO_FLAKY: This config setup is periodically flaky")
 	_, err = net.WaitForHeight(1)
 	require.NoError(t, err)
 	t.Cleanup(net.Cleanup)

testutil/testrelayer/relays.go

@@ -53,11 +53,10 @@ func NewUnsignedMinedRelay(
 		},
 	}
 
-	// TODO_TECHDEBT(@red-0ne): marshal using canonical codec.
 	relayBz, err := relay.Marshal()
 	require.NoError(t, err)
 
-	relayHashArr := protocol.GetHashFromBytes(relayBz)
+	relayHashArr := protocol.GetRelayHashFromBytes(relayBz)
 	relayHash := relayHashArr[:]
 
 	return &relayer.MinedRelay{
@@ -111,7 +110,7 @@ func NewSignedMinedRelay(
 	relayBz, err := relay.Marshal()
 	require.NoError(t, err)
 
-	relayHashArr := protocol.GetHashFromBytes(relayBz)
+	relayHashArr := protocol.GetRelayHashFromBytes(relayBz)
 	relayHash := relayHashArr[:]
 
 	return &relayer.MinedRelay{