From bdbc365e9b40e0448d2928165ffaa6164eb50b8e Mon Sep 17 00:00:00 2001 From: Bryan White Date: Sat, 6 Jul 2024 11:52:01 +0200 Subject: [PATCH] [Code Health] refactor: simplify `protocol.CountHashDifficultyBits()` (#656) Co-authored-by: Daniel Olshansky --- pkg/crypto/protocol/difficulty.go | 20 +++++++ pkg/crypto/protocol/difficulty_test.go | 51 +++++++++++++++++ pkg/crypto/protocol/errors.go | 5 ++ pkg/relayer/miner/gen/gen_fixtures.go | 6 +- pkg/relayer/miner/miner.go | 9 ++- pkg/relayer/protocol/difficulty.go | 48 ---------------- pkg/relayer/protocol/difficulty_test.go | 56 ------------------- pkg/relayer/protocol/errors.go | 8 --- x/proof/keeper/msg_server_submit_proof.go | 11 +--- .../keeper/msg_server_submit_proof_test.go | 7 +-- 10 files changed, 87 insertions(+), 134 deletions(-) create mode 100644 pkg/crypto/protocol/difficulty.go create mode 100644 pkg/crypto/protocol/difficulty_test.go create mode 100644 pkg/crypto/protocol/errors.go delete mode 100644 pkg/relayer/protocol/difficulty.go delete mode 100644 pkg/relayer/protocol/difficulty_test.go delete mode 100644 pkg/relayer/protocol/errors.go diff --git a/pkg/crypto/protocol/difficulty.go b/pkg/crypto/protocol/difficulty.go new file mode 100644 index 000000000..8901f0758 --- /dev/null +++ b/pkg/crypto/protocol/difficulty.go @@ -0,0 +1,20 @@ +package protocol + +import ( + "encoding/binary" + "math/bits" +) + +// 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[:])) +} diff --git a/pkg/crypto/protocol/difficulty_test.go b/pkg/crypto/protocol/difficulty_test.go new file mode 100644 index 000000000..90a9a2367 --- /dev/null +++ b/pkg/crypto/protocol/difficulty_test.go @@ -0,0 +1,51 @@ +package protocol_test + +import ( + "fmt" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/pokt-network/poktroll/pkg/crypto/protocol" +) + +func TestCountDifficultyBits(t *testing.T) { + tests := []struct { + bz []byte + difficulty int + }{ + { + bz: []byte{0b11111111}, + difficulty: 0, + }, + { + bz: []byte{0b01111111}, + difficulty: 1, + }, + { + bz: []byte{0, 255}, + difficulty: 8, + }, + { + bz: []byte{0, 0b01111111}, + difficulty: 9, + }, + { + bz: []byte{0, 0b00111111}, + difficulty: 10, + }, + { + bz: []byte{0, 0, 255}, + difficulty: 16, + }, + } + + 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) + }) + } +} diff --git a/pkg/crypto/protocol/errors.go b/pkg/crypto/protocol/errors.go new file mode 100644 index 000000000..da3ba4e79 --- /dev/null +++ b/pkg/crypto/protocol/errors.go @@ -0,0 +1,5 @@ +package protocol + +var ( + codespace = "crypto/protocol" +) diff --git a/pkg/relayer/miner/gen/gen_fixtures.go b/pkg/relayer/miner/gen/gen_fixtures.go index 358cd722e..0531e4266 100644 --- a/pkg/relayer/miner/gen/gen_fixtures.go +++ b/pkg/relayer/miner/gen/gen_fixtures.go @@ -17,11 +17,11 @@ import ( "sync" "time" + "github.com/pokt-network/poktroll/pkg/crypto/protocol" "github.com/pokt-network/poktroll/pkg/observable" "github.com/pokt-network/poktroll/pkg/observable/channel" "github.com/pokt-network/poktroll/pkg/relayer" "github.com/pokt-network/poktroll/pkg/relayer/miner" - "github.com/pokt-network/poktroll/pkg/relayer/protocol" servicetypes "github.com/pokt-network/poktroll/x/service/types" ) @@ -200,13 +200,13 @@ func exitOnError(errCh <-chan error) { // difficultyGTE returns true if the given hash has a difficulty greater than or // equal to flagDifficultyBitsThreshold. func difficultyGTE(hash []byte) bool { - return protocol.MustCountDifficultyBits(hash) >= flagDifficultyBitsThreshold + return protocol.CountDifficultyBits(hash) >= flagDifficultyBitsThreshold } // difficultyLT returns true if the given hash has a difficulty less than // flagDifficultyBitsThreshold. func difficultyLT(hash []byte) bool { - return protocol.MustCountDifficultyBits(hash) < flagDifficultyBitsThreshold + return protocol.CountDifficultyBits(hash) < flagDifficultyBitsThreshold } // getMarshaledRelayFmtLines performs two map operations followed by a collect. diff --git a/pkg/relayer/miner/miner.go b/pkg/relayer/miner/miner.go index d534bd9a5..c474d9c15 100644 --- a/pkg/relayer/miner/miner.go +++ b/pkg/relayer/miner/miner.go @@ -6,13 +6,13 @@ import ( "cosmossdk.io/depinject" "github.com/pokt-network/poktroll/pkg/client" + "github.com/pokt-network/poktroll/pkg/crypto/protocol" "github.com/pokt-network/poktroll/pkg/either" "github.com/pokt-network/poktroll/pkg/observable" "github.com/pokt-network/poktroll/pkg/observable/channel" "github.com/pokt-network/poktroll/pkg/observable/filter" "github.com/pokt-network/poktroll/pkg/observable/logging" "github.com/pokt-network/poktroll/pkg/relayer" - "github.com/pokt-network/poktroll/pkg/relayer/protocol" servicetypes "github.com/pokt-network/poktroll/x/service/types" ) @@ -112,11 +112,10 @@ func (mnr *miner) mapMineRelay( if err != nil { return either.Error[*relayer.MinedRelay](err), false } - relayHashArr := servicetypes.GetHashFromBytes(relayBz) - relayHash := relayHashArr[:] + relayHash := servicetypes.GetHashFromBytes(relayBz) // The relay IS NOT volume / reward applicable - if uint64(protocol.MustCountDifficultyBits(relayHash)) < mnr.relayDifficultyBits { + if uint64(protocol.CountHashDifficultyBits(relayHash)) < mnr.relayDifficultyBits { return either.Success[*relayer.MinedRelay](nil), true } @@ -124,6 +123,6 @@ func (mnr *miner) mapMineRelay( return either.Success(&relayer.MinedRelay{ Relay: *relay, Bytes: relayBz, - Hash: relayHash, + Hash: relayHash[:], }), false } diff --git a/pkg/relayer/protocol/difficulty.go b/pkg/relayer/protocol/difficulty.go deleted file mode 100644 index 4ab8b357c..000000000 --- a/pkg/relayer/protocol/difficulty.go +++ /dev/null @@ -1,48 +0,0 @@ -package protocol - -import "math/bits" - -// MustCountDifficultyBits returns the number of leading zero bits in the given -// byte slice. It panics if an error is encountered. -func MustCountDifficultyBits(bz []byte) int { - diff, err := CountHashDifficultyBits(bz) - if err != nil { - panic(err) - } - - return diff -} - -// CountHashDifficultyBits returns the number of leading zero bits in the given byte -// slice. It returns an error if the byte slice is all zero bits. -// -// TODO_BLOCKER(@Olshansk): Remove the forloop logic and replace with a simplified -// single method that accounts for the fact that block hashes/paths are always -// 32 bytes. We use Sha256 (32 bytes) and CosmosSDK defaults to 32 byte block -// hashes so specifying makes sense here. -// -// func CountHashDifficultyBits(bz [32]byte) int { -// return bits.LeadingZeros64(binary.LittleEndian.Uint64(bz)) -// } -// -// The above would mean we can replace MustCountDifficultyBits entirely. -func CountHashDifficultyBits(bz []byte) (int, error) { - bzLen := len(bz) - - var zeroBits int - for byteIdx, byteValue := range bz { - if byteValue != 0 { - zeroBits = bits.LeadingZeros8(byteValue) - if zeroBits == 8 { - // we already checked that byteValue != 0. - return 0, ErrDifficulty.Wrap("impossible code path") - } - - // We have byteIdx bytes that are all 0s and one byte that has - // zeroBits number of leading 0 bits. - return (byteIdx)*8 + zeroBits, nil - } - } - - return 0, ErrDifficulty.Wrapf("difficulty matches bytes length: %d; bytes (hex): % x", bzLen, bz) -} diff --git a/pkg/relayer/protocol/difficulty_test.go b/pkg/relayer/protocol/difficulty_test.go deleted file mode 100644 index b29e266be..000000000 --- a/pkg/relayer/protocol/difficulty_test.go +++ /dev/null @@ -1,56 +0,0 @@ -package protocol_test - -import ( - "fmt" - "testing" - - "github.com/stretchr/testify/require" - - "github.com/pokt-network/poktroll/pkg/relayer/protocol" -) - -func TestCountDifficultyBits(t *testing.T) { - tests := []struct { - bz []byte - difficulty int - }{ - { - bz: []byte{0b11111111, 255, 255, 255}, - difficulty: 0, - }, - { - bz: []byte{0b01111111, 255, 255, 255}, - difficulty: 1, - }, - { - bz: []byte{0, 255, 255, 255}, - difficulty: 8, - }, - { - bz: []byte{0, 0b01111111, 255, 255}, - difficulty: 9, - }, - { - bz: []byte{0, 0b00111111, 255, 255}, - difficulty: 10, - }, - { - bz: []byte{0, 0, 255, 255}, - difficulty: 16, - }, - } - - for _, test := range tests { - t.Run(fmt.Sprintf("difficulty_%d_zero_bits", test.difficulty), func(t *testing.T) { - actualDifficulty, err := protocol.CountHashDifficultyBits(test.bz) - require.NoError(t, err) - require.Equal(t, test.difficulty, actualDifficulty) - }) - } -} - -func TestCountDifficultyBits_Error(t *testing.T) { - _, err := protocol.CountHashDifficultyBits([]byte{0, 0, 0, 0}) - require.ErrorIs(t, err, protocol.ErrDifficulty) - require.ErrorContains(t, err, "difficulty matches bytes length") -} diff --git a/pkg/relayer/protocol/errors.go b/pkg/relayer/protocol/errors.go deleted file mode 100644 index 9f3de5c29..000000000 --- a/pkg/relayer/protocol/errors.go +++ /dev/null @@ -1,8 +0,0 @@ -package protocol - -import sdkerrors "cosmossdk.io/errors" - -var ( - ErrDifficulty = sdkerrors.New(codespace, 1, "difficulty error") - codespace = "relayer/protocol" -) diff --git a/x/proof/keeper/msg_server_submit_proof.go b/x/proof/keeper/msg_server_submit_proof.go index 465aa9da7..e8633fed3 100644 --- a/x/proof/keeper/msg_server_submit_proof.go +++ b/x/proof/keeper/msg_server_submit_proof.go @@ -18,7 +18,7 @@ import ( "google.golang.org/grpc/codes" "google.golang.org/grpc/status" - "github.com/pokt-network/poktroll/pkg/relayer/protocol" + "github.com/pokt-network/poktroll/pkg/crypto/protocol" "github.com/pokt-network/poktroll/telemetry" "github.com/pokt-network/poktroll/x/proof/types" servicetypes "github.com/pokt-network/poktroll/x/service/types" @@ -476,14 +476,7 @@ func verifyClosestProof( // function that can be used by both the proof and the miner packages. func validateMiningDifficulty(relayBz []byte, minRelayDifficultyBits uint64) error { relayHash := servicetypes.GetHashFromBytes(relayBz) - - relayDifficultyBits, err := protocol.CountHashDifficultyBits(relayHash[:]) - if err != nil { - return types.ErrProofInvalidRelay.Wrapf( - "error counting difficulty bits: %s", - err, - ) - } + relayDifficultyBits := protocol.CountHashDifficultyBits(relayHash) // TODO_MAINNET: Devise a test that tries to attack the network and ensure that there // is sufficient telemetry. diff --git a/x/proof/keeper/msg_server_submit_proof_test.go b/x/proof/keeper/msg_server_submit_proof_test.go index cd73c08c7..236f87ff3 100644 --- a/x/proof/keeper/msg_server_submit_proof_test.go +++ b/x/proof/keeper/msg_server_submit_proof_test.go @@ -16,10 +16,10 @@ import ( "google.golang.org/grpc/status" "github.com/pokt-network/poktroll/pkg/crypto" + "github.com/pokt-network/poktroll/pkg/crypto/protocol" "github.com/pokt-network/poktroll/pkg/crypto/rings" "github.com/pokt-network/poktroll/pkg/polylog/polyzero" "github.com/pokt-network/poktroll/pkg/relayer" - "github.com/pokt-network/poktroll/pkg/relayer/protocol" "github.com/pokt-network/poktroll/pkg/relayer/session" testutilevents "github.com/pokt-network/poktroll/testutil/events" keepertest "github.com/pokt-network/poktroll/testutil/keeper" @@ -1394,10 +1394,7 @@ func getClosestRelayDifficultyBits( require.NoError(t, err) // Count the number of leading 0s in the relay hash to determine its difficulty. - relayDifficultyBits, err := protocol.CountHashDifficultyBits(relayHash[:]) - require.NoError(t, err) - - return uint64(relayDifficultyBits) + return uint64(protocol.CountHashDifficultyBits(relayHash)) } // resetBlockHeightFn returns a function that resets the block height of the