From c08fdae7aacee7648be6cee1b694a152eab927ef Mon Sep 17 00:00:00 2001 From: Stephen Buttolph Date: Thu, 23 May 2024 10:51:09 -0400 Subject: [PATCH] Verify signatures during Parse (#3046) --- indexer/examples/p-chain/main.go | 3 +- indexer/examples/x-chain-blocks/main.go | 4 +- vms/proposervm/batched_vm.go | 2 +- vms/proposervm/block.go | 7 +- vms/proposervm/block/block.go | 45 +++---- vms/proposervm/block/block_test.go | 35 ++--- vms/proposervm/block/build_test.go | 12 +- vms/proposervm/block/option.go | 4 + vms/proposervm/block/option_test.go | 13 -- vms/proposervm/block/parse.go | 20 ++- vms/proposervm/block/parse_test.go | 172 ++++++++++++++---------- vms/proposervm/block_test.go | 49 ++++--- vms/proposervm/pre_fork_block.go | 12 +- vms/proposervm/state/block_state.go | 2 +- vms/proposervm/vm.go | 2 +- 15 files changed, 208 insertions(+), 174 deletions(-) delete mode 100644 vms/proposervm/block/option_test.go diff --git a/indexer/examples/p-chain/main.go b/indexer/examples/p-chain/main.go index 866424254fe..8c2a17c8636 100644 --- a/indexer/examples/p-chain/main.go +++ b/indexer/examples/p-chain/main.go @@ -9,6 +9,7 @@ import ( "time" "github.com/ava-labs/avalanchego/indexer" + "github.com/ava-labs/avalanchego/utils/constants" "github.com/ava-labs/avalanchego/wallet/subnet/primary" platformvmblock "github.com/ava-labs/avalanchego/vms/platformvm/block" @@ -33,7 +34,7 @@ func main() { } platformvmBlockBytes := container.Bytes - proposerVMBlock, err := proposervmblock.Parse(container.Bytes) + proposerVMBlock, err := proposervmblock.Parse(container.Bytes, constants.PlatformChainID) if err == nil { platformvmBlockBytes = proposerVMBlock.Block() } diff --git a/indexer/examples/x-chain-blocks/main.go b/indexer/examples/x-chain-blocks/main.go index 882f9a9c0ad..5b57d4d35e4 100644 --- a/indexer/examples/x-chain-blocks/main.go +++ b/indexer/examples/x-chain-blocks/main.go @@ -8,6 +8,7 @@ import ( "log" "time" + "github.com/ava-labs/avalanchego/ids" "github.com/ava-labs/avalanchego/indexer" "github.com/ava-labs/avalanchego/vms/proposervm/block" "github.com/ava-labs/avalanchego/wallet/chain/x/builder" @@ -19,6 +20,7 @@ import ( func main() { var ( uri = primary.LocalAPIURI + "/ext/index/X/block" + xChainID = ids.FromStringOrPanic("2eNy1mUFdmaxXNj1eQHUe7Np4gju9sJsEtWQ4MX3ToiNKuADed") client = indexer.NewClient(uri) ctx = context.Background() nextIndex uint64 @@ -31,7 +33,7 @@ func main() { continue } - proposerVMBlock, err := block.Parse(container.Bytes) + proposerVMBlock, err := block.Parse(container.Bytes, xChainID) if err != nil { log.Fatalf("failed to parse proposervm block: %s\n", err) } diff --git a/vms/proposervm/batched_vm.go b/vms/proposervm/batched_vm.go index ff1ce6a597a..853c858e6bb 100644 --- a/vms/proposervm/batched_vm.go +++ b/vms/proposervm/batched_vm.go @@ -101,7 +101,7 @@ func (vm *VM) BatchedParseBlock(ctx context.Context, blks [][]byte) ([]snowman.B ) for ; blocksIndex < len(blks); blocksIndex++ { blkBytes := blks[blocksIndex] - statelessBlock, err := statelessblock.Parse(blkBytes) + statelessBlock, err := statelessblock.Parse(blkBytes, vm.ctx.ChainID) if err != nil { break } diff --git a/vms/proposervm/block.go b/vms/proposervm/block.go index 9d48da49152..464acb9fc8c 100644 --- a/vms/proposervm/block.go +++ b/vms/proposervm/block.go @@ -36,6 +36,7 @@ var ( errTimeTooAdvanced = errors.New("time is too far advanced") errProposerWindowNotStarted = errors.New("proposer window hasn't started") errUnexpectedProposer = errors.New("unexpected proposer for current window") + errProposerMismatch = errors.New("proposer mismatch") errProposersNotActivated = errors.New("proposers haven't been activated yet") errPChainHeightTooLow = errors.New("block P-chain height is too low") ) @@ -152,9 +153,9 @@ func (p *postForkCommonComponents) Verify( return err } - // Verify the signature of the node - if err := child.SignedBlock.Verify(shouldHaveProposer, p.vm.ctx.ChainID); err != nil { - return err + hasProposer := child.SignedBlock.Proposer() != ids.EmptyNodeID + if shouldHaveProposer != hasProposer { + return fmt.Errorf("%w: shouldHaveProposer (%v) != hasProposer (%v)", errProposerMismatch, shouldHaveProposer, hasProposer) } p.vm.ctx.Log.Debug("verified post-fork block", diff --git a/vms/proposervm/block/block.go b/vms/proposervm/block/block.go index d99a569c96f..68da910e1db 100644 --- a/vms/proposervm/block/block.go +++ b/vms/proposervm/block/block.go @@ -17,9 +17,8 @@ import ( var ( _ SignedBlock = (*statelessBlock)(nil) - errUnexpectedProposer = errors.New("expected no proposer but one was provided") - errMissingProposer = errors.New("expected proposer but none was provided") - errInvalidCertificate = errors.New("invalid certificate") + errUnexpectedSignature = errors.New("signature provided when none was expected") + errInvalidCertificate = errors.New("invalid certificate") ) type Block interface { @@ -29,6 +28,7 @@ type Block interface { Bytes() []byte initialize(bytes []byte) error + verify(chainID ids.ID) error } type SignedBlock interface { @@ -36,9 +36,10 @@ type SignedBlock interface { PChainHeight() uint64 Timestamp() time.Time - Proposer() ids.NodeID - Verify(shouldHaveProposer bool, chainID ids.ID) error + // Proposer returns the ID of the node that proposed this block. If no node + // signed this block, [ids.EmptyNodeID] will be returned. + Proposer() ids.NodeID } type statelessUnsignedBlock struct { @@ -101,26 +102,12 @@ func (b *statelessBlock) initialize(bytes []byte) error { return nil } -func (b *statelessBlock) PChainHeight() uint64 { - return b.StatelessBlock.PChainHeight -} - -func (b *statelessBlock) Timestamp() time.Time { - return b.timestamp -} - -func (b *statelessBlock) Proposer() ids.NodeID { - return b.proposer -} - -func (b *statelessBlock) Verify(shouldHaveProposer bool, chainID ids.ID) error { - if !shouldHaveProposer { - if len(b.Signature) > 0 || len(b.StatelessBlock.Certificate) > 0 { - return errUnexpectedProposer +func (b *statelessBlock) verify(chainID ids.ID) error { + if len(b.StatelessBlock.Certificate) == 0 { + if len(b.Signature) > 0 { + return errUnexpectedSignature } return nil - } else if b.cert == nil { - return errMissingProposer } header, err := BuildHeader(chainID, b.StatelessBlock.ParentID, b.id) @@ -135,3 +122,15 @@ func (b *statelessBlock) Verify(shouldHaveProposer bool, chainID ids.ID) error { b.Signature, ) } + +func (b *statelessBlock) PChainHeight() uint64 { + return b.StatelessBlock.PChainHeight +} + +func (b *statelessBlock) Timestamp() time.Time { + return b.timestamp +} + +func (b *statelessBlock) Proposer() ids.NodeID { + return b.proposer +} diff --git a/vms/proposervm/block/block_test.go b/vms/proposervm/block/block_test.go index 8a8a57ae3b9..2b8918eb902 100644 --- a/vms/proposervm/block/block_test.go +++ b/vms/proposervm/block/block_test.go @@ -14,37 +14,22 @@ import ( "github.com/ava-labs/avalanchego/utils/units" ) -func equal(require *require.Assertions, chainID ids.ID, want, have SignedBlock) { +func equal(require *require.Assertions, want, have Block) { require.Equal(want.ID(), have.ID()) require.Equal(want.ParentID(), have.ParentID()) - require.Equal(want.PChainHeight(), have.PChainHeight()) - require.Equal(want.Timestamp(), have.Timestamp()) require.Equal(want.Block(), have.Block()) - require.Equal(want.Proposer(), have.Proposer()) require.Equal(want.Bytes(), have.Bytes()) - require.Equal(want.Verify(false, chainID), have.Verify(false, chainID)) - require.Equal(want.Verify(true, chainID), have.Verify(true, chainID)) -} - -func TestVerifyNoCertWithSignature(t *testing.T) { - parentID := ids.ID{1} - timestamp := time.Unix(123, 0) - pChainHeight := uint64(2) - innerBlockBytes := []byte{3} - - require := require.New(t) - - builtBlockIntf, err := BuildUnsigned(parentID, timestamp, pChainHeight, innerBlockBytes) - require.NoError(err) - - builtBlock := builtBlockIntf.(*statelessBlock) - builtBlock.Signature = []byte{0} - err = builtBlock.Verify(false, ids.Empty) - require.ErrorIs(err, errUnexpectedProposer) + signedWant, wantIsSigned := want.(SignedBlock) + signedHave, haveIsSigned := have.(SignedBlock) + require.Equal(wantIsSigned, haveIsSigned) + if !wantIsSigned { + return + } - err = builtBlock.Verify(true, ids.Empty) - require.ErrorIs(err, errMissingProposer) + require.Equal(signedWant.PChainHeight(), signedHave.PChainHeight()) + require.Equal(signedWant.Timestamp(), signedHave.Timestamp()) + require.Equal(signedWant.Proposer(), signedHave.Proposer()) } func TestBlockSizeLimit(t *testing.T) { diff --git a/vms/proposervm/block/build_test.go b/vms/proposervm/block/build_test.go index 5589a9ac95b..2ed9510c696 100644 --- a/vms/proposervm/block/build_test.go +++ b/vms/proposervm/block/build_test.go @@ -29,6 +29,7 @@ func TestBuild(t *testing.T) { cert, err := staking.ParseCertificate(tlsCert.Leaf.Raw) require.NoError(err) key := tlsCert.PrivateKey.(crypto.Signer) + nodeID := ids.NodeIDFromCert(cert) builtBlock, err := Build( parentID, @@ -45,11 +46,7 @@ func TestBuild(t *testing.T) { require.Equal(pChainHeight, builtBlock.PChainHeight()) require.Equal(timestamp, builtBlock.Timestamp()) require.Equal(innerBlockBytes, builtBlock.Block()) - - require.NoError(builtBlock.Verify(true, chainID)) - - err = builtBlock.Verify(false, chainID) - require.ErrorIs(err, errUnexpectedProposer) + require.Equal(nodeID, builtBlock.Proposer()) } func TestBuildUnsigned(t *testing.T) { @@ -68,11 +65,6 @@ func TestBuildUnsigned(t *testing.T) { require.Equal(timestamp, builtBlock.Timestamp()) require.Equal(innerBlockBytes, builtBlock.Block()) require.Equal(ids.EmptyNodeID, builtBlock.Proposer()) - - require.NoError(builtBlock.Verify(false, ids.Empty)) - - err = builtBlock.Verify(true, ids.Empty) - require.ErrorIs(err, errMissingProposer) } func TestBuildHeader(t *testing.T) { diff --git a/vms/proposervm/block/option.go b/vms/proposervm/block/option.go index c80651b621f..115b6d0b9f9 100644 --- a/vms/proposervm/block/option.go +++ b/vms/proposervm/block/option.go @@ -37,3 +37,7 @@ func (b *option) initialize(bytes []byte) error { b.bytes = bytes return nil } + +func (*option) verify(ids.ID) error { + return nil +} diff --git a/vms/proposervm/block/option_test.go b/vms/proposervm/block/option_test.go deleted file mode 100644 index d5af9c10007..00000000000 --- a/vms/proposervm/block/option_test.go +++ /dev/null @@ -1,13 +0,0 @@ -// Copyright (C) 2019-2024, Ava Labs, Inc. All rights reserved. -// See the file LICENSE for licensing terms. - -package block - -import "github.com/stretchr/testify/require" - -func equalOption(require *require.Assertions, want, have Block) { - require.Equal(want.ID(), have.ID()) - require.Equal(want.ParentID(), have.ParentID()) - require.Equal(want.Block(), have.Block()) - require.Equal(want.Bytes(), have.Bytes()) -} diff --git a/vms/proposervm/block/parse.go b/vms/proposervm/block/parse.go index cf275134d88..f6bc6387716 100644 --- a/vms/proposervm/block/parse.go +++ b/vms/proposervm/block/parse.go @@ -3,9 +3,25 @@ package block -import "fmt" +import ( + "fmt" -func Parse(bytes []byte) (Block, error) { + "github.com/ava-labs/avalanchego/ids" +) + +// Parse a block and verify that the signature attached to the block is valid +// for the certificate provided in the block. +func Parse(bytes []byte, chainID ids.ID) (Block, error) { + block, err := ParseWithoutVerification(bytes) + if err != nil { + return nil, err + } + return block, block.verify(chainID) +} + +// ParseWithoutVerification parses a block without verifying that the signature +// on the block is correct. +func ParseWithoutVerification(bytes []byte) (Block, error) { var block Block parsedVersion, err := Codec.Unmarshal(bytes, &block) if err != nil { diff --git a/vms/proposervm/block/parse_test.go b/vms/proposervm/block/parse_test.go index e894c1e7a05..ce1d5d97cbb 100644 --- a/vms/proposervm/block/parse_test.go +++ b/vms/proposervm/block/parse_test.go @@ -17,8 +17,6 @@ import ( ) func TestParse(t *testing.T) { - require := require.New(t) - parentID := ids.ID{1} timestamp := time.Unix(123, 0) pChainHeight := uint64(2) @@ -26,13 +24,13 @@ func TestParse(t *testing.T) { chainID := ids.ID{4} tlsCert, err := staking.NewTLSCert() - require.NoError(err) + require.NoError(t, err) cert, err := staking.ParseCertificate(tlsCert.Leaf.Raw) - require.NoError(err) + require.NoError(t, err) key := tlsCert.PrivateKey.(crypto.Signer) - builtBlock, err := Build( + signedBlock, err := Build( parentID, timestamp, pChainHeight, @@ -41,27 +39,106 @@ func TestParse(t *testing.T) { chainID, key, ) - require.NoError(err) - - builtBlockBytes := builtBlock.Bytes() - parsedBlockIntf, err := Parse(builtBlockBytes) - require.NoError(err) - - parsedBlock, ok := parsedBlockIntf.(SignedBlock) - require.True(ok) - - equal(require, chainID, builtBlock, parsedBlock) + require.NoError(t, err) + + unsignedBlock, err := BuildUnsigned(parentID, timestamp, pChainHeight, innerBlockBytes) + require.NoError(t, err) + + signedWithoutCertBlockIntf, err := BuildUnsigned(parentID, timestamp, pChainHeight, innerBlockBytes) + require.NoError(t, err) + signedWithoutCertBlock := signedWithoutCertBlockIntf.(*statelessBlock) + signedWithoutCertBlock.Signature = []byte{5} + + signedWithoutCertBlock.bytes, err = Codec.Marshal(CodecVersion, &signedWithoutCertBlockIntf) + require.NoError(t, err) + + optionBlock, err := BuildOption(parentID, innerBlockBytes) + require.NoError(t, err) + + tests := []struct { + name string + block Block + chainID ids.ID + expectedErr error + }{ + { + name: "correct chainID", + block: signedBlock, + chainID: chainID, + expectedErr: nil, + }, + { + name: "invalid chainID", + block: signedBlock, + chainID: ids.ID{5}, + expectedErr: staking.ErrECDSAVerificationFailure, + }, + { + name: "unsigned block", + block: unsignedBlock, + chainID: chainID, + expectedErr: nil, + }, + { + name: "invalid signature", + block: signedWithoutCertBlockIntf, + chainID: chainID, + expectedErr: errUnexpectedSignature, + }, + { + name: "option block", + block: optionBlock, + chainID: chainID, + expectedErr: nil, + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + require := require.New(t) + + blockBytes := test.block.Bytes() + parsedBlockWithoutVerification, err := ParseWithoutVerification(blockBytes) + require.NoError(err) + equal(require, test.block, parsedBlockWithoutVerification) + + parsedBlock, err := Parse(blockBytes, test.chainID) + require.ErrorIs(err, test.expectedErr) + if test.expectedErr == nil { + equal(require, test.block, parsedBlock) + } + }) + } } -func TestParseDuplicateExtension(t *testing.T) { - require := require.New(t) - - blockHex := "0000000000000100000000000000000000000000000000000000000000000000000000000000000000000000007b0000000000000002000004bd308204b9308202a1a003020102020100300d06092a864886f70d01010b050030003020170d3939313233313030303030305a180f32313232303830333233323835335a300030820222300d06092a864886f70d01010105000382020f003082020a0282020100c2b2de1c16924d9b9254a0d5b80a4bc5f9beaa4f4f40a0e4efb69eb9b55d7d37f8c82328c237d7c5b451f5427b487284fa3f365f9caa53c7fcfef8d7a461d743bd7d88129f2da62b877ebe9d6feabf1bd12923e6c12321382c782fc3bb6b6cb4986a937a1edc3814f4e621e1a62053deea8c7649e43edd97ab6b56315b00d9ab5026bb9c31fb042dc574ba83c54e720e0120fcba2e8a66b77839be3ece0d4a6383ef3f76aac952b49a15b65e18674cd1340c32cecbcbaf80ae45be001366cb56836575fb0ab51ea44bf7278817e99b6b180fdd110a49831a132968489822c56692161bbd372cf89d9b8ee5a734cff15303b3a960ee78d79e76662a701941d9ec084429f26707f767e9b1d43241c0e4f96655d95c1f4f4aa00add78eff6bf0a6982766a035bf0b465786632c5bb240788ca0fdf032d8815899353ea4bec5848fd30118711e5b356bde8a0da074cc25709623225e734ff5bd0cf65c40d9fd8fccf746d8f8f35145bcebcf378d2b086e57d78b11e84f47fa467c4d037f92bff6dd4e934e0189b58193f24c4222ffb72b5c06361cf68ca64345bc3e230cc0f40063ad5f45b1659c643662996328c2eeddcd760d6f7c9cbae081ccc065844f7ea78c858564a408979764de882793706acc67d88092790dff567ed914b03355330932616a0f26f994b963791f0b1dbd8df979db86d1ea490700a3120293c3c2b10bef10203010001a33c303a300e0603551d0f0101ff0404030204b030130603551d25040c300a06082b0601050507030230130603551d25040c300a06082b06010505070302300d06092a864886f70d01010b05000382020100a21a0d73ec9ef4eb39f810557ac70b0b775772b8bae5f42c98565bc50b5b2c57317aa9cb1da12f55d0aac7bb36a00cd4fd0d7384c4efa284b53520c5a3c4b8a65240b393eeab02c802ea146c0728c3481c9e8d3aaad9d4dd7607103dcfaa96da83460adbe18174ed5b71bde7b0a93d4fb52234a9ff54e3fd25c5b74790dfb090f2e59dc5907357f510cc3a0b70ccdb87aee214def794b316224f318b471ffa13b66e44b467670e881cb1628c99c048a503376d9b6d7b8eef2e7be47ff7d5c1d56221f4cf7fa2519b594cb5917815c64dc75d8d281bcc99b5a12899b08f2ca0f189857b64a1afc5963337f3dd6e79390e85221569f6dbbb13aadce06a3dfb5032f0cc454809627872cd7cd0cea5eba187723f07652c8abc3fc42bd62136fc66287f2cc19a7cb416923ad1862d7f820b55cacb65e43731cb6df780e2651e457a3438456aeeeb278ad9c0ad2e760f6c1cbe276eeb621c8a4e609b5f2d902beb3212e3e45df99497021ff536d0b56390c5d785a8bf7909f6b61bdc705d7d92ae22f58e7b075f164a0450d82d8286bf449072751636ab5185f59f518b845a75d112d6f7b65223479202cff67635e2ad88106bc8a0cc9352d87c5b182ac19a4680a958d814a093acf46730f87da0df6926291d02590f215041b44a0a1a32eeb3a52cddabc3d256689bace18a8d85e644cf9137cce3718f7caac1cb16ae06e874f4c701000000010300000200b8e3a4d9a4394bac714cb597f5ba1a81865185e35c782d0317e7abc0b52d49ff8e10f787bedf86f08148e3dbd2d2d478caa2a2893d31db7d5ee51339883fe84d3004440f16cb3797a7fab0f627d3ebd79217e995488e785cd6bb7b96b9d306f8109daa9cfc4162f9839f60fb965bcb3b56a5fa787549c153a4c80027398f73a617b90b7f24f437b140cd3ac832c0b75ec98b9423b275782988a9fd426937b8f82fbb0e88a622934643fb6335c1a080a4d13125544b04585d5f5295be7cd2c8be364246ea3d5df3e837b39a85074575a1fa2f4799050460110bdfb20795c8a9172a20f61b95e1c5c43eccd0c2c155b67385366142c63409cb3fb488e7aba6c8930f7f151abf1c24a54bd21c3f7a06856ea9db35beddecb30d2c61f533a3d0590bdbb438c6f2a2286dfc3c71b383354f0abad72771c2cc3687b50c2298783e53857cf26058ed78d0c1cf53786eb8d006a058ee3c85a7b2b836b5d03ef782709ce8f2725548e557b3de45a395a669a15f1d910e97015d22ac70020cab7e2531e8b1f739b023b49e742203e9e19a7fe0053826a9a2fe2e118d3b83498c2cb308573202ad41aa4a390aee4b6b5dd2164e5c5cd1b5f68b7d5632cf7dbb9a9139663c9aac53a74b2c6fc73cad80e228a186ba027f6f32f0182d62503e04fcced385f2e7d2e11c00940622ebd533b4d144689082f9777e5b16c36f9af9066e0ad6564d43" - blockBytes, err := hex.DecodeString(blockHex) - require.NoError(err) - - _, err = Parse(blockBytes) - require.NoError(err) +func TestParseBytes(t *testing.T) { + chainID := ids.ID{4} + tests := []struct { + name string + hex string + expectedErr error + }{ + { + name: "duplicate extensions in certificate", + hex: "0000000000000100000000000000000000000000000000000000000000000000000000000000000000000000007b0000000000000002000004bd308204b9308202a1a003020102020100300d06092a864886f70d01010b050030003020170d3939313233313030303030305a180f32313232303830333233323835335a300030820222300d06092a864886f70d01010105000382020f003082020a0282020100c2b2de1c16924d9b9254a0d5b80a4bc5f9beaa4f4f40a0e4efb69eb9b55d7d37f8c82328c237d7c5b451f5427b487284fa3f365f9caa53c7fcfef8d7a461d743bd7d88129f2da62b877ebe9d6feabf1bd12923e6c12321382c782fc3bb6b6cb4986a937a1edc3814f4e621e1a62053deea8c7649e43edd97ab6b56315b00d9ab5026bb9c31fb042dc574ba83c54e720e0120fcba2e8a66b77839be3ece0d4a6383ef3f76aac952b49a15b65e18674cd1340c32cecbcbaf80ae45be001366cb56836575fb0ab51ea44bf7278817e99b6b180fdd110a49831a132968489822c56692161bbd372cf89d9b8ee5a734cff15303b3a960ee78d79e76662a701941d9ec084429f26707f767e9b1d43241c0e4f96655d95c1f4f4aa00add78eff6bf0a6982766a035bf0b465786632c5bb240788ca0fdf032d8815899353ea4bec5848fd30118711e5b356bde8a0da074cc25709623225e734ff5bd0cf65c40d9fd8fccf746d8f8f35145bcebcf378d2b086e57d78b11e84f47fa467c4d037f92bff6dd4e934e0189b58193f24c4222ffb72b5c06361cf68ca64345bc3e230cc0f40063ad5f45b1659c643662996328c2eeddcd760d6f7c9cbae081ccc065844f7ea78c858564a408979764de882793706acc67d88092790dff567ed914b03355330932616a0f26f994b963791f0b1dbd8df979db86d1ea490700a3120293c3c2b10bef10203010001a33c303a300e0603551d0f0101ff0404030204b030130603551d25040c300a06082b0601050507030230130603551d25040c300a06082b06010505070302300d06092a864886f70d01010b05000382020100a21a0d73ec9ef4eb39f810557ac70b0b775772b8bae5f42c98565bc50b5b2c57317aa9cb1da12f55d0aac7bb36a00cd4fd0d7384c4efa284b53520c5a3c4b8a65240b393eeab02c802ea146c0728c3481c9e8d3aaad9d4dd7607103dcfaa96da83460adbe18174ed5b71bde7b0a93d4fb52234a9ff54e3fd25c5b74790dfb090f2e59dc5907357f510cc3a0b70ccdb87aee214def794b316224f318b471ffa13b66e44b467670e881cb1628c99c048a503376d9b6d7b8eef2e7be47ff7d5c1d56221f4cf7fa2519b594cb5917815c64dc75d8d281bcc99b5a12899b08f2ca0f189857b64a1afc5963337f3dd6e79390e85221569f6dbbb13aadce06a3dfb5032f0cc454809627872cd7cd0cea5eba187723f07652c8abc3fc42bd62136fc66287f2cc19a7cb416923ad1862d7f820b55cacb65e43731cb6df780e2651e457a3438456aeeeb278ad9c0ad2e760f6c1cbe276eeb621c8a4e609b5f2d902beb3212e3e45df99497021ff536d0b56390c5d785a8bf7909f6b61bdc705d7d92ae22f58e7b075f164a0450d82d8286bf449072751636ab5185f59f518b845a75d112d6f7b65223479202cff67635e2ad88106bc8a0cc9352d87c5b182ac19a4680a958d814a093acf46730f87da0df6926291d02590f215041b44a0a1a32eeb3a52cddabc3d256689bace18a8d85e644cf9137cce3718f7caac1cb16ae06e874f4c701000000010300000200b8e3a4d9a4394bac714cb597f5ba1a81865185e35c782d0317e7abc0b52d49ff8e10f787bedf86f08148e3dbd2d2d478caa2a2893d31db7d5ee51339883fe84d3004440f16cb3797a7fab0f627d3ebd79217e995488e785cd6bb7b96b9d306f8109daa9cfc4162f9839f60fb965bcb3b56a5fa787549c153a4c80027398f73a617b90b7f24f437b140cd3ac832c0b75ec98b9423b275782988a9fd426937b8f82fbb0e88a622934643fb6335c1a080a4d13125544b04585d5f5295be7cd2c8be364246ea3d5df3e837b39a85074575a1fa2f4799050460110bdfb20795c8a9172a20f61b95e1c5c43eccd0c2c155b67385366142c63409cb3fb488e7aba6c8930f7f151abf1c24a54bd21c3f7a06856ea9db35beddecb30d2c61f533a3d0590bdbb438c6f2a2286dfc3c71b383354f0abad72771c2cc3687b50c2298783e53857cf26058ed78d0c1cf53786eb8d006a058ee3c85a7b2b836b5d03ef782709ce8f2725548e557b3de45a395a669a15f1d910e97015d22ac70020cab7e2531e8b1f739b023b49e742203e9e19a7fe0053826a9a2fe2e118d3b83498c2cb308573202ad41aa4a390aee4b6b5dd2164e5c5cd1b5f68b7d5632cf7dbb9a9139663c9aac53a74b2c6fc73cad80e228a186ba027f6f32f0182d62503e04fcced385f2e7d2e11c00940622ebd533b4d144689082f9777e5b16c36f9af9066e0ad6564d43", + expectedErr: nil, + }, + { + name: "gibberish", + hex: "000102030405", + expectedErr: codec.ErrUnknownVersion, + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + require := require.New(t) + + bytes, err := hex.DecodeString(test.hex) + require.NoError(err) + + _, err = Parse(bytes, chainID) + require.ErrorIs(err, test.expectedErr) + }) + } } func TestParseHeader(t *testing.T) { @@ -85,50 +162,3 @@ func TestParseHeader(t *testing.T) { equalHeader(require, builtHeader, parsedHeader) } - -func TestParseOption(t *testing.T) { - require := require.New(t) - - parentID := ids.ID{1} - innerBlockBytes := []byte{3} - - builtOption, err := BuildOption(parentID, innerBlockBytes) - require.NoError(err) - - builtOptionBytes := builtOption.Bytes() - - parsedOption, err := Parse(builtOptionBytes) - require.NoError(err) - - equalOption(require, builtOption, parsedOption) -} - -func TestParseUnsigned(t *testing.T) { - require := require.New(t) - - parentID := ids.ID{1} - timestamp := time.Unix(123, 0) - pChainHeight := uint64(2) - innerBlockBytes := []byte{3} - - builtBlock, err := BuildUnsigned(parentID, timestamp, pChainHeight, innerBlockBytes) - require.NoError(err) - - builtBlockBytes := builtBlock.Bytes() - parsedBlockIntf, err := Parse(builtBlockBytes) - require.NoError(err) - - parsedBlock, ok := parsedBlockIntf.(SignedBlock) - require.True(ok) - - equal(require, ids.Empty, builtBlock, parsedBlock) -} - -func TestParseGibberish(t *testing.T) { - require := require.New(t) - - bytes := []byte{0, 1, 2, 3, 4, 5} - - _, err := Parse(bytes) - require.ErrorIs(err, codec.ErrUnknownVersion) -} diff --git a/vms/proposervm/block_test.go b/vms/proposervm/block_test.go index 1d16222f012..d8c867058f5 100644 --- a/vms/proposervm/block_test.go +++ b/vms/proposervm/block_test.go @@ -179,10 +179,12 @@ func TestPreDurangoValidatorNodeBlockBuiltDelaysTests(t *testing.T) { localTime := parentBlk.Timestamp().Add(proposer.MaxVerifyDelay - time.Second) proVM.Set(localTime) - childBlk, err := proVM.BuildBlock(ctx) + childBlkIntf, err := proVM.BuildBlock(ctx) require.NoError(err) - require.IsType(&postForkBlock{}, childBlk) - require.Equal(proVM.ctx.NodeID, childBlk.(*postForkBlock).Proposer()) // signed block + require.IsType(&postForkBlock{}, childBlkIntf) + + childBlk := childBlkIntf.(*postForkBlock) + require.Equal(proVM.ctx.NodeID, childBlk.Proposer()) // signed block } { @@ -191,34 +193,41 @@ func TestPreDurangoValidatorNodeBlockBuiltDelaysTests(t *testing.T) { localTime := parentBlk.Timestamp().Add(proposer.MaxVerifyDelay) proVM.Set(localTime) - childBlk, err := proVM.BuildBlock(ctx) + childBlkIntf, err := proVM.BuildBlock(ctx) require.NoError(err) - require.IsType(&postForkBlock{}, childBlk) - require.Equal(ids.EmptyNodeID, childBlk.(*postForkBlock).Proposer()) // signed block + require.IsType(&postForkBlock{}, childBlkIntf) + + childBlk := childBlkIntf.(*postForkBlock) + require.Equal(ids.EmptyNodeID, childBlk.Proposer()) // unsigned block } { - // Set local clock among MaxVerifyDelay and MaxBuildDelay from parent timestamp - // Check that child block is unsigned + // Set local clock between MaxVerifyDelay and MaxBuildDelay from parent + // timestamp. + // Check that child block is unsigned. localTime := parentBlk.Timestamp().Add((proposer.MaxVerifyDelay + proposer.MaxBuildDelay) / 2) proVM.Set(localTime) - childBlk, err := proVM.BuildBlock(ctx) + childBlkIntf, err := proVM.BuildBlock(ctx) require.NoError(err) - require.IsType(&postForkBlock{}, childBlk) - require.Equal(ids.EmptyNodeID, childBlk.(*postForkBlock).Proposer()) // unsigned so no proposer + require.IsType(&postForkBlock{}, childBlkIntf) + + childBlk := childBlkIntf.(*postForkBlock) + require.Equal(ids.EmptyNodeID, childBlk.Proposer()) // unsigned block } { - // Set local clock after MaxBuildDelay from parent timestamp - // Check that child block is unsigned + // Set local clock after MaxBuildDelay from parent timestamp. + // Check that child block is unsigned. localTime := parentBlk.Timestamp().Add(proposer.MaxBuildDelay) proVM.Set(localTime) - childBlk, err := proVM.BuildBlock(ctx) + childBlkIntf, err := proVM.BuildBlock(ctx) require.NoError(err) - require.IsType(&postForkBlock{}, childBlk) - require.Equal(ids.EmptyNodeID, childBlk.(*postForkBlock).Proposer()) // unsigned so no proposer + require.IsType(&postForkBlock{}, childBlkIntf) + + childBlk := childBlkIntf.(*postForkBlock) + require.Equal(ids.EmptyNodeID, childBlk.Proposer()) // unsigned block } } @@ -332,10 +341,12 @@ func TestPreDurangoNonValidatorNodeBlockBuiltDelaysTests(t *testing.T) { localTime := parentBlk.Timestamp().Add(proposer.MaxBuildDelay) proVM.Set(localTime) - childBlk, err := proVM.BuildBlock(ctx) + childBlkIntf, err := proVM.BuildBlock(ctx) require.NoError(err) - require.IsType(&postForkBlock{}, childBlk) - require.Equal(ids.EmptyNodeID, childBlk.(*postForkBlock).Proposer()) // unsigned so no proposer + require.IsType(&postForkBlock{}, childBlkIntf) + + childBlk := childBlkIntf.(*postForkBlock) + require.Equal(ids.EmptyNodeID, childBlk.Proposer()) // unsigned block } } diff --git a/vms/proposervm/pre_fork_block.go b/vms/proposervm/pre_fork_block.go index ffb618eed59..737659cacc0 100644 --- a/vms/proposervm/pre_fork_block.go +++ b/vms/proposervm/pre_fork_block.go @@ -5,18 +5,24 @@ package proposervm import ( "context" + "errors" "fmt" "time" "go.uber.org/zap" "github.com/ava-labs/avalanchego/database" + "github.com/ava-labs/avalanchego/ids" "github.com/ava-labs/avalanchego/snow/choices" "github.com/ava-labs/avalanchego/snow/consensus/snowman" "github.com/ava-labs/avalanchego/vms/proposervm/block" ) -var _ Block = (*preForkBlock)(nil) +var ( + _ Block = (*preForkBlock)(nil) + + errChildOfPreForkBlockHasProposer = errors.New("child of pre-fork block has proposer") +) type preForkBlock struct { snowman.Block @@ -167,8 +173,8 @@ func (b *preForkBlock) verifyPostForkChild(ctx context.Context, child *postForkB } // Verify the lack of signature on the node - if err := child.SignedBlock.Verify(false, b.vm.ctx.ChainID); err != nil { - return err + if child.SignedBlock.Proposer() != ids.EmptyNodeID { + return errChildOfPreForkBlockHasProposer } // Verify the inner block and track it as verified diff --git a/vms/proposervm/state/block_state.go b/vms/proposervm/state/block_state.go index 64a58885168..8e888332e9c 100644 --- a/vms/proposervm/state/block_state.go +++ b/vms/proposervm/state/block_state.go @@ -109,7 +109,7 @@ func (s *blockState) GetBlock(blkID ids.ID) (block.Block, choices.Status, error) } // The key was in the database - blk, err := block.Parse(blkWrapper.Block) + blk, err := block.ParseWithoutVerification(blkWrapper.Block) if err != nil { return nil, choices.Unknown, err } diff --git a/vms/proposervm/vm.go b/vms/proposervm/vm.go index ccb07fec83f..02af69b11dd 100644 --- a/vms/proposervm/vm.go +++ b/vms/proposervm/vm.go @@ -513,7 +513,7 @@ func (vm *VM) setLastAcceptedMetadata(ctx context.Context) error { } func (vm *VM) parsePostForkBlock(ctx context.Context, b []byte) (PostForkBlock, error) { - statelessBlock, err := statelessblock.Parse(b) + statelessBlock, err := statelessblock.Parse(b, vm.ctx.ChainID) if err != nil { return nil, err }