review fixes

go.mod

@@ -12,7 +12,6 @@ require (
 	github.com/deckarep/golang-set/v2 v2.1.0
 	github.com/dop251/goja v0.0.0-20230806174421-c933cf95e127
 	github.com/ethereum/go-ethereum v1.13.14
-	github.com/fjl/memsize v0.0.2
 	github.com/fsnotify/fsnotify v1.6.0
 	github.com/gballet/go-libpcsclite v0.0.0-20191108122812-4678299bea08
 	github.com/gballet/go-verkle v0.1.1-0.20231031103413-a67434b50f46

go.sum

@@ -185,8 +185,6 @@ github.com/ethereum/go-ethereum v1.13.14 h1:EwiY3FZP94derMCIam1iW4HFVrSgIcpsu0Hw
 github.com/ethereum/go-ethereum v1.13.14/go.mod h1:TN8ZiHrdJwSe8Cb6x+p0hs5CxhJZPbqB7hHkaUXcmIU=
 github.com/fasthttp-contrib/websocket v0.0.0-20160511215533-1f3b11f56072/go.mod h1:duJ4Jxv5lDcvg4QuQr0oowTf7dz4/CR8NtyCooz9HL8=
 github.com/fatih/structs v1.1.0/go.mod h1:9NiDSp5zOcgEDl+j00MP/WkGVPOlPRLejGD8Ga6PJ7M=
-github.com/fjl/memsize v0.0.2 h1:27txuSD9or+NZlnOWdKUxeBzTAUkWCVh+4Gf2dWFOzA=
-github.com/fjl/memsize v0.0.2/go.mod h1:VvhXpOYNQvB+uIk2RvXzuaQtkQJzzIx6lSBe1xv7hi0=
 github.com/frankban/quicktest v1.14.4 h1:g2rn0vABPOOXmZUj+vbmUp0lPoXEMuhTpIluN0XL9UY=
 github.com/frankban/quicktest v1.14.4/go.mod h1:4ptaffx2x8+WTWXmUCuVU6aPUX1/Mz7zb5vbUoiM6w0=
 github.com/fsnotify/fsnotify v1.4.7/go.mod h1:jwhsz4b93w/PPRr/qN1Yymfu8t87LnFCMoQvtojpjFo=

plugin/evm/vm_warp_test.go

@@ -882,6 +882,7 @@ func TestClearWarpDB(t *testing.T) {
 
 	// Restart VM with the same database default should not prune the warp db
 	vm = &VM{}
+	// we need new context since the previous one has registered metrics.
 	ctx, _, _, _, _ = setupGenesis(t, genesisJSONLatest)
 	err = vm.Initialize(context.Background(), ctx, db, genesisBytes, []byte{}, []byte{}, issuer, []*commonEng.Fx{}, &enginetest.Sender{})
 	require.NoError(t, err)

warp/backend.go

@@ -38,7 +38,7 @@ type Backend interface {
 	// GetMessageSignature validates the message and returns the signature of the requested message.
 	GetMessageSignature(message *avalancheWarp.UnsignedMessage) ([]byte, error)
 
-	// GetBlockSignature validates blockID and returns the signature of the requested message hash.
+	// GetBlockSignature returns the signature of a hash payload containing blockID if it's the ID of an accepted block.
 	GetBlockSignature(blockID ids.ID) ([]byte, error)
 
 	// GetMessage retrieves the [unsignedMessage] from the warp backend database if available
@@ -113,6 +113,7 @@ func (b *backend) initOffChainMessages(offchainMessages [][]byte) error {
 
 func (b *backend) AddMessage(unsignedMessage *avalancheWarp.UnsignedMessage) error {
 	messageID := unsignedMessage.ID()
+	log.Debug("Adding warp message to backend", "messageID", messageID)
 
 	// In the case when a node restarts, and possibly changes its bls key, the cache gets emptied but the database does not.
 	// So to avoid having incorrect signatures saved in the database after a bls key change, we save the full message in the database.
@@ -121,13 +122,9 @@ func (b *backend) AddMessage(unsignedMessage *avalancheWarp.UnsignedMessage) err
 		return fmt.Errorf("failed to put warp signature in db: %w", err)
 	}
 
-	sig, err := b.warpSigner.Sign(unsignedMessage)
-	if err != nil {
+	if _, err := b.signMessage(unsignedMessage); err != nil {
 		return fmt.Errorf("failed to sign warp message: %w", err)
 	}
-
-	b.signatureCache.Put(messageID, sig)
-	log.Debug("Adding warp message to backend", "messageID", messageID)
 	return nil
 }
 
@@ -140,7 +137,7 @@ func (b *backend) GetMessageSignature(unsignedMessage *avalancheWarp.UnsignedMes
 	}
 
 	if err := b.verifyMessage(unsignedMessage); err != nil {
-		return []byte{}, fmt.Errorf("failed to validate warp message: %w", err)
+		return nil, fmt.Errorf("failed to validate warp message: %w", err)
 	}
 	return b.signMessage(unsignedMessage)
 }
@@ -150,25 +147,25 @@ func (b *backend) GetBlockSignature(blockID ids.ID) ([]byte, error) {
 
 	blockHashPayload, err := payload.NewHash(blockID)
 	if err != nil {
-		return []byte{}, fmt.Errorf("failed to create new block hash payload: %w", err)
+		return nil, fmt.Errorf("failed to create new block hash payload: %w", err)
 	}
 
 	unsignedMessage, err := avalancheWarp.NewUnsignedMessage(b.networkID, b.sourceChainID, blockHashPayload.Bytes())
 	if err != nil {
-		return []byte{}, fmt.Errorf("failed to create new unsigned warp message: %w", err)
+		return nil, fmt.Errorf("failed to create new unsigned warp message: %w", err)
 	}
 
 	if sig, ok := b.signatureCache.Get(unsignedMessage.ID()); ok {
 		return sig, nil
 	}
 
 	if err := b.verifyBlockMessage(blockHashPayload); err != nil {
-		return []byte{}, fmt.Errorf("failed to validate block message: %w", err)
+		return nil, fmt.Errorf("failed to validate block message: %w", err)
 	}
 
 	sig, err := b.signMessage(unsignedMessage)
 	if err != nil {
-		return []byte{}, fmt.Errorf("failed to sign block message: %w", err)
+		return nil, fmt.Errorf("failed to sign block message: %w", err)
 	}
 	return sig, nil
 }
@@ -198,7 +195,7 @@ func (b *backend) GetMessage(messageID ids.ID) (*avalancheWarp.UnsignedMessage,
 func (b *backend) signMessage(unsignedMessage *avalancheWarp.UnsignedMessage) ([]byte, error) {
 	sig, err := b.warpSigner.Sign(unsignedMessage)
 	if err != nil {
-		return []byte{}, fmt.Errorf("failed to sign warp message: %w", err)
+		return nil, fmt.Errorf("failed to sign warp message: %w", err)
 	}
 
 	b.signatureCache.Put(unsignedMessage.ID(), sig)

warp/handlers/signature_request.go

@@ -55,7 +55,6 @@ func (s *SignatureRequestHandler) OnMessageSignatureRequest(ctx context.Context,
 		if err != nil {
 			log.Debug("Unknown warp signature requested", "messageID", signatureRequest.MessageID)
 			s.stats.IncMessageSignatureMiss()
-			signature = [bls.SignatureLen]byte{}
 		} else {
 			s.stats.IncMessageSignatureHit()
 			copy(signature[:], sig)
@@ -86,7 +85,6 @@ func (s *SignatureRequestHandler) OnBlockSignatureRequest(ctx context.Context, n
 	if err != nil {
 		log.Debug("Unknown warp signature requested", "blockID", request.BlockID)
 		s.stats.IncBlockSignatureMiss()
-		signature = [bls.SignatureLen]byte{}
 	} else {
 		s.stats.IncBlockSignatureHit()
 		copy(signature[:], sig)

warp/verifier_backend.go

@@ -21,10 +21,7 @@ const (
 
 // Verify implements the acp118.Verifier interface
 func (b *backend) Verify(_ context.Context, unsignedMessage *avalancheWarp.UnsignedMessage, _ []byte) *common.AppError {
-	if err := b.verifyMessage(unsignedMessage); err != nil {
-		return err
-	}
-	return nil
+	return b.verifyMessage(unsignedMessage)
 }
 
 // verifyMessage verifies the signature of the message
@@ -66,7 +63,8 @@ func (b *backend) verifyMessage(unsignedMessage *avalancheWarp.UnsignedMessage)
 	return nil
 }
 
-// verifyBlockMessage verifies the block message (payload.Hash)
+// verifyBlockMessage returns nil if blockHashPayload contains the ID
+// of an accepted block indicating it should be signed by the VM.
 func (b *backend) verifyBlockMessage(blockHashPayload *payload.Hash) *common.AppError {
 	blockID := blockHashPayload.Hash
 	_, err := b.blockClient.GetAcceptedBlock(context.TODO(), blockID)
@@ -81,9 +79,12 @@ func (b *backend) verifyBlockMessage(blockHashPayload *payload.Hash) *common.App
 	return nil
 }
 
-// verifyAddressedCall verifies the addressed call message
+// verifyAddressedCall returns nil if addressedCall is parseable to a known payload type and
+// passes type specific validation, indicating it should be signed by the VM.
+// Note currently there are no valid payload types so this call always returns common.AppError
+// with ParseErrCode.
 func (b *backend) verifyAddressedCall(addressedCall *payload.AddressedCall) *common.AppError {
-	// Further, parse the payload to see if it is a known type.
+	// Parse the payload to see if it is a known type.
 	parsed, err := messages.Parse(addressedCall.Payload)
 	if err != nil {
 		b.stats.IncMessageParseFail()

warp/verifier_backend_test.go

@@ -56,7 +56,6 @@ func TestAddressedCallSignatures(t *testing.T) {
 			},
 			verifyStats: func(t *testing.T, stats *verifierStats) {
 				require.EqualValues(t, 0, stats.messageParseFail.Snapshot().Count())
-				require.EqualValues(t, 0, stats.addressedCallSignatureValidationFail.Snapshot().Count())
 				require.EqualValues(t, 0, stats.blockSignatureValidationFail.Snapshot().Count())
 			},
 		},
@@ -66,7 +65,6 @@ func TestAddressedCallSignatures(t *testing.T) {
 			},
 			verifyStats: func(t *testing.T, stats *verifierStats) {
 				require.EqualValues(t, 0, stats.messageParseFail.Snapshot().Count())
-				require.EqualValues(t, 0, stats.addressedCallSignatureValidationFail.Snapshot().Count())
 				require.EqualValues(t, 0, stats.blockSignatureValidationFail.Snapshot().Count())
 			},
 		},
@@ -80,7 +78,6 @@ func TestAddressedCallSignatures(t *testing.T) {
 			},
 			verifyStats: func(t *testing.T, stats *verifierStats) {
 				require.EqualValues(t, 1, stats.messageParseFail.Snapshot().Count())
-				require.EqualValues(t, 0, stats.addressedCallSignatureValidationFail.Snapshot().Count())
 				require.EqualValues(t, 0, stats.blockSignatureValidationFail.Snapshot().Count())
 			},
 			err: &common.AppError{Code: ParseErrCode},
@@ -147,17 +144,19 @@ func TestBlockSignatures(t *testing.T) {
 	require.NoError(t, err)
 
 	warpSigner := avalancheWarp.NewSigner(blsSecretKey, snowCtx.NetworkID, snowCtx.ChainID)
-	blkID := ids.GenerateTestID()
-	blockClient := warptest.MakeBlockClient(blkID)
-
-	unknownBlockID := ids.GenerateTestID()
+	knownBlkID := ids.GenerateTestID()
+	blockClient := warptest.MakeBlockClient(knownBlkID)
 
 	toMessageBytes := func(id ids.ID) []byte {
 		idPayload, err := payload.NewHash(id)
-		require.NoError(t, err)
+		if err != nil {
+			panic(err)
+		}
 
 		msg, err := avalancheWarp.NewUnsignedMessage(snowCtx.NetworkID, snowCtx.ChainID, idPayload.Bytes())
-		require.NoError(t, err)
+		if err != nil {
+			panic(err)
+		}
 
 		return msg.Bytes()
 	}
@@ -169,26 +168,25 @@ func TestBlockSignatures(t *testing.T) {
 	}{
 		"known block": {
 			setup: func() (request []byte, expectedResponse []byte) {
-				hashPayload, err := payload.NewHash(blkID)
+				hashPayload, err := payload.NewHash(knownBlkID)
 				require.NoError(t, err)
 				unsignedMessage, err := avalancheWarp.NewUnsignedMessage(snowCtx.NetworkID, snowCtx.ChainID, hashPayload.Bytes())
 				require.NoError(t, err)
 				signature, err := warpSigner.Sign(unsignedMessage)
 				require.NoError(t, err)
-				return toMessageBytes(blkID), signature[:]
+				return toMessageBytes(knownBlkID), signature[:]
 			},
 			verifyStats: func(t *testing.T, stats *verifierStats) {
-				require.EqualValues(t, 0, stats.addressedCallSignatureValidationFail.Snapshot().Count())
 				require.EqualValues(t, 0, stats.blockSignatureValidationFail.Snapshot().Count())
 				require.EqualValues(t, 0, stats.messageParseFail.Snapshot().Count())
 			},
 		},
 		"unknown block": {
 			setup: func() (request []byte, expectedResponse []byte) {
+				unknownBlockID := ids.GenerateTestID()
 				return toMessageBytes(unknownBlockID), nil
 			},
 			verifyStats: func(t *testing.T, stats *verifierStats) {
-				require.EqualValues(t, 0, stats.addressedCallSignatureValidationFail.Snapshot().Count())
 				require.EqualValues(t, 1, stats.blockSignatureValidationFail.Snapshot().Count())
 				require.EqualValues(t, 0, stats.messageParseFail.Snapshot().Count())
 			},

warp/stats.go → warp/verifier_stats.go

@@ -9,24 +9,17 @@ import (
 
 type verifierStats struct {
 	messageParseFail metrics.Counter
-	// AddressedCall metrics
-	addressedCallSignatureValidationFail metrics.Counter
 	// BlockRequest metrics
 	blockSignatureValidationFail metrics.Counter
 }
 
 func newVerifierStats() *verifierStats {
 	return &verifierStats{
-		messageParseFail:                     metrics.NewRegisteredCounter("message_parse_fail", nil),
-		addressedCallSignatureValidationFail: metrics.NewRegisteredCounter("addressed_call_signature_validation_fail", nil),
-		blockSignatureValidationFail:         metrics.NewRegisteredCounter("block_signature_validation_fail", nil),
+		messageParseFail:             metrics.NewRegisteredCounter("message_parse_fail", nil),
+		blockSignatureValidationFail: metrics.NewRegisteredCounter("block_signature_validation_fail", nil),
 	}
 }
 
-func (h *verifierStats) IncAddressedCallSignatureValidationFail() {
-	h.addressedCallSignatureValidationFail.Inc(1)
-}
-
 func (h *verifierStats) IncBlockSignatureValidationFail() {
 	h.blockSignatureValidationFail.Inc(1)
 }