Skip to content

Commit

Permalink
fix unit tests for update msgUpdateTSS
Browse files Browse the repository at this point in the history
  • Loading branch information
kingpinXD committed Jul 7, 2024
1 parent 0dca846 commit 1469b73
Show file tree
Hide file tree
Showing 15 changed files with 85 additions and 31 deletions.
16 changes: 5 additions & 11 deletions cmd/zetaclientd/keygen_tss.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,9 @@ func GenerateTss(
return errors.New("unexpected state for TSS generation")
}

// keygenTss generates a new TSS using the keygen request and the TSS server.
// If the keygen is successful, the function returns the new TSS pubkey.
// If the keygen is unsuccessful, the function posts blame and returns an error.
func keygenTss(keyGen observertypes.Keygen, tssServer tss.TssServer, zetacoreClient interfaces.ZetacoreClient, keygenLogger zerolog.Logger) (string, error) {
keygenLogger.Info().Msgf("Keygen at blocknum %d , TSS signers %s ", keyGen.BlockNumber, keyGen.GranteePubkeys)
var req keygen.Request
Expand Down Expand Up @@ -148,20 +151,11 @@ func keygenTss(keyGen observertypes.Keygen, tssServer tss.TssServer, zetacoreCli
keygenLogger.Error().Msgf("keygen fail: reason %s ", err.Error())
return "", err
}
// Keygen succeed! Report TSS address
keygenLogger.Debug().Msgf("Keygen success! keygen response: %v", res)
// Keygen succeed
keygenLogger.Info().Msgf("Keygen success! keygen response: %v", res)
return res.PubKey, nil
}

func SetTSSPubKey(tss *mc.TSS, logger zerolog.Logger) error {
err := tss.InsertPubKey(tss.CurrentPubkey)
if err != nil {
logger.Error().Msgf("SetPubKey fail")
return err
}
logger.Info().Msgf("TSS address in hex: %s", tss.EVMAddress().Hex())
return nil
}
func TestTSS(pubkey string, tssServer tss.TssServer, logger zerolog.Logger) error {
keygenLogger := logger.With().Str("module", "test-keygen").Logger()
keygenLogger.Info().Msgf("KeyGen success ! Doing a Key-sign test")
Expand Down
5 changes: 0 additions & 5 deletions cmd/zetae2e/local/local.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,6 @@ const (
flagTestMigration = "test-migration"
flagSkipBitcoinSetup = "skip-bitcoin-setup"
flagSkipHeaderProof = "skip-header-proof"

flagPostMigration = "post-migration"
)

var (
Expand Down Expand Up @@ -70,7 +68,6 @@ func NewLocalCmd() *cobra.Command {
cmd.Flags().Bool(flagSkipBitcoinSetup, false, "set to true to skip bitcoin wallet setup")
cmd.Flags().Bool(flagSkipHeaderProof, false, "set to true to skip header proof tests")
cmd.Flags().Bool(flagTestMigration, false, "set to true to skip migration tests")
cmd.Flags().Bool(flagPostMigration, false, "set to true to run post migration tests")

return cmd
}
Expand Down Expand Up @@ -261,8 +258,6 @@ func localE2ETest(cmd *cobra.Command, _ []string) {
e2etests.TestEtherWithdrawRestrictedName,
}

light = true

if !light {
erc20Tests = append(erc20Tests, erc20AdvancedTests...)
zetaTests = append(zetaTests, zetaAdvancedTests...)
Expand Down
1 change: 1 addition & 0 deletions pkg/gas/gas_limits.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
const (
// EVMSend is the gas limit required to transfer tokens on an EVM based chain
EVMSend = 100_000

// TODO: Move gas limits from zeta-client to this file
// https://github.com/zeta-chain/node/issues/1606
)
Expand Down
8 changes: 7 additions & 1 deletion x/crosschain/keeper/cctx_orchestrator_validate_inbound.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,12 +60,18 @@ func (k Keeper) ValidateInbound(
// CheckMigration checks if the sender is a TSS address and returns an error if it is.
// If the sender is an older TSS address, this means that it is a migration transfer, and we do not need to treat this as a deposit.
func (k Keeper) CheckMigration(ctx sdk.Context, msg *types.MsgVoteInbound) error {
additionalChains := k.GetAuthorityKeeper().GetAdditionalChainList(ctx)
// Ignore cctx originating from zeta chain/zevm for this check as there is no TSS in zeta chain
if chains.IsZetaChain(msg.SenderChainId, additionalChains) {
return nil
}

historicalTssList := k.zetaObserverKeeper.GetAllTSS(ctx)
chain, found := k.zetaObserverKeeper.GetSupportedChainFromChainID(ctx, msg.SenderChainId)
if !found {
return observertypes.ErrSupportedChains.Wrapf("chain not found for chainID %d", msg.SenderChainId)
}
additionalChains := k.GetAuthorityKeeper().GetAdditionalChainList(ctx)

for _, tss := range historicalTssList {
if chains.IsEVMChain(chain.ChainId, additionalChains) {
ethTssAddress, err := crypto.GetTssAddrEVM(tss.TssPubkey)
Expand Down
24 changes: 24 additions & 0 deletions x/crosschain/keeper/cctx_orchestrator_validate_inbound_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ func TestKeeper_CheckMigration(t *testing.T) {
})

observerMock := keepertest.GetCrosschainObserverMock(t, k)
authorityMock := keepertest.GetCrosschainAuthorityMock(t, k)
chain := chains.Chain{
ChainId: 999,
}
Expand All @@ -84,6 +85,7 @@ func TestKeeper_CheckMigration(t *testing.T) {
// Set up mocks
observerMock.On("GetAllTSS", ctx).Return(tssList)
observerMock.On("GetSupportedChainFromChainID", ctx, chain.ChainId).Return(chain, false)
authorityMock.On("GetAdditionalChainList", ctx).Return([]chains.Chain{})

msg := types.MsgVoteInbound{
SenderChainId: chain.ChainId,
Expand Down Expand Up @@ -237,4 +239,26 @@ func TestKeeper_CheckMigration(t *testing.T) {
err := k.CheckMigration(ctx, &msg)
require.ErrorContains(t, err, "no Bitcoin net params for chain ID: 999")
})

t.Run("returns early if sender chain is zeta chain", func(t *testing.T) {
k, ctx, _, _ := keepertest.CrosschainKeeperWithMocks(t,
keepertest.CrosschainMockOptions{
UseAuthorityMock: true,
})

authorityMock := keepertest.GetCrosschainAuthorityMock(t, k)
chain := chains.ZetaChainTestnet
sender := sample.AccAddress()

// Set up mocks
authorityMock.On("GetAdditionalChainList", ctx).Return([]chains.Chain{})

msg := types.MsgVoteInbound{
SenderChainId: chain.ChainId,
Sender: sender,
}

err := k.CheckMigration(ctx, &msg)
require.NoError(t, err)
})
}
4 changes: 2 additions & 2 deletions x/crosschain/keeper/msg_server_migrate_tss_funds.go
Original file line number Diff line number Diff line change
Expand Up @@ -181,8 +181,8 @@ func (k Keeper) MigrateTSSFundsForChain(
),
)
}
evmFee = evmFee.Mul(sdkmath.NewUint(2))
cctx.GetCurrentOutboundParam().Amount = amount.Sub(evmFee)

cctx.GetCurrentOutboundParam().Amount = amount.Sub(evmFee.Add(sdkmath.NewUintFromString(types.TSSMigrationBufferAmountEVM)))
}
// Set the sender and receiver addresses for Bitcoin chain
if chains.IsBitcoinChain(chainID, additionalChains) {
Expand Down
3 changes: 1 addition & 2 deletions x/crosschain/keeper/msg_server_migrate_tss_funds_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (
"github.com/ethereum/go-ethereum/crypto"
"github.com/stretchr/testify/mock"
"github.com/stretchr/testify/require"

"github.com/zeta-chain/zetacore/pkg/chains"
"github.com/zeta-chain/zetacore/pkg/gas"
keepertest "github.com/zeta-chain/zetacore/testutil/keeper"
Expand Down Expand Up @@ -386,7 +385,7 @@ func TestMsgServer_MigrateTssFunds(t *testing.T) {
cctx, found := k.GetCrossChainTx(ctx, index)
require.True(t, found)
feeCalculated := sdk.NewUint(cctx.GetCurrentOutboundParam().GasLimit).
Mul(sdkmath.NewUintFromString(cctx.GetCurrentOutboundParam().GasPrice))
Mul(sdkmath.NewUintFromString(cctx.GetCurrentOutboundParam().GasPrice)).Add(sdkmath.NewUintFromString(crosschaintypes.TSSMigrationBufferAmountEVM))
require.Equal(t, cctx.GetCurrentOutboundParam().Amount.String(), amount.Sub(feeCalculated).String())
})

Expand Down
10 changes: 6 additions & 4 deletions x/crosschain/keeper/msg_server_remove_outbound_tracker.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@ package keeper
import (
"context"

errorsmod "cosmossdk.io/errors"
sdk "github.com/cosmos/cosmos-sdk/types"
authoritytypes "github.com/zeta-chain/zetacore/x/authority/types"

"github.com/zeta-chain/zetacore/x/crosschain/types"
)
Expand All @@ -16,10 +18,10 @@ func (k msgServer) RemoveOutboundTracker(
msg *types.MsgRemoveOutboundTracker,
) (*types.MsgRemoveOutboundTrackerResponse, error) {
ctx := sdk.UnwrapSDKContext(goCtx)
//err := k.GetAuthorityKeeper().CheckAuthorization(ctx, msg)
//if err != nil {
// return nil, errorsmod.Wrap(authoritytypes.ErrUnauthorized, err.Error())
//}
err := k.GetAuthorityKeeper().CheckAuthorization(ctx, msg)
if err != nil {
return nil, errorsmod.Wrap(authoritytypes.ErrUnauthorized, err.Error())
}

k.RemoveOutboundTrackerFromStore(ctx, msg.ChainId, msg.Nonce)
return &types.MsgRemoveOutboundTrackerResponse{}, nil
Expand Down
2 changes: 1 addition & 1 deletion x/crosschain/keeper/msg_server_update_tss.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ func (k msgServer) UpdateTssAddress(
if len(k.zetaObserverKeeper.GetSupportedForeignChains(ctx)) != len(tssMigrators) {
return nil, errorsmod.Wrap(
types.ErrUnableToUpdateTss,
"cannot update tss address not enough migrations have been created and completed",
"cannot update tss address incorrect number of migrations have been created and completed",
)
}

Expand Down
6 changes: 3 additions & 3 deletions x/crosschain/keeper/msg_server_update_tss_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ func TestMsgServer_UpdateTssAddress(t *testing.T) {
k.GetObserverKeeper().SetTSSHistory(ctx, tssOld)
k.GetObserverKeeper().SetTSSHistory(ctx, tssNew)
k.GetObserverKeeper().SetTSS(ctx, tssOld)
for _, chain := range k.GetObserverKeeper().GetSupportedChains(ctx) {
for _, chain := range k.GetObserverKeeper().GetSupportedForeignChains(ctx) {
index := chain.ChainName.String() + "_migration_tx_index"
k.GetObserverKeeper().SetFundMigrator(ctx, types.TssFundMigratorInfo{
ChainId: chain.ChainId,
Expand All @@ -78,7 +78,7 @@ func TestMsgServer_UpdateTssAddress(t *testing.T) {
require.Equal(
t,
len(k.GetObserverKeeper().GetAllTssFundMigrators(ctx)),
len(k.GetObserverKeeper().GetSupportedChains(ctx)),
len(k.GetObserverKeeper().GetSupportedForeignChains(ctx)),
)

msg := crosschaintypes.MsgUpdateTssAddress{
Expand Down Expand Up @@ -224,7 +224,7 @@ func TestMsgServer_UpdateTssAddress(t *testing.T) {
}
keepertest.MockCheckAuthorization(&authorityMock.Mock, &msg, nil)
_, err := msgServer.UpdateTssAddress(ctx, &msg)
require.ErrorContains(t, err, "cannot update tss address not enough migrations have been created and completed")
require.ErrorContains(t, err, "cannot update tss address incorrect number of migrations have been created and completed: unable to update TSS address")
require.ErrorIs(t, err, crosschaintypes.ErrUnableToUpdateTss)
tss, found := k.GetObserverKeeper().GetTSS(ctx)
require.True(t, found)
Expand Down
2 changes: 2 additions & 0 deletions x/crosschain/types/keys.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ const (
//TssMigrationGasMultiplierEVM is multiplied to the median gas price to get the gas price for the tss migration . This is done to avoid the tss migration tx getting stuck in the mempool
TssMigrationGasMultiplierEVM = "2.5"

TSSMigrationBufferAmountEVM = "2100000000"

// CCTXIndexLength is the length of a crosschain transaction index
CCTXIndexLength = 66
)
Expand Down
16 changes: 14 additions & 2 deletions zetaclient/chains/evm/observer/outbound.go
Original file line number Diff line number Diff line change
Expand Up @@ -365,9 +365,21 @@ func (ob *Observer) checkConfirmedTx(txHash string, nonce uint64) (*ethtypes.Rec
return nil, nil, false
}
if from != ob.TSS().EVMAddress() { // must be TSS address
log.Error().Msgf("confirmTxByHash: sender %s for outbound %s chain %d is not TSS address %s",
// If from is not TSS address, check if it is one of the previous TSS addresses We can still try to confirm a tx which was broadcast by an old TSS
log.Info().Msgf("confirmTxByHash: sender %s for outbound %s chain %d is not current TSS address %s",

Check warning on line 369 in zetaclient/chains/evm/observer/outbound.go

View check run for this annotation

Codecov / codecov/patch

zetaclient/chains/evm/observer/outbound.go#L369

Added line #L369 was not covered by tests
from.Hex(), transaction.Hash().Hex(), ob.Chain().ChainId, ob.TSS().EVMAddress().Hex())
return nil, nil, false
addressList := ob.TSS().EVMAddressList()
isOldTssAddress := false
for _, addr := range addressList {
if from == addr {
isOldTssAddress = true

Check warning on line 375 in zetaclient/chains/evm/observer/outbound.go

View check run for this annotation

Codecov / codecov/patch

zetaclient/chains/evm/observer/outbound.go#L371-L375

Added lines #L371 - L375 were not covered by tests
}
}
if !isOldTssAddress {
log.Error().Msgf("confirmTxByHash: sender %s for outbound %s chain %d is not current or old TSS address. Current TSS %s",
from.Hex(), transaction.Hash().Hex(), ob.Chain().ChainId, ob.TSS().EVMAddress().Hex())
return nil, nil, false

Check warning on line 381 in zetaclient/chains/evm/observer/outbound.go

View check run for this annotation

Codecov / codecov/patch

zetaclient/chains/evm/observer/outbound.go#L378-L381

Added lines #L378 - L381 were not covered by tests
}
}
if transaction.Nonce() != nonce { // must match cctx nonce
log.Error().
Expand Down
2 changes: 2 additions & 0 deletions zetaclient/chains/interfaces/interfaces.go
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,8 @@ type TSSSigner interface {
SignBatch(digests [][]byte, height uint64, nonce uint64, chainID int64) ([][65]byte, error)

EVMAddress() ethcommon.Address

EVMAddressList() []ethcommon.Address
BTCAddress() string
BTCAddressWitnessPubkeyHash() *btcutil.AddressWitnessPubKeyHash
PubKeyCompressedBytes() []byte
Expand Down
4 changes: 4 additions & 0 deletions zetaclient/testutils/mocks/tss_signer.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,10 @@ func (s *TSS) EVMAddress() ethcommon.Address {
return crypto.PubkeyToAddress(s.PrivKey.PublicKey)
}

func (s *TSS) EVMAddressList() []ethcommon.Address {
return []ethcommon.Address{s.EVMAddress()}
}

func (s *TSS) BTCAddress() string {
// force use btcAddress if set
if s.btcAddress != "" {
Expand Down
13 changes: 13 additions & 0 deletions zetaclient/tss/tss_signer.go
Original file line number Diff line number Diff line change
Expand Up @@ -437,6 +437,19 @@ func (tss *TSS) EVMAddress() ethcommon.Address {
return addr
}

func (tss *TSS) EVMAddressList() []ethcommon.Address {
var addresses []ethcommon.Address

Check failure on line 441 in zetaclient/tss/tss_signer.go

View workflow job for this annotation

GitHub Actions / lint

Consider pre-allocating `addresses` (prealloc)
for _, key := range tss.Keys {
addr, err := GetTssAddrEVM(key.PubkeyInBech32)
if err != nil {
log.Error().Err(err).Msg("getKeyAddr error")
return nil

Check warning on line 446 in zetaclient/tss/tss_signer.go

View check run for this annotation

Codecov / codecov/patch

zetaclient/tss/tss_signer.go#L440-L446

Added lines #L440 - L446 were not covered by tests
}
addresses = append(addresses, addr)

Check warning on line 448 in zetaclient/tss/tss_signer.go

View check run for this annotation

Codecov / codecov/patch

zetaclient/tss/tss_signer.go#L448

Added line #L448 was not covered by tests
}
return addresses

Check warning on line 450 in zetaclient/tss/tss_signer.go

View check run for this annotation

Codecov / codecov/patch

zetaclient/tss/tss_signer.go#L450

Added line #L450 was not covered by tests
}

// BTCAddress generates a bech32 p2wpkh address from pubkey
func (tss *TSS) BTCAddress() string {
addr, err := GetTssAddrBTC(tss.CurrentPubkey, tss.BitcoinChainID)
Expand Down

0 comments on commit 1469b73

Please sign in to comment.