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

Remove stale expiries in AdvanceTimeTo #3415

Merged
merged 9 commits into from
Sep 25, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
24 changes: 24 additions & 0 deletions vms/platformvm/txs/executor/state_changes.go
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,30 @@ 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
expiryIterator, err := parentState.GetExpiryIterator()
if err != nil {
return false, err
}
defer expiryIterator.Release()

newChainTimeUnix := uint64(newChainTime.Unix())
for expiryIterator.Next() {
expiry := expiryIterator.Value()
if expiry.Timestamp > newChainTimeUnix {
Copy link
Contributor

Choose a reason for hiding this comment

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

(No action required) Maybe add a comment indicating that expiryIterator is ordered ascending by Timestamp such that the first unexpired Expiry implies that all subsequent Expiries are also unexpired?

break
}

changes.DeleteExpiry(expiry)
}

changes.SetTimestamp(newChainTime)
return changed, changes.Apply(parentState)
}
Expand Down
110 changes: 110 additions & 0 deletions vms/platformvm/txs/executor/state_changes_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,12 @@ 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/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 @@ -98,3 +101,110 @@ 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())
)

tests := []struct {
name string
initialExpiries []state.ExpiryEntry
expectedExpiries []state.ExpiryEntry
}{
{
name: "no expiries",
},
{
name: "unexpired expiry",
initialExpiries: []state.ExpiryEntry{
{
Timestamp: newTimeUnix + 1,
marun marked this conversation as resolved.
Show resolved Hide resolved
ValidationID: ids.ID{1},
},
},
expectedExpiries: []state.ExpiryEntry{
{
Timestamp: newTimeUnix + 1,
ValidationID: ids.ID{1},
},
},
},
{
name: "unexpired expiry at new time",
initialExpiries: []state.ExpiryEntry{
{
Timestamp: newTimeUnix,
ValidationID: ids.GenerateTestID(),
marun marked this conversation as resolved.
Show resolved Hide resolved
},
},
},
{
name: "unexpired expiry at previous time",
initialExpiries: []state.ExpiryEntry{
{
Timestamp: newTimeUnix - 1,
ValidationID: ids.GenerateTestID(),
},
},
},
{
name: "limit expiries removed",
initialExpiries: []state.ExpiryEntry{
{
Timestamp: newTimeUnix,
ValidationID: ids.GenerateTestID(),
},
{
Timestamp: newTimeUnix + 1,
ValidationID: ids.ID{1},
},
},
expectedExpiries: []state.ExpiryEntry{
{
Timestamp: newTimeUnix + 1,
ValidationID: ids.ID{1},
},
},
},
}
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)
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),
)
})
}
}