Skip to content

Commit

Permalink
Remove stale expiries in AdvanceTimeTo (#3415)
Browse files Browse the repository at this point in the history
  • Loading branch information
@StephenButtolph
StephenButtolph authored Sep 25, 2024
1 parent 0a46687 commit 4680312
Show file tree
Hide file tree
Showing 4 changed files with 205 additions and 39 deletions.
26 changes: 12 additions & 14 deletions vms/platformvm/block/executor/proposal_block_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"github.com/ava-labs/avalanchego/utils/constants"
"github.com/ava-labs/avalanchego/utils/crypto/bls"
"github.com/ava-labs/avalanchego/utils/crypto/secp256k1"
"github.com/ava-labs/avalanchego/utils/iterator"
"github.com/ava-labs/avalanchego/utils/iterator/iteratormock"
"github.com/ava-labs/avalanchego/utils/timer/mockable"
"github.com/ava-labs/avalanchego/vms/components/avax"
Expand Down Expand Up @@ -205,24 +206,21 @@ func TestBanffProposalBlockTimeVerification(t *testing.T) {
nextStakerTxID := nextStakerTx.ID()
onParentAccept.EXPECT().GetTx(nextStakerTxID).Return(nextStakerTx, status.Processing, nil)

currentStakersIt := iteratormock.NewIterator[*state.Staker](ctrl)
currentStakersIt.EXPECT().Next().Return(true).AnyTimes()
currentStakersIt.EXPECT().Value().Return(&state.Staker{
TxID: nextStakerTxID,
EndTime: nextStakerTime,
NextTime: nextStakerTime,
Priority: txs.PrimaryNetworkValidatorCurrentPriority,
onParentAccept.EXPECT().GetCurrentStakerIterator().DoAndReturn(func() (iterator.Iterator[*state.Staker], error) {
return iterator.FromSlice(
&state.Staker{
TxID: nextStakerTxID,
EndTime: nextStakerTime,
NextTime: nextStakerTime,
Priority: txs.PrimaryNetworkValidatorCurrentPriority,
},
), nil
}).AnyTimes()
currentStakersIt.EXPECT().Release().AnyTimes()
onParentAccept.EXPECT().GetCurrentStakerIterator().Return(currentStakersIt, nil).AnyTimes()
onParentAccept.EXPECT().GetPendingStakerIterator().Return(iterator.Empty[*state.Staker]{}, nil).AnyTimes()
onParentAccept.EXPECT().GetExpiryIterator().Return(iterator.Empty[state.ExpiryEntry]{}, nil).AnyTimes()

onParentAccept.EXPECT().GetDelegateeReward(constants.PrimaryNetworkID, unsignedNextStakerTx.NodeID()).Return(uint64(0), nil).AnyTimes()

pendingStakersIt := iteratormock.NewIterator[*state.Staker](ctrl)
pendingStakersIt.EXPECT().Next().Return(false).AnyTimes() // no pending stakers
pendingStakersIt.EXPECT().Release().AnyTimes()
onParentAccept.EXPECT().GetPendingStakerIterator().Return(pendingStakersIt, nil).AnyTimes()

env.mockedState.EXPECT().GetUptime(gomock.Any).Return(
time.Microsecond, /*upDuration*/
time.Time{}, /*lastUpdated*/
Expand Down
27 changes: 12 additions & 15 deletions vms/platformvm/block/executor/standard_block_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import (
"github.com/ava-labs/avalanchego/upgrade/upgradetest"
"github.com/ava-labs/avalanchego/utils/constants"
"github.com/ava-labs/avalanchego/utils/crypto/secp256k1"
"github.com/ava-labs/avalanchego/utils/iterator/iteratormock"
"github.com/ava-labs/avalanchego/utils/iterator"
"github.com/ava-labs/avalanchego/vms/components/avax"
"github.com/ava-labs/avalanchego/vms/components/gas"
"github.com/ava-labs/avalanchego/vms/platformvm/block"
Expand Down Expand Up @@ -118,22 +118,19 @@ func TestBanffStandardBlockTimeVerification(t *testing.T) {
nextStakerTime := chainTime.Add(executor.SyncBound).Add(-1 * time.Second)

// store just once current staker to mark next staker time.
currentStakerIt := iteratormock.NewIterator[*state.Staker](ctrl)
currentStakerIt.EXPECT().Next().Return(true).AnyTimes()
currentStakerIt.EXPECT().Value().Return(
&state.Staker{
NextTime: nextStakerTime,
Priority: txs.PrimaryNetworkValidatorCurrentPriority,
},
).AnyTimes()
currentStakerIt.EXPECT().Release().Return().AnyTimes()
onParentAccept.EXPECT().GetCurrentStakerIterator().Return(currentStakerIt, nil).AnyTimes()
onParentAccept.EXPECT().GetCurrentStakerIterator().DoAndReturn(func() (iterator.Iterator[*state.Staker], error) {
return iterator.FromSlice(
&state.Staker{
NextTime: nextStakerTime,
Priority: txs.PrimaryNetworkValidatorCurrentPriority,
},
), nil
}).AnyTimes()

// no pending stakers
pendingIt := iteratormock.NewIterator[*state.Staker](ctrl)
pendingIt.EXPECT().Next().Return(false).AnyTimes()
pendingIt.EXPECT().Release().Return().AnyTimes()
onParentAccept.EXPECT().GetPendingStakerIterator().Return(pendingIt, nil).AnyTimes()
onParentAccept.EXPECT().GetPendingStakerIterator().Return(iterator.Empty[*state.Staker]{}, nil).AnyTimes()
// no expiries
onParentAccept.EXPECT().GetExpiryIterator().Return(iterator.Empty[state.ExpiryEntry]{}, nil).AnyTimes()

onParentAccept.EXPECT().GetTimestamp().Return(chainTime).AnyTimes()
onParentAccept.EXPECT().GetFeeState().Return(gas.State{}).AnyTimes()
Expand Down
76 changes: 66 additions & 10 deletions vms/platformvm/txs/executor/state_changes.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,12 +77,30 @@ func VerifyNewChainTime(

// AdvanceTimeTo applies all state changes to [parentState] resulting from
// advancing the chain time to [newChainTime].
//
// Returns true iff the validator set changed.
func AdvanceTimeTo(
backend *Backend,
parentState state.Chain,
newChainTime time.Time,
) (bool, error) {
diff, changed, err := advanceTimeTo(backend, parentState, newChainTime)
if err != nil {
return false, err
}
return changed, diff.Apply(parentState)
}

// advanceTimeTo returns the state diff on top of parentState resulting from
// advancing the chain time to newChainTime. It also returns a boolean
// indicating if the validator set changed.
//
// parentState is not modified.
func advanceTimeTo(
backend *Backend,
parentState state.Chain,
newChainTime time.Time,
) (state.Diff, bool, error) {
// We promote pending stakers to current stakers first and remove
// completed stakers from the current staker set. We assume that any
// promoted staker will not immediately be removed from the current staker
Expand All @@ -93,17 +111,21 @@ func AdvanceTimeTo(

changes, err := state.NewDiffOn(parentState)
if err != nil {
return false, err
return nil, false, err
}

// Promote any pending stakers to current if [StartTime] <= [newChainTime].
//
// Invariant: It is not safe to modify the state while iterating over it,
// so we use the parentState's iterator rather than the changes iterator.
// ParentState must not be modified before this iterator is released.
pendingStakerIterator, err := parentState.GetPendingStakerIterator()
if err != nil {
return false, err
return nil, false, err
}
defer pendingStakerIterator.Release()

var changed bool
// Promote any pending stakers to current if [StartTime] <= [newChainTime].
for pendingStakerIterator.Next() {
stakerToRemove := pendingStakerIterator.Value()
if stakerToRemove.StartTime.After(newChainTime) {
Expand All @@ -116,7 +138,7 @@ func AdvanceTimeTo(

if stakerToRemove.Priority == txs.SubnetPermissionedValidatorPendingPriority {
if err := changes.PutCurrentValidator(&stakerToAdd); err != nil {
return false, err
return nil, false, err
}
changes.DeletePendingValidator(stakerToRemove)
changed = true
Expand All @@ -125,12 +147,12 @@ func AdvanceTimeTo(

supply, err := changes.GetCurrentSupply(stakerToRemove.SubnetID)
if err != nil {
return false, err
return nil, false, err
}

rewards, err := GetRewardsCalculator(backend, parentState, stakerToRemove.SubnetID)
if err != nil {
return false, err
return nil, false, err
}

potentialReward := rewards.Calculate(
Expand All @@ -147,7 +169,7 @@ func AdvanceTimeTo(
switch stakerToRemove.Priority {
case txs.PrimaryNetworkValidatorPendingPriority, txs.SubnetPermissionlessValidatorPendingPriority:
if err := changes.PutCurrentValidator(&stakerToAdd); err != nil {
return false, err
return nil, false, err
}
changes.DeletePendingValidator(stakerToRemove)

Expand All @@ -156,16 +178,20 @@ func AdvanceTimeTo(
changes.DeletePendingDelegator(stakerToRemove)

default:
return false, fmt.Errorf("expected staker priority got %d", stakerToRemove.Priority)
return nil, false, fmt.Errorf("expected staker priority got %d", stakerToRemove.Priority)
}

changed = true
}

// Remove any current stakers whose [EndTime] <= [newChainTime].
//
// Invariant: It is not safe to modify the state while iterating over it,
// so we use the parentState's iterator rather than the changes iterator.
// ParentState must not be modified before this iterator is released.
currentStakerIterator, err := parentState.GetCurrentStakerIterator()
if err != nil {
return false, err
return nil, false, err
}
defer currentStakerIterator.Release()

Expand Down Expand Up @@ -201,8 +227,38 @@ func AdvanceTimeTo(
changes.SetFeeState(feeState)
}

// Remove all expiries whose timestamp now implies they can never be
// re-issued.
//
// The expiry timestamp is the time at which it is no longer valid, so any
// expiry with a timestamp less than or equal to the new chain time can be
// removed.
//
// Ref: https://github.com/avalanche-foundation/ACPs/tree/main/ACPs/77-reinventing-subnets#registersubnetvalidatortx
//
// The expiry iterator is sorted in order of increasing timestamp.
//
// Invariant: It is not safe to modify the state while iterating over it,
// so we use the parentState's iterator rather than the changes iterator.
// ParentState must not be modified before this iterator is released.
expiryIterator, err := parentState.GetExpiryIterator()
if err != nil {
return nil, false, err
}
defer expiryIterator.Release()

newChainTimeUnix := uint64(newChainTime.Unix())
for expiryIterator.Next() {
expiry := expiryIterator.Value()
if expiry.Timestamp > newChainTimeUnix {
break
}

changes.DeleteExpiry(expiry)
}

changes.SetTimestamp(newChainTime)
return changed, changes.Apply(parentState)
return changes, changed, nil
}

func GetRewardsCalculator(
Expand Down
115 changes: 115 additions & 0 deletions vms/platformvm/txs/executor/state_changes_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,13 @@ import (

"github.com/stretchr/testify/require"

"github.com/ava-labs/avalanchego/ids"
"github.com/ava-labs/avalanchego/upgrade/upgradetest"
"github.com/ava-labs/avalanchego/utils/iterator"
"github.com/ava-labs/avalanchego/utils/timer/mockable"
"github.com/ava-labs/avalanchego/vms/components/gas"
"github.com/ava-labs/avalanchego/vms/platformvm/config"
"github.com/ava-labs/avalanchego/vms/platformvm/genesis/genesistest"
"github.com/ava-labs/avalanchego/vms/platformvm/state"
"github.com/ava-labs/avalanchego/vms/platformvm/state/statetest"
)
Expand Down Expand Up @@ -99,3 +102,115 @@ func TestAdvanceTimeTo_UpdatesFeeState(t *testing.T) {
})
}
}

func TestAdvanceTimeTo_RemovesStaleExpiries(t *testing.T) {
var (
currentTime = genesistest.DefaultValidatorStartTime
newTime = currentTime.Add(3 * time.Second)
newTimeUnix = uint64(newTime.Unix())

unexpiredTime = newTimeUnix + 1
expiredTime = newTimeUnix
previouslyExpiredTime = newTimeUnix - 1
validationID = ids.GenerateTestID()
)

tests := []struct {
name string
initialExpiries []state.ExpiryEntry
expectedExpiries []state.ExpiryEntry
}{
{
name: "no expiries",
},
{
name: "unexpired expiry",
initialExpiries: []state.ExpiryEntry{
{
Timestamp: unexpiredTime,
ValidationID: validationID,
},
},
expectedExpiries: []state.ExpiryEntry{
{
Timestamp: unexpiredTime,
ValidationID: validationID,
},
},
},
{
name: "unexpired expiry at new time",
initialExpiries: []state.ExpiryEntry{
{
Timestamp: expiredTime,
ValidationID: ids.GenerateTestID(),
},
},
},
{
name: "unexpired expiry at previous time",
initialExpiries: []state.ExpiryEntry{
{
Timestamp: previouslyExpiredTime,
ValidationID: ids.GenerateTestID(),
},
},
},
{
name: "limit expiries removed",
initialExpiries: []state.ExpiryEntry{
{
Timestamp: expiredTime,
ValidationID: ids.GenerateTestID(),
},
{
Timestamp: unexpiredTime,
ValidationID: validationID,
},
},
expectedExpiries: []state.ExpiryEntry{
{
Timestamp: unexpiredTime,
ValidationID: validationID,
},
},
},
}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
var (
require = require.New(t)
s = statetest.New(t, statetest.Config{})
)

// Ensure the invariant that [newTime <= nextStakerChangeTime] on
// AdvanceTimeTo is maintained.
nextStakerChangeTime, err := state.GetNextStakerChangeTime(s, mockable.MaxTime)
require.NoError(err)
require.False(newTime.After(nextStakerChangeTime))

for _, expiry := range test.initialExpiries {
s.PutExpiry(expiry)
}

validatorsModified, err := AdvanceTimeTo(
&Backend{
Config: &config.Config{
UpgradeConfig: upgradetest.GetConfig(upgradetest.Latest),
},
},
s,
newTime,
)
require.NoError(err)
require.False(validatorsModified)

expiryIterator, err := s.GetExpiryIterator()
require.NoError(err)
require.Equal(
test.expectedExpiries,
iterator.ToSlice(expiryIterator),
)
})
}
}

0 comments on commit 4680312

Please sign in to comment.