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.

Sign up for GitHub

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

Configuration support fully specified source to destination mapping #59

Merged
merged 34 commits into from
Oct 27, 2023
Merged

Configuration support fully specified source to destination mapping #59

Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
9ab0d98
add allowed destination chain id list
gwen917 Oct 13, 2023
618a185
add unit tests
gwen917 Oct 13, 2023
fddb357
fix comments
gwen917 Oct 13, 2023
8b5eacd
fix typo and config validation
gwen917 Oct 16, 2023
62d4fac
fix PR comments
gwen917 Oct 16, 2023
4c0d019
fix PR comments
gwen917 Oct 16, 2023
4240369
fix
gwen917 Oct 16, 2023
199b3e7
add global cfg
cam-schultz Oct 17, 2023
6f56a22
add source chain ID to log
cam-schultz Oct 17, 2023
724178f
combine source subnet config validation
cam-schultz Oct 17, 2023
5b6a211
add config readme
cam-schultz Oct 17, 2023
89ab862
rename supported destinations
cam-schultz Oct 17, 2023
836bce8
calculare allowed destinations on startup
cam-schultz Oct 17, 2023
c22b7bd
clarify error msg
cam-schultz Oct 17, 2023
be3f30f
fix typo
cam-schultz Oct 19, 2023
b3318d6
Merge branch 'main' into p2p-config
cam-schultz Oct 19, 2023
5e75b5f
readme cleanup
cam-schultz Oct 20, 2023
6973bdb
cleanup readme
cam-schultz Oct 24, 2023
c80f97b
validate dst chains in source validation
cam-schultz Oct 24, 2023
947d5d7
clarify comment
cam-schultz Oct 24, 2023
9ae1307
correct json tag
cam-schultz Oct 24, 2023
5b5decb
Merge branch 'main' into p2p-config
cam-schultz Oct 24, 2023
50dd208
Merge branch 'main' into p2p-config
cam-schultz Oct 25, 2023
c05368e
remove default p-chain node
cam-schultz Oct 25, 2023
6c8a869
denote api methods in readme
cam-schultz Oct 25, 2023
77fe281
remove global config
cam-schultz Oct 25, 2023
cb82c07
move supported destination check to relayer
cam-schultz Oct 25, 2023
50548ac
check supported destination unit test
cam-schultz Oct 26, 2023
468fa3f
lazily initialize source id fields
cam-schultz Oct 26, 2023
03b790d
remove redundant validation step
cam-schultz Oct 26, 2023
5630533
Merge branch 'main' into p1p-config
cam-schultz Oct 26, 2023
87634cb
use require in test
cam-schultz Oct 26, 2023
0e5eee6
store source ids on initialization
cam-schultz Oct 26, 2023
8498771
combine duplicate iterations
cam-schultz Oct 27, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
84 changes: 84 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,90 @@ The relayer binary accepts a path to a JSON configuration file as the sole argum
./build/awm-relayer --config-file path-to-config
```

### Configuration
Copy link

Choose a reason for hiding this comment

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

Love this new README section 🙌


The relayer is configured via a JSON file, the path to which is passed in via the `--config-file` command line argument. The following configuration options are available:

`"log-level": "debug" | "info" | "warn" | "error" | "fatal" | "panic"`
- The log level for the relayer. Defaults to `info`.

`"network-id": integer`
- The ID of the Avalanche network to which the relayer will connect. Defaults to `1` (Mainnet).

`"p-chain-api-url": string`
- The URL of the Avalanche P-Chain API node to which the relayer will connect. Defaults to `https://api.avax.network`.

`"encrypt-connection": boolean`
- Whether or not to encrypt the connection to the P-Chain API node. Defaults to `true`.

`"storage-location": string`
- The path to the directory in which the relayer will store its state. Defaults to `./awm-relayer-storage`.

`"source-subnets": []SourceSubnets`
- The list of source subnets to support. Each `SourceSubnet` has the following configuration:

`"subnet-id": string`
- cb58-encoded Subnet ID

`"blockchain-id": string`
- cb58-encoded Blockchain ID

`"vm": string`
- The VM type of the source subnet.

`"api-node-host": string`
- The host of the source subnet's API node.

`"api-node-port": integer`
- The port of the source subnet's API node.

`"encrypt-connection": boolean`
- Whether or not to encrypt the connection to the source subnet's API node.

`"rpc-endpoint": string`
- The RPC endpoint of the source subnet's API node. If provided, `api-node-host`, `api-node-port`, and `encrypt-connection` are ignored

`"ws-endpoint": string`
- The WebSocket endpoint of the source subnet's API node. If provided, `api-node-host`, `api-node-port`, and `encrypt-connection` are ignored

`"message-contracts": map[string]MessageProtocolConfig`
- Map of contract addresses to the config options of the protocol at that address. Each `MessageProtocolConfig` consists of a unique `message-format` name, and the raw JSON `settings`

`"supported-destinations": []string`
- List of destination subnet IDs that the source subnet supports. If empty, then all destinations are supported.

`"destination-subnets": []DestinationSubnets`
- The list of destination subnets to support. Each `DestinationSubnet` has the following configuration:

`"subnet-id": string`
- cb58-encoded Subnet ID

`"blockchain-id": string`
- cb58-encoded Blockchain ID

`"vm": string`
- The VM type of the source subnet.

`"api-node-host": string`
- The host of the source subnet's API node.

`"api-node-port": integer`
- The port of the source subnet's API node.

`"encrypt-connection": boolean`
- Whether or not to encrypt the connection to the source subnet's API node.

`"rpc-endpoint": string`
- The RPC endpoint of the source subnet's API node. If provided, `api-node-host`, `api-node-port`, and `encrypt-connection` are ignored

`"ws-endpoint": string`
- The WebSocket endpoint of the source subnet's API node. If provided, `api-node-host`, `api-node-port`, and `encrypt-connection` are ignored

`"account-private-key": string`
- The hex-encoded private key to use for signing transactions on the destination subnet. May be provided by the environment variable `ACCOUNT_PRIVATE_KEY`. Each `destination-subnet` may use a separate private key by appending the blockchain ID to the private key environment variable name, e.g. `ACCOUNT_PRIVATE_KEY_11111111111111111111111111111111LpoYY`



## Architecture

**Note:** The relayer in its current state supports Teleporter messages between `subnet-evm` instances. A handful of abstractions have been added to make the relayer extensible to other Warp message formats and VM types, but this work is ongoing.
Expand Down
117 changes: 81 additions & 36 deletions config/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@ import (
"github.com/spf13/viper"
)

// global config singleton
var globalConfig Config

const (
relayerPrivateKeyBytes = 32
accountPrivateKeyEnvVarName = "ACCOUNT_PRIVATE_KEY"
Expand All @@ -37,15 +40,19 @@ type MessageProtocolConfig struct {
Settings map[string]interface{} `mapstructure:"settings" json:"settings"`
}
type SourceSubnet struct {
SubnetID string `mapstructure:"subnet-id" json:"subnet-id"`
ChainID string `mapstructure:"chain-id" json:"chain-id"`
VM string `mapstructure:"vm" json:"vm"`
APINodeHost string `mapstructure:"api-node-host" json:"api-node-host"`
APINodePort uint32 `mapstructure:"api-node-port" json:"api-node-port"`
EncryptConnection bool `mapstructure:"encrypt-connection" json:"encrypt-connection"`
RPCEndpoint string `mapstructure:"rpc-endpoint" json:"rpc-endpoint"`
WSEndpoint string `mapstructure:"ws-endpoint" json:"ws-endpoint"`
MessageContracts map[string]MessageProtocolConfig `mapstructure:"message-contracts" json:"message-contracts"`
SubnetID string `mapstructure:"subnet-id" json:"subnet-id"`
ChainID string `mapstructure:"chain-id" json:"chain-id"`
VM string `mapstructure:"vm" json:"vm"`
APINodeHost string `mapstructure:"api-node-host" json:"api-node-host"`
APINodePort uint32 `mapstructure:"api-node-port" json:"api-node-port"`
EncryptConnection bool `mapstructure:"encrypt-connection" json:"encrypt-connection"`
RPCEndpoint string `mapstructure:"rpc-endpoint" json:"rpc-endpoint"`
WSEndpoint string `mapstructure:"ws-endpoint" json:"ws-endpoint"`
MessageContracts map[string]MessageProtocolConfig `mapstructure:"message-contracts" json:"message-contracts"`
SupportedDestinations []string `mapstructure:"supported-destinations" json:"allowed-destinations"`
cam-schultz marked this conversation as resolved.
Show resolved Hide resolved

// convenience field
cam-schultz marked this conversation as resolved.
Show resolved Hide resolved
supportedDestinationsMap map[ids.ID]bool
}

type DestinationSubnet struct {
Expand Down Expand Up @@ -152,6 +159,8 @@ func BuildConfig(v *viper.Viper) (Config, bool, error) {
}
cfg.PChainAPIURL = pChainapiUrl

globalConfig = cfg

return cfg, optionOverwritten, nil
}

Expand All @@ -165,17 +174,8 @@ func (c *Config) Validate() error {
if _, err := url.ParseRequestURI(c.PChainAPIURL); err != nil {
return err
}
sourceChains := set.NewSet[string](len(c.SourceSubnets))
for _, s := range c.SourceSubnets {
if err := s.Validate(); err != nil {
return err
}
if sourceChains.Contains(s.ChainID) {
return fmt.Errorf("configured source subnets must have unique chain IDs")
}
sourceChains.Add(s.ChainID)
}

// Validate the destination chains
destinationChains := set.NewSet[string](len(c.DestinationSubnets))
for _, s := range c.DestinationSubnets {
if err := s.Validate(); err != nil {
Expand All @@ -187,27 +187,31 @@ func (c *Config) Validate() error {
destinationChains.Add(s.ChainID)
}

return nil
}

// GetSourceIDs returns the Subnet and Chain IDs of all subnets configured as a source
func (cfg *Config) GetSourceIDs() ([]ids.ID, []ids.ID, error) {
var sourceSubnetIDs []ids.ID
var sourceChainIDs []ids.ID
for _, s := range cfg.SourceSubnets {
subnetID, err := ids.FromString(s.SubnetID)
if err != nil {
return nil, nil, fmt.Errorf("invalid subnetID in configuration. error: %v", err)
// Validate the source chains, and validate that the allowed destinations are configured as destinations
sourceChains := set.NewSet[string](len(c.SourceSubnets))
for _, s := range c.SourceSubnets {
if err := s.Validate(); err != nil {
return err
}
sourceSubnetIDs = append(sourceSubnetIDs, subnetID)
if sourceChains.Contains(s.ChainID) {
return fmt.Errorf("configured source subnets must have unique chain IDs")
}
sourceChains.Add(s.ChainID)

chainID, err := ids.FromString(s.ChainID)
if err != nil {
return nil, nil, fmt.Errorf("invalid subnetID in configuration. error: %v", err)
for _, blockchainID := range s.SupportedDestinations {
cam-schultz marked this conversation as resolved.
Show resolved Hide resolved
if !destinationChains.Contains(blockchainID) {
return fmt.Errorf("configured source subnet %s has a supported destination blockchain ID %s that is not configured as a destination blockchain",
s.SubnetID,
blockchainID)
}
}
sourceChainIDs = append(sourceChainIDs, chainID)
}
return sourceSubnetIDs, sourceChainIDs, nil

return nil
}

func (s *SourceSubnet) GetSupportedDestinations() map[ids.ID]bool {
return s.supportedDestinationsMap
}

func (s *SourceSubnet) Validate() error {
Expand Down Expand Up @@ -244,6 +248,23 @@ func (s *SourceSubnet) Validate() error {
}
}

// Validate the allowed destinations
gwen917 marked this conversation as resolved.
Show resolved Hide resolved
for _, chainIDs := range s.SupportedDestinations {
if _, err := ids.FromString(chainIDs); err != nil {
return fmt.Errorf("invalid chainID in source subnet configuration. Provided ID: %s", chainIDs)
}
}
cam-schultz marked this conversation as resolved.
Show resolved Hide resolved

// Store the allowed destinations for future use
s.supportedDestinationsMap = make(map[ids.ID]bool)
for _, chainIDStr := range s.SupportedDestinations {
chainID, err := ids.FromString(chainIDStr)
if err != nil {
return fmt.Errorf("invalid chainID in configuration. error: %v", err)
}
s.supportedDestinationsMap[chainID] = true
}

return nil
}

Expand Down Expand Up @@ -373,3 +394,27 @@ func (s *DestinationSubnet) GetRelayerAccountInfo() (*ecdsa.PrivateKey, common.A
pkBytes = append(pkBytes, pk.PublicKey.Y.Bytes()...)
return pk, common.BytesToAddress(crypto.Keccak256(pkBytes)), nil
}

//
// Global config getters
//

// GetSourceIDs returns the Subnet and Chain IDs of all subnets configured as a source
func GetSourceIDs() ([]ids.ID, []ids.ID, error) {
Copy link

Choose a reason for hiding this comment

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

why did we move this to using a global config variable vs a receiver function?

Copy link
Collaborator

Choose a reason for hiding this comment

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

After it's constructed, the configuration is a static singleton, so this pattern makes access more straightforward. The alternative is passing the option from the config down the call stack to where it's used, unnecessarily complicating interfaces along the way.

Copy link

Choose a reason for hiding this comment

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

I have a preference, though not a strong preference, to stay away from the global variables and to instead pass in the configs, similar to previously how we had global var for destination clients. This also helps prevent in the future from accidentally ovewriting values in the global config

Copy link
Collaborator

Choose a reason for hiding this comment

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

On second thought, I agree we should pass the config down the callstack rather than have a global variable. I've made this change.

var sourceSubnetIDs []ids.ID
var sourceChainIDs []ids.ID
for _, s := range globalConfig.SourceSubnets {
subnetID, err := ids.FromString(s.SubnetID)
if err != nil {
return nil, nil, fmt.Errorf("invalid subnetID in configuration. error: %v", err)
}
sourceSubnetIDs = append(sourceSubnetIDs, subnetID)

chainID, err := ids.FromString(s.ChainID)
if err != nil {
return nil, nil, fmt.Errorf("invalid subnetID in configuration. error: %v", err)
}
sourceChainIDs = append(sourceChainIDs, chainID)
}
return sourceSubnetIDs, sourceChainIDs, nil
}
2 changes: 1 addition & 1 deletion main/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ func main() {

// Initialize the global app request network
logger.Info("Initializing app request network")
sourceSubnetIDs, sourceChainIDs, err := cfg.GetSourceIDs()
sourceSubnetIDs, sourceChainIDs, err := config.GetSourceIDs()
if err != nil {
logger.Error(
"Failed to get source IDs",
Expand Down
2 changes: 2 additions & 0 deletions messages/message_manager.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ func NewMessageManager(
messageProtocolAddress common.Hash,
messageProtocolConfig config.MessageProtocolConfig,
destinationClients map[ids.ID]vms.DestinationClient,
supportedDestinationsChainIDs map[ids.ID]bool,
) (MessageManager, error) {
format := messageProtocolConfig.MessageFormat
switch config.ParseMessageProtocol(format) {
Expand All @@ -44,6 +45,7 @@ func NewMessageManager(
messageProtocolAddress,
messageProtocolConfig,
destinationClients,
supportedDestinationsChainIDs,
)
default:
return nil, fmt.Errorf("invalid message format %s", format)
Expand Down
19 changes: 19 additions & 0 deletions messages/teleporter/message_manager.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ type messageManager struct {
// The cache is keyed by the Warp message ID, NOT the Teleporter message ID
teleporterMessageCache *cache.LRU[ids.ID, *TeleporterMessage]
destinationClients map[ids.ID]vms.DestinationClient
supportedDestinationss map[ids.ID]bool
cam-schultz marked this conversation as resolved.
Show resolved Hide resolved

logger logging.Logger
}
Expand All @@ -43,6 +44,7 @@ func NewMessageManager(
messageProtocolAddress common.Hash,
messageProtocolConfig config.MessageProtocolConfig,
destinationClients map[ids.ID]vms.DestinationClient,
supportedDestinationss map[ids.ID]bool,
) (*messageManager, error) {
// Marshal the map and unmarshal into the Teleporter config
data, err := json.Marshal(messageProtocolConfig.Settings)
Expand Down Expand Up @@ -71,6 +73,7 @@ func NewMessageManager(
teleporterMessageCache: teleporterMessageCache,
destinationClients: destinationClients,
logger: logger,
supportedDestinationss: supportedDestinationss,
}, nil
}

Expand Down Expand Up @@ -105,6 +108,22 @@ func (m *messageManager) ShouldSendMessage(warpMessageInfo *vmtypes.WarpMessageI
if !ok {
return false, fmt.Errorf("relayer not configured to deliver to destination. destinationChainID=%s", destinationChainID.String())
}

// If supportedDestinationss is empty, then all destinations are allowed
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems opposite than the expected behavior (one normally assumes that if the allow list is empty, then nothing is allowed).

Copy link
Collaborator

Choose a reason for hiding this comment

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

This is similar to how the "allowedRelayerAddresses" field is handled in TeleporterMessenger: https://github.com/ava-labs/teleporter/blob/main/contracts/src/Teleporter/TeleporterMessenger.sol#L126

The expected use case here is that if the supportedDestinations JSON key is not provided, we don't filter any of the configured destinations. This would be useful if the relayer operator wants messages from subnet A to be relayed to anybody, but messages from subnet B to only be relayed to subnet C, for example.

// If supportedDestinationss is not empty, then only the allowed destinations are allowed
if len(m.supportedDestinationss) > 0 {
if allowed, exist := m.supportedDestinationss[destinationChainID]; !exist || !allowed {
m.logger.Info(
"Relayer not configured to relay between source and destination",
zap.String("sourceChainID", warpMessageInfo.WarpUnsignedMessage.SourceChainID.String()),
zap.String("destinationChainID", destinationChainID.String()),
zap.String("warpMessageID", warpMessageInfo.WarpUnsignedMessage.ID().String()),
zap.String("teleporterMessageID", teleporterMessage.MessageID.String()),
)
return false, nil
Copy link
Collaborator

Choose a reason for hiding this comment

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

In the case above where the destination chain ID does not have a destination client, we return false and a non-nil error. I noticed this case is different in that we return false, nil. Inspecting the code, it looks like it's incorrect to return an error in the case above (unless I'm missing another check on whether or not the destination chain ID is supported elsewhere).

@cam-schultz could you confirm this?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah, that case should not be an error. With the upcoming subnet-evm changes (as well as anycasting support in the future), I think ShouldSendMessage is the right place to check against the list of configured destinations.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thinking about it some more, we can clean up this implementation by using the configured allowed destinations as a filter in on the list of destination clients in NewRelayer before the call to NewMessageManager. We'd then pass in the filtered list of destination clients. In relayer.go, we'd add something like:

var filteredDestinationClients map[ids.ID]ethclient.Client
allowedDestinationChainIDs, err := sourceSubnetInfo.GetAllowedDestinations()
if len(m.allowedDestinations) > 0 { 
    filteredDestinationClients := make(map[ids.ID]ethclient.Client)
    for _, id := range m.allowedDestinations {
        filteredDestinationClients[id] = destinationClients[id]
    }
} else {
    filteredDestinationClients = destinationClients
}
...
messageManager, err := messages.NewMessageManager(logger, addressHash, config, filteredDestinationClients, allowedDestinationChainIDs)

This would move logic for deciding which destinations are and are not valid to the relayer constructor which ones once, rather than the message manager ShouldSendMessage implementation, which runs for each message. When processing a message log, we could then reference the allowed destinations for that source subnet here

Copy link

Choose a reason for hiding this comment

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

would agree that we should move the destination client filtering logic to NewRelayer to prevent checking each message.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Moved 👍

}
}

senderAddress := destinationClient.SenderAddress()
if !isAllowedRelayer(teleporterMessage.AllowedRelayerAddresses, senderAddress) {
m.logger.Info(
Expand Down
Loading