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

Pull apart writeCurrentStakers method - Part 1 #2074

Closed
wants to merge 75 commits into from
Closed
Changes from 4 commits
Commits
Show all changes
75 commits
Select commit Hold shift + click to select a range
1c12d39
pulled weight diff computation apart from writeCurrentStaker method
abi87 Sep 22, 2023
c5061a4
pulled bls diff computation apart from writeCurrentStaker method
abi87 Sep 22, 2023
b0cd9cb
nits
abi87 Sep 22, 2023
fe93ba1
nit
abi87 Sep 22, 2023
34d5602
Merge branch 'dev' into make_write_current_stakers_testable
abi87 Sep 22, 2023
cddf51c
Merge branch 'dev' into make_write_current_stakers_testable
abi87 Sep 22, 2023
9b77852
Merge branch 'dev' into make_write_current_stakers_testable
abi87 Sep 23, 2023
407cb74
Merge branch 'dev' into make_write_current_stakers_testable
abi87 Sep 25, 2023
8caf284
nit
abi87 Sep 25, 2023
96d0e56
nit
abi87 Sep 26, 2023
112a2e2
Merge branch 'dev' into make_write_current_stakers_testable
abi87 Sep 27, 2023
460d5c9
Merge branch 'dev' into make_write_current_stakers_testable
abi87 Sep 28, 2023
14b33a0
Merge branch 'dev' into make_write_current_stakers_testable
abi87 Sep 29, 2023
590bb02
improve iterator release
abi87 Sep 29, 2023
37d1d5e
Merge branch 'dev' into make_write_current_stakers_testable
abi87 Sep 30, 2023
6373786
Merge branch 'dev' into make_write_current_stakers_testable
abi87 Oct 4, 2023
3d7cc37
Merge branch 'dev' into make_write_current_stakers_testable
abi87 Oct 5, 2023
405632e
Merge branch 'dev' into make_write_current_stakers_testable
abi87 Oct 6, 2023
b281f22
Merge branch 'dev' into make_write_current_stakers_testable
abi87 Oct 7, 2023
3eaa5c0
Merge branch 'dev' into make_write_current_stakers_testable
abi87 Oct 11, 2023
25804f4
Merge branch 'dev' into make_write_current_stakers_testable
abi87 Oct 12, 2023
5bdd4f5
Merge branch 'dev' into make_write_current_stakers_testable
abi87 Oct 15, 2023
e5fd062
Merge branch 'dev' into make_write_current_stakers_testable
abi87 Oct 17, 2023
c809c46
Merge branch 'dev' into make_write_current_stakers_testable
abi87 Oct 17, 2023
4c3f70b
Merge branch 'dev' into make_write_current_stakers_testable
abi87 Oct 18, 2023
a30a7f8
Merge branch 'dev' into make_write_current_stakers_testable
abi87 Oct 19, 2023
22e9e57
Merge branch 'dev' into make_write_current_stakers_testable
abi87 Oct 23, 2023
47a0f8a
Merge branch 'dev' into make_write_current_stakers_testable
abi87 Oct 25, 2023
fd774e9
Merge branch 'dev' into make_write_current_stakers_testable
abi87 Oct 26, 2023
6723f87
Merge branch 'dev' into make_write_current_stakers_testable
abi87 Oct 29, 2023
1ce6610
Merge branch 'dev' into make_write_current_stakers_testable
abi87 Nov 2, 2023
348bbcd
Merge branch 'dev' into make_write_current_stakers_testable
abi87 Nov 3, 2023
38d21b1
Merge branch 'dev' into make_write_current_stakers_testable
abi87 Nov 4, 2023
a159219
Merge branch 'dev' into make_write_current_stakers_testable
abi87 Nov 6, 2023
5af4b8c
Merge branch 'dev' into make_write_current_stakers_testable
abi87 Nov 6, 2023
5289800
Merge branch 'dev' into make_write_current_stakers_testable
abi87 Nov 6, 2023
824c73d
Merge branch 'dev' into make_write_current_stakers_testable
abi87 Nov 8, 2023
189760b
Merge branch 'dev' into make_write_current_stakers_testable
abi87 Nov 9, 2023
a894112
Merge branch 'dev' into make_write_current_stakers_testable
abi87 Nov 9, 2023
eec2f0a
Merge branch 'dev' into make_write_current_stakers_testable
abi87 Nov 9, 2023
df3f29d
Merge branch 'dev' into make_write_current_stakers_testable
abi87 Nov 13, 2023
5127e47
Merge branch 'dev' into make_write_current_stakers_testable
abi87 Nov 14, 2023
5c737ad
specialized naming of some structs
abi87 Nov 14, 2023
c1f43fd
Merge branch 'dev' into make_write_current_stakers_testable
abi87 Nov 15, 2023
847818f
Merge branch 'dev' into make_write_current_stakers_testable
abi87 Nov 16, 2023
a3fa4a5
Merge branch 'dev' into make_write_current_stakers_testable
abi87 Nov 16, 2023
9db4ce5
nits
abi87 Nov 16, 2023
cbc4b78
fix
abi87 Nov 16, 2023
18c9c05
Merge branch 'dev' into make_write_current_stakers_testable
abi87 Nov 17, 2023
3c73950
Merge branch 'dev' into make_write_current_stakers_testable
abi87 Nov 18, 2023
b837541
Merge branch 'dev' into make_write_current_stakers_testable
abi87 Nov 21, 2023
f99dd6a
Merge branch 'dev' into make_write_current_stakers_testable
abi87 Nov 22, 2023
9a2f2c7
Merge branch 'dev' into make_write_current_stakers_testable
abi87 Nov 23, 2023
789670e
Merge branch 'dev' into make_write_current_stakers_testable
abi87 Nov 27, 2023
1731817
Merge branch 'dev' into make_write_current_stakers_testable
abi87 Nov 28, 2023
9fdd336
Merge branch 'dev' into make_write_current_stakers_testable
abi87 Nov 29, 2023
f528832
Merge branch 'dev' into make_write_current_stakers_testable
abi87 Nov 30, 2023
bee3ea3
Merge branch 'dev' into make_write_current_stakers_testable
abi87 Dec 1, 2023
4e915af
Merge branch 'dev' into make_write_current_stakers_testable
abi87 Dec 3, 2023
04cb730
Merge branch 'dev' into make_write_current_stakers_testable
abi87 Dec 4, 2023
7b0fe01
Merge branch 'dev' into make_write_current_stakers_testable
abi87 Dec 5, 2023
3c1bc14
Merge branch 'dev' into make_write_current_stakers_testable
abi87 Dec 6, 2023
56d9aa3
Merge branch 'dev' into make_write_current_stakers_testable
abi87 Dec 7, 2023
cb97bd7
Merge branch 'dev' into make_write_current_stakers_testable
abi87 Dec 8, 2023
7f666ed
Merge branch 'dev' into make_write_current_stakers_testable
abi87 Dec 9, 2023
5dd86b9
Merge branch 'dev' into make_write_current_stakers_testable
abi87 Dec 11, 2023
38a2e45
Merge branch 'dev' into make_write_current_stakers_testable
abi87 Dec 12, 2023
75a74da
Merge branch 'dev' into make_write_current_stakers_testable
abi87 Dec 13, 2023
6563d8b
Merge branch 'dev' into make_write_current_stakers_testable
abi87 Dec 14, 2023
fd8ca3f
Merge branch 'dev' into make_write_current_stakers_testable
abi87 Dec 15, 2023
e622e22
Merge branch 'dev' into make_write_current_stakers_testable
abi87 Dec 18, 2023
22f1462
Merge branch 'dev' into make_write_current_stakers_testable
abi87 Dec 19, 2023
c35754a
Merge branch 'dev' into make_write_current_stakers_testable
abi87 Dec 19, 2023
6794aa4
Merge branch 'dev' into make_write_current_stakers_testable
abi87 Dec 20, 2023
ed0f398
Merge branch 'dev' into make_write_current_stakers_testable
abi87 Jan 3, 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
240 changes: 158 additions & 82 deletions vms/platformvm/state/state.go
Original file line number Diff line number Diff line change
Expand Up @@ -1672,10 +1672,17 @@ func (s *state) initValidatorSets() error {
}

func (s *state) write(updateValidators bool, height uint64) error {
weightDiffs, blsKeyDiffs, err := s.processCurrentStakers()
if err != nil {
return err
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is the part where stakers-related content is scanned to extract specific information to be stored in different parts of the pchain state


errs := wrappers.Errs{}
errs.Add(
s.writeBlocks(),
s.writeCurrentStakers(updateValidators, height),
s.writeCurrentStakers(updateValidators),
s.writeWeightDiffs(height, weightDiffs),
s.writeBlsKeyDiffs(height, blsKeyDiffs),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

these are some of methods where we only write the information collected in processCurrentStakers

s.writePendingStakers(),
s.WriteValidatorMetadata(s.currentValidatorList, s.currentSubnetValidatorList), // Must be called after writeCurrentStakers
s.writeTXs(),
Expand All @@ -1690,6 +1697,155 @@ func (s *state) write(updateValidators bool, height uint64) error {
return errs.Err
}

func (s *state) writeWeightDiffs(height uint64, weightDiffs map[subnetNodePair]*ValidatorWeightDiff) error {
for k, weightDiff := range weightDiffs {
if weightDiff.Amount == 0 {
continue
}

err := s.flatValidatorWeightDiffsDB.Put(
marshalDiffKey(k.subnetID, height, k.nodeID),
marshalWeightDiff(weightDiff),
)
if err != nil {
return err
}

// TODO: Remove this once we no longer support version rollbacks.
prefixStruct := heightWithSubnet{
Height: height,
SubnetID: k.subnetID,
}
prefixBytes, err := block.GenesisCodec.Marshal(block.Version, prefixStruct)
if err != nil {
return fmt.Errorf("failed to create prefix bytes: %w", err)
}
rawNestedWeightDiffDB := prefixdb.New(prefixBytes, s.nestedValidatorWeightDiffsDB)
nestedWeightDiffDB := linkeddb.NewDefault(rawNestedWeightDiffDB)

weightDiffBytes, err := block.GenesisCodec.Marshal(block.Version, weightDiff)
if err != nil {
return fmt.Errorf("failed to serialize validator weight diff: %w", err)
}
if err := nestedWeightDiffDB.Put(k.nodeID[:], weightDiffBytes); err != nil {
return err
}
}
return nil
}

func (s *state) writeBlsKeyDiffs(height uint64, blsKeyDiffs map[ids.NodeID]*bls.PublicKey) error {
for nodeID, blsKey := range blsKeyDiffs {
key := marshalDiffKey(constants.PrimaryNetworkID, height, nodeID)
blsKeyBytes := []byte{}

Choose a reason for hiding this comment

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

Nit. Don't think this actually makes a difference, since we treat nil/[]byte{} values as being the same, but still.

Suggested change
blsKeyBytes := []byte{}
var blsKeyBytes []byte

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

if blsKey != nil {
// Note: in flatValidatorPublicKeyDiffsDB we store the
// uncompressed public key here as it is
// significantly more efficient to parse when applying diffs.
blsKeyBytes = blsKey.Serialize()
}
if err := s.flatValidatorPublicKeyDiffsDB.Put(key, blsKeyBytes); err != nil {
return fmt.Errorf("failed to add bls key diffs: %w", err)
}

// TODO: Remove this once we no longer support version rollbacks.
if blsKey != nil {
heightBytes := database.PackUInt64(height)
rawNestedPublicKeyDiffDB := prefixdb.New(heightBytes, s.nestedValidatorPublicKeyDiffsDB)
nestedPKDiffDB := linkeddb.NewDefault(rawNestedPublicKeyDiffDB)

Choose a reason for hiding this comment

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

We don't need to compute these every loop iteration

Copy link
Contributor Author

Choose a reason for hiding this comment

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

pulled out of the loop


// Note: We store the compressed public key here.
pkBytes := bls.PublicKeyToBytes(blsKey)
if err := nestedPKDiffDB.Put(nodeID[:], pkBytes); err != nil {
return err
}
}
}
return nil
}

type subnetNodePair struct {
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we can call this subnet validator? or just validator?

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 got plenty of both 😅. How about subnetNodeKey?

subnetID ids.ID
nodeID ids.NodeID
}

func (s *state) processCurrentStakers() (
map[subnetNodePair]*ValidatorWeightDiff,
map[ids.NodeID]*bls.PublicKey,
error,
) {
var (
outputWeights = make(map[subnetNodePair]*ValidatorWeightDiff)
outputBlsKey = make(map[ids.NodeID]*bls.PublicKey)
)

for subnetID, subnetValidatorDiffs := range s.currentStakers.validatorDiffs {
// for now, let writeCurrentStakers consume s.currentStakers.validatorDiffs
for nodeID, validatorDiff := range subnetValidatorDiffs {
weightKey := subnetNodePair{
subnetID: subnetID,
nodeID: nodeID,
}

// make sure there is an entry for delegators even in case
// there are no validators modified.
outputWeights[weightKey] = &ValidatorWeightDiff{
Decrease: validatorDiff.validatorStatus == deleted,
}

switch validatorDiff.validatorStatus {
case added:
var (
weight = validatorDiff.validator.Weight
blkKey = validatorDiff.validator.PublicKey

Choose a reason for hiding this comment

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

is this supposed to be blsKey?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

)

outputWeights[weightKey].Amount = weight

if blkKey != nil {
// Record that the public key for the validator is being
// added. This means the prior value for the public key was
// nil.
outputBlsKey[nodeID] = nil
}

case deleted:
var (
weight = validatorDiff.validator.Weight
blkKey = validatorDiff.validator.PublicKey
)
outputWeights[weightKey].Amount = weight
if blkKey != nil {
// Record that the public key for the validator is being
// removed. This means we must record the prior value of the
// public key.
outputBlsKey[nodeID] = blkKey
}

default:
// nothing to do
}

addedDelegatorIterator := NewTreeIterator(validatorDiff.addedDelegators)
defer addedDelegatorIterator.Release()
for addedDelegatorIterator.Next() {
staker := addedDelegatorIterator.Value()

if err := outputWeights[weightKey].Add(false, staker.Weight); err != nil {

Choose a reason for hiding this comment

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

Suggested change
if err := outputWeights[weightKey].Add(false, staker.Weight); err != nil {
if err := outputWeights[weightKey].Add(false /*negative*/, staker.Weight); err != nil {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

return nil, nil, fmt.Errorf("failed to increase node weight diff: %w", err)
}
}

for _, staker := range validatorDiff.deletedDelegators {
if err := outputWeights[weightKey].Add(true, staker.Weight); err != nil {

Choose a reason for hiding this comment

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

Suggested change
if err := outputWeights[weightKey].Add(true, staker.Weight); err != nil {
if err := outputWeights[weightKey].Add(true /*negative*/, staker.Weight); err != nil {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

return nil, nil, fmt.Errorf("failed to decrease node weight diff: %w", err)
}
}
}
}
return outputWeights, outputBlsKey, nil
}

func (s *state) Close() error {
errs := wrappers.Errs{}
errs.Add(
Expand Down Expand Up @@ -1905,11 +2061,7 @@ func (s *state) GetBlockIDAtHeight(height uint64) (ids.ID, error) {
return blkID, nil
}

func (s *state) writeCurrentStakers(updateValidators bool, height uint64) error {
heightBytes := database.PackUInt64(height)
rawNestedPublicKeyDiffDB := prefixdb.New(heightBytes, s.nestedValidatorPublicKeyDiffsDB)
nestedPKDiffDB := linkeddb.NewDefault(rawNestedPublicKeyDiffDB)

func (s *state) writeCurrentStakers(updateValidators bool) error {
for subnetID, validatorDiffs := range s.currentStakers.validatorDiffs {
Copy link
Contributor

Choose a reason for hiding this comment

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

  1. I feel like now we shouldn't be using s.currentStakers and instead be using the returned computed diffs from processCurrentStakers()
  2. Do we still need this function? Looks like we're applying the weight + key diffs in the other two new functions we're adding

Copy link
Contributor Author

@abi87 abi87 Nov 14, 2023

Choose a reason for hiding this comment

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

As you suggest, the plan is to split writeCurrentStakers into two parts:

  • one where we pull out info to store
  • another where we store it.

The "store it" component won't need to access s.currentStakers; it will only access pulled info, as you suggest.
I just splitted this whole process into three PRs to simplify diffs checking and this is just first one.
See #2079 and #2086 for the final result

delete(s.currentStakers.validatorDiffs, subnetID)

Expand All @@ -1921,17 +2073,6 @@ func (s *state) writeCurrentStakers(updateValidators bool, height uint64) error
delegatorDB = s.currentDelegatorList
}

prefixStruct := heightWithSubnet{
Height: height,
SubnetID: subnetID,
}
prefixBytes, err := block.GenesisCodec.Marshal(block.Version, prefixStruct)
if err != nil {
return fmt.Errorf("failed to create prefix bytes: %w", err)
}
rawNestedWeightDiffDB := prefixdb.New(prefixBytes, s.nestedValidatorWeightDiffsDB)
nestedWeightDiffDB := linkeddb.NewDefault(rawNestedWeightDiffDB)

// Record the change in weight and/or public key for each validator.
for nodeID, validatorDiff := range validatorDiffs {
// Copy [nodeID] so it doesn't get overwritten next iteration.
Expand All @@ -1945,21 +2086,6 @@ func (s *state) writeCurrentStakers(updateValidators bool, height uint64) error
staker := validatorDiff.validator
weightDiff.Amount = staker.Weight

// Invariant: Only the Primary Network contains non-nil public
// keys.
if staker.PublicKey != nil {
// Record that the public key for the validator is being
// added. This means the prior value for the public key was
// nil.
err := s.flatValidatorPublicKeyDiffsDB.Put(
marshalDiffKey(constants.PrimaryNetworkID, height, nodeID),
nil,
)
if err != nil {
return err
}
}

// The validator is being added.
//
// Invariant: It's impossible for a delegator to have been
Expand Down Expand Up @@ -1988,34 +2114,6 @@ func (s *state) writeCurrentStakers(updateValidators bool, height uint64) error
staker := validatorDiff.validator
weightDiff.Amount = staker.Weight

// Invariant: Only the Primary Network contains non-nil public
// keys.
if staker.PublicKey != nil {
// Record that the public key for the validator is being
// removed. This means we must record the prior value of the
// public key.
//
// Note: We store the uncompressed public key here as it is
// significantly more efficient to parse when applying
// diffs.
err := s.flatValidatorPublicKeyDiffsDB.Put(
marshalDiffKey(constants.PrimaryNetworkID, height, nodeID),
staker.PublicKey.Serialize(),
)
if err != nil {
return err
}

// TODO: Remove this once we no longer support version
// rollbacks.
//
// Note: We store the compressed public key here.
pkBytes := bls.PublicKeyToBytes(staker.PublicKey)
if err := nestedPKDiffDB.Put(nodeID[:], pkBytes); err != nil {
return err
}
}

if err := validatorDB.Delete(staker.TxID[:]); err != nil {
return fmt.Errorf("failed to delete current staker: %w", err)
}
Expand All @@ -2032,28 +2130,6 @@ func (s *state) writeCurrentStakers(updateValidators bool, height uint64) error
return err
}

if weightDiff.Amount == 0 {
// No weight change to record; go to next validator.
Copy link

@danlaine danlaine Nov 16, 2023

Choose a reason for hiding this comment

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

Isn't this a behavior change? In the existing code, we continue here if the weight didn't change. In the new code, we call one of s.cfg.Validators.RemoveWeight, s.cfg.Validators.AddStaker or s.cfg.Validators.AddWeight

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@danlaine, you're right. I moved this condition to writeWeightDiffs but it should have been kept here too since it guards the cfg.Validators set update.
Rewritten the if/else as a switch as the linter suggest (and you do too next PR)

continue
}

err = s.flatValidatorWeightDiffsDB.Put(
marshalDiffKey(subnetID, height, nodeID),
marshalWeightDiff(weightDiff),
)
if err != nil {
return err
}

// TODO: Remove this once we no longer support version rollbacks.
weightDiffBytes, err := block.GenesisCodec.Marshal(block.Version, weightDiff)
if err != nil {
return fmt.Errorf("failed to serialize validator weight diff: %w", err)
}
if err := nestedWeightDiffDB.Put(nodeID[:], weightDiffBytes); err != nil {
return err
}

// TODO: Move the validator set management out of the state package
if !updateValidators {
continue
Expand Down
Loading