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

Pchain bls key diff fix #1584

Merged
merged 33 commits into from
Jun 8, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
44fa8e9
fix
abi87 Jun 5, 2023
b608a16
nits
joshua-kim Jun 5, 2023
6bc2ed0
nit
joshua-kim Jun 5, 2023
04d4e9c
simplify
joshua-kim Jun 5, 2023
c509fe1
Revert "simplify"
joshua-kim Jun 5, 2023
8df0224
Update vms/platformvm/validators_set_property_test.go
joshua-kim Jun 5, 2023
5831ec4
Update vms/platformvm/validators_set_property_test.go
joshua-kim Jun 5, 2023
8430ee1
Update vms/platformvm/validators_set_property_test.go
joshua-kim Jun 5, 2023
284000d
Update vms/platformvm/validators_set_property_test.go
joshua-kim Jun 5, 2023
ae6e944
Update vms/platformvm/validators_set_property_test.go
joshua-kim Jun 5, 2023
e51ab31
Update vms/platformvm/validators_set_property_test.go
joshua-kim Jun 5, 2023
33cae54
Update vms/platformvm/validators_set_property_test.go
joshua-kim Jun 5, 2023
d63f5df
Update vms/platformvm/vm_regression_test.go
joshua-kim Jun 5, 2023
b71c0bc
nits
joshua-kim Jun 5, 2023
1db7996
nits
joshua-kim Jun 5, 2023
071559d
nit
abi87 Jun 6, 2023
0c96716
nits
abi87 Jun 6, 2023
b897764
nits
abi87 Jun 6, 2023
8c0a2a9
Merge branch 'dev' into pchain_bls_diffs_fix
abi87 Jun 6, 2023
b1734aa
nit
abi87 Jun 6, 2023
584dec0
Merge branch 'dev' into pchain_bls_diffs_fix
abi87 Jun 6, 2023
f09ecd5
Merge branch 'dev' into pchain_bls_diffs_fix
abi87 Jun 6, 2023
f6d9ce9
nits
abi87 Jun 6, 2023
efbbf20
cleanup
abi87 Jun 6, 2023
7c39fbe
Merge branch 'dev' into pchain_bls_diffs_fix
hexfusion Jun 6, 2023
b0e038f
nit
abi87 Jun 7, 2023
6540524
nit
abi87 Jun 7, 2023
3b2f786
Merge branch 'dev' into pchain_bls_diffs_fix
StephenButtolph Jun 7, 2023
6b49006
Pchain bls diffs fix cleanup (#1594)
StephenButtolph Jun 8, 2023
0c1d5e5
Merge branch 'dev' into pchain_bls_diffs_fix
StephenButtolph Jun 8, 2023
6286ca7
format nits
StephenButtolph Jun 8, 2023
acac984
import nit
StephenButtolph Jun 8, 2023
fa43300
nit naming
StephenButtolph Jun 8, 2023
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
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ require (
github.com/huin/goupnp v1.0.3
github.com/jackpal/gateway v1.0.6
github.com/jackpal/go-nat-pmp v1.0.2
github.com/leanovate/gopter v0.2.9
github.com/mr-tron/base58 v1.2.0
github.com/nbutton23/zxcvbn-go v0.0.0-20180912185939-ae427f1e4c1d
github.com/onsi/ginkgo/v2 v2.4.0
Expand Down
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,8 @@ github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ=
github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI=
github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY=
github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE=
github.com/leanovate/gopter v0.2.9 h1:fQjYxZaynp97ozCzfOyOuAGOU4aU/z37zf/tOujFk7c=
github.com/leanovate/gopter v0.2.9/go.mod h1:U2L/78B+KVFIx2VmW6onHJQzXtFb+p5y3y2Sh+Jxxv8=
github.com/magiconair/properties v1.8.6 h1:5ibWZ6iY0NctNGWo87LalDlEZ6R41TqbbDamhfG/Qzo=
github.com/magiconair/properties v1.8.6/go.mod h1:y3VJvCyxH9uVvJTWEGAELF3aiYNyPKd5NZ3oSwXrF60=
github.com/mattn/go-colorable v0.1.12 h1:jF+Du6AlPIjs2BiUiQlKOX0rt3SujHxPnksPKZbaA40=
Expand Down
134 changes: 86 additions & 48 deletions vms/platformvm/validators/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -164,100 +164,138 @@ func (m *manager) GetValidatorSet(ctx context.Context, height uint64, subnetID i
return nil, err
}
}
currentSubnetValidatorList := currentSubnetValidators.List()
subnetSet := make(map[ids.NodeID]*validators.GetValidatorOutput, len(currentSubnetValidatorList))
abi87 marked this conversation as resolved.
Show resolved Hide resolved
for _, vdr := range currentSubnetValidatorList {
subnetSet[vdr.NodeID] = &validators.GetValidatorOutput{
NodeID: vdr.NodeID,
// PublicKey will be picked from primary validators
Weight: vdr.Weight,
}
}

currentPrimaryNetworkValidators, ok := m.cfg.Validators.Get(constants.PrimaryNetworkID)
if !ok {
// This should never happen
return nil, ErrMissingValidator
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know not changed in this PR but this stuff freaks me out. Should this be tracked by a metric with an alert? Not finding the validator set for a primary chain during runtime sounds like stop the world event. Do we need to add any details on possible invariants, like maybe bootstrapping? Should this shutdown the node or could it be transient?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm definitely happy to add ERROR logs for stuff like this

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

started here will rebase after this merges #1592

}

currentSubnetValidatorList := currentSubnetValidators.List()
vdrSet := make(map[ids.NodeID]*validators.GetValidatorOutput, len(currentSubnetValidatorList))
for _, vdr := range currentSubnetValidatorList {
primaryVdr, ok := currentPrimaryNetworkValidators.Get(vdr.NodeID)
if !ok {
// This should never happen
return nil, fmt.Errorf("%w: %s", ErrMissingValidator, vdr.NodeID)
}
vdrSet[vdr.NodeID] = &validators.GetValidatorOutput{
currentPrimaryValidatorList := currentPrimaryNetworkValidators.List()
primarySet := make(map[ids.NodeID]*validators.GetValidatorOutput, len(currentPrimaryValidatorList))
abi87 marked this conversation as resolved.
Show resolved Hide resolved
for _, vdr := range currentPrimaryValidatorList {
primarySet[vdr.NodeID] = &validators.GetValidatorOutput{
NodeID: vdr.NodeID,
PublicKey: primaryVdr.PublicKey,
PublicKey: vdr.PublicKey,
Weight: vdr.Weight,
}

// fill PK from primary network
if _, found := subnetSet[vdr.NodeID]; found {
subnetSet[vdr.NodeID].PublicKey = vdr.PublicKey
}
abi87 marked this conversation as resolved.
Show resolved Hide resolved
}
joshua-kim marked this conversation as resolved.
Show resolved Hide resolved

for diffHeight := lastAcceptedHeight; diffHeight > height; diffHeight-- {
err := m.applyValidatorDiffs(vdrSet, subnetID, diffHeight)
err := m.applyValidatorDiffs(subnetSet, primarySet, subnetID, diffHeight)
if err != nil {
return nil, err
}
}

// cache the validator set
validatorSetsCache.Put(height, vdrSet)
validatorSetsCache.Put(height, subnetSet)

endTime := m.clk.Time()
m.metrics.IncValidatorSetsCreated()
m.metrics.AddValidatorSetsDuration(endTime.Sub(startTime))
m.metrics.AddValidatorSetsHeightDiff(lastAcceptedHeight - height)
return vdrSet, nil
return subnetSet, nil
}

func (m *manager) applyValidatorDiffs(
abi87 marked this conversation as resolved.
Show resolved Hide resolved
vdrSet map[ids.NodeID]*validators.GetValidatorOutput,
targetSet, primarySet map[ids.NodeID]*validators.GetValidatorOutput,
abi87 marked this conversation as resolved.
Show resolved Hide resolved
subnetID ids.ID,
height uint64,
) error {
weightDiffs, err := m.state.GetValidatorWeightDiffs(height, subnetID)
// fully rebuild primary network validators at [height]
primaryWeightDiffs, err := m.state.GetValidatorWeightDiffs(height, constants.PlatformChainID)
if err != nil {
return err
}

for nodeID, weightDiff := range weightDiffs {
vdr, ok := vdrSet[nodeID]
if !ok {
// This node isn't in the current validator set.
vdr = &validators.GetValidatorOutput{
NodeID: nodeID,
}
vdrSet[nodeID] = vdr
}

// The weight of this node changed at this block.
if weightDiff.Decrease {
// The validator's weight was decreased at this block, so in the
// prior block it was higher.
vdr.Weight, err = math.Add64(vdr.Weight, weightDiff.Amount)
} else {
// The validator's weight was increased at this block, so in the
// prior block it was lower.
vdr.Weight, err = math.Sub(vdr.Weight, weightDiff.Amount)
}
for nodeID, weightDiff := range primaryWeightDiffs {
err := rebuildWeight(primarySet, nodeID, weightDiff)
if err != nil {
return err
}

if vdr.Weight == 0 {
// The validator's weight was 0 before this block so
// they weren't in the validator set.
delete(vdrSet, nodeID)
}
}

pkDiffs, err := m.state.GetValidatorPublicKeyDiffs(height)
if err != nil {
return err
}

for nodeID, pk := range pkDiffs {
// pkDiffs includes all primary network key diffs, if we are
// fetching a subnet's validator set, we should ignore non-subnet
// validators.
if vdr, ok := vdrSet[nodeID]; ok {
if vdr, ok := primarySet[nodeID]; ok {
// The validator's public key was removed at this block, so it
// was in the validator set before.
vdr.PublicKey = pk
}
}

// rebuild weights of target validators
targetWeightDiffs, err := m.state.GetValidatorWeightDiffs(height, subnetID)
if err != nil {
return err
}
for nodeID, weightDiff := range targetWeightDiffs {
err := rebuildWeight(targetSet, nodeID, weightDiff)
if err != nil {
return err
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems this result may be cacheable up to here?
don't have to fix as part of this PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there are more efficient ways to store and use diffs. We'll work on them in a separate PR


// rebuild public key of target validators, just peeking in primary validators set
for nodeID, vdr := range targetSet {
if primary, found := primarySet[nodeID]; found {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need this check since all subnet validators are also primary network validators

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we don't, you're right. But a bug breaking this invariant would cause a panic if the found is dropped right? I guess I am just trying to be defensive. I feel we usually do this, but happy to switch if I am wrong

vdr.PublicKey = primary.PublicKey
}
}

return nil
}

func rebuildWeight(
targetSet map[ids.NodeID]*validators.GetValidatorOutput,
nodeID ids.NodeID,
weightDiff *state.ValidatorWeightDiff,
) error {
vdr, ok := targetSet[nodeID]
if !ok {
// This node isn't in the current validator set.
vdr = &validators.GetValidatorOutput{
NodeID: nodeID,
}
targetSet[nodeID] = vdr
}

// The weight of this node changed at this block.
var err error
if weightDiff.Decrease {
// The validator's weight was decreased at this block, so in the
// prior block it was higher.
vdr.Weight, err = math.Add64(vdr.Weight, weightDiff.Amount)
} else {
// The validator's weight was increased at this block, so in the
// prior block it was lower.
vdr.Weight, err = math.Sub(vdr.Weight, weightDiff.Amount)
}
if err != nil {
return err
}

if vdr.Weight == 0 {
// The validator's weight was 0 before this block so
// they weren't in the validator set.
delete(targetSet, nodeID)
}
return nil
}

Expand Down
Loading