diff --git a/vms/platformvm/block/builder/builder.go b/vms/platformvm/block/builder/builder.go index 35ad20adc16c..aa4804cae6d1 100644 --- a/vms/platformvm/block/builder/builder.go +++ b/vms/platformvm/block/builder/builder.go @@ -31,9 +31,15 @@ import ( txexecutor "github.com/ava-labs/avalanchego/vms/platformvm/txs/executor" ) -// targetBlockSize is maximum number of transaction bytes to place into a -// StandardBlock -const targetBlockSize = 128 * units.KiB +const ( + // targetBlockSize is maximum number of transaction bytes to place into a + // StandardBlock + targetBlockSize = 128 * units.KiB + + // maxTimeToSleep is the maximum time to sleep between checking if a block + // should be produced. + maxTimeToSleep = time.Hour +) var ( _ Builder = (*builder)(nil) @@ -174,12 +180,13 @@ func (b *builder) durationToSleep() (time.Duration, error) { return 0, fmt.Errorf("%w: %s", errMissingPreferredState, preferredID) } - nextStakerChangeTime, err := state.GetNextStakerChangeTime(preferredState) + now := b.txExecutorBackend.Clk.Time() + maxTimeToAwake := now.Add(maxTimeToSleep) + nextStakerChangeTime, err := state.GetNextStakerChangeTime(preferredState, maxTimeToAwake) if err != nil { return 0, fmt.Errorf("%w of %s: %w", errCalculatingNextStakerTime, preferredID, err) } - now := b.txExecutorBackend.Clk.Time() return nextStakerChangeTime.Sub(now), nil } diff --git a/vms/platformvm/block/executor/proposal_block_test.go b/vms/platformvm/block/executor/proposal_block_test.go index 60520a83ccf2..41553c151309 100644 --- a/vms/platformvm/block/executor/proposal_block_test.go +++ b/vms/platformvm/block/executor/proposal_block_test.go @@ -21,6 +21,7 @@ import ( "github.com/ava-labs/avalanchego/utils/crypto/bls" "github.com/ava-labs/avalanchego/utils/crypto/secp256k1" "github.com/ava-labs/avalanchego/utils/iterator/iteratormock" + "github.com/ava-labs/avalanchego/utils/timer/mockable" "github.com/ava-labs/avalanchego/vms/components/avax" "github.com/ava-labs/avalanchego/vms/components/gas" "github.com/ava-labs/avalanchego/vms/platformvm/block" @@ -279,7 +280,7 @@ func TestBanffProposalBlockTimeVerification(t *testing.T) { block := env.blkManager.NewBlock(statelessProposalBlock) err = block.Verify(context.Background()) - require.ErrorIs(err, errChildBlockEarlierThanParent) + require.ErrorIs(err, executor.ErrChildBlockEarlierThanParent) } { @@ -1377,7 +1378,7 @@ func TestAddValidatorProposalBlock(t *testing.T) { // Advance time until next staker change time is [validatorEndTime] for { - nextStakerChangeTime, err := state.GetNextStakerChangeTime(env.state) + nextStakerChangeTime, err := state.GetNextStakerChangeTime(env.state, mockable.MaxTime) require.NoError(err) if nextStakerChangeTime.Equal(validatorEndTime) { break diff --git a/vms/platformvm/block/executor/standard_block_test.go b/vms/platformvm/block/executor/standard_block_test.go index fa64eee74697..065bd2098616 100644 --- a/vms/platformvm/block/executor/standard_block_test.go +++ b/vms/platformvm/block/executor/standard_block_test.go @@ -212,7 +212,7 @@ func TestBanffStandardBlockTimeVerification(t *testing.T) { require.NoError(err) block := env.blkManager.NewBlock(banffChildBlk) err = block.Verify(context.Background()) - require.ErrorIs(err, errChildBlockEarlierThanParent) + require.ErrorIs(err, executor.ErrChildBlockEarlierThanParent) } { diff --git a/vms/platformvm/block/executor/verifier.go b/vms/platformvm/block/executor/verifier.go index 532dc4d4b6f6..abcbc566a303 100644 --- a/vms/platformvm/block/executor/verifier.go +++ b/vms/platformvm/block/executor/verifier.go @@ -27,7 +27,6 @@ var ( errApricotBlockIssuedAfterFork = errors.New("apricot block issued after fork") errBanffStandardBlockWithoutChanges = errors.New("BanffStandardBlock performs no state changes") errIncorrectBlockHeight = errors.New("incorrect block height") - errChildBlockEarlierThanParent = errors.New("proposed timestamp before current chain time") errOptionBlockTimestampNotMatchingParent = errors.New("option block proposed timestamp not matching parent block one") ) @@ -278,26 +277,11 @@ func (v *verifier) banffNonOptionBlock(b block.BanffBlock) error { } newChainTime := b.Timestamp() - parentChainTime := parentState.GetTimestamp() - if newChainTime.Before(parentChainTime) { - return fmt.Errorf( - "%w: proposed timestamp (%s), chain time (%s)", - errChildBlockEarlierThanParent, - newChainTime, - parentChainTime, - ) - } - - nextStakerChangeTime, err := state.GetNextStakerChangeTime(parentState) - if err != nil { - return fmt.Errorf("could not verify block timestamp: %w", err) - } - now := v.txExecutorBackend.Clk.Time() return executor.VerifyNewChainTime( newChainTime, - nextStakerChangeTime, now, + parentState, ) } diff --git a/vms/platformvm/state/chain_time_helpers.go b/vms/platformvm/state/chain_time_helpers.go index 8b861f69a265..0a2fbfa8ba17 100644 --- a/vms/platformvm/state/chain_time_helpers.go +++ b/vms/platformvm/state/chain_time_helpers.go @@ -7,7 +7,7 @@ import ( "fmt" "time" - "github.com/ava-labs/avalanchego/database" + "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" @@ -24,7 +24,10 @@ func NextBlockTime(state Chain, clk *mockable.Clock) (time.Time, bool, error) { } // [timestamp] = max(now, parentTime) - nextStakerChangeTime, err := GetNextStakerChangeTime(state) + // If the NextStakerChangeTime is after timestamp, then we shouldn't return + // that the time was capped. + nextStakerChangeTimeCap := timestamp.Add(time.Second) + nextStakerChangeTime, err := GetNextStakerChangeTime(state, nextStakerChangeTimeCap) if err != nil { return time.Time{}, false, fmt.Errorf("failed getting next staker change time: %w", err) } @@ -39,37 +42,33 @@ func NextBlockTime(state Chain, clk *mockable.Clock) (time.Time, bool, error) { } // GetNextStakerChangeTime returns the next time a staker will be either added -// or removed to/from the current validator set. -func GetNextStakerChangeTime(state Chain) (time.Time, error) { - currentStakerIterator, err := state.GetCurrentStakerIterator() +// or removed to/from the current validator set. If the next staker change time +// is further in the future than [defaultTime], then [defaultTime] is returned. +func GetNextStakerChangeTime(state Chain, defaultTime time.Time) (time.Time, error) { + currentIterator, err := state.GetCurrentStakerIterator() if err != nil { return time.Time{}, err } - defer currentStakerIterator.Release() + defer currentIterator.Release() - pendingStakerIterator, err := state.GetPendingStakerIterator() + pendingIterator, err := state.GetPendingStakerIterator() if err != nil { return time.Time{}, err } - defer pendingStakerIterator.Release() + defer pendingIterator.Release() - hasCurrentStaker := currentStakerIterator.Next() - hasPendingStaker := pendingStakerIterator.Next() - switch { - case hasCurrentStaker && hasPendingStaker: - nextCurrentTime := currentStakerIterator.Value().NextTime - nextPendingTime := pendingStakerIterator.Value().NextTime - if nextCurrentTime.Before(nextPendingTime) { - return nextCurrentTime, nil + for _, it := range []iterator.Iterator[*Staker]{currentIterator, pendingIterator} { + // If the iterator is empty, skip it + if !it.Next() { + continue + } + + time := it.Value().NextTime + if time.Before(defaultTime) { + defaultTime = time } - return nextPendingTime, nil - case hasCurrentStaker: - return currentStakerIterator.Value().NextTime, nil - case hasPendingStaker: - return pendingStakerIterator.Value().NextTime, nil - default: - return time.Time{}, database.ErrNotFound } + return defaultTime, nil } // PickFeeCalculator creates either a static or a dynamic fee calculator, diff --git a/vms/platformvm/state/chain_time_helpers_test.go b/vms/platformvm/state/chain_time_helpers_test.go index 3a6304aaeea5..7e1a1786eb61 100644 --- a/vms/platformvm/state/chain_time_helpers_test.go +++ b/vms/platformvm/state/chain_time_helpers_test.go @@ -5,16 +5,131 @@ package state import ( "testing" + "time" "github.com/stretchr/testify/require" "github.com/ava-labs/avalanchego/database/memdb" "github.com/ava-labs/avalanchego/genesis" + "github.com/ava-labs/avalanchego/ids" "github.com/ava-labs/avalanchego/upgrade/upgradetest" + "github.com/ava-labs/avalanchego/utils/constants" + "github.com/ava-labs/avalanchego/utils/timer/mockable" "github.com/ava-labs/avalanchego/vms/platformvm/config" + "github.com/ava-labs/avalanchego/vms/platformvm/genesis/genesistest" + "github.com/ava-labs/avalanchego/vms/platformvm/txs" "github.com/ava-labs/avalanchego/vms/platformvm/txs/fee" ) +func TestNextBlockTime(t *testing.T) { + tests := []struct { + name string + chainTime time.Time + now time.Time + expectedTime time.Time + expectedCapped bool + }{ + { + name: "parent time is after now", + chainTime: genesistest.DefaultValidatorStartTime, + now: genesistest.DefaultValidatorStartTime.Add(-time.Second), + expectedTime: genesistest.DefaultValidatorStartTime, + expectedCapped: false, + }, + { + name: "parent time is before now", + chainTime: genesistest.DefaultValidatorStartTime, + now: genesistest.DefaultValidatorStartTime.Add(time.Second), + expectedTime: genesistest.DefaultValidatorStartTime.Add(time.Second), + expectedCapped: false, + }, + { + name: "now is at next staker change time", + chainTime: genesistest.DefaultValidatorStartTime, + now: genesistest.DefaultValidatorEndTime, + expectedTime: genesistest.DefaultValidatorEndTime, + expectedCapped: true, + }, + { + name: "now is after next staker change time", + chainTime: genesistest.DefaultValidatorStartTime, + now: genesistest.DefaultValidatorEndTime.Add(time.Second), + expectedTime: genesistest.DefaultValidatorEndTime, + expectedCapped: true, + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + var ( + require = require.New(t) + s = newTestState(t, memdb.New()) + clk mockable.Clock + ) + + s.SetTimestamp(test.chainTime) + clk.Set(test.now) + + actualTime, actualCapped, err := NextBlockTime(s, &clk) + require.NoError(err) + require.Equal(test.expectedTime.Local(), actualTime.Local()) + require.Equal(test.expectedCapped, actualCapped) + }) + } +} + +func TestGetNextStakerChangeTime(t *testing.T) { + tests := []struct { + name string + pending []*Staker + maxTime time.Time + expected time.Time + }{ + { + name: "only current validators", + maxTime: mockable.MaxTime, + expected: genesistest.DefaultValidatorEndTime, + }, + { + name: "current and pending validators", + pending: []*Staker{ + { + TxID: ids.GenerateTestID(), + NodeID: ids.GenerateTestNodeID(), + PublicKey: nil, + SubnetID: constants.PrimaryNetworkID, + Weight: 1, + StartTime: genesistest.DefaultValidatorStartTime.Add(time.Second), + EndTime: genesistest.DefaultValidatorEndTime, + NextTime: genesistest.DefaultValidatorStartTime.Add(time.Second), + Priority: txs.PrimaryNetworkValidatorPendingPriority, + }, + }, + maxTime: mockable.MaxTime, + expected: genesistest.DefaultValidatorStartTime.Add(time.Second), + }, + { + name: "restricted timestamp", + maxTime: genesistest.DefaultValidatorStartTime, + expected: genesistest.DefaultValidatorStartTime, + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + var ( + require = require.New(t) + s = newTestState(t, memdb.New()) + ) + for _, staker := range test.pending { + require.NoError(s.PutPendingValidator(staker)) + } + + actual, err := GetNextStakerChangeTime(s, test.maxTime) + require.NoError(err) + require.Equal(test.expected.Local(), actual.Local()) + }) + } +} + func TestPickFeeCalculator(t *testing.T) { var ( createAssetTxFee = genesis.LocalParams.CreateAssetTxFee diff --git a/vms/platformvm/txs/executor/advance_time_test.go b/vms/platformvm/txs/executor/advance_time_test.go index c301b8bde8bb..fa7d3583c68f 100644 --- a/vms/platformvm/txs/executor/advance_time_test.go +++ b/vms/platformvm/txs/executor/advance_time_test.go @@ -99,12 +99,13 @@ func TestAdvanceTimeTxUpdatePrimaryNetworkStakers(t *testing.T) { require.True(ok) } -// Ensure semantic verification fails when proposed timestamp is at or before current timestamp +// Ensure semantic verification fails when proposed timestamp is before the +// current timestamp func TestAdvanceTimeTxTimestampTooEarly(t *testing.T) { require := require.New(t) env := newEnvironment(t, upgradetest.ApricotPhase5) - tx, err := newAdvanceTimeTx(t, env.state.GetTimestamp()) + tx, err := newAdvanceTimeTx(t, env.state.GetTimestamp().Add(-time.Second)) require.NoError(err) onCommitState, err := state.NewDiff(lastAcceptedID, env) @@ -122,10 +123,11 @@ func TestAdvanceTimeTxTimestampTooEarly(t *testing.T) { Tx: tx, } err = tx.Unsigned.Visit(&executor) - require.ErrorIs(err, ErrChildBlockNotAfterParent) + require.ErrorIs(err, ErrChildBlockEarlierThanParent) } -// Ensure semantic verification fails when proposed timestamp is after next validator set change time +// Ensure semantic verification fails when proposed timestamp is after next +// validator set change time func TestAdvanceTimeTxTimestampTooLate(t *testing.T) { require := require.New(t) env := newEnvironment(t, upgradetest.ApricotPhase5) @@ -166,8 +168,8 @@ func TestAdvanceTimeTxTimestampTooLate(t *testing.T) { env.ctx.Lock.Lock() defer env.ctx.Lock.Unlock() - // fast forward clock to 10 seconds before genesis validators stop validating - env.clk.Set(genesistest.DefaultValidatorEndTime.Add(-10 * time.Second)) + // fast forward clock to when genesis validators stop validating + env.clk.Set(genesistest.DefaultValidatorEndTime) { // Proposes advancing timestamp to 1 second after genesis validators stop validating diff --git a/vms/platformvm/txs/executor/proposal_tx_executor.go b/vms/platformvm/txs/executor/proposal_tx_executor.go index 573d6199e2a3..aa8064f2fcd8 100644 --- a/vms/platformvm/txs/executor/proposal_tx_executor.go +++ b/vms/platformvm/txs/executor/proposal_tx_executor.go @@ -34,7 +34,6 @@ var ( ErrRemoveStakerTooEarly = errors.New("attempting to remove staker before their end time") ErrRemoveWrongStaker = errors.New("attempting to remove wrong staker") - ErrChildBlockNotAfterParent = errors.New("proposed timestamp not after current chain time") ErrInvalidState = errors.New("generated output isn't valid state") ErrShouldBePermissionlessStaker = errors.New("expected permissionless staker") ErrWrongTxType = errors.New("wrong transaction type") @@ -270,34 +269,17 @@ func (e *ProposalTxExecutor) AdvanceTimeTx(tx *txs.AdvanceTimeTx) error { ) } - parentChainTime := e.OnCommitState.GetTimestamp() - if !newChainTime.After(parentChainTime) { - return fmt.Errorf( - "%w, proposed timestamp (%s), chain time (%s)", - ErrChildBlockNotAfterParent, - parentChainTime, - parentChainTime, - ) - } - - // Only allow timestamp to move forward as far as the time of next staker - // set change time - nextStakerChangeTime, err := state.GetNextStakerChangeTime(e.OnCommitState) - if err != nil { - return err - } - now := e.Clk.Time() if err := VerifyNewChainTime( newChainTime, - nextStakerChangeTime, now, + e.OnCommitState, ); err != nil { return err } // Note that state doesn't change if this proposal is aborted - _, err = AdvanceTimeTo(e.Backend, e.OnCommitState, newChainTime) + _, err := AdvanceTimeTo(e.Backend, e.OnCommitState, newChainTime) return err } diff --git a/vms/platformvm/txs/executor/state_changes.go b/vms/platformvm/txs/executor/state_changes.go index 55f940fca509..a30a422506b9 100644 --- a/vms/platformvm/txs/executor/state_changes.go +++ b/vms/platformvm/txs/executor/state_changes.go @@ -16,35 +16,35 @@ import ( ) var ( + ErrChildBlockEarlierThanParent = errors.New("proposed timestamp before current chain time") ErrChildBlockAfterStakerChangeTime = errors.New("proposed timestamp later than next staker change time") ErrChildBlockBeyondSyncBound = errors.New("proposed timestamp is too far in the future relative to local time") ) -// VerifyNewChainTime returns nil if the [newChainTime] is a valid chain time -// given the wall clock time ([now]) and when the next staking set change occurs -// ([nextStakerChangeTime]). +// VerifyNewChainTime returns nil if the [newChainTime] is a valid chain time. // Requires: -// - [newChainTime] <= [nextStakerChangeTime]: so that no staking set changes -// are skipped. +// - [newChainTime] >= [currentChainTime]: to ensure chain time advances +// monotonically. // - [newChainTime] <= [now] + [SyncBound]: to ensure chain time approximates // "real" time. +// - [newChainTime] <= [nextStakerChangeTime]: so that no staking set changes +// are skipped. func VerifyNewChainTime( - newChainTime, - nextStakerChangeTime, + newChainTime time.Time, now time.Time, + currentState state.Chain, ) error { - // Only allow timestamp to move as far forward as the time of the next - // staker set change - if newChainTime.After(nextStakerChangeTime) { + currentChainTime := currentState.GetTimestamp() + if newChainTime.Before(currentChainTime) { return fmt.Errorf( - "%w, proposed timestamp (%s), next staker change time (%s)", - ErrChildBlockAfterStakerChangeTime, + "%w: proposed timestamp (%s), chain time (%s)", + ErrChildBlockEarlierThanParent, newChainTime, - nextStakerChangeTime, + currentChainTime, ) } - // Only allow timestamp to reasonably far forward + // Only allow timestamp to be reasonably far forward maxNewChainTime := now.Add(SyncBound) if newChainTime.After(maxNewChainTime) { return fmt.Errorf( @@ -54,6 +54,24 @@ func VerifyNewChainTime( now, ) } + + // nextStakerChangeTime is calculated last to ensure that the function is + // able to be calculated efficiently. + nextStakerChangeTime, err := state.GetNextStakerChangeTime(currentState, newChainTime) + if err != nil { + return fmt.Errorf("could not verify block timestamp: %w", err) + } + + // Only allow timestamp to move as far forward as the time of the next + // staker set change + if newChainTime.After(nextStakerChangeTime) { + return fmt.Errorf( + "%w, proposed timestamp (%s), next staker change time (%s)", + ErrChildBlockAfterStakerChangeTime, + newChainTime, + nextStakerChangeTime, + ) + } return nil } diff --git a/vms/platformvm/txs/executor/state_changes_test.go b/vms/platformvm/txs/executor/state_changes_test.go index 5588f4b7da73..d642e2ed1481 100644 --- a/vms/platformvm/txs/executor/state_changes_test.go +++ b/vms/platformvm/txs/executor/state_changes_test.go @@ -10,6 +10,7 @@ import ( "github.com/stretchr/testify/require" "github.com/ava-labs/avalanchego/upgrade/upgradetest" + "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/state" @@ -75,7 +76,7 @@ func TestAdvanceTimeTo_UpdatesFeeState(t *testing.T) { // Ensure the invariant that [nextTime <= nextStakerChangeTime] on // AdvanceTimeTo is maintained. - nextStakerChangeTime, err := state.GetNextStakerChangeTime(s) + nextStakerChangeTime, err := state.GetNextStakerChangeTime(s, mockable.MaxTime) require.NoError(err) require.False(nextTime.After(nextStakerChangeTime))