Verify signatures during Parse (#3046)

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()
 		}

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)
 		}

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
 		}

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",

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,16 +28,18 @@ type Block interface {
 	Bytes() []byte
 
 	initialize(bytes []byte) error
+	verify(chainID ids.ID) error
 }
 
 type SignedBlock interface {
 	Block
 
 	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
+}

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) {

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) {

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
+}

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 {