Skip to content

Commit

Permalink
refactor: removing unnecessary pointers in context structure (#1831)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
skosito authored Mar 8, 2024
1 parent 9aacfb7 commit f8e8565
Show file tree
Hide file tree
Showing 20 changed files with 341 additions and 83 deletions.
1 change: 1 addition & 0 deletions changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
6 changes: 3 additions & 3 deletions cmd/zetaclientd/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
}

Expand Down
2 changes: 1 addition & 1 deletion cmd/zetaclientd/p2p_diagnostics.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
5 changes: 3 additions & 2 deletions cmd/zetaclientd/start.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
16 changes: 7 additions & 9 deletions cmd/zetaclientd/start_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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,
}
Expand Down
4 changes: 2 additions & 2 deletions cmd/zetaclientd/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
10 changes: 5 additions & 5 deletions zetaclient/app_context/app_context.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,30 +9,30 @@ 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,
config: config,
}
}

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()

Expand Down
2 changes: 1 addition & 1 deletion zetaclient/bitcoin/bitcoin_client_rpc_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions zetaclient/common/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
12 changes: 6 additions & 6 deletions zetaclient/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,32 +36,32 @@ 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)

// read config
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
if cfg.KeyringBackend == KeyringBackendUndefined {
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
Expand All @@ -75,7 +75,7 @@ func Load(path string) (*Config, error) {
return cfg, nil
}

func LoadComplianceConfig(cfg *Config) {
func LoadComplianceConfig(cfg Config) {
restrictedAddressBook = cfg.GetRestrictedAddressBook()
}

Expand Down
4 changes: 2 additions & 2 deletions zetaclient/config/config_chain.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
},
Expand Down
57 changes: 25 additions & 32 deletions zetaclient/config/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 {
Expand Down
13 changes: 8 additions & 5 deletions zetaclient/core_context/zeta_core_context.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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{}
Expand All @@ -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,
Expand Down Expand Up @@ -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 {
Expand Down
Loading

0 comments on commit f8e8565

Please sign in to comment.