Skip to content

Commit

Permalink
Fix RemoveSubnetValidatorTx Weight Diff Calculation (ava-labs#2510)
Browse files Browse the repository at this point in the history
  • Loading branch information
@StephenButtolph
StephenButtolph authored Jan 23, 2023
1 parent b7b31a0 commit 4c368b1
Show file tree
Hide file tree
Showing 7 changed files with 442 additions and 87 deletions.
22 changes: 10 additions & 12 deletions vms/platformvm/state/diff.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -439,12 +439,11 @@ func (d *diff) Apply(baseState State) {
}
for _, subnetValidatorDiffs := range d.currentStakerDiffs.validatorDiffs {
for _, validatorDiff := range subnetValidatorDiffs {
if validatorDiff.validatorModified {
if validatorDiff.validatorDeleted {
baseState.DeleteCurrentValidator(validatorDiff.validator)
} else {
baseState.PutCurrentValidator(validatorDiff.validator)
}
if validatorDiff.validatorAdded {
baseState.PutCurrentValidator(validatorDiff.validator)
}
if validatorDiff.validatorDeleted {
baseState.DeleteCurrentValidator(validatorDiff.validator)
}

addedDelegatorIterator := NewTreeIterator(validatorDiff.addedDelegators)
Expand All @@ -460,12 +459,11 @@ func (d *diff) Apply(baseState State) {
}
for _, subnetValidatorDiffs := range d.pendingStakerDiffs.validatorDiffs {
for _, validatorDiff := range subnetValidatorDiffs {
if validatorDiff.validatorModified {
if validatorDiff.validatorDeleted {
baseState.DeletePendingValidator(validatorDiff.validator)
} else {
baseState.PutPendingValidator(validatorDiff.validator)
}
if validatorDiff.validatorAdded {
baseState.PutPendingValidator(validatorDiff.validator)
}
if validatorDiff.validatorDeleted {
baseState.DeletePendingValidator(validatorDiff.validator)
}

addedDelegatorIterator := NewTreeIterator(validatorDiff.addedDelegators)
Expand Down
2 changes: 2 additions & 0 deletions vms/platformvm/state/diff_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ func TestDiffCurrentValidator(t *testing.T) {
d.DeleteCurrentValidator(currentValidator)

// Make sure the deletion worked
state.EXPECT().GetCurrentValidator(currentValidator.SubnetID, currentValidator.NodeID).Return(nil, database.ErrNotFound).Times(1)
_, err = d.GetCurrentValidator(currentValidator.SubnetID, currentValidator.NodeID)
require.ErrorIs(err, database.ErrNotFound)
}
Expand Down Expand Up @@ -146,6 +147,7 @@ func TestDiffPendingValidator(t *testing.T) {
d.DeletePendingValidator(pendingValidator)

// Make sure the deletion worked
state.EXPECT().GetPendingValidator(pendingValidator.SubnetID, pendingValidator.NodeID).Return(nil, database.ErrNotFound).Times(1)
_, err = d.GetPendingValidator(pendingValidator.SubnetID, pendingValidator.NodeID)
require.ErrorIs(err, database.ErrNotFound)
}
Expand Down
44 changes: 24 additions & 20 deletions vms/platformvm/state/stakers.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -128,8 +128,7 @@ func (v *baseStakers) PutValidator(staker *Staker) {
validator.validator = staker

validatorDiff := v.getOrCreateValidatorDiff(staker.SubnetID, staker.NodeID)
validatorDiff.validatorModified = true
validatorDiff.validatorDeleted = false
validatorDiff.validatorAdded = true
validatorDiff.validator = staker

v.stakers.ReplaceOrInsert(staker)
Expand All @@ -141,7 +140,6 @@ func (v *baseStakers) DeleteValidator(staker *Staker) {
v.pruneValidator(staker.SubnetID, staker.NodeID)

validatorDiff := v.getOrCreateValidatorDiff(staker.SubnetID, staker.NodeID)
validatorDiff.validatorModified = true
validatorDiff.validatorDeleted = true
validatorDiff.validator = staker

Expand Down Expand Up @@ -249,8 +247,9 @@ type diffStakers struct {
}

type diffValidator struct {
validatorModified bool
// [validatorDeleted] implies [validatorModified]
// Invariant: [validatorAdded] and [validatorDeleted] will not be set at the
// same time.
validatorAdded bool
validatorDeleted bool
validator *Staker

Expand All @@ -266,6 +265,8 @@ type diffValidator struct {
// 2. If the validator was removed in this diff, [nil, true] will be returned.
// 3. If the validator was not modified by this diff, [nil, false] will be
// returned.
//
// Invariant: Assumes that the validator will never be removed and then added.
func (s *diffStakers) GetValidator(subnetID ids.ID, nodeID ids.NodeID) (*Staker, bool) {
subnetValidatorDiffs, ok := s.validatorDiffs[subnetID]
if !ok {
Expand All @@ -277,20 +278,19 @@ func (s *diffStakers) GetValidator(subnetID ids.ID, nodeID ids.NodeID) (*Staker,
return nil, false
}

if !validatorDiff.validatorModified {
return nil, false
}

if validatorDiff.validatorDeleted {
switch {
case validatorDiff.validatorAdded:
return validatorDiff.validator, true
case validatorDiff.validatorDeleted:
return nil, true
default:
return nil, false
}
return validatorDiff.validator, true
}

func (s *diffStakers) PutValidator(staker *Staker) {
validatorDiff := s.getOrCreateDiff(staker.SubnetID, staker.NodeID)
validatorDiff.validatorModified = true
validatorDiff.validatorDeleted = false
validatorDiff.validatorAdded = true
validatorDiff.validator = staker

if s.addedStakers == nil {
Expand All @@ -301,14 +301,18 @@ func (s *diffStakers) PutValidator(staker *Staker) {

func (s *diffStakers) DeleteValidator(staker *Staker) {
validatorDiff := s.getOrCreateDiff(staker.SubnetID, staker.NodeID)
validatorDiff.validatorModified = true
validatorDiff.validatorDeleted = true
validatorDiff.validator = staker

if s.deletedStakers == nil {
s.deletedStakers = make(map[ids.ID]*Staker)
if validatorDiff.validatorAdded {
validatorDiff.validatorAdded = false
s.addedStakers.Delete(validatorDiff.validator)
validatorDiff.validator = nil
} else {
validatorDiff.validatorDeleted = true
validatorDiff.validator = staker
if s.deletedStakers == nil {
s.deletedStakers = make(map[ids.ID]*Staker)
}
s.deletedStakers[staker.TxID] = staker
}
s.deletedStakers[staker.TxID] = staker
}

func (s *diffStakers) GetDelegatorIterator(
Expand Down
22 changes: 19 additions & 3 deletions vms/platformvm/state/stakers_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -166,14 +166,30 @@ func TestDiffStakersValidator(t *testing.T) {

v.DeleteValidator(staker)

returnedStaker, ok = v.GetValidator(staker.SubnetID, staker.NodeID)
require.True(ok)
require.Nil(returnedStaker)
_, ok = v.GetValidator(staker.SubnetID, staker.NodeID)
require.False(ok)

stakerIterator = v.GetStakerIterator(EmptyIterator)
assertIteratorsEqual(t, NewSliceIterator(delegator), stakerIterator)
}

func TestDiffStakersDeleteValidator(t *testing.T) {
require := require.New(t)
staker := newTestStaker()
delegator := newTestStaker()

v := diffStakers{}

_, ok := v.GetValidator(ids.GenerateTestID(), delegator.NodeID)
require.False(ok)

v.DeleteValidator(staker)

returnedStaker, ok := v.GetValidator(staker.SubnetID, staker.NodeID)
require.True(ok)
require.Nil(returnedStaker)
}

func TestDiffStakersDelegator(t *testing.T) {
staker := newTestStaker()
delegator := newTestStaker()
Expand Down
100 changes: 48 additions & 52 deletions vms/platformvm/state/state.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1592,58 +1592,55 @@ func (s *state) writeCurrentStakers(updateValidators bool, height uint64) error
// Copy [nodeID] so it doesn't get overwritten next iteration.
nodeID := nodeID

var (
weightDiff = &ValidatorWeightDiff{}
isNewValidator bool
)
if validatorDiff.validatorModified {
// This validator is being added or removed.
weightDiff := &ValidatorWeightDiff{
Decrease: validatorDiff.validatorDeleted,
}
switch {
case validatorDiff.validatorAdded:
staker := validatorDiff.validator

weightDiff.Decrease = validatorDiff.validatorDeleted
weightDiff.Amount = staker.Weight

if validatorDiff.validatorDeleted {
// Invariant: Only the Primary Network contains non-nil
// public keys.
if staker.PublicKey != nil {
// Record the public key of the validator being removed.
pkDiffs[nodeID] = staker.PublicKey

pkBytes := bls.PublicKeyToBytes(staker.PublicKey)
if err := pkDiffDB.Put(nodeID[:], pkBytes); err != nil {
return err
}
}
// The validator is being added.
vdr := &uptimeAndReward{
txID: staker.TxID,
lastUpdated: staker.StartTime,

if err := validatorDB.Delete(staker.TxID[:]); err != nil {
return fmt.Errorf("failed to delete current staker: %w", err)
}
UpDuration: 0,
LastUpdated: uint64(staker.StartTime.Unix()),
PotentialReward: staker.PotentialReward,
}

s.validatorUptimes.DeleteUptime(nodeID, subnetID)
} else {
// The validator is being added.
vdr := &uptimeAndReward{
txID: staker.TxID,
lastUpdated: staker.StartTime,

UpDuration: 0,
LastUpdated: uint64(staker.StartTime.Unix()),
PotentialReward: staker.PotentialReward,
}
vdrBytes, err := blocks.GenesisCodec.Marshal(blocks.Version, vdr)
if err != nil {
return fmt.Errorf("failed to serialize current validator: %w", err)
}

vdrBytes, err := blocks.GenesisCodec.Marshal(blocks.Version, vdr)
if err != nil {
return fmt.Errorf("failed to serialize current validator: %w", err)
}
if err = validatorDB.Put(staker.TxID[:], vdrBytes); err != nil {
return fmt.Errorf("failed to write current validator to list: %w", err)
}

if err = validatorDB.Put(staker.TxID[:], vdrBytes); err != nil {
return fmt.Errorf("failed to write current validator to list: %w", err)
s.validatorUptimes.LoadUptime(nodeID, subnetID, vdr)
case validatorDiff.validatorDeleted:
staker := validatorDiff.validator
weightDiff.Amount = staker.Weight

// Invariant: Only the Primary Network contains non-nil
// public keys.
if staker.PublicKey != nil {
// Record the public key of the validator being removed.
pkDiffs[nodeID] = staker.PublicKey

pkBytes := bls.PublicKeyToBytes(staker.PublicKey)
if err := pkDiffDB.Put(nodeID[:], pkBytes); err != nil {
return err
}
}

s.validatorUptimes.LoadUptime(nodeID, subnetID, vdr)
isNewValidator = true
if err := validatorDB.Delete(staker.TxID[:]); err != nil {
return fmt.Errorf("failed to delete current staker: %w", err)
}

s.validatorUptimes.DeleteUptime(nodeID, subnetID)
}

err := writeCurrentDelegatorDiff(
Expand Down Expand Up @@ -1683,7 +1680,7 @@ func (s *state) writeCurrentStakers(updateValidators bool, height uint64) error
if weightDiff.Decrease {
err = validators.RemoveWeight(s.cfg.Validators, subnetID, nodeID, weightDiff.Amount)
} else {
if isNewValidator {
if validatorDiff.validatorAdded {
staker := validatorDiff.validator
err = validators.Add(
s.cfg.Validators,
Expand Down Expand Up @@ -1781,17 +1778,16 @@ func writePendingDiff(
pendingDelegatorList linkeddb.LinkedDB,
validatorDiff *diffValidator,
) error {
if validatorDiff.validatorModified {
staker := validatorDiff.validator

var err error
if validatorDiff.validatorDeleted {
err = pendingValidatorList.Delete(staker.TxID[:])
} else {
err = pendingValidatorList.Put(staker.TxID[:], nil)
if validatorDiff.validatorAdded {
err := pendingValidatorList.Put(validatorDiff.validator.TxID[:], nil)
if err != nil {
return fmt.Errorf("failed to add pending validator: %w", err)
}
}
if validatorDiff.validatorDeleted {
err := pendingValidatorList.Delete(validatorDiff.validator.TxID[:])
if err != nil {
return fmt.Errorf("failed to update pending validator: %w", err)
return fmt.Errorf("failed to delete pending validator: %w", err)
}
}

Expand Down
Loading

0 comments on commit 4c368b1

Please sign in to comment.