Skip to content

Fix delegator state inconsistency on put/delete pairs #5331

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.

Sign up for GitHub

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

Open
DracoLi wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Fix delegator state inconsistency on put/delete pairs #5331

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9dbf1bd
fix(platformvm): cancel out matching delegator put/delete pairs
DracoLi May 10, 2026
a0ba33e
remove unneeded releases
DracoLi May 11, 2026
e07df27
chore: inline verify
DracoLi May 19, 2026
80b421a
fix(platformvm): preserve fields on delegator re-add
DracoLi May 23, 2026
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 vms/platformvm/state/BUILD.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ go_test(
"//cache/lru",
"//codec",
"//database",
"//database/linkeddb",
"//database/memdb",
"//genesis",
"//ids",
Expand Down
4 changes: 4 additions & 0 deletions vms/platformvm/state/diff_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -464,6 +464,7 @@ func TestDiffCurrentDelegator(t *testing.T) {
// The iterator should have the 1 delegator we put in [d]
require.True(gotCurrentDelegatorIter.Next())
require.Equal(gotCurrentDelegatorIter.Value(), currentDelegator)
gotCurrentDelegatorIter.Release()

// Delete the current delegator
require.NoError(d.DeleteCurrentDelegator(currentDelegator))
Expand All @@ -472,6 +473,7 @@ func TestDiffCurrentDelegator(t *testing.T) {
// The iterator should have no elements.
gotCurrentDelegatorIter, err = d.GetCurrentDelegatorIterator(currentDelegator.SubnetID, currentDelegator.NodeID)
require.NoError(err)
defer gotCurrentDelegatorIter.Release()
require.False(gotCurrentDelegatorIter.Next())
}

Expand All @@ -498,6 +500,7 @@ func TestDiffPendingDelegator(t *testing.T) {
// The iterator should have the 1 delegator we put in [d]
require.True(gotPendingDelegatorIter.Next())
require.Equal(gotPendingDelegatorIter.Value(), pendingDelegator)
gotPendingDelegatorIter.Release()

// Delete the pending delegator
d.DeletePendingDelegator(pendingDelegator)
Expand All @@ -506,6 +509,7 @@ func TestDiffPendingDelegator(t *testing.T) {
// The iterator should have no elements.
gotPendingDelegatorIter, err = d.GetPendingDelegatorIterator(pendingDelegator.SubnetID, pendingDelegator.NodeID)
require.NoError(err)
defer gotPendingDelegatorIter.Release()
require.False(gotPendingDelegatorIter.Next())
}

Expand Down
38 changes: 35 additions & 3 deletions vms/platformvm/state/stakers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,10 +203,16 @@ func (v *baseStakers) PutDelegator(staker *Staker) {
validator.delegators.ReplaceOrInsert(staker)

validatorDiff := v.getOrCreateValidatorDiff(staker.SubnetID, staker.NodeID)
if validatorDiff.addedDelegators == nil {
validatorDiff.addedDelegators = btree.NewG(defaultTreeDegree, (*Staker).Less)
// Cancel a prior delete only when the re-added staker is identical.
// A mismatch (e.g. different weight) is recorded as a replacement.
if existing, ok := validatorDiff.deletedDelegators[staker.TxID]; ok && existing.Equals(staker) {
Copy link
Copy Markdown
Contributor

@yacovm yacovm May 24, 2026

Choose a reason for hiding this comment

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

when I revert this change, the tests still pass 🫠

delete(validatorDiff.deletedDelegators, staker.TxID)
} else {
if validatorDiff.addedDelegators == nil {
validatorDiff.addedDelegators = btree.NewG(defaultTreeDegree, (*Staker).Less)
}
validatorDiff.addedDelegators.ReplaceOrInsert(staker)
}
validatorDiff.addedDelegators.ReplaceOrInsert(staker)

v.stakers.ReplaceOrInsert(staker)
}
Expand All @@ -219,6 +225,14 @@ func (v *baseStakers) DeleteDelegator(staker *Staker) {
v.pruneValidator(staker.SubnetID, staker.NodeID)

validatorDiff := v.getOrCreateValidatorDiff(staker.SubnetID, staker.NodeID)
// An add followed by a delete of the same staker is a no-op.
// Undo the addition instead of recording a delete.
if validatorDiff.addedDelegators != nil {
if _, ok := validatorDiff.addedDelegators.Delete(staker); ok {
v.stakers.Delete(staker)
return
}
}
if validatorDiff.deletedDelegators == nil {
validatorDiff.deletedDelegators = make(map[ids.ID]*Staker)
}
Expand Down Expand Up @@ -449,6 +463,15 @@ func (s *diffStakers) GetDelegatorIterator(

func (s *diffStakers) PutDelegator(staker *Staker) {
validatorDiff := s.getOrCreateDiff(staker.SubnetID, staker.NodeID)
// Cancel a prior delete only when the re-added staker is identical.
// A mismatch (e.g. different weight) is recorded as a replacement
// rather than silently dropping the updated fields.
if existing, ok := validatorDiff.deletedDelegators[staker.TxID]; ok && existing.Equals(staker) {
delete(validatorDiff.deletedDelegators, staker.TxID)
delete(s.deletedStakers, staker.TxID)
return
}

if validatorDiff.addedDelegators == nil {
validatorDiff.addedDelegators = btree.NewG(defaultTreeDegree, (*Staker).Less)
}
Expand All @@ -462,6 +485,15 @@ func (s *diffStakers) PutDelegator(staker *Staker) {

func (s *diffStakers) DeleteDelegator(staker *Staker) {
validatorDiff := s.getOrCreateDiff(staker.SubnetID, staker.NodeID)
// An add followed by a delete of the same staker is a no-op on the diff.
// Undo the addition instead of recording a delete.
if validatorDiff.addedDelegators != nil {
if _, ok := validatorDiff.addedDelegators.Delete(staker); ok {
s.addedStakers.Delete(staker)
return
}
}

if validatorDiff.deletedDelegators == nil {
validatorDiff.deletedDelegators = make(map[ids.ID]*Staker)
}
Expand Down
146 changes: 146 additions & 0 deletions vms/platformvm/state/stakers_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ import (
"github.com/stretchr/testify/require"

"github.com/ava-labs/avalanchego/database"
"github.com/ava-labs/avalanchego/database/linkeddb"
"github.com/ava-labs/avalanchego/database/memdb"
"github.com/ava-labs/avalanchego/ids"
"github.com/ava-labs/avalanchego/utils/constants"
"github.com/ava-labs/avalanchego/utils/crypto/bls/signer/localsigner"
Expand Down Expand Up @@ -416,6 +418,150 @@ func TestDiffStakersDelegator(t *testing.T) {
)
}

func TestDiffDelegatorCancelOut(t *testing.T) {
setupState := func(t *testing.T) (*State, *Staker) {
t.Helper()
state := newTestState(t, memdb.New())
subnetID := ids.GenerateTestID()
nodeID := ids.GenerateTestNodeID()
require.NoError(t, state.PutCurrentValidator(newTestStaker(subnetID, nodeID)))
return state, newTestStaker(subnetID, nodeID)
}

currentDelegators := func(t *testing.T, state *State, d *Staker) []*Staker {
t.Helper()
iter, err := state.GetCurrentDelegatorIterator(d.SubnetID, d.NodeID)
require.NoError(t, err)
defer iter.Release()
return iterator.ToSlice(iter)
}

t.Run("delete then put identical delegator leaves state unchanged", func(t *testing.T) {
state, delegator := setupState(t)
require.NoError(t, state.PutCurrentDelegator(delegator))

diff, err := NewDiffOn(state, StakerAdditionAfterDeletionAllowed)
require.NoError(t, err)
require.NoError(t, diff.DeleteCurrentDelegator(delegator))
require.NoError(t, diff.PutCurrentDelegator(delegator))
require.NoError(t, diff.Apply(state))

require.Equal(t, []*Staker{delegator}, currentDelegators(t, state, delegator))
})

t.Run("delete then put with updated weight applies the replacement", func(t *testing.T) {
state, delegator := setupState(t)
delegator.Weight = 2
require.NoError(t, state.PutCurrentDelegator(delegator))

updated := *delegator
updated.Weight = delegator.Weight - 1

diff, err := NewDiffOn(state, StakerAdditionAfterDeletionAllowed)
require.NoError(t, err)
require.NoError(t, diff.DeleteCurrentDelegator(delegator))
require.NoError(t, diff.PutCurrentDelegator(&updated))
require.NoError(t, diff.Apply(state))

require.Equal(t, []*Staker{&updated}, currentDelegators(t, state, delegator))
})

t.Run("put then delete identical delegator leaves state empty", func(t *testing.T) {
state, delegator := setupState(t)

diff, err := NewDiffOn(state, StakerAdditionAfterDeletionAllowed)
require.NoError(t, err)
require.NoError(t, diff.PutCurrentDelegator(delegator))
require.NoError(t, diff.DeleteCurrentDelegator(delegator))
require.NoError(t, diff.Apply(state))

require.Empty(t, currentDelegators(t, state, delegator))
})

t.Run("put then delete with mismatched weight leaves state empty", func(t *testing.T) {
state, delegator := setupState(t)
delegator.Weight = 2

mismatched := *delegator
mismatched.Weight = delegator.Weight - 1

diff, err := NewDiffOn(state, StakerAdditionAfterDeletionAllowed)
require.NoError(t, err)
require.NoError(t, diff.PutCurrentDelegator(delegator))
require.NoError(t, diff.DeleteCurrentDelegator(&mismatched))
require.NoError(t, diff.Apply(state))

require.Empty(t, currentDelegators(t, state, delegator))
})

t.Run("pending: delete then put identical delegator leaves state unchanged", func(t *testing.T) {
state, delegator := setupState(t)
state.PutPendingDelegator(delegator)

diff, err := NewDiffOn(state, StakerAdditionAfterDeletionAllowed)
require.NoError(t, err)
diff.DeletePendingDelegator(delegator)
diff.PutPendingDelegator(delegator)
require.NoError(t, diff.Apply(state))

iter, err := state.GetPendingDelegatorIterator(delegator.SubnetID, delegator.NodeID)
require.NoError(t, err)
defer iter.Release()
require.Equal(t, []*Staker{delegator}, iterator.ToSlice(iter))
})
}

func TestBaseStakersDelegatorCancelOutPersistence(t *testing.T) {
delegator := newTestStaker(constants.PrimaryNetworkID, ids.GenerateTestNodeID())

writeDiff := func(t *testing.T, v *baseStakers, list linkeddb.LinkedDB) {
t.Helper()
diff := v.validatorDiffs[delegator.SubnetID][delegator.NodeID]
require.NotNil(t, diff)
require.NoError(t, writeCurrentDelegatorDiff(list, diff, CodecVersion1))
delete(v.validatorDiffs[delegator.SubnetID], delegator.NodeID)
}

t.Run("delete then put identical persisted delegator", func(t *testing.T) {
list := linkeddb.NewDefault(memdb.New())
v := newBaseStakers()

v.PutDelegator(delegator)
writeDiff(t, v, list)

has, err := list.Has(delegator.TxID[:])
require.NoError(t, err)
require.True(t, has)

// Delete then re-add. The cancel-out must keep the DB entry.
v.DeleteDelegator(delegator)
v.PutDelegator(delegator)
writeDiff(t, v, list)

has, err = list.Has(delegator.TxID[:])
require.NoError(t, err)
require.True(t, has)
})

t.Run("put then delete identical delegator never persists", func(t *testing.T) {
list := linkeddb.NewDefault(memdb.New())
v := newBaseStakers()

// Put then delete in the same diff cycle. The cancel-out must not
// write the delegator to the DB.
v.PutDelegator(delegator)
v.DeleteDelegator(delegator)
writeDiff(t, v, list)

has, err := list.Has(delegator.TxID[:])
require.NoError(t, err)
require.False(t, has)

iter := v.GetDelegatorIterator(delegator.SubnetID, delegator.NodeID)
require.Empty(t, iterator.ToSlice(iter))
})
}

func newTestStaker(subnetID ids.ID, nodeID ids.NodeID) *Staker {
startTime := time.Time{}
endTime := startTime.Add(genesistest.DefaultValidatorDuration)
Expand Down
Loading