Browse Source

wormchain: allow hot-swapping validator address association when guar… (#3576)

* wormchain: allow hot-swapping validator address association when guardian set size is 1

* Replace panics with require in tests, add additional context in comments, simplify checks and code flow
Nikhil Suri 1 year ago
parent
commit
5525caad08

+ 16 - 8
wormchain/x/wormhole/keeper/msg_server_register_account_as_guardian.go

@@ -12,7 +12,12 @@ import (
 	wormholesdk "github.com/wormhole-foundation/wormhole/sdk"
 	wormholesdk "github.com/wormhole-foundation/wormhole/sdk"
 )
 )
 
 
-// TODO(csongor): high-level overview of what this does
+// This function is used to onboard Wormhole Guardians as Validators on Wormchain.
+// It creates a 1:1 association between a Guardian addresss and a Wormchain validator address.
+// There is also a special case -- when the size of the Guardian set is 1, the Guardian is allowed to "hot-swap" their validator address in the mapping.
+// We include the special case to make it easier to shuffle things in testnets and local devnets.
+// 1. Guardian signs their validator address -- SIGNATURE=$(guardiand admin sign-wormchain-address <wormhole...>)
+// 2. Guardian submits $SIGNATURE to Wormchain via this handler, using their new validator address as the signer of the Wormchain tx.
 func (k msgServer) RegisterAccountAsGuardian(goCtx context.Context, msg *types.MsgRegisterAccountAsGuardian) (*types.MsgRegisterAccountAsGuardianResponse, error) {
 func (k msgServer) RegisterAccountAsGuardian(goCtx context.Context, msg *types.MsgRegisterAccountAsGuardian) (*types.MsgRegisterAccountAsGuardianResponse, error) {
 	ctx := sdk.UnwrapSDKContext(goCtx)
 	ctx := sdk.UnwrapSDKContext(goCtx)
 
 
@@ -40,16 +45,19 @@ func (k msgServer) RegisterAccountAsGuardian(goCtx context.Context, msg *types.M
 	// we don't allow registration of arbitrary public keys, since that would
 	// we don't allow registration of arbitrary public keys, since that would
 	// enable a DoS vector
 	// enable a DoS vector
 	latestGuardianSetIndex := k.Keeper.GetLatestGuardianSetIndex(ctx)
 	latestGuardianSetIndex := k.Keeper.GetLatestGuardianSetIndex(ctx)
-	consensusGuardianSetIndex, found := k.GetConsensusGuardianSetIndex(ctx)
-
-	if found && latestGuardianSetIndex == consensusGuardianSetIndex.Index {
-		return nil, types.ErrConsensusSetNotUpdatable
+	latestGuardianSet, guardianSetFound := k.Keeper.GetGuardianSet(ctx, latestGuardianSetIndex)
+	if !guardianSetFound {
+		return nil, types.ErrGuardianSetNotFound
 	}
 	}
 
 
-	latestGuardianSet, found := k.Keeper.GetGuardianSet(ctx, latestGuardianSetIndex)
+	consensusGuardianSetIndex, consensusIndexFound := k.GetConsensusGuardianSetIndex(ctx)
+	if !consensusIndexFound {
+		return nil, types.ErrConsensusSetUndefined
+	}
 
 
-	if !found {
-		return nil, types.ErrGuardianSetNotFound
+	// If the size of the guardian set is 1, allow hot-swapping the validator address.
+	if consensusIndexFound && latestGuardianSetIndex == consensusGuardianSetIndex.Index && len(latestGuardianSet.Keys) > 1 {
+		return nil, types.ErrConsensusSetNotUpdatable
 	}
 	}
 
 
 	if !latestGuardianSet.ContainsKey(guardianKeyAddr) {
 	if !latestGuardianSet.ContainsKey(guardianKeyAddr) {

+ 88 - 0
wormchain/x/wormhole/keeper/msg_server_register_account_as_guardian_test.go

@@ -0,0 +1,88 @@
+package keeper_test
+
+import (
+	"testing"
+
+	sdk "github.com/cosmos/cosmos-sdk/types"
+	"github.com/ethereum/go-ethereum/crypto"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	keepertest "github.com/wormhole-foundation/wormchain/testutil/keeper"
+	"github.com/wormhole-foundation/wormchain/x/wormhole/keeper"
+	"github.com/wormhole-foundation/wormchain/x/wormhole/types"
+	wormholesdk "github.com/wormhole-foundation/wormhole/sdk"
+	"github.com/wormhole-foundation/wormhole/sdk/vaa"
+)
+
+// hot-swap validator address when guardian set size is 1 (for testnets and local devnets)
+func TestRegisterAccountAsGuardianHotSwap(t *testing.T) {
+	// setup -- create guardian set of size 1
+	k, ctx := keepertest.WormholeKeeper(t)
+	guardians, privateKeys := createNGuardianValidator(k, ctx, 1)
+	k.SetConfig(ctx, types.Config{
+		GovernanceEmitter:     vaa.GovernanceEmitter[:],
+		GovernanceChain:       uint32(vaa.GovernanceChain),
+		ChainId:               uint32(vaa.ChainIDWormchain),
+		GuardianSetExpiration: 86400,
+	})
+	newValAddr_bz := [20]byte{}
+	newValAddr := sdk.AccAddress(newValAddr_bz[:])
+
+	set := createNewGuardianSet(k, ctx, guardians)
+	k.SetConsensusGuardianSetIndex(ctx, types.ConsensusGuardianSetIndex{Index: set.Index})
+
+	// execute msg_server function to associate new validator address account with the guardian address
+	context := sdk.WrapSDKContext(ctx)
+	msgServer := keeper.NewMsgServerImpl(*k)
+
+	// sign the new validator address as the new validator address
+	addrHash := crypto.Keccak256Hash(wormholesdk.SignedWormchainAddressPrefix, newValAddr)
+	sig, err := crypto.Sign(addrHash[:], privateKeys[0])
+	require.NoErrorf(t, err, "failed to sign wormchain address: %v", err)
+
+	_, err = msgServer.RegisterAccountAsGuardian(context, &types.MsgRegisterAccountAsGuardian{
+		Signer:    newValAddr.String(),
+		Signature: sig,
+	})
+	require.NoError(t, err)
+
+	// assert that the guardian validator has the new validator address
+	newGuardian, newGuardianFound := k.GetGuardianValidator(ctx, guardians[0].GuardianKey)
+	require.Truef(t, newGuardianFound, "expected guardian not found in the keeper store")
+
+	assert.Equal(t, newValAddr.Bytes(), newGuardian.ValidatorAddr)
+}
+
+// disallow hot-swapping validator addresses when guardian set size is >1
+func TestRegisterAccountAsGuardianBlockHotSwap(t *testing.T) {
+	// setup -- create guardian set of size 2
+	k, ctx := keepertest.WormholeKeeper(t)
+	guardians, privateKeys := createNGuardianValidator(k, ctx, 2)
+	k.SetConfig(ctx, types.Config{
+		GovernanceEmitter:     vaa.GovernanceEmitter[:],
+		GovernanceChain:       uint32(vaa.GovernanceChain),
+		ChainId:               uint32(vaa.ChainIDWormchain),
+		GuardianSetExpiration: 86400,
+	})
+	newValAddr_bz := [20]byte{}
+	newValAddr := sdk.AccAddress(newValAddr_bz[:])
+
+	set := createNewGuardianSet(k, ctx, guardians)
+	k.SetConsensusGuardianSetIndex(ctx, types.ConsensusGuardianSetIndex{Index: set.Index})
+
+	// execute msg_server function to associate new validator address account with the guardian address
+	context := sdk.WrapSDKContext(ctx)
+	msgServer := keeper.NewMsgServerImpl(*k)
+
+	// sign the new validator address as the new validator address
+	addrHash := crypto.Keccak256Hash(wormholesdk.SignedWormchainAddressPrefix, newValAddr)
+	sig, err := crypto.Sign(addrHash[:], privateKeys[0])
+	require.NoErrorf(t, err, "failed to sign wormchain address: %v", err)
+
+	// assert that we are unable to associate the guardian address with a new validator address when the set size is >1
+	_, err = msgServer.RegisterAccountAsGuardian(context, &types.MsgRegisterAccountAsGuardian{
+		Signer:    newValAddr.String(),
+		Signature: sig,
+	})
+	assert.Error(t, types.ErrConsensusSetNotUpdatable, err)
+}