Skip to content

Commit

Permalink
Replace Guardian Key with abstracted Guardian Signer (wormhole-founda…
Browse files Browse the repository at this point in the history 
…tion#4120)

* node: add guardiansigner node/pkg

* node: replace use of guardian key with guardian signer

* node: replace use of vaa.AddSigner with guardian signer

* node: add nolint for armor import and fix test

* node: handle error returned from signing

* apply draft review suggestions

* apply pr reviews

* apply pr reviews

* apply pr reviews

* apply pr reviews

---------

Co-authored-by: pleasew8t <[email protected]>
  • Loading branch information
@pleasew8t
pleasew8t and pleasew8t authored Oct 17, 2024
1 parent a0dd60f commit e82db71
Show file tree
Hide file tree
Showing 26 changed files with 568 additions and 196 deletions.
15 changes: 9 additions & 6 deletions node/cmd/guardiand/adminclient.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import (
"github.com/spf13/pflag"
"golang.org/x/crypto/sha3"

"github.com/certusone/wormhole/node/pkg/common"
"github.com/certusone/wormhole/node/pkg/guardiansigner"
gossipv1 "github.com/certusone/wormhole/node/pkg/proto/gossip/v1"
publicrpcv1 "github.com/certusone/wormhole/node/pkg/proto/publicrpc/v1"
"github.com/wormhole-foundation/wormhole/sdk"
Expand Down Expand Up @@ -102,7 +102,7 @@ var AdminCmd = &cobra.Command{
}

var AdminClientSignWormchainAddress = &cobra.Command{
Use: "sign-wormchain-address [/path/to/guardianKey] [wormchain-validator-address]",
Use: "sign-wormchain-address [vaa-signer-uri] [wormchain-validator-address]",
Short: "Sign a wormchain validator address. Only sign the address that you control the key for and will be for your validator.",
RunE: runSignWormchainValidatorAddress,
Args: cobra.ExactArgs(2),
Expand Down Expand Up @@ -236,22 +236,25 @@ func getPublicRPCServiceClient(ctx context.Context, addr string) (*grpc.ClientCo
}

func runSignWormchainValidatorAddress(cmd *cobra.Command, args []string) error {
guardianKeyPath := args[0]
guardianSignerUri := args[0]
wormchainAddress := args[1]
if !strings.HasPrefix(wormchainAddress, "wormhole") || strings.HasPrefix(wormchainAddress, "wormholeval") {
return errors.New("must provide a bech32 address that has 'wormhole' prefix")
}
gk, err := common.LoadGuardianKey(guardianKeyPath, *unsafeDevnetMode)

guardianSigner, err := guardiansigner.NewGuardianSignerFromUri(guardianSignerUri, *unsafeDevnetMode)
if err != nil {
return fmt.Errorf("failed to load guardian key: %w", err)
return fmt.Errorf("failed to create new guardian signer from uri: %w", err)
}

addr, err := types.GetFromBech32(wormchainAddress, "wormhole")
if err != nil {
return fmt.Errorf("failed to decode wormchain address: %w", err)
}

// Hash and sign address
addrHash := crypto.Keccak256Hash(sdk.SignedWormchainAddressPrefix, addr)
sig, err := crypto.Sign(addrHash[:], gk)
sig, err := guardianSigner.Sign(addrHash.Bytes())
if err != nil {
return fmt.Errorf("failed to sign wormchain address: %w", err)
}
Expand Down
49 changes: 35 additions & 14 deletions node/cmd/guardiand/node.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"syscall"
"time"

"github.com/certusone/wormhole/node/pkg/guardiansigner"
"github.com/certusone/wormhole/node/pkg/watchers"
"github.com/certusone/wormhole/node/pkg/watchers/ibc"
ethcrypto "github.com/ethereum/go-ethereum/crypto"
Expand Down Expand Up @@ -62,8 +63,9 @@ var (

statusAddr *string

guardianKeyPath *string
solanaContract *string
guardianKeyPath *string
guardianSignerUri *string
solanaContract *string

ethRPC *string
ethContract *string
Expand Down Expand Up @@ -269,7 +271,8 @@ func init() {

dataDir = NodeCmd.Flags().String("dataDir", "", "Data directory")

guardianKeyPath = NodeCmd.Flags().String("guardianKey", "", "Path to guardian key (required)")
guardianKeyPath = NodeCmd.Flags().String("guardianKey", "", "Path to guardian key")
guardianSignerUri = NodeCmd.Flags().String("guardianSignerUri", "", "Guardian signer URI")
solanaContract = NodeCmd.Flags().String("solanaContract", "", "Address of the Solana program (required)")

ethRPC = node.RegisterFlagWithValidationOrFail(NodeCmd, "ethRPC", "Ethereum RPC URL", "ws://eth-devnet:8545", []string{"ws", "wss"})
Expand Down Expand Up @@ -628,7 +631,22 @@ func runNode(cmd *cobra.Command, args []string) {
logger.Fatal("Please specify --nodeKey")
}
if *guardianKeyPath == "" {
logger.Fatal("Please specify --guardianKey")
// This if-statement is nested, since checking if both are empty at once will always result in the else-branch
// being executed if at least one is specified. For example, in the case where the signer URI is specified and
// the guardianKeyPath not, then the else-statement will create an empty `file://` URI.
if *guardianSignerUri == "" {
logger.Fatal("Please specify --guardianKey or --guardianSignerUri")
}
} else {
// To avoid confusion, require that only guardianKey or guardianSignerUri can be specified
if *guardianSignerUri != "" {
logger.Fatal("Please only specify --guardianKey or --guardianSignerUri")
}

// If guardianKeyPath is set, set guardianSignerUri to the file signer URI, pointing to guardianKeyPath.
// This ensures that the signer-abstracted guardian has backwards compatibility with guardians that would
// just like to ignore the new guardianSignerUri altogether.
*guardianSignerUri = fmt.Sprintf("file://%s", *guardianKeyPath)
}
if *adminSocketPath == "" {
logger.Fatal("Please specify --adminSocket")
Expand All @@ -650,20 +668,23 @@ func runNode(cmd *cobra.Command, args []string) {

// In devnet mode, we generate a deterministic guardian key and write it to disk.
if env == common.UnsafeDevNet {
err := devnet.GenerateAndStoreDevnetGuardianKey(*guardianKeyPath)
if err != nil {
logger.Fatal("failed to generate devnet guardian key", zap.Error(err))
// Only if the signer is file-based should we generate the deterministic key and write it to disk
if st, _, _ := guardiansigner.ParseSignerUri(*guardianSignerUri); st == guardiansigner.FileSignerType {
err := devnet.GenerateAndStoreDevnetGuardianKey(*guardianKeyPath)
if err != nil {
logger.Fatal("failed to generate devnet guardian key", zap.Error(err))
}
}
}

// Load guardian key
gk, err := common.LoadGuardianKey(*guardianKeyPath, env == common.UnsafeDevNet)
// Create the Guardian Signer
guardianSigner, err := guardiansigner.NewGuardianSignerFromUri(*guardianSignerUri, env == common.UnsafeDevNet)
if err != nil {
logger.Fatal("failed to load guardian key", zap.Error(err))
logger.Fatal("failed to create a new guardian signer", zap.Error(err))
}

logger.Info("Loaded guardian key", zap.String(
"address", ethcrypto.PubkeyToAddress(gk.PublicKey).String()))
"address", ethcrypto.PubkeyToAddress(guardianSigner.PublicKey()).String()))

// Load p2p private key
var p2pKey libp2p_crypto.PrivKey
Expand Down Expand Up @@ -719,7 +740,7 @@ func runNode(cmd *cobra.Command, args []string) {
labels := map[string]string{
"node_name": *nodeName,
"node_key": peerID.String(),
"guardian_addr": ethcrypto.PubkeyToAddress(gk.PublicKey).String(),
"guardian_addr": ethcrypto.PubkeyToAddress(guardianSigner.PublicKey()).String(),
"network": *p2pNetworkID,
"version": version.Version(),
}
Expand Down Expand Up @@ -1060,7 +1081,7 @@ func runNode(cmd *cobra.Command, args []string) {
info.PromRemoteURL = *promRemoteURL
info.Labels = map[string]string{
"node_name": *nodeName,
"guardian_addr": ethcrypto.PubkeyToAddress(gk.PublicKey).String(),
"guardian_addr": ethcrypto.PubkeyToAddress(guardianSigner.PublicKey()).String(),
"network": *p2pNetworkID,
"version": version.Version(),
"product": "wormhole",
Expand Down Expand Up @@ -1585,7 +1606,7 @@ func runNode(cmd *cobra.Command, args []string) {

guardianNode := node.NewGuardianNode(
env,
gk,
guardianSigner,
)

guardianOptions := []*node.GuardianOption{
Expand Down
45 changes: 23 additions & 22 deletions node/hack/accountant/send_obs.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,12 @@ package main

import (
"context"
"crypto/ecdsa"
"encoding/hex"
"time"

"github.com/certusone/wormhole/node/pkg/accountant"
"github.com/certusone/wormhole/node/pkg/common"
"github.com/certusone/wormhole/node/pkg/guardiansigner"
"github.com/certusone/wormhole/node/pkg/wormconn"
"github.com/wormhole-foundation/wormhole/sdk/vaa"

Expand All @@ -26,7 +26,7 @@ func main() {
wormchainURL := string("localhost:9090")
wormchainKeyPath := string("./dev.wormchain.key")
contract := "wormhole14hj2tavq8fpesdwxxcu44rty3hh90vhujrvcmstl4zr3txmfvw9srrg465"
guardianKeyPath := string("./dev.guardian.key")
guardianSignerUri := string("file://dev.guardian.key")

wormchainKey, err := wormconn.LoadWormchainPrivKey(wormchainKeyPath, "test0000")
if err != nil {
Expand All @@ -44,40 +44,41 @@ func main() {
zap.String("senderAddress", wormchainConn.SenderAddress()),
)

logger.Info("Loading guardian key", zap.String("guardianKeyPath", guardianKeyPath))
gk, err := common.LoadGuardianKey(guardianKeyPath, true)
logger.Info("Initializing guardian signer", zap.String("guardianSignerUri", guardianSignerUri))
guardianSigner, err := guardiansigner.NewGuardianSignerFromUri(guardianSignerUri, true)

if err != nil {
logger.Fatal("failed to load guardian key", zap.Error(err))
}

sequence := uint64(time.Now().Unix())
timestamp := time.Now()

if !testSubmit(ctx, logger, gk, wormchainConn, contract, "0000000000000000000000000290fb167208af455bb137780163b7b7a9a10c16", timestamp, sequence, true, "Submit should succeed") {
if !testSubmit(ctx, logger, guardianSigner, wormchainConn, contract, "0000000000000000000000000290fb167208af455bb137780163b7b7a9a10c16", timestamp, sequence, true, "Submit should succeed") {
return
}

if !testSubmit(ctx, logger, gk, wormchainConn, contract, "0000000000000000000000000290fb167208af455bb137780163b7b7a9a10c16", timestamp, sequence, true, "Already commited should succeed") {
if !testSubmit(ctx, logger, guardianSigner, wormchainConn, contract, "0000000000000000000000000290fb167208af455bb137780163b7b7a9a10c16", timestamp, sequence, true, "Already commited should succeed") {
return
}

sequence += 10
if !testSubmit(ctx, logger, gk, wormchainConn, contract, "0000000000000000000000000290fb167208af455bb137780163b7b7a9a10c17", timestamp, sequence, false, "Bad emitter address should fail") {
if !testSubmit(ctx, logger, guardianSigner, wormchainConn, contract, "0000000000000000000000000290fb167208af455bb137780163b7b7a9a10c17", timestamp, sequence, false, "Bad emitter address should fail") {
return
}

sequence += 10
if !testBatch(ctx, logger, gk, wormchainConn, contract, timestamp, sequence) {
if !testBatch(ctx, logger, guardianSigner, wormchainConn, contract, timestamp, sequence) {
return
}

sequence += 10
if !testBatchWithcommitted(ctx, logger, gk, wormchainConn, contract, timestamp, sequence) {
if !testBatchWithcommitted(ctx, logger, guardianSigner, wormchainConn, contract, timestamp, sequence) {
return
}

sequence += 10
if !testBatchWithDigestError(ctx, logger, gk, wormchainConn, contract, timestamp, sequence) {
if !testBatchWithDigestError(ctx, logger, guardianSigner, wormchainConn, contract, timestamp, sequence) {
return
}

Expand All @@ -87,7 +88,7 @@ func main() {
func testSubmit(
ctx context.Context,
logger *zap.Logger,
gk *ecdsa.PrivateKey,
guardianSigner guardiansigner.GuardianSigner,
wormchainConn *wormconn.ClientConn,
contract string,
emitterAddressStr string,
Expand All @@ -112,7 +113,7 @@ func testSubmit(
}

msgs := []*common.MessagePublication{&msg}
txResp, err := submit(ctx, logger, gk, wormchainConn, contract, msgs)
txResp, err := submit(ctx, logger, guardianSigner, wormchainConn, contract, msgs)
if err != nil {
logger.Error("failed to broadcast Observation request", zap.String("test", tag), zap.Error(err))
return false
Expand Down Expand Up @@ -151,7 +152,7 @@ func testSubmit(
func testBatch(
ctx context.Context,
logger *zap.Logger,
gk *ecdsa.PrivateKey,
guardianSigner guardiansigner.GuardianSigner,
wormchainConn *wormconn.ClientConn,
contract string,
timestamp time.Time,
Expand Down Expand Up @@ -191,7 +192,7 @@ func testBatch(
}
msgs = append(msgs, &msg2)

txResp, err := submit(ctx, logger, gk, wormchainConn, contract, msgs)
txResp, err := submit(ctx, logger, guardianSigner, wormchainConn, contract, msgs)
if err != nil {
logger.Error("failed to broadcast Observation request", zap.Error(err))
return false
Expand Down Expand Up @@ -229,7 +230,7 @@ func testBatch(
func testBatchWithcommitted(
ctx context.Context,
logger *zap.Logger,
gk *ecdsa.PrivateKey,
guardianSigner guardiansigner.GuardianSigner,
wormchainConn *wormconn.ClientConn,
contract string,
timestamp time.Time,
Expand All @@ -256,7 +257,7 @@ func testBatchWithcommitted(
}
msgs = append(msgs, &msg1)

_, err := submit(ctx, logger, gk, wormchainConn, contract, msgs)
_, err := submit(ctx, logger, guardianSigner, wormchainConn, contract, msgs)
if err != nil {
logger.Error("failed to submit initial observation that should work", zap.Error(err))
return false
Expand All @@ -283,7 +284,7 @@ func testBatchWithcommitted(
msg3 := msg1
msgs = append(msgs, &msg3)

txResp, err := submit(ctx, logger, gk, wormchainConn, contract, msgs)
txResp, err := submit(ctx, logger, guardianSigner, wormchainConn, contract, msgs)
if err != nil {
logger.Error("failed to broadcast Observation request", zap.Error(err))
return false
Expand Down Expand Up @@ -321,7 +322,7 @@ func testBatchWithcommitted(
func testBatchWithDigestError(
ctx context.Context,
logger *zap.Logger,
gk *ecdsa.PrivateKey,
guardianSigner guardiansigner.GuardianSigner,
wormchainConn *wormconn.ClientConn,
contract string,
timestamp time.Time,
Expand All @@ -348,7 +349,7 @@ func testBatchWithDigestError(
}
msgs = append(msgs, &msg1)

_, err := submit(ctx, logger, gk, wormchainConn, contract, msgs)
_, err := submit(ctx, logger, guardianSigner, wormchainConn, contract, msgs)
if err != nil {
logger.Error("failed to submit initial observation that should work", zap.Error(err))
return false
Expand Down Expand Up @@ -376,7 +377,7 @@ func testBatchWithDigestError(
msg3.Nonce = msg3.Nonce + 1
msgs = append(msgs, &msg3)

txResp, err := submit(ctx, logger, gk, wormchainConn, contract, msgs)
txResp, err := submit(ctx, logger, guardianSigner, wormchainConn, contract, msgs)
if err != nil {
logger.Error("failed to submit second observation that should work", zap.Error(err))
return false
Expand Down Expand Up @@ -429,13 +430,13 @@ func testBatchWithDigestError(
func submit(
ctx context.Context,
logger *zap.Logger,
gk *ecdsa.PrivateKey,
guardianSigner guardiansigner.GuardianSigner,
wormchainConn *wormconn.ClientConn,
contract string,
msgs []*common.MessagePublication,
) (*sdktx.BroadcastTxResponse, error) {
gsIndex := uint32(0)
guardianIndex := uint32(0)

return accountant.SubmitObservationsToContract(ctx, logger, gk, gsIndex, guardianIndex, wormchainConn, contract, accountant.SubmitObservationPrefix, msgs)
return accountant.SubmitObservationsToContract(ctx, logger, guardianSigner, gsIndex, guardianIndex, wormchainConn, contract, accountant.SubmitObservationPrefix, msgs)
}
10 changes: 5 additions & 5 deletions node/pkg/accountant/accountant.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,13 @@ package accountant

import (
"context"
"crypto/ecdsa"
"fmt"
"sync"
"time"

"github.com/certusone/wormhole/node/pkg/common"
"github.com/certusone/wormhole/node/pkg/db"
"github.com/certusone/wormhole/node/pkg/guardiansigner"
gossipv1 "github.com/certusone/wormhole/node/pkg/proto/gossip/v1"
"github.com/certusone/wormhole/node/pkg/supervisor"
sdktypes "github.com/cosmos/cosmos-sdk/types"
Expand Down Expand Up @@ -83,7 +83,7 @@ type Accountant struct {
wsUrl string
wormchainConn AccountantWormchainConn
enforceFlag bool
gk *ecdsa.PrivateKey
guardianSigner guardiansigner.GuardianSigner
gst *common.GuardianSetState
guardianAddr ethCommon.Address
msgChan chan<- *common.MessagePublication
Expand Down Expand Up @@ -120,7 +120,7 @@ func NewAccountant(
enforceFlag bool, // whether or not accountant should be enforced
nttContract string, // the address of the NTT smart contract on wormchain
nttWormchainConn AccountantWormchainConn, // used for communicating with the NTT smart contract
gk *ecdsa.PrivateKey, // the guardian key used for signing observation requests
guardianSigner guardiansigner.GuardianSigner, // the guardian signer used for signing observation requests
gst *common.GuardianSetState, // used to get the current guardian set index when sending observation requests
msgChan chan<- *common.MessagePublication, // the channel where transfers received by the accountant runnable should be published
env common.Environment, // Controls the set of token bridges to be monitored
Expand All @@ -134,9 +134,9 @@ func NewAccountant(
wsUrl: wsUrl,
wormchainConn: wormchainConn,
enforceFlag: enforceFlag,
gk: gk,
guardianSigner: guardianSigner,
gst: gst,
guardianAddr: ethCrypto.PubkeyToAddress(gk.PublicKey),
guardianAddr: ethCrypto.PubkeyToAddress(guardianSigner.PublicKey()),
msgChan: msgChan,
tokenBridges: make(validEmitters),
pendingTransfers: make(map[string]*pendingEntry),
Expand Down
Loading

0 comments on commit e82db71

Please sign in to comment.