From f8e856570acdcf3c23e15f49c8c6b74548d19187 Mon Sep 17 00:00:00 2001 From: skosito Date: Fri, 8 Mar 2024 08:04:49 +0000 Subject: [PATCH 1/2] refactor: removing unnecessary pointers in context structure (#1831) * Fix keygen not found case * Add test for empty core context * Add tests for creating new core context with config chain params * Add update core context unit tests * Cleanup pointers from config * Cleanup keygen core context pointer * Add to changelog * Fix lint and masked cfg * Fix lint * Cleanup * Cleanup * Revert * Add todo * PR comments * PR comments * Fixes after merge --- changelog.md | 1 + cmd/zetaclientd/main.go | 6 +- cmd/zetaclientd/p2p_diagnostics.go | 2 +- cmd/zetaclientd/start.go | 5 +- cmd/zetaclientd/start_utils.go | 16 +- cmd/zetaclientd/utils.go | 4 +- zetaclient/app_context/app_context.go | 10 +- zetaclient/bitcoin/bitcoin_client_rpc_test.go | 2 +- zetaclient/common/utils_test.go | 4 +- zetaclient/config/config.go | 12 +- zetaclient/config/config_chain.go | 4 +- zetaclient/config/types.go | 57 ++-- zetaclient/core_context/zeta_core_context.go | 13 +- .../core_context/zeta_core_context_test.go | 262 ++++++++++++++++++ zetaclient/evm/evm_signer_test.go | 2 +- zetaclient/evm/inbounds_test.go | 12 +- zetaclient/keys/keys.go | 2 +- zetaclient/keys/keys_test.go | 4 +- zetaclient/testutils/testdata.go | 4 +- zetaclient/tss/tss_signer.go | 2 +- 20 files changed, 341 insertions(+), 83 deletions(-) create mode 100644 zetaclient/core_context/zeta_core_context_test.go diff --git a/changelog.md b/changelog.md index 5ebc2da24d..d4b8145915 100644 --- a/changelog.md +++ b/changelog.md @@ -14,6 +14,7 @@ * [1511](https://github.com/zeta-chain/node/pull/1511) - move ballot voting logic from `crosschain` to `observer` * [1783](https://github.com/zeta-chain/node/pull/1783) - refactor zetaclient metrics naming and structure * [1774](https://github.com/zeta-chain/node/pull/1774) - split params and config in zetaclient +* [1831](https://github.com/zeta-chain/node/pull/1831) - removing unnecessary pointers in context structure ### Features diff --git a/cmd/zetaclientd/main.go b/cmd/zetaclientd/main.go index 767b1fd78e..07654a0375 100644 --- a/cmd/zetaclientd/main.go +++ b/cmd/zetaclientd/main.go @@ -60,7 +60,7 @@ func SetupConfigForTest() { } -func InitLogger(cfg *config.Config) (clientcommon.ClientLogger, error) { +func InitLogger(cfg config.Config) (clientcommon.ClientLogger, error) { // open compliance log file file, err := OpenComplianceLogFile(cfg) if err != nil { @@ -92,10 +92,10 @@ func InitLogger(cfg *config.Config) (clientcommon.ClientLogger, error) { }, nil } -func OpenComplianceLogFile(cfg *config.Config) (*os.File, error) { +func OpenComplianceLogFile(cfg config.Config) (*os.File, error) { // use zetacore home as default logPath := cfg.ZetaCoreHome - if cfg.ComplianceConfig != nil && cfg.ComplianceConfig.LogPath != "" { + if cfg.ComplianceConfig.LogPath != "" { logPath = cfg.ComplianceConfig.LogPath } diff --git a/cmd/zetaclientd/p2p_diagnostics.go b/cmd/zetaclientd/p2p_diagnostics.go index 38bc269bc6..72a1580224 100644 --- a/cmd/zetaclientd/p2p_diagnostics.go +++ b/cmd/zetaclientd/p2p_diagnostics.go @@ -26,7 +26,7 @@ import ( "github.com/zeta-chain/zetacore/zetaclient/config" ) -func RunDiagnostics(startLogger zerolog.Logger, peers p2p.AddrList, bridgePk cryptotypes.PrivKey, cfg *config.Config) error { +func RunDiagnostics(startLogger zerolog.Logger, peers p2p.AddrList, bridgePk cryptotypes.PrivKey, cfg config.Config) error { startLogger.Warn().Msg("P2P Diagnostic mode enabled") startLogger.Warn().Msgf("seed peer: %s", peers) diff --git a/cmd/zetaclientd/start.go b/cmd/zetaclientd/start.go index b0b181a105..3816699c78 100644 --- a/cmd/zetaclientd/start.go +++ b/cmd/zetaclientd/start.go @@ -191,8 +191,9 @@ func start(_ *cobra.Command, _ []string) error { // For existing keygen, this should directly proceed to the next step ticker := time.NewTicker(time.Second * 1) for range ticker.C { - if appContext.ZetaCoreContext().GetKeygen().Status != observerTypes.KeygenStatus_KeyGenSuccess { - startLogger.Info().Msgf("Waiting for TSS Keygen to be a success, current status %s", appContext.ZetaCoreContext().GetKeygen().Status) + keyGen := appContext.ZetaCoreContext().GetKeygen() + if keyGen.Status != observerTypes.KeygenStatus_KeyGenSuccess { + startLogger.Info().Msgf("Waiting for TSS Keygen to be a success, current status %s", keyGen.Status) continue } break diff --git a/cmd/zetaclientd/start_utils.go b/cmd/zetaclientd/start_utils.go index 042f2fb20d..e0aa59002e 100644 --- a/cmd/zetaclientd/start_utils.go +++ b/cmd/zetaclientd/start_utils.go @@ -13,12 +13,12 @@ import ( "google.golang.org/grpc" ) -func waitForZetaCore(configData *config.Config, logger zerolog.Logger) { +func waitForZetaCore(config config.Config, logger zerolog.Logger) { // wait until zetacore is up logger.Debug().Msg("Waiting for ZetaCore to open 9090 port...") for { _, err := grpc.Dial( - fmt.Sprintf("%s:9090", configData.ZetaCoreURL), + fmt.Sprintf("%s:9090", config.ZetaCoreURL), grpc.WithInsecure(), ) if err != nil { @@ -54,20 +54,18 @@ func validatePeer(seedPeer string) error { // maskCfg sensitive fields are masked, currently only the EVM endpoints and bitcoin credentials, // // other fields can be added. -func maskCfg(cfg *config.Config) string { - maskedCfg := config.NewConfig() +func maskCfg(cfg config.Config) string { + maskedCfg := cfg - // Deep copy since cfg contains some references - *maskedCfg = *cfg - maskedCfg.BitcoinConfig = &config.BTCConfig{ + maskedCfg.BitcoinConfig = config.BTCConfig{ RPCUsername: cfg.BitcoinConfig.RPCUsername, RPCPassword: cfg.BitcoinConfig.RPCPassword, RPCHost: cfg.BitcoinConfig.RPCHost, RPCParams: cfg.BitcoinConfig.RPCParams, } - maskedCfg.EVMChainConfigs = map[int64]*config.EVMConfig{} + maskedCfg.EVMChainConfigs = map[int64]config.EVMConfig{} for key, val := range cfg.EVMChainConfigs { - maskedCfg.EVMChainConfigs[key] = &config.EVMConfig{ + maskedCfg.EVMChainConfigs[key] = config.EVMConfig{ Chain: val.Chain, Endpoint: val.Endpoint, } diff --git a/cmd/zetaclientd/utils.go b/cmd/zetaclientd/utils.go index 6dafd72312..f926f8118e 100644 --- a/cmd/zetaclientd/utils.go +++ b/cmd/zetaclientd/utils.go @@ -22,7 +22,7 @@ func CreateAuthzSigner(granter string, grantee sdk.AccAddress) { authz.SetupAuthZSignerList(granter, grantee) } -func CreateZetaBridge(cfg *config.Config, telemetry *metrics.TelemetryServer, hotkeyPassword string) (*zetabridge.ZetaCoreBridge, error) { +func CreateZetaBridge(cfg config.Config, telemetry *metrics.TelemetryServer, hotkeyPassword string) (*zetabridge.ZetaCoreBridge, error) { hotKey := cfg.AuthzHotkey if cfg.HsmMode { hotKey = cfg.HsmHotKey @@ -115,7 +115,7 @@ func CreateChainClientMap( if !evmChainParams.IsSupported { continue } - co, err := evm.NewEVMChainClient(appContext, bridge, tss, dbpath, loggers, *evmConfig, ts) + co, err := evm.NewEVMChainClient(appContext, bridge, tss, dbpath, loggers, evmConfig, ts) if err != nil { loggers.Std.Error().Err(err).Msgf("NewEVMChainClient error for chain %s", evmConfig.Chain.String()) continue diff --git a/zetaclient/app_context/app_context.go b/zetaclient/app_context/app_context.go index deab613618..d73f48d9d0 100644 --- a/zetaclient/app_context/app_context.go +++ b/zetaclient/app_context/app_context.go @@ -9,13 +9,13 @@ import ( // AppContext contains global app structs like config, core context and logger type AppContext struct { coreContext *corecontext.ZetaCoreContext - config *config.Config + config config.Config } // NewAppContext creates and returns new AppContext func NewAppContext( coreContext *corecontext.ZetaCoreContext, - config *config.Config, + config config.Config, ) *AppContext { return &AppContext{ coreContext: coreContext, @@ -23,16 +23,16 @@ func NewAppContext( } } -func (a *AppContext) Config() *config.Config { +func (a AppContext) Config() config.Config { return a.config } -func (a *AppContext) ZetaCoreContext() *corecontext.ZetaCoreContext { +func (a AppContext) ZetaCoreContext() *corecontext.ZetaCoreContext { return a.coreContext } // GetBTCChainAndConfig returns btc chain and config if enabled -func (a *AppContext) GetBTCChainAndConfig() (common.Chain, config.BTCConfig, bool) { +func (a AppContext) GetBTCChainAndConfig() (common.Chain, config.BTCConfig, bool) { btcConfig, configEnabled := a.Config().GetBTCConfig() btcChain, _, paramsEnabled := a.ZetaCoreContext().GetBTCChainParams() diff --git a/zetaclient/bitcoin/bitcoin_client_rpc_test.go b/zetaclient/bitcoin/bitcoin_client_rpc_test.go index d40a08152c..7f8c060dce 100644 --- a/zetaclient/bitcoin/bitcoin_client_rpc_test.go +++ b/zetaclient/bitcoin/bitcoin_client_rpc_test.go @@ -46,7 +46,7 @@ func (suite *BitcoinClientTestSuite) SetupTest() { tss := interfaces.TestSigner{ PrivKey: privateKey, } - appContext := appcontext.NewAppContext(&corecontext.ZetaCoreContext{}, &config.Config{}) + appContext := appcontext.NewAppContext(&corecontext.ZetaCoreContext{}, config.Config{}) client, err := NewBitcoinClient(appContext, common.BtcRegtestChain(), nil, tss, "/tmp", clientcommon.DefaultLoggers(), nil) suite.Require().NoError(err) suite.BitcoinChainClient = client diff --git a/zetaclient/common/utils_test.go b/zetaclient/common/utils_test.go index 41b3f0c4cf..b7fa29b9bf 100644 --- a/zetaclient/common/utils_test.go +++ b/zetaclient/common/utils_test.go @@ -18,8 +18,8 @@ func TestCctxRestricted(t *testing.T) { require.NoError(t, err) // create config - cfg := &config.Config{ - ComplianceConfig: &config.ComplianceConfig{}, + cfg := config.Config{ + ComplianceConfig: config.ComplianceConfig{}, } t.Run("should return true if sender is restricted", func(t *testing.T) { diff --git a/zetaclient/config/config.go b/zetaclient/config/config.go index 08a9d4c002..4cb0e6b21b 100644 --- a/zetaclient/config/config.go +++ b/zetaclient/config/config.go @@ -36,12 +36,12 @@ func Save(config *Config, path string) error { } // Load loads ZetaClient config from a filepath -func Load(path string) (*Config, error) { +func Load(path string) (Config, error) { // retrieve file file := filepath.Join(path, folder, filename) file, err := filepath.Abs(file) if err != nil { - return nil, err + return Config{}, err } file = filepath.Clean(file) @@ -49,11 +49,11 @@ func Load(path string) (*Config, error) { cfg := NewConfig() input, err := os.ReadFile(file) if err != nil { - return nil, err + return Config{}, err } err = json.Unmarshal(input, &cfg) if err != nil { - return nil, err + return Config{}, err } // read keyring backend and use test by default @@ -61,7 +61,7 @@ func Load(path string) (*Config, error) { cfg.KeyringBackend = KeyringBackendTest } if cfg.KeyringBackend != KeyringBackendFile && cfg.KeyringBackend != KeyringBackendTest { - return nil, fmt.Errorf("invalid keyring backend %s", cfg.KeyringBackend) + return Config{}, fmt.Errorf("invalid keyring backend %s", cfg.KeyringBackend) } // fields sanitization @@ -75,7 +75,7 @@ func Load(path string) (*Config, error) { return cfg, nil } -func LoadComplianceConfig(cfg *Config) { +func LoadComplianceConfig(cfg Config) { restrictedAddressBook = cfg.GetRestrictedAddressBook() } diff --git a/zetaclient/config/config_chain.go b/zetaclient/config/config_chain.go index 2596c554a5..ea497a1821 100644 --- a/zetaclient/config/config_chain.go +++ b/zetaclient/config/config_chain.go @@ -38,14 +38,14 @@ func New() Config { } } -var bitcoinConfigRegnet = &BTCConfig{ +var bitcoinConfigRegnet = BTCConfig{ RPCUsername: "e2e", RPCPassword: "123", RPCHost: "bitcoin:18443", RPCParams: "regtest", } -var evmChainsConfigs = map[int64]*EVMConfig{ +var evmChainsConfigs = map[int64]EVMConfig{ common.EthChain().ChainId: { Chain: common.EthChain(), }, diff --git a/zetaclient/config/types.go b/zetaclient/config/types.go index 376c06dc54..200066f109 100644 --- a/zetaclient/config/types.go +++ b/zetaclient/config/types.go @@ -73,69 +73,61 @@ type Config struct { HsmMode bool `json:"HsmMode"` HsmHotKey string `json:"HsmHotKey"` - EVMChainConfigs map[int64]*EVMConfig `json:"EVMChainConfigs"` - BitcoinConfig *BTCConfig `json:"BitcoinConfig"` + EVMChainConfigs map[int64]EVMConfig `json:"EVMChainConfigs"` + BitcoinConfig BTCConfig `json:"BitcoinConfig"` // compliance config - ComplianceConfig *ComplianceConfig `json:"ComplianceConfig"` + ComplianceConfig ComplianceConfig `json:"ComplianceConfig"` } -func NewConfig() *Config { - return &Config{ +func NewConfig() Config { + return Config{ cfgLock: &sync.RWMutex{}, - EVMChainConfigs: make(map[int64]*EVMConfig), + EVMChainConfigs: make(map[int64]EVMConfig), } } -func (c *Config) String() string { - c.cfgLock.RLock() - defer c.cfgLock.RUnlock() - s, err := json.MarshalIndent(c, "", "\t") - if err != nil { - return "" - } - return string(s) -} - -func (c *Config) GetEVMConfig(chainID int64) (EVMConfig, bool) { +func (c Config) GetEVMConfig(chainID int64) (EVMConfig, bool) { c.cfgLock.RLock() defer c.cfgLock.RUnlock() evmCfg, found := c.EVMChainConfigs[chainID] - return *evmCfg, found + return evmCfg, found } -func (c *Config) GetAllEVMConfigs() map[int64]*EVMConfig { +func (c Config) GetAllEVMConfigs() map[int64]EVMConfig { c.cfgLock.RLock() defer c.cfgLock.RUnlock() // deep copy evm configs - copied := make(map[int64]*EVMConfig, len(c.EVMChainConfigs)) + copied := make(map[int64]EVMConfig, len(c.EVMChainConfigs)) for chainID, evmConfig := range c.EVMChainConfigs { - copied[chainID] = &EVMConfig{} - *copied[chainID] = *evmConfig + copied[chainID] = evmConfig } return copied } -func (c *Config) GetBTCConfig() (BTCConfig, bool) { +func (c Config) GetBTCConfig() (BTCConfig, bool) { c.cfgLock.RLock() defer c.cfgLock.RUnlock() - if c.BitcoinConfig == nil { // bitcoin is not enabled - return BTCConfig{}, false + return c.BitcoinConfig, c.BitcoinConfig != (BTCConfig{}) +} + +func (c Config) String() string { + s, err := json.MarshalIndent(c, "", "\t") + if err != nil { + return "" } - return *c.BitcoinConfig, true + return string(s) } // GetRestrictedAddressBook returns a map of restricted addresses // Note: the restricted address book contains both ETH and BTC addresses -func (c *Config) GetRestrictedAddressBook() map[string]bool { +func (c Config) GetRestrictedAddressBook() map[string]bool { restrictedAddresses := make(map[string]bool) - if c.ComplianceConfig != nil { - for _, address := range c.ComplianceConfig.RestrictedAddresses { - if address != "" { - restrictedAddresses[strings.ToLower(address)] = true - } + for _, address := range c.ComplianceConfig.RestrictedAddresses { + if address != "" { + restrictedAddresses[strings.ToLower(address)] = true } } return restrictedAddresses @@ -147,6 +139,7 @@ func (c *Config) GetKeyringBackend() KeyringBackend { return c.KeyringBackend } +// TODO: remove this duplicated function https://github.com/zeta-chain/node/issues/1838 // ValidateChainParams performs some basic checks on core params func ValidateChainParams(chainParams *observertypes.ChainParams) error { if chainParams == nil { diff --git a/zetaclient/core_context/zeta_core_context.go b/zetaclient/core_context/zeta_core_context.go index 27a88b8951..9eb3de1f43 100644 --- a/zetaclient/core_context/zeta_core_context.go +++ b/zetaclient/core_context/zeta_core_context.go @@ -15,7 +15,7 @@ import ( // these are initialized and updated at runtime at every height type ZetaCoreContext struct { coreContextLock *sync.RWMutex - keygen *observertypes.Keygen + keygen observertypes.Keygen chainsEnabled []common.Chain evmChainParams map[int64]*observertypes.ChainParams bitcoinChainParams *observertypes.ChainParams @@ -24,7 +24,7 @@ type ZetaCoreContext struct { // NewZetaCoreContext creates and returns new ZetaCoreContext // it is initializing chain params from provided config -func NewZetaCoreContext(cfg *config.Config) *ZetaCoreContext { +func NewZetaCoreContext(cfg config.Config) *ZetaCoreContext { evmChainParams := make(map[int64]*observertypes.ChainParams) for _, e := range cfg.EVMChainConfigs { evmChainParams[e.Chain.ChainId] = &observertypes.ChainParams{} @@ -45,9 +45,12 @@ func NewZetaCoreContext(cfg *config.Config) *ZetaCoreContext { func (c *ZetaCoreContext) GetKeygen() observertypes.Keygen { c.coreContextLock.RLock() defer c.coreContextLock.RUnlock() - copiedPubkeys := make([]string, len(c.keygen.GranteePubkeys)) - copy(copiedPubkeys, c.keygen.GranteePubkeys) + var copiedPubkeys []string + if c.keygen.GranteePubkeys != nil { + copiedPubkeys = make([]string, len(c.keygen.GranteePubkeys)) + copy(copiedPubkeys, c.keygen.GranteePubkeys) + } return observertypes.Keygen{ Status: c.keygen.Status, GranteePubkeys: copiedPubkeys, @@ -145,7 +148,7 @@ func (c *ZetaCoreContext) Update( } } } - c.keygen = keygen + c.keygen = *keygen c.chainsEnabled = newChains // update chain params for bitcoin if it has config in file if c.bitcoinChainParams != nil && btcChainParams != nil { diff --git a/zetaclient/core_context/zeta_core_context_test.go b/zetaclient/core_context/zeta_core_context_test.go new file mode 100644 index 0000000000..e7690f0c23 --- /dev/null +++ b/zetaclient/core_context/zeta_core_context_test.go @@ -0,0 +1,262 @@ +package corecontext_test + +import ( + "testing" + + "github.com/stretchr/testify/require" + "github.com/zeta-chain/zetacore/common" + observertypes "github.com/zeta-chain/zetacore/x/observer/types" + clientcommon "github.com/zeta-chain/zetacore/zetaclient/common" + "github.com/zeta-chain/zetacore/zetaclient/config" + corecontext "github.com/zeta-chain/zetacore/zetaclient/core_context" +) + +func TestNewZetaCoreContext(t *testing.T) { + t.Run("should create new zeta core context with empty config", func(t *testing.T) { + testCfg := config.NewConfig() + + zetaContext := corecontext.NewZetaCoreContext(testCfg) + require.NotNil(t, zetaContext) + + // assert keygen + keyGen := zetaContext.GetKeygen() + require.Equal(t, observertypes.Keygen{}, keyGen) + + // assert enabled chains + require.Empty(t, len(zetaContext.GetEnabledChains())) + + // assert current tss pubkey + require.Equal(t, "", zetaContext.GetCurrentTssPubkey()) + + // assert btc chain params + chain, btcChainParams, btcChainParamsFound := zetaContext.GetBTCChainParams() + require.Equal(t, common.Chain{}, chain) + require.False(t, btcChainParamsFound) + require.Equal(t, &observertypes.ChainParams{}, btcChainParams) + + // assert evm chain params + allEVMChainParams := zetaContext.GetAllEVMChainParams() + require.Empty(t, allEVMChainParams) + }) + + t.Run("should create new zeta core context with config containing evm chain params", func(t *testing.T) { + testCfg := config.NewConfig() + testCfg.EVMChainConfigs = map[int64]config.EVMConfig{ + 1: { + Chain: common.Chain{ + ChainName: 1, + ChainId: 1, + }, + }, + 2: { + Chain: common.Chain{ + ChainName: 2, + ChainId: 2, + }, + }, + } + zetaContext := corecontext.NewZetaCoreContext(testCfg) + require.NotNil(t, zetaContext) + + // assert evm chain params + allEVMChainParams := zetaContext.GetAllEVMChainParams() + require.Equal(t, 2, len(allEVMChainParams)) + require.Equal(t, &observertypes.ChainParams{}, allEVMChainParams[1]) + require.Equal(t, &observertypes.ChainParams{}, allEVMChainParams[2]) + + evmChainParams1, found := zetaContext.GetEVMChainParams(1) + require.True(t, found) + require.Equal(t, &observertypes.ChainParams{}, evmChainParams1) + + evmChainParams2, found := zetaContext.GetEVMChainParams(2) + require.True(t, found) + require.Equal(t, &observertypes.ChainParams{}, evmChainParams2) + }) + + t.Run("should create new zeta core context with config containing btc config", func(t *testing.T) { + testCfg := config.NewConfig() + testCfg.BitcoinConfig = config.BTCConfig{ + RPCUsername: "test username", + RPCPassword: "test password", + RPCHost: "test host", + RPCParams: "test params", + } + zetaContext := corecontext.NewZetaCoreContext(testCfg) + require.NotNil(t, zetaContext) + + // assert btc chain params panic because chain params are not yet updated + assertPanic(t, func() { + zetaContext.GetBTCChainParams() + }, "BTCChain is missing for chainID 0") + }) +} + +func TestUpdateZetaCoreContext(t *testing.T) { + t.Run("should update core context after being created from empty config", func(t *testing.T) { + testCfg := config.NewConfig() + + zetaContext := corecontext.NewZetaCoreContext(testCfg) + require.NotNil(t, zetaContext) + + keyGenToUpdate := observertypes.Keygen{ + Status: observertypes.KeygenStatus_KeyGenSuccess, + GranteePubkeys: []string{"testpubkey1"}, + } + enabledChainsToUpdate := []common.Chain{ + { + ChainName: 1, + ChainId: 1, + }, + { + ChainName: 2, + ChainId: 2, + }, + } + evmChainParamsToUpdate := map[int64]*observertypes.ChainParams{ + 1: { + ChainId: 1, + }, + 2: { + ChainId: 2, + }, + } + btcChainParamsToUpdate := &observertypes.ChainParams{ + ChainId: 3, + } + tssPubKeyToUpdate := "tsspubkeytest" + loggers := clientcommon.DefaultLoggers() + zetaContext.Update( + &keyGenToUpdate, + enabledChainsToUpdate, + evmChainParamsToUpdate, + btcChainParamsToUpdate, + tssPubKeyToUpdate, + false, + loggers.Std, + ) + + // assert keygen updated + keyGen := zetaContext.GetKeygen() + require.Equal(t, keyGenToUpdate, keyGen) + + // assert enabled chains updated + require.Equal(t, enabledChainsToUpdate, zetaContext.GetEnabledChains()) + + // assert current tss pubkey updated + require.Equal(t, tssPubKeyToUpdate, zetaContext.GetCurrentTssPubkey()) + + // assert btc chain params still empty because they were not specified in config + chain, btcChainParams, btcChainParamsFound := zetaContext.GetBTCChainParams() + require.Equal(t, common.Chain{}, chain) + require.False(t, btcChainParamsFound) + require.Equal(t, &observertypes.ChainParams{}, btcChainParams) + + // assert evm chain params still empty because they were not specified in config + allEVMChainParams := zetaContext.GetAllEVMChainParams() + require.Empty(t, allEVMChainParams) + }) + + t.Run("should update core context after being created from config with evm and btc chain params", func(t *testing.T) { + testCfg := config.NewConfig() + testCfg.EVMChainConfigs = map[int64]config.EVMConfig{ + 1: { + Chain: common.Chain{ + ChainName: 1, + ChainId: 1, + }, + }, + 2: { + Chain: common.Chain{ + ChainName: 2, + ChainId: 2, + }, + }, + } + testCfg.BitcoinConfig = config.BTCConfig{ + RPCUsername: "test username", + RPCPassword: "test password", + RPCHost: "test host", + RPCParams: "test params", + } + + zetaContext := corecontext.NewZetaCoreContext(testCfg) + require.NotNil(t, zetaContext) + + keyGenToUpdate := observertypes.Keygen{ + Status: observertypes.KeygenStatus_KeyGenSuccess, + GranteePubkeys: []string{"testpubkey1"}, + } + enabledChainsToUpdate := []common.Chain{ + { + ChainName: 1, + ChainId: 1, + }, + { + ChainName: 2, + ChainId: 2, + }, + } + evmChainParamsToUpdate := map[int64]*observertypes.ChainParams{ + 1: { + ChainId: 1, + }, + 2: { + ChainId: 2, + }, + } + + testBtcChain := common.BtcTestNetChain() + btcChainParamsToUpdate := &observertypes.ChainParams{ + ChainId: testBtcChain.ChainId, + } + tssPubKeyToUpdate := "tsspubkeytest" + loggers := clientcommon.DefaultLoggers() + zetaContext.Update( + &keyGenToUpdate, + enabledChainsToUpdate, + evmChainParamsToUpdate, + btcChainParamsToUpdate, + tssPubKeyToUpdate, + false, + loggers.Std, + ) + + // assert keygen updated + keyGen := zetaContext.GetKeygen() + require.Equal(t, keyGenToUpdate, keyGen) + + // assert enabled chains updated + require.Equal(t, enabledChainsToUpdate, zetaContext.GetEnabledChains()) + + // assert current tss pubkey updated + require.Equal(t, tssPubKeyToUpdate, zetaContext.GetCurrentTssPubkey()) + + // assert btc chain params + chain, btcChainParams, btcChainParamsFound := zetaContext.GetBTCChainParams() + require.Equal(t, testBtcChain, chain) + require.True(t, btcChainParamsFound) + require.Equal(t, btcChainParamsToUpdate, btcChainParams) + + // assert evm chain params + allEVMChainParams := zetaContext.GetAllEVMChainParams() + require.Equal(t, evmChainParamsToUpdate, allEVMChainParams) + + evmChainParams1, found := zetaContext.GetEVMChainParams(1) + require.True(t, found) + require.Equal(t, evmChainParamsToUpdate[1], evmChainParams1) + + evmChainParams2, found := zetaContext.GetEVMChainParams(2) + require.True(t, found) + require.Equal(t, evmChainParamsToUpdate[2], evmChainParams2) + }) +} + +func assertPanic(t *testing.T, f func(), errorLog string) { + defer func() { + r := recover() + if r != nil { + require.Contains(t, r, errorLog) + } + }() + f() +} diff --git a/zetaclient/evm/evm_signer_test.go b/zetaclient/evm/evm_signer_test.go index e685128902..711db544d1 100644 --- a/zetaclient/evm/evm_signer_test.go +++ b/zetaclient/evm/evm_signer_test.go @@ -52,7 +52,7 @@ func getNewEvmChainClient() (*ChainClient, error) { tss := stub.NewTSSMainnet() evmcfg := config.EVMConfig{Chain: corecommon.BscMainnetChain(), Endpoint: "http://localhost:8545"} - cfg.EVMChainConfigs[corecommon.BscMainnetChain().ChainId] = &evmcfg + cfg.EVMChainConfigs[corecommon.BscMainnetChain().ChainId] = evmcfg coreCTX := corecontext.NewZetaCoreContext(cfg) appCTX := appcontext.NewAppContext(coreCTX, cfg) diff --git a/zetaclient/evm/inbounds_test.go b/zetaclient/evm/inbounds_test.go index 15a3efbe76..472cdc65ae 100644 --- a/zetaclient/evm/inbounds_test.go +++ b/zetaclient/evm/inbounds_test.go @@ -209,8 +209,8 @@ func TestEVM_BuildInboundVoteMsgForZetaSentEvent(t *testing.T) { event := testutils.ParseReceiptZetaSent(receipt, connector) // create test compliance config - cfg := &config.Config{ - ComplianceConfig: &config.ComplianceConfig{}, + cfg := config.Config{ + ComplianceConfig: config.ComplianceConfig{}, } t.Run("should return vote msg for archived ZetaSent event", func(t *testing.T) { @@ -256,8 +256,8 @@ func TestEVM_BuildInboundVoteMsgForDepositedEvent(t *testing.T) { sender := ethcommon.HexToAddress(tx.From) // create test compliance config - cfg := &config.Config{ - ComplianceConfig: &config.ComplianceConfig{}, + cfg := config.Config{ + ComplianceConfig: config.ComplianceConfig{}, } t.Run("should return vote msg for archived Deposited event", func(t *testing.T) { @@ -302,8 +302,8 @@ func TestEVM_BuildInboundVoteMsgForTokenSentToTSS(t *testing.T) { // create test compliance config ob := MockEVMClient(common.EthChain(), nil, 1, stub.MockChainParams(1, 1)) - cfg := &config.Config{ - ComplianceConfig: &config.ComplianceConfig{}, + cfg := config.Config{ + ComplianceConfig: config.ComplianceConfig{}, } t.Run("should return vote msg for archived gas token transfer to TSS", func(t *testing.T) { diff --git a/zetaclient/keys/keys.go b/zetaclient/keys/keys.go index 974743098f..e7e40c2bdc 100644 --- a/zetaclient/keys/keys.go +++ b/zetaclient/keys/keys.go @@ -44,7 +44,7 @@ func GetGranteeKeyName(signerName string) string { } // GetKeyringKeybase return keyring and key info -func GetKeyringKeybase(cfg *config.Config, hotkeyPassword string) (ckeys.Keyring, string, error) { +func GetKeyringKeybase(cfg config.Config, hotkeyPassword string) (ckeys.Keyring, string, error) { granteeName := cfg.AuthzHotkey chainHomeFolder := cfg.ZetaCoreHome logger := log.Logger.With().Str("module", "GetKeyringKeybase").Logger() diff --git a/zetaclient/keys/keys_test.go b/zetaclient/keys/keys_test.go index 5cbdeec5d5..2b37f5a63c 100644 --- a/zetaclient/keys/keys_test.go +++ b/zetaclient/keys/keys_test.go @@ -63,7 +63,7 @@ func (*KeysSuite) setupKeysForTest(c *C) string { func (ks *KeysSuite) TestGetKeyringKeybase(c *C) { keyring.Debug = true - cfg := &config.Config{ + cfg := config.Config{ AuthzHotkey: "bob", ZetaCoreHome: "/Users/test/.zetacored/", } @@ -83,7 +83,7 @@ func (ks *KeysSuite) TestNewKeys(c *C) { c.Assert(err, IsNil) }() - cfg := &config.Config{ + cfg := config.Config{ AuthzHotkey: signerNameForTest, ZetaCoreHome: folder, } diff --git a/zetaclient/testutils/testdata.go b/zetaclient/testutils/testdata.go index 8918476217..0676824514 100644 --- a/zetaclient/testutils/testdata.go +++ b/zetaclient/testutils/testdata.go @@ -50,8 +50,8 @@ func LoadObjectFromJSONFile(obj interface{}, filename string) error { return decoder.Decode(&obj) } -func ComplianceConfigTest() *config.ComplianceConfig { - return &config.ComplianceConfig{ +func ComplianceConfigTest() config.ComplianceConfig { + return config.ComplianceConfig{ RestrictedAddresses: []string{RestrictedEVMAddressTest, RestrictedBtcAddressTest}, } } diff --git a/zetaclient/tss/tss_signer.go b/zetaclient/tss/tss_signer.go index f9abdb78ee..b95baadf1b 100644 --- a/zetaclient/tss/tss_signer.go +++ b/zetaclient/tss/tss_signer.go @@ -127,7 +127,7 @@ func NewTSS( return &newTss, nil } -func SetupTSSServer(peer p2p.AddrList, privkey tmcrypto.PrivKey, preParams *keygen.LocalPreParams, cfg *config.Config, tssPassword string) (*tss.TssServer, error) { +func SetupTSSServer(peer p2p.AddrList, privkey tmcrypto.PrivKey, preParams *keygen.LocalPreParams, cfg config.Config, tssPassword string) (*tss.TssServer, error) { bootstrapPeers := peer log.Info().Msgf("Peers AddrList %v", bootstrapPeers) From 828529c2b3c5805378d357debc35c5a7fbab05e1 Mon Sep 17 00:00:00 2001 From: Lucas Bertrand Date: Sat, 9 Mar 2024 11:19:45 +0100 Subject: [PATCH 2/2] fix: rebase 14.0.1 (#1861) * remove observerslash amount from param keys for emissions * fix lint * fix: test * add tracked task * fix tests * fix tests --------- Co-authored-by: Tanmay --- changelog.md | 4 ++++ x/emissions/abci.go | 19 ++++++++++++++++--- x/emissions/abci_test.go | 4 +++- x/emissions/genesis_test.go | 7 ++++++- x/emissions/keeper/params_test.go | 17 +---------------- x/emissions/types/keys.go | 6 ++++++ x/emissions/types/params.go | 27 +++++++++------------------ 7 files changed, 45 insertions(+), 39 deletions(-) diff --git a/changelog.md b/changelog.md index d4b8145915..db924b3ee5 100644 --- a/changelog.md +++ b/changelog.md @@ -30,6 +30,10 @@ * [1787](https://github.com/zeta-chain/node/pull/1787) - add unit tests for cross-chain evm hooks and e2e test failed withdraw to BTC legacy address * [1840](https://github.com/zeta-chain/node/pull/1840) - fix code coverage test failures ignored in CI +### Fixes + +* [1861](https://github.com/zeta-chain/node/pull/1861) - fix `ObserverSlashAmount` invalid read + ### Chores * [1814](https://github.com/zeta-chain/node/pull/1814) - fix code coverage ignore for protobuf generated files diff --git a/x/emissions/abci.go b/x/emissions/abci.go index 2fb0561bb6..96be4ae613 100644 --- a/x/emissions/abci.go +++ b/x/emissions/abci.go @@ -22,6 +22,14 @@ func BeginBlocker(ctx sdk.Context, keeper keeper.Keeper) { observerRewards := sdk.MustNewDecFromStr(keeper.GetParams(ctx).ObserverEmissionPercentage).Mul(blockRewards).TruncateInt() tssSignerRewards := sdk.MustNewDecFromStr(keeper.GetParams(ctx).TssSignerEmissionPercentage).Mul(blockRewards).TruncateInt() + // TODO : Replace hardcoded slash amount with a parameter + // https://github.com/zeta-chain/node/pull/1861 + slashAmount, ok := sdkmath.NewIntFromString(types.ObserverSlashAmount) + if !ok { + ctx.Logger().Error(fmt.Sprintf("Error while parsing observer slash amount %s", types.ObserverSlashAmount)) + return + } + // Use a tmpCtx, which is a cache-wrapped context to avoid writing to the store // We commit only if all three distributions are successful, if not the funds stay in the emission pool tmpCtx, commit := ctx.CacheContext() @@ -30,7 +38,7 @@ func BeginBlocker(ctx sdk.Context, keeper keeper.Keeper) { ctx.Logger().Error(fmt.Sprintf("Error while distributing validator rewards %s", err)) return } - err = DistributeObserverRewards(tmpCtx, observerRewards, keeper) + err = DistributeObserverRewards(tmpCtx, observerRewards, keeper, slashAmount) if err != nil { ctx.Logger().Error(fmt.Sprintf("Error while distributing observer rewards %s", err)) return @@ -63,7 +71,12 @@ func DistributeValidatorRewards(ctx sdk.Context, amount sdkmath.Int, bankKeeper // NotVoted or Unsuccessful votes are slashed // rewards given or slashed amounts are in azeta -func DistributeObserverRewards(ctx sdk.Context, amount sdkmath.Int, keeper keeper.Keeper) error { +func DistributeObserverRewards( + ctx sdk.Context, + amount sdkmath.Int, + keeper keeper.Keeper, + slashAmount sdkmath.Int, +) error { rewardsDistributer := map[string]int64{} totalRewardsUnits := int64(0) @@ -113,7 +126,7 @@ func DistributeObserverRewards(ctx sdk.Context, amount sdkmath.Int, keeper keepe continue } if observerRewardUnits < 0 { - slashAmount := keeper.GetParams(ctx).ObserverSlashAmount + keeper.SlashObserverEmission(ctx, observerAddress.String(), slashAmount) finalDistributionList = append(finalDistributionList, &types.ObserverEmission{ EmissionType: types.EmissionType_Slash, diff --git a/x/emissions/abci_test.go b/x/emissions/abci_test.go index 07240d1450..a4a66b34a0 100644 --- a/x/emissions/abci_test.go +++ b/x/emissions/abci_test.go @@ -153,6 +153,7 @@ func TestBeginBlocker(t *testing.T) { } func TestDistributeObserverRewards(t *testing.T) { + keepertest.SetConfig(false) observerSet := sample.ObserverSet(4) tt := []struct { @@ -286,8 +287,9 @@ func TestDistributeObserverRewards(t *testing.T) { ctx = ctx.WithBlockHeight(100) // Distribute the rewards and check if the rewards are distributed correctly - err = emissionsModule.DistributeObserverRewards(ctx, tc.totalRewardsForBlock, *k) + err = emissionsModule.DistributeObserverRewards(ctx, tc.totalRewardsForBlock, *k, tc.slashAmount) require.NoError(t, err) + for i, observer := range observerSet.ObserverList { observerEmission, found := k.GetWithdrawableEmission(ctx, observer) require.True(t, found, "withdrawable emission not found for observer %d", i) diff --git a/x/emissions/genesis_test.go b/x/emissions/genesis_test.go index e1bebddee5..79edd3bf90 100644 --- a/x/emissions/genesis_test.go +++ b/x/emissions/genesis_test.go @@ -3,6 +3,8 @@ package emissions_test import ( "testing" + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/stretchr/testify/require" keepertest "github.com/zeta-chain/zetacore/testutil/keeper" "github.com/zeta-chain/zetacore/testutil/nullify" @@ -12,8 +14,11 @@ import ( ) func TestGenesis(t *testing.T) { + params := types.DefaultParams() + params.ObserverSlashAmount = sdk.Int{} + genesisState := types.GenesisState{ - Params: types.DefaultParams(), + Params: params, WithdrawableEmissions: []types.WithdrawableEmissions{ sample.WithdrawableEmissions(t), sample.WithdrawableEmissions(t), diff --git a/x/emissions/keeper/params_test.go b/x/emissions/keeper/params_test.go index f861a944a3..e73031b824 100644 --- a/x/emissions/keeper/params_test.go +++ b/x/emissions/keeper/params_test.go @@ -3,7 +3,6 @@ package keeper_test import ( "testing" - sdkmath "cosmossdk.io/math" "github.com/stretchr/testify/require" keepertest "github.com/zeta-chain/zetacore/testutil/keeper" emissionstypes "github.com/zeta-chain/zetacore/x/emissions/types" @@ -26,7 +25,6 @@ func TestKeeper_GetParams(t *testing.T) { ObserverEmissionPercentage: "00.25", TssSignerEmissionPercentage: "00.25", DurationFactorConstant: "0.001877876953694702", - ObserverSlashAmount: sdkmath.NewInt(100000000000000000), }, isPanic: "", }, @@ -41,7 +39,6 @@ func TestKeeper_GetParams(t *testing.T) { ObserverEmissionPercentage: "00.25", TssSignerEmissionPercentage: "00.25", DurationFactorConstant: "0.001877876953694702", - ObserverSlashAmount: sdkmath.NewInt(-10), }, isPanic: "slash amount cannot be less than 0", }, @@ -56,7 +53,6 @@ func TestKeeper_GetParams(t *testing.T) { ObserverEmissionPercentage: "00.25", TssSignerEmissionPercentage: "00.25", DurationFactorConstant: "0.001877876953694702", - ObserverSlashAmount: sdkmath.NewInt(100000000000000000), }, isPanic: "max bond factor cannot be higher that 0.25", }, @@ -71,7 +67,6 @@ func TestKeeper_GetParams(t *testing.T) { ObserverEmissionPercentage: "00.25", TssSignerEmissionPercentage: "00.25", DurationFactorConstant: "0.001877876953694702", - ObserverSlashAmount: sdkmath.NewInt(100000000000000000), }, isPanic: "min bond factor cannot be lower that 0.75", }, @@ -86,7 +81,6 @@ func TestKeeper_GetParams(t *testing.T) { ObserverEmissionPercentage: "00.25", TssSignerEmissionPercentage: "00.25", DurationFactorConstant: "0.001877876953694702", - ObserverSlashAmount: sdkmath.NewInt(100000000000000000), }, isPanic: "invalid block time", }, @@ -101,7 +95,6 @@ func TestKeeper_GetParams(t *testing.T) { ObserverEmissionPercentage: "00.25", TssSignerEmissionPercentage: "00.25", DurationFactorConstant: "0.001877876953694702", - ObserverSlashAmount: sdkmath.NewInt(100000000000000000), }, isPanic: "block time cannot be less than or equal to 0", }, @@ -116,7 +109,6 @@ func TestKeeper_GetParams(t *testing.T) { ObserverEmissionPercentage: "00.25", TssSignerEmissionPercentage: "00.25", DurationFactorConstant: "0.001877876953694702", - ObserverSlashAmount: sdkmath.NewInt(100000000000000000), }, isPanic: "target bond ratio cannot be more than 100 percent", }, @@ -131,7 +123,6 @@ func TestKeeper_GetParams(t *testing.T) { ObserverEmissionPercentage: "00.25", TssSignerEmissionPercentage: "00.25", DurationFactorConstant: "0.001877876953694702", - ObserverSlashAmount: sdkmath.NewInt(100000000000000000), }, isPanic: "target bond ratio cannot be less than 0 percent", }, @@ -146,7 +137,6 @@ func TestKeeper_GetParams(t *testing.T) { ObserverEmissionPercentage: "00.25", TssSignerEmissionPercentage: "00.25", DurationFactorConstant: "0.001877876953694702", - ObserverSlashAmount: sdkmath.NewInt(100000000000000000), }, isPanic: "validator emission percentage cannot be more than 100 percent", }, @@ -161,7 +151,6 @@ func TestKeeper_GetParams(t *testing.T) { ObserverEmissionPercentage: "00.25", TssSignerEmissionPercentage: "00.25", DurationFactorConstant: "0.001877876953694702", - ObserverSlashAmount: sdkmath.NewInt(100000000000000000), }, isPanic: "validator emission percentage cannot be less than 0 percent", }, @@ -176,7 +165,6 @@ func TestKeeper_GetParams(t *testing.T) { ObserverEmissionPercentage: "-00.25", TssSignerEmissionPercentage: "00.25", DurationFactorConstant: "0.001877876953694702", - ObserverSlashAmount: sdkmath.NewInt(100000000000000000), }, isPanic: "observer emission percentage cannot be less than 0 percent", }, @@ -191,7 +179,6 @@ func TestKeeper_GetParams(t *testing.T) { ObserverEmissionPercentage: "150.25", TssSignerEmissionPercentage: "00.25", DurationFactorConstant: "0.001877876953694702", - ObserverSlashAmount: sdkmath.NewInt(100000000000000000), }, isPanic: "observer emission percentage cannot be more than 100 percent", }, @@ -206,12 +193,11 @@ func TestKeeper_GetParams(t *testing.T) { ObserverEmissionPercentage: "00.25", TssSignerEmissionPercentage: "102.22", DurationFactorConstant: "0.001877876953694702", - ObserverSlashAmount: sdkmath.NewInt(100000000000000000), }, isPanic: "tss emission percentage cannot be more than 100 percent", }, { - name: "tss signer percentage too loo", + name: "tss signer percentage too low", params: emissionstypes.Params{ MaxBondFactor: "1.25", MinBondFactor: "0.75", @@ -221,7 +207,6 @@ func TestKeeper_GetParams(t *testing.T) { ObserverEmissionPercentage: "00.25", TssSignerEmissionPercentage: "-102.22", DurationFactorConstant: "0.001877876953694702", - ObserverSlashAmount: sdkmath.NewInt(100000000000000000), }, isPanic: "tss emission percentage cannot be less than 0 percent", }, diff --git a/x/emissions/types/keys.go b/x/emissions/types/keys.go index 9f1fa705fc..21b07cb854 100644 --- a/x/emissions/types/keys.go +++ b/x/emissions/types/keys.go @@ -29,6 +29,12 @@ const ( EmissionScheduledYears = 4 AvgBlockTime = "5.7" + + // ObserverSlashAmount is the amount of tokens to be slashed from observer in case of incorrect vote + // it is set to 0.1 ZETA + // TODO: replace this with a parameter + // https://github.com/zeta-chain/node/pull/1861 + ObserverSlashAmount = "100000000000000000" ) func KeyPrefix(p string) []byte { diff --git a/x/emissions/types/params.go b/x/emissions/types/params.go index aa896f77e8..96267b0a70 100644 --- a/x/emissions/types/params.go +++ b/x/emissions/types/params.go @@ -4,7 +4,6 @@ import ( "fmt" "strconv" - sdkmath "cosmossdk.io/math" sdk "github.com/cosmos/cosmos-sdk/types" paramtypes "github.com/cosmos/cosmos-sdk/x/params/types" "gopkg.in/yaml.v2" @@ -20,11 +19,6 @@ func ParamKeyTable() paramtypes.KeyTable { // NewParams creates a new Params instance func NewParams() Params { - defaultSlashAmount := sdk.ZeroInt() - intSlashAmount, ok := sdkmath.NewIntFromString("100000000000000000") - if ok { - defaultSlashAmount = intSlashAmount - } return Params{ MaxBondFactor: "1.25", MinBondFactor: "0.75", @@ -34,7 +28,11 @@ func NewParams() Params { ObserverEmissionPercentage: "00.25", TssSignerEmissionPercentage: "00.25", DurationFactorConstant: "0.001877876953694702", - ObserverSlashAmount: defaultSlashAmount, + + // ObserverSlashAmount is currently disabled + // TODO: enable this param + // https://github.com/zeta-chain/node/issues/1862 + ObserverSlashAmount: sdk.Int{}, } } @@ -54,7 +52,10 @@ func (p *Params) ParamSetPairs() paramtypes.ParamSetPairs { paramtypes.NewParamSetPair(KeyPrefix(ParamObserverEmissionPercentage), &p.ObserverEmissionPercentage, validateObserverEmissionPercentage), paramtypes.NewParamSetPair(KeyPrefix(ParamTssSignerEmissionPercentage), &p.TssSignerEmissionPercentage, validateTssEmissonPercentage), paramtypes.NewParamSetPair(KeyPrefix(ParamDurationFactorConstant), &p.DurationFactorConstant, validateDurationFactorConstant), - paramtypes.NewParamSetPair(KeyPrefix(ParamObserverSlashAmount), &p.ObserverSlashAmount, validateObserverSlashAmount), + + // TODO: enable this param + // https://github.com/zeta-chain/node/pull/1861 + //paramtypes.NewParamSetPair(KeyPrefix(ParamObserverSlashAmount), &p.ObserverSlashAmount, validateObserverSlashAmount), } } @@ -72,16 +73,6 @@ func (p Params) String() string { return string(out) } -func validateObserverSlashAmount(i interface{}) error { - v, ok := i.(sdkmath.Int) - if !ok { - return fmt.Errorf("invalid parameter type: %T", i) - } - if v.LT(sdk.ZeroInt()) { - return fmt.Errorf("slash amount cannot be less than 0") - } - return nil -} func validateDurationFactorConstant(i interface{}) error { _, ok := i.(string) if !ok {