Skip to content

Commit

Permalink
Sort RewardDelegators when marshaling a validator (pokt-network#1591)
Browse files Browse the repository at this point in the history
## Description

This is a fix for a regression introduced by pokt-network#1581.

When a node stores a validator with the new `RewardDelegators` field,
its marshaled product is not deterministic because the order of map
iteration is not deterministic. As a result, the merkle tree of
application.db can differ between nodes, leading to consensus failure.

The proposed fix is to sort the keys of `RewardDelegators` by using the
stable marshaler.

<!-- reviewpad:summarize:start -->
### Summary generated by Reviewpad on 15 Jan 24 18:21 UTC
This pull request includes changes to the `nodes.proto` file and a file
diff.

The changes in the `nodes.proto` file include:
- Added an option `stable_marshaler` with a value of `true`. This option
is used to make the order of `RewardDelegators` deterministic.
- Updated the `Address` field to include additional options
`(gogoproto.casttype)`, `(gogoproto.moretags)`, and
`(gogoproto.jsontag)`.
- Updated the `PublicKey` field to include additional options
`(gogoproto.moretags)` and `(gogoproto.jsontag)`.
- Updated the `jailed` field to include an additional option
`(gogoproto.jsontag)`.

The file diff includes the following changes:
- Imported the "encoding/hex", "math/rand", "reflect", "strconv", and
"time" packages.
- Added a test case for marshalling RewardDelegators.
- Added the "Test_Marshal_RewardDelegators" function.
- Created variables and constants within the test function.
- Generated random numbers to be used as reward delegator addresses.
- Created and initialized a map, "delegatorMap", with random numbers as
keys and their corresponding modulo 10 values as values.
- Created a "Validator" struct with various fields and assigned values
to them.
- Marshaled the validator struct and ensured that the resulting hash is
the same for every iteration of the test.

Please let me know if there is anything specific you would like me to
focus on.
<!-- reviewpad:summarize:end -->
  • Loading branch information
msmania authored Jan 16, 2024
1 parent 69a035d commit e78e940
Show file tree
Hide file tree
Showing 3 changed files with 142 additions and 68 deletions.
3 changes: 3 additions & 0 deletions proto/x/nodes/nodes.proto
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@ message ProtoValidator {
option (gogoproto.goproto_stringer) = true;
option (gogoproto.goproto_getters) = false;

// This option is to make the order of RewardDelegators deterministic
option (gogoproto.stable_marshaler) = true;

bytes Address = 1 [(gogoproto.casttype) = "github.com/pokt-network/pocket-core/types.Address", (gogoproto.moretags) = "yaml:\"address\"", (gogoproto.jsontag) = "address"];
bytes PublicKey = 2 [(gogoproto.moretags) = "yaml:\"public_key\"", (gogoproto.jsontag) = "public_key"];
bool jailed = 3 [(gogoproto.jsontag) = "jailed"];
Expand Down
69 changes: 69 additions & 0 deletions x/nodes/keeper/validator_test.go
Original file line number Diff line number Diff line change
@@ -1,10 +1,15 @@
package keeper

import (
"encoding/hex"
"fmt"
"math/rand"
"reflect"
"strconv"
"testing"
"time"

"github.com/pokt-network/pocket-core/crypto"
sdk "github.com/pokt-network/pocket-core/types"
"github.com/pokt-network/pocket-core/x/nodes/types"
"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -150,6 +155,70 @@ func Test_sortNoLongerStakedValidators(t *testing.T) {
}
}

// Verifies the marshaled product of a validator is the same regardless of
// the order of RewardDelegators. If it is not, a key of the merkle tree in
// application.db is not deterministic, that will cause consensus failure.
func Test_Marshal_RewardDelegators(t *testing.T) {
numOfIterations := 100
numOfDelegators := 100

chains := []string{"0001", "0002", "005A", "005B", "005C", "005D"}
url := "https://test.pokt.network:443"
stake := int64(18000000000)
pubKey, _ := crypto.NewPublicKey("0b787e54e66b3db3a3396c2322d8314287e08990f7696c490153b078bada7e94")
nodeAddr := sdk.Address(pubKey.PubKey().Address())
outputAddr, _ := sdk.AddressFromHex("42846261e1798fc08e1dfd97325af7b280f815b0")

// Generate a bunch of random numbers to use as reward delegator addressees
randNums := make([]int, numOfDelegators)
for j := 0; j < numOfDelegators; j++ {
randNums[j] = rand.Int()
}

var valHash string

for i := 0; i < numOfIterations; i++ {
// Initialize `delegatorMap` with a different order of the pre-created
// random numbers. In every iteration, the value of `delegatorMap` is the
// same, but the order of its range loop (`for k, v := range delegatorMap`)
// is usually impacted by the order of values inserted.
// This behavior is not guaranteed as https://go.dev/ref/spec#For_statements
// says "The iteration order over maps is not specified and is not
// guaranteed to be the same from one iteration to the next", but it's
// ok for testing purpose.
rand.Shuffle(len(randNums), func(i, j int) {
randNums[i], randNums[j] = randNums[j], randNums[i]
})
delegatorMap := map[string]uint32{}
for randNum := range randNums {
delegatorMap[strconv.Itoa(randNum)] = uint32(randNum % 10)
}

val := types.Validator{
Address: nodeAddr,
PublicKey: pubKey,
Jailed: false,
Status: sdk.Staked,
Chains: chains,
UnstakingCompletionTime: time.Time{},
ServiceURL: url,
StakedTokens: sdk.NewInt(stake),
OutputAddress: outputAddr,
RewardDelegators: delegatorMap,
}
valBytes, err := val.Marshal()
assert.Nil(t, err)

// First test iteration when valhash hasn't been set yet
if len(valHash) == 0 {
valHash = hex.EncodeToString(valBytes)
}

// Make sure the hash is always the same regardless of the order of randNums
assert.Equal(t, hex.EncodeToString(valBytes), valHash)
}
}

// There are two versions of structs to represent a validator.
// - LegacyValidator - the original version
// - Validator - LegacyValidator + OutputAddress + Delegators (since 0.11)
Expand Down
138 changes: 70 additions & 68 deletions x/nodes/types/nodes.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit e78e940

Please sign in to comment.