Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Proof Validation] Move proof validation to session settlement #703

Merged
merged 38 commits into from
Jul 25, 2024
Merged
Show file tree
Hide file tree
Changes from 31 commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
ffb6902
refactor: difficulty in terms of target hash
bryanchriswhite Jul 8, 2024
ceb283d
fix: e2e test
bryanchriswhite Jul 16, 2024
0ca9b98
fix: e2e test
bryanchriswhite Jul 16, 2024
38d1985
chore: review feedback improvements
bryanchriswhite Jul 17, 2024
1c66b4f
Merge remote-tracking branch 'pokt/main' into refactor/difficulty/tar…
bryanchriswhite Jul 17, 2024
5089b94
Merge branch 'main' into refactor/difficulty/target-hash
bryanchriswhite Jul 17, 2024
cc6468d
refactor: protocol.NewRelayHasher, .RelayHashSize
bryanchriswhite Jul 18, 2024
5675f81
fix: ComputeNewDifficultyTargetHash()
bryanchriswhite Jul 19, 2024
440ecc6
chore: review fee
bryanchriswhite Jul 19, 2024
ee59e0d
refactor: use big.Floats on-chain & cleanup
bryanchriswhite Jul 19, 2024
69d97d3
chore: review improvements
bryanchriswhite Jul 19, 2024
5a3cfd8
[TODO] chore: update `smt.MerkleRoot#Sum()` error handling (#672)
bryanchriswhite Jul 19, 2024
e2802ac
Merge remote-tracking branch 'pokt/main' into refactor/difficulty/tar…
bryanchriswhite Jul 19, 2024
a5f9b61
Last review of #690
Olshansk Jul 19, 2024
c365a4f
Self review
Olshansk Jul 20, 2024
09d4cb0
Fix flaky tests
Olshansk Jul 22, 2024
36d55a3
Minor nits
Olshansk Jul 22, 2024
9cf20eb
Move the verification layer
Olshansk Jul 23, 2024
cb166cd
Update testutil/network/network.go
Olshansk Jul 23, 2024
49ce8b7
Merge branch 'refactor/difficulty/target-hash-review' into pr_690_fol…
Olshansk Jul 23, 2024
a52a5e8
Working on one last test
Olshansk Jul 23, 2024
ac4a212
Add a TODO_MAINNET
Olshansk Jul 23, 2024
8e0d26c
Merge branch 'refactor/difficulty/target-hash-review' into pr_690_fol…
Olshansk Jul 23, 2024
f16ba64
WIP
Olshansk Jul 23, 2024
6dbbcd5
Not functioning but temp WIP
Olshansk Jul 23, 2024
58591c4
The test passes
Olshansk Jul 24, 2024
d21f9fc
About to do a self review
Olshansk Jul 24, 2024
99783a8
Review of #690 by @olshansk (#699)
Olshansk Jul 24, 2024
8fe0e87
git merge refactor/difficulty/target-hash
Olshansk Jul 24, 2024
0147e99
Finished self review
Olshansk Jul 24, 2024
f7b4e5c
Missing )
Olshansk Jul 24, 2024
54b0261
Merge with main
Olshansk Jul 24, 2024
ba2058c
Merge with main
Olshansk Jul 24, 2024
2d60b8d
Replied to some PR comments
Olshansk Jul 24, 2024
e2aa6fd
Reply to all review comments
Olshansk Jul 24, 2024
a4d28e3
Fix unit test
Olshansk Jul 24, 2024
6bf702e
Merge branch 'main' into pr_690_followups
Olshansk Jul 25, 2024
9e8e8e8
Omit nil check
Olshansk Jul 25, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 22 additions & 4 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 ###
Expand Down Expand Up @@ -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 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 ###
##############
Expand Down
167 changes: 93 additions & 74 deletions api/poktroll/proof/params.pulsar.go

Large diffs are not rendered by default.

195 changes: 159 additions & 36 deletions api/poktroll/tokenomics/event.pulsar.go

Large diffs are not rendered by default.

5 changes: 3 additions & 2 deletions e2e/tests/parse_params_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
package e2e

import (
"encoding/hex"
"fmt"
"strconv"

Expand Down Expand Up @@ -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:
Expand Down
14 changes: 7 additions & 7 deletions e2e/tests/update_params.feature
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
5 changes: 3 additions & 2 deletions e2e/tests/update_params_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
package e2e

import (
"encoding/hex"
"encoding/json"
"fmt"
"reflect"
Expand Down Expand Up @@ -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]
Expand Down
1 change: 0 additions & 1 deletion pkg/client/events/query_client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion pkg/client/interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions pkg/client/query/accquerier.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,9 @@ func (aq *accQuerier) GetPubKeyFromAddress(ctx context.Context, address string)
if err != nil {
return nil, err
}
if acc == nil {
return nil, ErrQueryAccountNotFound.Wrapf("address: %s", address)
}

// If the account's public key is nil, then return an error.
pubKey := acc.GetPubKey()
Expand Down
54 changes: 40 additions & 14 deletions pkg/crypto/protocol/difficulty.go
Original file line number Diff line number Diff line change
@@ -1,20 +1,46 @@
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.
// TODO_MAINNET: Can this cause an integer overflow?
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
}
103 changes: 79 additions & 24 deletions pkg/crypto/protocol/difficulty_test.go
Original file line number Diff line number Diff line change
@@ -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))
})
}
}
15 changes: 15 additions & 0 deletions pkg/crypto/protocol/hash.go
Original file line number Diff line number Diff line change
@@ -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
}
13 changes: 8 additions & 5 deletions pkg/crypto/protocol/hasher.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
)
10 changes: 10 additions & 0 deletions pkg/crypto/rand/integer.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
Loading
Loading