From 1340ccef2844b8739fea5652ce21ad57ac422834 Mon Sep 17 00:00:00 2001 From: Stephen Buttolph Date: Thu, 7 Mar 2024 15:16:23 -0500 Subject: [PATCH 1/3] Remove pre-Durango block building logic and verification (#2823) --- vms/platformvm/block/builder/builder.go | 43 ++++++------------- .../block/executor/proposal_block_test.go | 17 -------- vms/platformvm/block/executor/verifier.go | 17 +++----- 3 files changed, 19 insertions(+), 58 deletions(-) diff --git a/vms/platformvm/block/builder/builder.go b/vms/platformvm/block/builder/builder.go index 77f39fbd0296..1a7f2f0556e6 100644 --- a/vms/platformvm/block/builder/builder.go +++ b/vms/platformvm/block/builder/builder.go @@ -263,6 +263,19 @@ func buildBlock( forceAdvanceTime bool, parentState state.Chain, ) (block.Block, error) { + blockTxs, err := packBlockTxs( + parentID, + parentState, + builder.Mempool, + builder.txExecutorBackend, + builder.blkManager, + timestamp, + targetBlockSize, + ) + if err != nil { + return nil, fmt.Errorf("failed to pack block txs: %w", err) + } + // Try rewarding stakers whose staking period ends at the new chain time. // This is done first to prioritize advancing the timestamp as quickly as // possible. @@ -276,23 +289,6 @@ 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, - targetBlockSize, - ) - if err != nil { - return nil, fmt.Errorf("failed to pack block txs: %w", err) - } - } - return block.NewBanffProposalBlock( timestamp, parentID, @@ -302,19 +298,6 @@ func buildBlock( ) } - blockTxs, err := packBlockTxs( - parentID, - parentState, - builder.Mempool, - builder.txExecutorBackend, - builder.blkManager, - timestamp, - targetBlockSize, - ) - 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. if len(blockTxs) == 0 && !forceAdvanceTime { builder.txExecutorBackend.Ctx.Log.Debug("no pending txs to issue into a block") diff --git a/vms/platformvm/block/executor/proposal_block_test.go b/vms/platformvm/block/executor/proposal_block_test.go index 984c8081ae4f..7d987f952196 100644 --- a/vms/platformvm/block/executor/proposal_block_test.go +++ b/vms/platformvm/block/executor/proposal_block_test.go @@ -334,23 +334,6 @@ func TestBanffProposalBlockTimeVerification(t *testing.T) { require.ErrorIs(err, executor.ErrAdvanceTimeTxIssuedAfterBanff) } - { - // include too many transactions - statelessProposalBlock, err := block.NewBanffProposalBlock( - nextStakerTime, - parentID, - banffParentBlk.Height()+1, - blkTx, - []*txs.Tx{}, - ) - require.NoError(err) - - statelessProposalBlock.Transactions = []*txs.Tx{blkTx} - block := env.blkManager.NewBlock(statelessProposalBlock) - err = block.Verify(context.Background()) - require.ErrorIs(err, errBanffProposalBlockWithMultipleTransactions) - } - { // valid statelessProposalBlock, err := block.NewBanffProposalBlock( diff --git a/vms/platformvm/block/executor/verifier.go b/vms/platformvm/block/executor/verifier.go index b35d2ecdd55c..926a7acf4fa0 100644 --- a/vms/platformvm/block/executor/verifier.go +++ b/vms/platformvm/block/executor/verifier.go @@ -22,12 +22,11 @@ var ( 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") - errOptionBlockTimestampNotMatchingParent = errors.New("option block proposed timestamp not matching parent block one") + 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") ) // verifier handles the logic for verifying a block. @@ -51,11 +50,6 @@ func (v *verifier) BanffCommitBlock(b *block.BanffCommitBlock) error { } func (v *verifier) BanffProposalBlock(b *block.BanffProposalBlock) error { - nextChainTime := b.Timestamp() - if !v.txExecutorBackend.Config.IsDurangoActivated(nextChainTime) && len(b.Transactions) != 0 { - return errBanffProposalBlockWithMultipleTransactions - } - if err := v.banffNonOptionBlock(b); err != nil { return err } @@ -67,6 +61,7 @@ func (v *verifier) BanffProposalBlock(b *block.BanffProposalBlock) error { } // Advance the time to [nextChainTime]. + nextChainTime := b.Timestamp() if _, err := executor.AdvanceTimeTo(v.txExecutorBackend, onDecisionState, nextChainTime); err != nil { return err } From 50ca08e6f92bdb46256df6b39a17707125a8fddb Mon Sep 17 00:00:00 2001 From: Stephen Buttolph Date: Thu, 7 Mar 2024 15:30:34 -0500 Subject: [PATCH 2/3] Remove pre-Durango checks in BLS key verification (#2824) --- network/peer/peer.go | 10 +---- network/peer/peer_test.go | 91 ++------------------------------------- 2 files changed, 4 insertions(+), 97 deletions(-) diff --git a/network/peer/peer.go b/network/peer/peer.go index c747c1d816b7..a78aae2996a8 100644 --- a/network/peer/peer.go +++ b/network/peer/peer.go @@ -716,8 +716,7 @@ func (p *peer) shouldDisconnect() bool { return false } - postDurango := p.Clock.Time().After(version.GetDurangoTime(constants.MainnetID)) - if postDurango && p.ip.BLSSignature == nil { + if p.ip.BLSSignature == nil { p.Log.Debug("disconnecting from peer", zap.String("reason", "missing BLS signature"), zap.Stringer("nodeID", p.id), @@ -725,13 +724,6 @@ func (p *peer) shouldDisconnect() bool { return true } - // If Durango hasn't activated on mainnet yet, we don't require BLS - // signatures to be provided. However, if they are provided, verify that - // they are correct. - if p.ip.BLSSignature == nil { - return false - } - validSignature := bls.VerifyProofOfPossession( vdr.PublicKey, p.ip.BLSSignature, diff --git a/network/peer/peer_test.go b/network/peer/peer_test.go index e52273fc6fbe..45d2fc60a234 100644 --- a/network/peer/peer_test.go +++ b/network/peer/peer_test.go @@ -29,7 +29,6 @@ import ( "github.com/ava-labs/avalanchego/utils/math/meter" "github.com/ava-labs/avalanchego/utils/resource" "github.com/ava-labs/avalanchego/utils/set" - "github.com/ava-labs/avalanchego/utils/timer/mockable" "github.com/ava-labs/avalanchego/version" ) @@ -591,14 +590,9 @@ func TestShouldDisconnect(t *testing.T) { expectedShouldDisconnect: false, }, { - name: "past durango without a signature", + name: "peer without signature", initialPeer: &peer{ Config: &Config{ - Clock: func() mockable.Clock { - clk := mockable.Clock{} - clk.Set(mockable.MaxTime) - return clk - }(), Log: logging.NoLog{}, VersionCompatibility: version.GetCompatibility(constants.UnitTestID), Validators: func() validators.Manager { @@ -619,11 +613,6 @@ func TestShouldDisconnect(t *testing.T) { }, expectedPeer: &peer{ Config: &Config{ - Clock: func() mockable.Clock { - clk := mockable.Clock{} - clk.Set(mockable.MaxTime) - return clk - }(), Log: logging.NoLog{}, VersionCompatibility: version.GetCompatibility(constants.UnitTestID), Validators: func() validators.Manager { @@ -645,68 +634,9 @@ func TestShouldDisconnect(t *testing.T) { expectedShouldDisconnect: true, }, { - name: "pre durango without a signature", + name: "peer with invalid signature", initialPeer: &peer{ Config: &Config{ - Clock: func() mockable.Clock { - clk := mockable.Clock{} - clk.Set(time.Time{}) - return clk - }(), - Log: logging.NoLog{}, - VersionCompatibility: version.GetCompatibility(constants.UnitTestID), - Validators: func() validators.Manager { - vdrs := validators.NewManager() - require.NoError(t, vdrs.AddStaker( - constants.PrimaryNetworkID, - peerID, - bls.PublicFromSecretKey(blsKey), - txID, - 1, - )) - return vdrs - }(), - }, - id: peerID, - version: version.CurrentApp, - ip: &SignedIP{}, - }, - expectedPeer: &peer{ - Config: &Config{ - Clock: func() mockable.Clock { - clk := mockable.Clock{} - clk.Set(time.Time{}) - return clk - }(), - Log: logging.NoLog{}, - VersionCompatibility: version.GetCompatibility(constants.UnitTestID), - Validators: func() validators.Manager { - vdrs := validators.NewManager() - require.NoError(t, vdrs.AddStaker( - constants.PrimaryNetworkID, - peerID, - bls.PublicFromSecretKey(blsKey), - txID, - 1, - )) - return vdrs - }(), - }, - id: peerID, - version: version.CurrentApp, - ip: &SignedIP{}, - }, - expectedShouldDisconnect: false, - }, - { - name: "pre durango with an invalid signature", - initialPeer: &peer{ - Config: &Config{ - Clock: func() mockable.Clock { - clk := mockable.Clock{} - clk.Set(time.Time{}) - return clk - }(), Log: logging.NoLog{}, VersionCompatibility: version.GetCompatibility(constants.UnitTestID), Validators: func() validators.Manager { @@ -729,11 +659,6 @@ func TestShouldDisconnect(t *testing.T) { }, expectedPeer: &peer{ Config: &Config{ - Clock: func() mockable.Clock { - clk := mockable.Clock{} - clk.Set(time.Time{}) - return clk - }(), Log: logging.NoLog{}, VersionCompatibility: version.GetCompatibility(constants.UnitTestID), Validators: func() validators.Manager { @@ -757,14 +682,9 @@ func TestShouldDisconnect(t *testing.T) { expectedShouldDisconnect: true, }, { - name: "pre durango with a valid signature", + name: "peer with valid signature", initialPeer: &peer{ Config: &Config{ - Clock: func() mockable.Clock { - clk := mockable.Clock{} - clk.Set(time.Time{}) - return clk - }(), Log: logging.NoLog{}, VersionCompatibility: version.GetCompatibility(constants.UnitTestID), Validators: func() validators.Manager { @@ -787,11 +707,6 @@ func TestShouldDisconnect(t *testing.T) { }, expectedPeer: &peer{ Config: &Config{ - Clock: func() mockable.Clock { - clk := mockable.Clock{} - clk.Set(time.Time{}) - return clk - }(), Log: logging.NoLog{}, VersionCompatibility: version.GetCompatibility(constants.UnitTestID), Validators: func() validators.Manager { From a593cc4147b305153dba99655773c3f934dfe703 Mon Sep 17 00:00:00 2001 From: Dhruba Basu <7675102+dhrubabasu@users.noreply.github.com> Date: Thu, 7 Mar 2024 17:45:11 -0500 Subject: [PATCH 3/3] [snow/networking] Enforce `PreferredIDAtHeight` in `Chits` messages (#2827) --- snow/networking/handler/handler.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/snow/networking/handler/handler.go b/snow/networking/handler/handler.go index 7a62dd7d928b..8878e4d2f770 100644 --- a/snow/networking/handler/handler.go +++ b/snow/networking/handler/handler.go @@ -716,9 +716,7 @@ func (h *handler) handleSyncMsg(ctx context.Context, msg Message) error { zap.String("field", "PreferredIDAtHeight"), zap.Error(err), ) - // TODO: Require this field to be populated correctly after v1.11.x - // is activated. - preferredIDAtHeight = preferredID + return engine.QueryFailed(ctx, nodeID, msg.RequestId) } acceptedID, err := ids.ToID(msg.AcceptedId)