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

Sort RewardDelegators when marshaling a validator #1591

Merged
merged 2 commits into from
Jan 16, 2024

Conversation

msmania
Copy link
Contributor

@msmania msmania commented Jan 14, 2024

Description

This is a fix for a regression introduced by #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.

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 reviewpad bot added small Pull request is small waiting-for-review labels Jan 14, 2024
@msmania msmania self-assigned this Jan 14, 2024
@msmania msmania added this to the Network Quality of Life milestone Jan 14, 2024
@msmania msmania force-pushed the toshi/bugfix-sort-RewardDelegators branch from f3bca76 to 23b00b4 Compare January 14, 2024 23:02
@msmania msmania requested a review from Olshansk January 15, 2024 00:07
@Olshansk
Copy link
Member

@msmania Friendly reminder to update ticket metadata
Screenshot 2024-01-15 at 11 22 56 AM

x/nodes/keeper/validator_test.go Show resolved Hide resolved
x/nodes/keeper/validator_test.go Show resolved Hide resolved
x/nodes/keeper/validator_test.go Outdated Show resolved Hide resolved
x/nodes/types/nodes.pb.go Outdated Show resolved Hide resolved
@msmania
Copy link
Contributor Author

msmania commented Jan 15, 2024

@msmania Friendly reminder to update ticket metadata

So the process is to set a project AND fill the Release field in it?

@reviewpad reviewpad bot added medium Pull request is medium and removed small Pull request is small labels Jan 15, 2024
@msmania msmania requested a review from Olshansk January 15, 2024 18:40
@msmania msmania merged commit e78e940 into staging Jan 16, 2024
5 checks passed
@msmania msmania deleted the toshi/bugfix-sort-RewardDelegators branch January 16, 2024 17:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working medium Pull request is medium waiting-for-review
Projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.

2 participants