diff --git a/vms/platformvm/block/builder/builder.go b/vms/platformvm/block/builder/builder.go index 53602ef3ed93..fc272453aba5 100644 --- a/vms/platformvm/block/builder/builder.go +++ b/vms/platformvm/block/builder/builder.go @@ -14,10 +14,12 @@ import ( "github.com/ava-labs/avalanchego/ids" "github.com/ava-labs/avalanchego/snow/consensus/snowman" + "github.com/ava-labs/avalanchego/utils/set" "github.com/ava-labs/avalanchego/utils/timer/mockable" "github.com/ava-labs/avalanchego/utils/units" "github.com/ava-labs/avalanchego/vms/platformvm/block" "github.com/ava-labs/avalanchego/vms/platformvm/state" + "github.com/ava-labs/avalanchego/vms/platformvm/status" "github.com/ava-labs/avalanchego/vms/platformvm/txs" "github.com/ava-labs/avalanchego/vms/platformvm/txs/mempool" @@ -192,16 +194,9 @@ func (b *builder) ShutdownBlockTimer() { // This method removes the transactions from the returned // blocks from the mempool. func (b *builder) BuildBlock(context.Context) (snowman.Block, error) { - defer func() { - // If we need to advance the chain's timestamp in a standard block, but - // we build an invalid block, then we need to re-trigger block building. - // - // TODO: Remove once we are guaranteed to build a valid block. - b.ResetBlockTimer() - // If there are still transactions in the mempool, then we need to - // re-trigger block building. - b.Mempool.RequestBuildBlock(false /*=emptyBlockPermitted*/) - }() + // If there are still transactions in the mempool, then we need to + // re-trigger block building. + defer b.Mempool.RequestBuildBlock(false /*=emptyBlockPermitted*/) b.txExecutorBackend.Ctx.Log.Debug("starting to attempt to build a block") @@ -259,38 +254,41 @@ func buildBlock( return nil, fmt.Errorf("could not build tx to reward staker: %w", err) } + var blockTxs []*txs.Tx + // TODO: Cleanup post-Durango + if builder.txExecutorBackend.Config.IsDurangoActivated(timestamp) { + blockTxs, err = packBlockTxs( + parentID, + parentState, + builder.Mempool, + builder.txExecutorBackend, + builder.blkManager, + timestamp, + ) + if err != nil { + return nil, fmt.Errorf("failed to pack block txs: %w", err) + } + } + return block.NewBanffProposalBlock( timestamp, parentID, height, rewardValidatorTx, - []*txs.Tx{}, // TODO: Populate with StandardBlock txs + blockTxs, ) } - // Clean out the mempool's transactions with invalid timestamps. - droppedStakerTxIDs := builder.Mempool.DropExpiredStakerTxs(timestamp.Add(txexecutor.SyncBound)) - for _, txID := range droppedStakerTxIDs { - builder.txExecutorBackend.Ctx.Log.Debug("dropping tx", - zap.Stringer("txID", txID), - zap.Error(err), - ) - } - - var ( - blockTxs []*txs.Tx - remainingSize = targetBlockSize + blockTxs, err := packBlockTxs( + parentID, + parentState, + builder.Mempool, + builder.txExecutorBackend, + builder.blkManager, + timestamp, ) - - for { - tx, exists := builder.Mempool.Peek() - if !exists || len(tx.Bytes()) > remainingSize { - break - } - builder.Mempool.Remove([]*txs.Tx{tx}) - - remainingSize -= len(tx.Bytes()) - blockTxs = append(blockTxs, tx) + if err != nil { + return nil, fmt.Errorf("failed to pack block txs: %w", err) } // If there is no reason to build a block, don't. @@ -308,6 +306,89 @@ func buildBlock( ) } +func packBlockTxs( + parentID ids.ID, + parentState state.Chain, + mempool mempool.Mempool, + backend *txexecutor.Backend, + manager blockexecutor.Manager, + timestamp time.Time, +) ([]*txs.Tx, error) { + stateDiff, err := state.NewDiffOn(parentState) + if err != nil { + return nil, err + } + + changes, err := txexecutor.AdvanceTimeTo(backend, stateDiff, timestamp) + if err != nil { + return nil, err + } + changes.Apply(stateDiff) + stateDiff.SetTimestamp(timestamp) + + var ( + blockTxs []*txs.Tx + inputs set.Set[ids.ID] + remainingSize = targetBlockSize + ) + + for { + tx, exists := mempool.Peek() + if !exists { + break + } + txSize := len(tx.Bytes()) + if txSize > remainingSize { + break + } + mempool.Remove([]*txs.Tx{tx}) + + // Invariant: [tx] has already been syntactically verified. + + txDiff, err := state.NewDiffOn(stateDiff) + if err != nil { + return nil, err + } + + executor := &txexecutor.StandardTxExecutor{ + Backend: backend, + State: txDiff, + Tx: tx, + } + + err = tx.Unsigned.Visit(executor) + if err != nil { + txID := tx.ID() + mempool.MarkDropped(txID, err) + continue + } + + if inputs.Overlaps(executor.Inputs) { + txID := tx.ID() + mempool.MarkDropped(txID, blockexecutor.ErrConflictingBlockTxs) + continue + } + err = manager.VerifyUniqueInputs(parentID, executor.Inputs) + if err != nil { + txID := tx.ID() + mempool.MarkDropped(txID, err) + continue + } + inputs.Union(executor.Inputs) + + txDiff.AddTx(tx, status.Committed) + err = txDiff.Apply(stateDiff) + if err != nil { + return nil, err + } + + remainingSize -= txSize + blockTxs = append(blockTxs, tx) + } + + return blockTxs, nil +} + // getNextStakerToReward returns the next staker txID to remove from the staking // set with a RewardValidatorTx rather than an AdvanceTimeTx. [chainTimestamp] // is the timestamp of the chain at the time this validator would be getting diff --git a/vms/platformvm/block/builder/builder_test.go b/vms/platformvm/block/builder/builder_test.go index ee603c484622..4b683bf9e1e3 100644 --- a/vms/platformvm/block/builder/builder_test.go +++ b/vms/platformvm/block/builder/builder_test.go @@ -288,6 +288,102 @@ func TestBuildBlockForceAdvanceTime(t *testing.T) { require.Equal(nextTime.Unix(), standardBlk.Timestamp().Unix()) } +func TestBuildBlockDropExpiredStakerTxs(t *testing.T) { + require := require.New(t) + + env := newEnvironment(t) + env.ctx.Lock.Lock() + defer func() { + require.NoError(shutdownEnvironment(env)) + env.ctx.Lock.Unlock() + }() + + var ( + now = env.backend.Clk.Time() + defaultValidatorStake = 100 * units.MilliAvax + + // Add a validator with StartTime in the future within [MaxFutureStartTime] + validatorStartTime = now.Add(txexecutor.MaxFutureStartTime - 1*time.Second) + validatorEndTime = validatorStartTime.Add(360 * 24 * time.Hour) + ) + + tx1, err := env.txBuilder.NewAddValidatorTx( + defaultValidatorStake, + uint64(validatorStartTime.Unix()), + uint64(validatorEndTime.Unix()), + ids.GenerateTestNodeID(), + preFundedKeys[0].PublicKey().Address(), + reward.PercentDenominator, + []*secp256k1.PrivateKey{preFundedKeys[0]}, + preFundedKeys[0].PublicKey().Address(), + ) + require.NoError(err) + require.NoError(env.mempool.Add(tx1)) + tx1ID := tx1.ID() + require.True(env.mempool.Has(tx1ID)) + + // Add a validator with StartTime before current chain time + validator2StartTime := now.Add(-5 * time.Second) + validator2EndTime := validator2StartTime.Add(360 * 24 * time.Hour) + + tx2, err := env.txBuilder.NewAddValidatorTx( + defaultValidatorStake, + uint64(validator2StartTime.Unix()), + uint64(validator2EndTime.Unix()), + ids.GenerateTestNodeID(), + preFundedKeys[1].PublicKey().Address(), + reward.PercentDenominator, + []*secp256k1.PrivateKey{preFundedKeys[1]}, + preFundedKeys[1].PublicKey().Address(), + ) + require.NoError(err) + require.NoError(env.mempool.Add(tx2)) + tx2ID := tx2.ID() + require.True(env.mempool.Has(tx2ID)) + + // Add a validator with StartTime in the future past [MaxFutureStartTime] + validator3StartTime := now.Add(txexecutor.MaxFutureStartTime + 5*time.Second) + validator3EndTime := validator2StartTime.Add(360 * 24 * time.Hour) + + tx3, err := env.txBuilder.NewAddValidatorTx( + defaultValidatorStake, + uint64(validator3StartTime.Unix()), + uint64(validator3EndTime.Unix()), + ids.GenerateTestNodeID(), + preFundedKeys[2].PublicKey().Address(), + reward.PercentDenominator, + []*secp256k1.PrivateKey{preFundedKeys[2]}, + preFundedKeys[2].PublicKey().Address(), + ) + require.NoError(err) + require.NoError(env.mempool.Add(tx3)) + tx3ID := tx3.ID() + require.True(env.mempool.Has(tx3ID)) + + // Only tx1 should be in a built block + blkIntf, err := env.Builder.BuildBlock(context.Background()) + require.NoError(err) + + require.IsType(&blockexecutor.Block{}, blkIntf) + blk := blkIntf.(*blockexecutor.Block) + require.Len(blk.Txs(), 1) + require.Equal(tx1ID, blk.Txs()[0].ID()) + + // Mempool should have none of the txs + require.False(env.mempool.Has(tx1ID)) + require.False(env.mempool.Has(tx2ID)) + require.False(env.mempool.Has(tx3ID)) + + // Only tx2 and tx3 should be dropped + require.NoError(env.mempool.GetDropReason(tx1ID)) + + tx2DropReason := env.mempool.GetDropReason(tx2ID) + require.ErrorIs(tx2DropReason, txexecutor.ErrTimestampNotBeforeStartTime) + + tx3DropReason := env.mempool.GetDropReason(tx3ID) + require.ErrorIs(tx3DropReason, txexecutor.ErrFutureStakeTime) +} + func TestPreviouslyDroppedTxsCanBeReAddedToMempool(t *testing.T) { require := require.New(t) diff --git a/vms/platformvm/block/executor/verifier.go b/vms/platformvm/block/executor/verifier.go index 4e81c8470ba3..37129f8cc03a 100644 --- a/vms/platformvm/block/executor/verifier.go +++ b/vms/platformvm/block/executor/verifier.go @@ -20,12 +20,13 @@ import ( var ( _ block.Visitor = (*verifier)(nil) + ErrConflictingBlockTxs = errors.New("block contains conflicting transactions") + errApricotBlockIssuedAfterFork = errors.New("apricot block issued after fork") errBanffProposalBlockWithMultipleTransactions = errors.New("BanffProposalBlock contains multiple transactions") errBanffStandardBlockWithoutChanges = errors.New("BanffStandardBlock performs no state changes") errIncorrectBlockHeight = errors.New("incorrect block height") errChildBlockEarlierThanParent = errors.New("proposed timestamp before current chain time") - errConflictingBatchTxs = errors.New("block contains conflicting transactions") errOptionBlockTimestampNotMatchingParent = errors.New("option block proposed timestamp not matching parent block one") ) @@ -468,7 +469,7 @@ func (v *verifier) processStandardTxs(txs []*txs.Tx, state state.Diff, parentID } // ensure it doesn't overlap with current input batch if inputs.Overlaps(txExecutor.Inputs) { - return nil, nil, nil, errConflictingBatchTxs + return nil, nil, nil, ErrConflictingBlockTxs } // Add UTXOs to batch inputs.Union(txExecutor.Inputs) diff --git a/vms/platformvm/txs/mempool/mempool.go b/vms/platformvm/txs/mempool/mempool.go index d4783adf5a4c..7e7aee09520a 100644 --- a/vms/platformvm/txs/mempool/mempool.go +++ b/vms/platformvm/txs/mempool/mempool.go @@ -6,7 +6,6 @@ package mempool import ( "errors" "fmt" - "time" "github.com/prometheus/client_golang/prometheus" @@ -53,12 +52,6 @@ type Mempool interface { // Peek returns the oldest tx in the mempool. Peek() (tx *txs.Tx, exists bool) - // Drops all [txs.Staker] transactions whose [StartTime] is before - // [minStartTime] from [mempool]. The dropped tx ids are returned. - // - // TODO: Remove once [StartTime] field is ignored in staker txs - DropExpiredStakerTxs(minStartTime time.Time) []ids.ID - // RequestBuildBlock notifies the consensus engine that a block should be // built. If [emptyBlockPermitted] is true, the notification will be sent // regardless of whether there are no transactions in the mempool. If not, @@ -229,38 +222,3 @@ func (m *mempool) RequestBuildBlock(emptyBlockPermitted bool) { default: } } - -// Drops all [txs.Staker] transactions whose [StartTime] is before -// [minStartTime] from [mempool]. The dropped tx ids are returned. -// -// TODO: Remove once [StartTime] field is ignored in staker txs -func (m *mempool) DropExpiredStakerTxs(minStartTime time.Time) []ids.ID { - var droppedTxIDs []ids.ID - - txIter := m.unissuedTxs.NewIterator() - for txIter.Next() { - tx := txIter.Value() - stakerTx, ok := tx.Unsigned.(txs.ScheduledStaker) - if !ok { - continue - } - - startTime := stakerTx.StartTime() - if !startTime.Before(minStartTime) { - continue - } - - txID := tx.ID() - err := fmt.Errorf( - "synchrony bound (%s) is later than staker start time (%s)", - minStartTime, - startTime, - ) - - m.Remove([]*txs.Tx{tx}) - m.MarkDropped(txID, err) // cache tx as dropped - droppedTxIDs = append(droppedTxIDs, txID) - } - - return droppedTxIDs -} diff --git a/vms/platformvm/txs/mempool/mempool_test.go b/vms/platformvm/txs/mempool/mempool_test.go index 9877b0384288..2f6b9782d2fe 100644 --- a/vms/platformvm/txs/mempool/mempool_test.go +++ b/vms/platformvm/txs/mempool/mempool_test.go @@ -199,29 +199,6 @@ func generateAddValidatorTx(startTime uint64, endTime uint64) (*txs.Tx, error) { return txs.NewSigned(utx, txs.Codec, nil) } -func TestDropExpiredStakerTxs(t *testing.T) { - require := require.New(t) - - registerer := prometheus.NewRegistry() - mempool, err := New("mempool", registerer, nil) - require.NoError(err) - - tx1, err := generateAddValidatorTx(10, 20) - require.NoError(err) - require.NoError(mempool.Add(tx1)) - - tx2, err := generateAddValidatorTx(8, 20) - require.NoError(err) - require.NoError(mempool.Add(tx2)) - - tx3, err := generateAddValidatorTx(15, 20) - require.NoError(err) - require.NoError(mempool.Add(tx3)) - - minStartTime := time.Unix(9, 0) - require.Len(mempool.DropExpiredStakerTxs(minStartTime), 1) -} - func TestPeekTxs(t *testing.T) { require := require.New(t) diff --git a/vms/platformvm/txs/mempool/mock_mempool.go b/vms/platformvm/txs/mempool/mock_mempool.go index 5ccb1ec1820f..384959d46694 100644 --- a/vms/platformvm/txs/mempool/mock_mempool.go +++ b/vms/platformvm/txs/mempool/mock_mempool.go @@ -9,7 +9,6 @@ package mempool import ( reflect "reflect" - time "time" ids "github.com/ava-labs/avalanchego/ids" txs "github.com/ava-labs/avalanchego/vms/platformvm/txs" @@ -53,20 +52,6 @@ func (mr *MockMempoolMockRecorder) Add(arg0 interface{}) *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Add", reflect.TypeOf((*MockMempool)(nil).Add), arg0) } -// DropExpiredStakerTxs mocks base method. -func (m *MockMempool) DropExpiredStakerTxs(arg0 time.Time) []ids.ID { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "DropExpiredStakerTxs", arg0) - ret0, _ := ret[0].([]ids.ID) - return ret0 -} - -// DropExpiredStakerTxs indicates an expected call of DropExpiredStakerTxs. -func (mr *MockMempoolMockRecorder) DropExpiredStakerTxs(arg0 interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DropExpiredStakerTxs", reflect.TypeOf((*MockMempool)(nil).DropExpiredStakerTxs), arg0) -} - // Get mocks base method. func (m *MockMempool) Get(arg0 ids.ID) *txs.Tx { m.ctrl.T.Helper()