From 5525caad0804c43171bbdb8dda3e3e2eac7e8be4 Mon Sep 17 00:00:00 2001 From: Nikhil Suri Date: Tue, 5 Dec 2023 14:52:56 -0800 Subject: [PATCH] =?UTF-8?q?wormchain:=20allow=20hot-swapping=20validator?= =?UTF-8?q?=20address=20association=20when=20guar=E2=80=A6=20(#3576)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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 --- ...msg_server_register_account_as_guardian.go | 24 +++-- ...erver_register_account_as_guardian_test.go | 88 +++++++++++++++++++ 2 files changed, 104 insertions(+), 8 deletions(-) create mode 100644 wormchain/x/wormhole/keeper/msg_server_register_account_as_guardian_test.go diff --git a/wormchain/x/wormhole/keeper/msg_server_register_account_as_guardian.go b/wormchain/x/wormhole/keeper/msg_server_register_account_as_guardian.go index 1a60c7223..4cef14435 100644 --- a/wormchain/x/wormhole/keeper/msg_server_register_account_as_guardian.go +++ b/wormchain/x/wormhole/keeper/msg_server_register_account_as_guardian.go @@ -12,7 +12,12 @@ import ( 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 ) +// 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) { 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 // enable a DoS vector 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) { diff --git a/wormchain/x/wormhole/keeper/msg_server_register_account_as_guardian_test.go b/wormchain/x/wormhole/keeper/msg_server_register_account_as_guardian_test.go new file mode 100644 index 000000000..8011491b9 --- /dev/null +++ b/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) +}