Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Types and refactoring for ACP-77 ValidatorUptime messages #1292

Open
wants to merge 14 commits into
base: master
Choose a base branch
from
7 changes: 3 additions & 4 deletions plugin/evm/vm_warp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,10 +110,9 @@ func TestSendWarpMessage(t *testing.T) {
logData := receipts[0].Logs[0].Data
unsignedMessage, err := warp.UnpackSendWarpEventDataToMessage(logData)
require.NoError(err)
unsignedMessageID := unsignedMessage.ID()

// Verify the signature cannot be fetched before the block is accepted
_, err = vm.warpBackend.GetMessageSignature(unsignedMessageID)
_, err = vm.warpBackend.GetMessageSignature(unsignedMessage)
require.Error(err)
_, err = vm.warpBackend.GetBlockSignature(blk.ID())
require.Error(err)
Expand All @@ -123,7 +122,7 @@ func TestSendWarpMessage(t *testing.T) {
vm.blockChain.DrainAcceptorQueue()

// Verify the message signature after accepting the block.
rawSignatureBytes, err := vm.warpBackend.GetMessageSignature(unsignedMessageID)
rawSignatureBytes, err := vm.warpBackend.GetMessageSignature(unsignedMessage)
require.NoError(err)
blsSignature, err := bls.SignatureFromBytes(rawSignatureBytes[:])
require.NoError(err)
Expand Down Expand Up @@ -596,7 +595,7 @@ func TestMessageSignatureRequestsToVM(t *testing.T) {
// Add the known message and get its signature to confirm.
err = vm.warpBackend.AddMessage(warpMessage)
require.NoError(t, err)
signature, err := vm.warpBackend.GetMessageSignature(warpMessage.ID())
signature, err := vm.warpBackend.GetMessageSignature(warpMessage)
require.NoError(t, err)

tests := map[string]struct {
Expand Down
45 changes: 39 additions & 6 deletions warp/backend.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"github.com/ava-labs/avalanchego/utils/crypto/bls"
avalancheWarp "github.com/ava-labs/avalanchego/vms/platformvm/warp"
"github.com/ava-labs/avalanchego/vms/platformvm/warp/payload"
"github.com/ava-labs/subnet-evm/warp/messages"
"github.com/ethereum/go-ethereum/ethdb"
"github.com/ethereum/go-ethereum/log"
)
Expand All @@ -36,8 +37,8 @@ type Backend interface {
// AddMessage signs [unsignedMessage] and adds it to the warp backend database
AddMessage(unsignedMessage *avalancheWarp.UnsignedMessage) error

// GetMessageSignature returns the signature of the requested message hash.
GetMessageSignature(messageID ids.ID) ([bls.SignatureLen]byte, error)
// GetMessageSignature returns the signature of the requested message.
GetMessageSignature(message *avalancheWarp.UnsignedMessage) ([bls.SignatureLen]byte, error)

// GetBlockSignature returns the signature of the requested message hash.
GetBlockSignature(blockID ids.ID) ([bls.SignatureLen]byte, error)
Expand Down Expand Up @@ -142,15 +143,16 @@ func (b *backend) AddMessage(unsignedMessage *avalancheWarp.UnsignedMessage) err
return nil
}

func (b *backend) GetMessageSignature(messageID ids.ID) ([bls.SignatureLen]byte, error) {
func (b *backend) GetMessageSignature(unsignedMessage *avalancheWarp.UnsignedMessage) ([bls.SignatureLen]byte, error) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why we replaced messageID with unsignedMessage? it seems we only use it for the messageID.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it is so the backend can sign arbitrary messages on the fly (it will need to inspect their contents, such as for validator uptime message, so knowing the ID aka hash is not enough).
I didn't want to make another struct that has access to the bls signer key.
Do you suggest a different code organization here?

messageID := unsignedMessage.ID()

log.Debug("Getting warp message from backend", "messageID", messageID)
if sig, ok := b.messageSignatureCache.Get(messageID); ok {
return sig, nil
}

unsignedMessage, err := b.GetMessage(messageID)
if err != nil {
return [bls.SignatureLen]byte{}, fmt.Errorf("failed to get warp message %s from db: %w", messageID.String(), err)
if err := b.ValidateMessage(unsignedMessage); err != nil {
return [bls.SignatureLen]byte{}, fmt.Errorf("failed to validate warp message: %w", err)
}

var signature [bls.SignatureLen]byte
Expand All @@ -164,6 +166,37 @@ func (b *backend) GetMessageSignature(messageID ids.ID) ([bls.SignatureLen]byte,
return signature, nil
}

func (b *backend) ValidateMessage(unsignedMessage *avalancheWarp.UnsignedMessage) error {
// Known on-chain messages should be signed
if _, err := b.GetMessage(unsignedMessage.ID()); err == nil {
return nil
}

// Try to parse the payload as an AddressedCall
addressedCall, err := payload.ParseAddressedCall(unsignedMessage.Payload)
if err != nil {
return fmt.Errorf("failed to parse unknown message as AddressedCall: %w", err)
}
Comment on lines +176 to +179
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what happens if that message type is not supposed to comfort any types. i.e a supposed to be a pure addressedcall without any specific type? (like current teleporter messages)


// Further, parse the payload to see if it is a known type.
parsed, err := messages.Parse(addressedCall.Payload)
if err != nil {
return fmt.Errorf("failed to parse unknown message: %w", err)
}

// Check if the message is a known type that can be signed on demand
signable, ok := parsed.(messages.Signable)
if !ok {
return fmt.Errorf("parsed message is not Signable: %T", signable)
}

// Check if the message should be signed according to its type
if err := signable.VerifyMesssage(addressedCall.SourceAddress); err != nil {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we don't need to sort it out right not but seems we might need a handler to take different interfaces (VM, providers etc). One example could be the uptime request needs the uptime manager (and/or the state) to verify the message.

return fmt.Errorf("failed to verify Signable message: %w", err)
}
return nil
}

func (b *backend) GetBlockSignature(blockID ids.ID) ([bls.SignatureLen]byte, error) {
log.Debug("Getting block from backend", "blockID", blockID)
if sig, ok := b.blockSignatureCache.Get(blockID); ok {
Expand Down
25 changes: 10 additions & 15 deletions warp/backend_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import (
"github.com/ava-labs/avalanchego/ids"
"github.com/ava-labs/avalanchego/utils"
"github.com/ava-labs/avalanchego/utils/crypto/bls"
"github.com/ava-labs/avalanchego/utils/hashing"
avalancheWarp "github.com/ava-labs/avalanchego/vms/platformvm/warp"
"github.com/ava-labs/avalanchego/vms/platformvm/warp/payload"
"github.com/ava-labs/subnet-evm/warp/warptest"
Expand Down Expand Up @@ -49,18 +48,17 @@ func TestClearDB(t *testing.T) {

// use multiple messages to test that all messages get cleared
payloads := [][]byte{[]byte("test1"), []byte("test2"), []byte("test3"), []byte("test4"), []byte("test5")}
messageIDs := []ids.ID{}
messages := make([]*avalancheWarp.UnsignedMessage, 0, len(payloads))

// add all messages
for _, payload := range payloads {
unsignedMsg, err := avalancheWarp.NewUnsignedMessage(networkID, sourceChainID, payload)
require.NoError(t, err)
messageID := hashing.ComputeHash256Array(unsignedMsg.Bytes())
messageIDs = append(messageIDs, messageID)
messages = append(messages, unsignedMsg)
err = backend.AddMessage(unsignedMsg)
require.NoError(t, err)
// ensure that the message was added
_, err = backend.GetMessageSignature(messageID)
_, err = backend.GetMessageSignature(unsignedMsg)
require.NoError(t, err)
}

Expand All @@ -74,9 +72,9 @@ func TestClearDB(t *testing.T) {
require.False(t, it.Next())

// ensure all messages have been deleted
for _, messageID := range messageIDs {
_, err := backend.GetMessageSignature(messageID)
require.ErrorContains(t, err, "failed to get warp message")
for _, message := range messages {
_, err := backend.GetMessageSignature(message)
require.ErrorContains(t, err, "failed to validate warp message")
}
}

Expand All @@ -94,8 +92,7 @@ func TestAddAndGetValidMessage(t *testing.T) {
require.NoError(t, err)

// Verify that a signature is returned successfully, and compare to expected signature.
messageID := testUnsignedMessage.ID()
signature, err := backend.GetMessageSignature(messageID)
signature, err := backend.GetMessageSignature(testUnsignedMessage)
require.NoError(t, err)

expectedSig, err := warpSigner.Sign(testUnsignedMessage)
Expand All @@ -113,8 +110,7 @@ func TestAddAndGetUnknownMessage(t *testing.T) {
require.NoError(t, err)

// Try getting a signature for a message that was not added.
messageID := testUnsignedMessage.ID()
_, err = backend.GetMessageSignature(messageID)
_, err = backend.GetMessageSignature(testUnsignedMessage)
require.Error(t, err)
}

Expand Down Expand Up @@ -162,8 +158,7 @@ func TestZeroSizedCache(t *testing.T) {
require.NoError(t, err)

// Verify that a signature is returned successfully, and compare to expected signature.
messageID := testUnsignedMessage.ID()
signature, err := backend.GetMessageSignature(messageID)
signature, err := backend.GetMessageSignature(testUnsignedMessage)
require.NoError(t, err)

expectedSig, err := warpSigner.Sign(testUnsignedMessage)
Expand Down Expand Up @@ -192,7 +187,7 @@ func TestOffChainMessages(t *testing.T) {
require.NoError(err)
require.Equal(testUnsignedMessage.Bytes(), msg.Bytes())

signature, err := b.GetMessageSignature(testUnsignedMessage.ID())
signature, err := b.GetMessageSignature(testUnsignedMessage)
require.NoError(err)
expectedSignatureBytes, err := warpSigner.Sign(msg)
require.NoError(err)
Expand Down
15 changes: 11 additions & 4 deletions warp/handlers/signature_request.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,13 +45,20 @@ func (s *SignatureRequestHandler) OnMessageSignatureRequest(ctx context.Context,
s.stats.UpdateMessageSignatureRequestTime(time.Since(startTime))
}()

signature, err := s.backend.GetMessageSignature(signatureRequest.MessageID)
var signature [bls.SignatureLen]byte
unsignedMessage, err := s.backend.GetMessage(signatureRequest.MessageID)
if err != nil {
log.Debug("Unknown warp signature requested", "messageID", signatureRequest.MessageID)
log.Debug("Unknown warp message requested", "messageID", signatureRequest.MessageID)
s.stats.IncMessageSignatureMiss()
signature = [bls.SignatureLen]byte{}
} else {
s.stats.IncMessageSignatureHit()
signature, err = s.backend.GetMessageSignature(unsignedMessage)
ceyonur marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
log.Debug("Unknown warp signature requested", "messageID", signatureRequest.MessageID)
s.stats.IncMessageSignatureMiss()
signature = [bls.SignatureLen]byte{}
} else {
s.stats.IncMessageSignatureHit()
}
}

response := message.SignatureResponse{Signature: signature}
Expand Down
14 changes: 7 additions & 7 deletions warp/handlers/signature_request_p2p.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,10 @@ const (
ErrFailedToMarshal
)

type AddressedCallHandler interface {
GetMessageSignature(*avalancheWarp.UnsignedMessage) ([bls.SignatureLen]byte, error)
}

// SignatureRequestHandlerP2P serves warp signature requests using the p2p
// framework from avalanchego. It is a peer.RequestHandler for
// message.MessageSignatureRequest.
Expand Down Expand Up @@ -79,11 +83,7 @@ func (s *SignatureRequestHandlerP2P) AppRequest(
var sig [bls.SignatureLen]byte
switch p := parsed.(type) {
case *payload.AddressedCall:
// Note we pass the unsigned message ID to GetMessageSignature since
// that is what the backend expects.
// However, we verify the types and format of the payload to ensure
// the message conforms to the ACP-118 spec.
sig, err = s.GetMessageSignature(unsignedMessage.ID())
sig, err = s.GetMessageSignature(unsignedMessage)
if err != nil {
s.stats.IncMessageSignatureMiss()
} else {
Expand Down Expand Up @@ -122,7 +122,7 @@ func (s *SignatureRequestHandlerP2P) AppRequest(
return respBytes, nil
}

func (s *SignatureRequestHandlerP2P) GetMessageSignature(messageID ids.ID) ([bls.SignatureLen]byte, error) {
func (s *SignatureRequestHandlerP2P) GetMessageSignature(message *avalancheWarp.UnsignedMessage) ([bls.SignatureLen]byte, error) {
startTime := time.Now()
s.stats.IncMessageSignatureRequest()

Expand All @@ -131,7 +131,7 @@ func (s *SignatureRequestHandlerP2P) GetMessageSignature(messageID ids.ID) ([bls
s.stats.UpdateMessageSignatureRequestTime(time.Since(startTime))
}()

return s.backend.GetMessageSignature(messageID)
return s.backend.GetMessageSignature(message)
}

func (s *SignatureRequestHandlerP2P) GetBlockSignature(blockID ids.ID) ([bls.SignatureLen]byte, error) {
Expand Down
5 changes: 2 additions & 3 deletions warp/handlers/signature_request_p2p_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,10 @@ func TestMessageSignatureHandlerP2P(t *testing.T) {
require.NoError(t, err)
msg, err := avalancheWarp.NewUnsignedMessage(snowCtx.NetworkID, snowCtx.ChainID, offchainPayload.Bytes())
require.NoError(t, err)
messageID := msg.ID()
require.NoError(t, backend.AddMessage(msg))
signature, err := backend.GetMessageSignature(messageID)
signature, err := backend.GetMessageSignature(msg)
require.NoError(t, err)
offchainSignature, err := backend.GetMessageSignature(offchainMessage.ID())
offchainSignature, err := backend.GetMessageSignature(offchainMessage)
require.NoError(t, err)

unknownPayload, err := payload.NewAddressedCall([]byte{0, 0, 0}, []byte("unknown message"))
Expand Down
4 changes: 2 additions & 2 deletions warp/handlers/signature_request_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,9 @@ func TestMessageSignatureHandler(t *testing.T) {
require.NoError(t, err)
messageID := msg.ID()
require.NoError(t, backend.AddMessage(msg))
signature, err := backend.GetMessageSignature(messageID)
signature, err := backend.GetMessageSignature(msg)
require.NoError(t, err)
offchainSignature, err := backend.GetMessageSignature(offchainMessage.ID())
offchainSignature, err := backend.GetMessageSignature(offchainMessage)
require.NoError(t, err)

unknownMessageID := ids.GenerateTestID()
Expand Down
33 changes: 33 additions & 0 deletions warp/messages/codec.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
// (c) 2024, Ava Labs, Inc. All rights reserved.
// See the file LICENSE for licensing terms.

package messages

import (
"errors"

"github.com/ava-labs/avalanchego/codec"
"github.com/ava-labs/avalanchego/codec/linearcodec"
"github.com/ava-labs/avalanchego/utils/units"
)

const (
CodecVersion = 0

MaxMessageSize = 24 * units.KiB
)

var Codec codec.Manager

func init() {
Codec = codec.NewManager(MaxMessageSize)
lc := linearcodec.NewDefault()

err := errors.Join(
lc.RegisterType(&ValidatorUptime{}),
Codec.RegisterCodec(CodecVersion, lc),
)
if err != nil {
panic(err)
}
}
45 changes: 45 additions & 0 deletions warp/messages/payload.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
// (c) 2024, Ava Labs, Inc. All rights reserved.
// See the file LICENSE for licensing terms.

package messages

import (
"errors"
"fmt"
)

var errWrongType = errors.New("wrong payload type")

// Payload provides a common interface for all payloads implemented by this
// package.
type Payload interface {
// Bytes returns the binary representation of this payload.
Bytes() []byte

// initialize the payload with the provided binary representation.
initialize(b []byte)
}

// Signable is an optional interface that payloads can implement to allow
// on-the-fly signing of incoming messages by the warp backend.
type Signable interface {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've made this an optional interface but we can make it required and have non-implementers always return an error if there is a preference for that

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how do we make it required?

VerifyMesssage(sourceAddress []byte) error
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀

}

func Parse(bytes []byte) (Payload, error) {
var payload Payload
if _, err := Codec.Unmarshal(bytes, &payload); err != nil {
return nil, err
}
payload.initialize(bytes)
return payload, nil
}

func initialize(p Payload) error {
bytes, err := Codec.Marshal(CodecVersion, &p)
if err != nil {
return fmt.Errorf("couldn't marshal %T payload: %w", p, err)
}
p.initialize(bytes)
return nil
}
Loading
Loading