From 7b8881b5fb6680572373c23dbc0b2fd4cb5f0607 Mon Sep 17 00:00:00 2001 From: Ekaterina Lebedeva Date: Tue, 6 Feb 2024 02:58:00 +0300 Subject: [PATCH 1/2] [#905] Make notary support enabled by default In morph client notary support is enabled by default. Added method to disable notary support. Removed usage of deprecated method enabling notary support from code where it's possible. Signed-off-by: Ekaterina Lebedeva --- cmd/frostfs-node/config.go | 2 +- cmd/frostfs-node/morph.go | 17 ++++++------- cmd/frostfs-node/netmap.go | 20 +++++++++------- pkg/innerring/initialization.go | 22 +++-------------- pkg/innerring/innerring.go | 12 +++++++++- pkg/morph/client/constructor.go | 12 ++++++++++ pkg/morph/client/notary.go | 40 ++++++++++++++++++++++++------- pkg/morph/client/notifications.go | 2 +- 8 files changed, 77 insertions(+), 50 deletions(-) diff --git a/cmd/frostfs-node/config.go b/cmd/frostfs-node/config.go index 50219a8c7..e662d3fd3 100644 --- a/cmd/frostfs-node/config.go +++ b/cmd/frostfs-node/config.go @@ -552,7 +552,7 @@ func (c *cfgGRPC) dropConnection(endpoint string) { type cfgMorph struct { client *client.Client - notaryEnabled bool + notaryDisabled bool // TTL of Sidechain cached values. Non-positive value disables caching. cacheTTL time.Duration diff --git a/cmd/frostfs-node/morph.go b/cmd/frostfs-node/morph.go index 698fb3b83..64b5172a1 100644 --- a/cmd/frostfs-node/morph.go +++ b/cmd/frostfs-node/morph.go @@ -48,6 +48,7 @@ func initMorphComponents(ctx context.Context, c *cfg) { }), client.WithSwitchInterval(morphconfig.SwitchInterval(c.appCfg)), client.WithMorphCacheMetrics(c.metricsCollector.MorphCacheMetrics()), + client.WithNotaryOptions(client.WithProxyContract(c.cfgMorph.proxyScriptHash)), ) if err != nil { c.log.Info(logs.FrostFSNodeFailedToCreateNeoRPCClient, @@ -68,21 +69,17 @@ func initMorphComponents(ctx context.Context, c *cfg) { } c.cfgMorph.client = cli - c.cfgMorph.notaryEnabled = cli.ProbeNotary() + c.cfgMorph.notaryDisabled = !cli.ProbeNotary() lookupScriptHashesInNNS(c) // smart contract auto negotiation - if c.cfgMorph.notaryEnabled { - err = c.cfgMorph.client.EnableNotarySupport( - client.WithProxyContract( - c.cfgMorph.proxyScriptHash, - ), - ) + if c.cfgMorph.notaryDisabled { + err := c.cfgMorph.client.DisableNotarySupport() fatalOnErr(err) } c.log.Info(logs.FrostFSNodeNotarySupport, - zap.Bool("sidechain_enabled", c.cfgMorph.notaryEnabled), + zap.Bool("sidechain_disabled", c.cfgMorph.notaryDisabled), ) wrap, err := nmClient.NewFromMorph(c.cfgMorph.client, c.cfgNetmap.scriptHash, 0, nmClient.TryNotary()) @@ -112,7 +109,7 @@ func initMorphComponents(ctx context.Context, c *cfg) { func makeAndWaitNotaryDeposit(ctx context.Context, c *cfg) { // skip notary deposit in non-notary environments - if !c.cfgMorph.notaryEnabled { + if c.cfgMorph.notaryDisabled { return } @@ -295,7 +292,7 @@ func lookupScriptHashesInNNS(c *cfg) { ) for _, t := range targets { - if t.nnsName == client.NNSProxyContractName && !c.cfgMorph.notaryEnabled { + if t.nnsName == client.NNSProxyContractName && c.cfgMorph.notaryDisabled { continue // ignore proxy contract if notary disabled } diff --git a/cmd/frostfs-node/netmap.go b/cmd/frostfs-node/netmap.go index b21e842c5..80bf7801c 100644 --- a/cmd/frostfs-node/netmap.go +++ b/cmd/frostfs-node/netmap.go @@ -202,16 +202,18 @@ func addNewEpochNotificationHandlers(c *cfg) { c.handleLocalNodeInfo(ni) }) - if c.cfgMorph.notaryEnabled { - addNewEpochAsyncNotificationHandler(c, func(ev event.Event) { - _, err := makeNotaryDeposit(c) - if err != nil { - c.log.Error(logs.FrostFSNodeCouldNotMakeNotaryDeposit, - zap.String("error", err.Error()), - ) - } - }) + if c.cfgMorph.notaryDisabled { + return } + + addNewEpochAsyncNotificationHandler(c, func(ev event.Event) { + _, err := makeNotaryDeposit(c) + if err != nil { + c.log.Error(logs.FrostFSNodeCouldNotMakeNotaryDeposit, + zap.String("error", err.Error()), + ) + } + }) } // bootstrapNode adds current node to the Network map. diff --git a/pkg/innerring/initialization.go b/pkg/innerring/initialization.go index f4d9b4169..c8c62b559 100644 --- a/pkg/innerring/initialization.go +++ b/pkg/innerring/initialization.go @@ -16,7 +16,6 @@ import ( nodevalidator "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/innerring/processors/netmap/nodevalidation" addrvalidator "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/innerring/processors/netmap/nodevalidation/maddress" statevalidation "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/innerring/processors/netmap/nodevalidation/state" - "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/morph/client" balanceClient "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/morph/client/balance" "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/morph/client/container" frostfsClient "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/morph/client/frostfs" @@ -107,7 +106,7 @@ func (s *Server) initMainnet(ctx context.Context, cfg *viper.Viper, morphChain * mainnetChain.from = fromMainChainBlock // create mainnet client - s.mainnetClient, err = createClient(ctx, mainnetChain, errChan) + s.mainnetClient, err = createClient(ctx, mainnetChain, errChan, s.contracts.processing, s.morphClient.Committee) if err != nil { return err } @@ -118,25 +117,10 @@ func (s *Server) initMainnet(ctx context.Context, cfg *viper.Viper, morphChain * } func (s *Server) enableNotarySupport() error { - // enable notary support in the side client - err := s.morphClient.EnableNotarySupport( - client.WithProxyContract(s.contracts.proxy), - ) - if err != nil { - return fmt.Errorf("could not enable side chain notary support: %w", err) - } - s.morphListener.EnableNotarySupport(s.contracts.proxy, s.morphClient.Committee, s.morphClient) if !s.mainNotaryConfig.disabled { - // enable notary support in the main client - err := s.mainnetClient.EnableNotarySupport( - client.WithProxyContract(s.contracts.processing), - client.WithAlphabetSource(s.morphClient.Committee), - ) - if err != nil { - return fmt.Errorf("could not enable main chain notary support: %w", err) - } + return fmt.Errorf("main chain notary support is disabled") } return nil @@ -470,7 +454,7 @@ func (s *Server) initMorph(ctx context.Context, cfg *viper.Viper, errChan chan<- } // create morph client - s.morphClient, err = createClient(ctx, morphChain, errChan) + s.morphClient, err = createClient(ctx, morphChain, errChan, s.contracts.proxy, nil) if err != nil { return nil, err } diff --git a/pkg/innerring/innerring.go b/pkg/innerring/innerring.go index 5d7dc5a52..260c24177 100644 --- a/pkg/innerring/innerring.go +++ b/pkg/innerring/innerring.go @@ -444,7 +444,7 @@ func createListener(ctx context.Context, cli *client.Client, p *chainParams) (ev return listener, err } -func createClient(ctx context.Context, p *chainParams, errChan chan<- error) (*client.Client, error) { +func createClient(ctx context.Context, p *chainParams, errChan chan<- error, proxyHash util.Uint160, src client.AlphabetKeys) (*client.Client, error) { // config name left unchanged for compatibility, may be its better to rename it to "endpoints" or "clients" var endpoints []client.Endpoint @@ -473,6 +473,15 @@ func createClient(ctx context.Context, p *chainParams, errChan chan<- error) (*c return nil, fmt.Errorf("%s chain client endpoints not provided", p.name) } + notaryOpts := client.WithNotaryOptions(client.WithProxyContract(proxyHash)) + + if src != nil { + notaryOpts = client.WithNotaryOptions( + client.WithProxyContract(proxyHash), + client.WithAlphabetSource(src), + ) + } + return client.New( ctx, p.key, @@ -485,6 +494,7 @@ func createClient(ctx context.Context, p *chainParams, errChan chan<- error) (*c }), client.WithSwitchInterval(p.cfg.GetDuration(p.name+".switch_interval")), client.WithMorphCacheMetrics(p.morphCacheMetric), + notaryOpts, ) } diff --git a/pkg/morph/client/constructor.go b/pkg/morph/client/constructor.go index 50a9572d4..623f45987 100644 --- a/pkg/morph/client/constructor.go +++ b/pkg/morph/client/constructor.go @@ -48,6 +48,8 @@ type cfg struct { switchInterval time.Duration morphCacheMetrics metrics.MorphCacheMetrics + + notaryOpts []NotaryOption } const ( @@ -157,6 +159,10 @@ func New(ctx context.Context, key *keys.PrivateKey, opts ...Option) (*Client, er } cli.setActor(act) + if err = cli.enableNotarySupport(cfg.notaryOpts...); err != nil { + return nil, err + } + go cli.closeWaiter(ctx) return cli, nil @@ -311,3 +317,9 @@ func WithMorphCacheMetrics(morphCacheMetrics metrics.MorphCacheMetrics) Option { c.morphCacheMetrics = morphCacheMetrics } } + +func WithNotaryOptions(opts ...NotaryOption) Option { + return func(c *cfg) { + c.notaryOpts = append(c.notaryOpts, opts...) + } +} diff --git a/pkg/morph/client/notary.go b/pkg/morph/client/notary.go index 5b817a805..8f317974f 100644 --- a/pkg/morph/client/notary.go +++ b/pkg/morph/client/notary.go @@ -61,11 +61,14 @@ const ( notaryExpirationOfMethod = "expirationOf" setDesignateMethod = "designateAsRole" - notaryBalanceErrMsg = "can't fetch notary balance" - notaryNotEnabledPanicMsg = "notary support was not enabled on this client" + notaryBalanceErrMsg = "can't fetch notary balance" + notaryDisabledErrMsg = "notary support was not enabled on this client" ) -var errUnexpectedItems = errors.New("invalid number of NEO VM arguments on stack") +var ( + errUnexpectedItems = errors.New("invalid number of NEO VM arguments on stack") + ErrNotaryDisabled = errors.New(notaryDisabledErrMsg) +) func defaultNotaryConfig(c *Client) *notaryCfg { return ¬aryCfg{ @@ -78,7 +81,7 @@ func defaultNotaryConfig(c *Client) *notaryCfg { // EnableNotarySupport creates notary structure in client that provides // ability for client to get alphabet keys from committee or provided source // and use proxy contract script hash to create tx for notary contract. -func (c *Client) EnableNotarySupport(opts ...NotaryOption) error { +func (c *Client) enableNotarySupport(opts ...NotaryOption) error { c.switchLock.RLock() defer c.switchLock.RUnlock() @@ -114,6 +117,25 @@ func (c *Client) EnableNotarySupport(opts ...NotaryOption) error { return nil } +// Deprecated: Notary support is enabled by default during client construction. +func (c *Client) EnableNotarySupport(opts ...NotaryOption) error { + return c.enableNotarySupport(opts...) +} + +// DisableNotarySupport removes notary structure in client. +func (c *Client) DisableNotarySupport() error { + c.switchLock.Lock() + defer c.switchLock.Unlock() + + if c.inactive { + return ErrConnectionLost + } + + c.notary = nil + + return nil +} + // IsNotaryEnabled returns true if EnableNotarySupport has been successfully // called before. func (c *Client) IsNotaryEnabled() bool { @@ -149,7 +171,7 @@ func (c *Client) DepositNotary(amount fixedn.Fixed8, delta uint32) (res util.Uin } if c.notary == nil { - panic(notaryNotEnabledPanicMsg) + return util.Uint256{}, fmt.Errorf(notaryDisabledErrMsg) } bc, err := c.rpcActor.GetBlockCount() @@ -184,7 +206,7 @@ func (c *Client) DepositEndlessNotary(amount fixedn.Fixed8) (res util.Uint256, e } if c.notary == nil { - panic(notaryNotEnabledPanicMsg) + return util.Uint256{}, ErrNotaryDisabled } // till value refers to a block height and it is uint32 value in neo-go @@ -234,7 +256,7 @@ func (c *Client) GetNotaryDeposit() (res int64, err error) { } if c.notary == nil { - panic(notaryNotEnabledPanicMsg) + return 0, ErrNotaryDisabled } sh := c.acc.PrivateKey().PublicKey().GetScriptHash() @@ -286,7 +308,7 @@ func (c *Client) UpdateNotaryList(prm UpdateNotaryListPrm) error { } if c.notary == nil { - panic(notaryNotEnabledPanicMsg) + return ErrNotaryDisabled } nonce, vub, err := c.CalculateNonceAndVUB(&prm.hash) @@ -334,7 +356,7 @@ func (c *Client) UpdateNeoFSAlphabetList(prm UpdateAlphabetListPrm) error { } if c.notary == nil { - panic(notaryNotEnabledPanicMsg) + return ErrNotaryDisabled } nonce, vub, err := c.CalculateNonceAndVUB(&prm.hash) diff --git a/pkg/morph/client/notifications.go b/pkg/morph/client/notifications.go index 35204bb36..4bdd71ca8 100644 --- a/pkg/morph/client/notifications.go +++ b/pkg/morph/client/notifications.go @@ -63,7 +63,7 @@ func (c *Client) ReceiveBlocks(ch chan<- *block.Block) (string, error) { // connection to any of passed RPC endpoints. func (c *Client) ReceiveNotaryRequests(txSigner util.Uint160, ch chan<- *result.NotaryRequestEvent) (string, error) { if c.notary == nil { - panic(notaryNotEnabledPanicMsg) + panic(notaryDisabledErrMsg) } c.switchLock.Lock() -- 2.45.2 From ff0f029788fd99f4a3931c2e6cae1d8f10e2b212 Mon Sep 17 00:00:00 2001 From: Ekaterina Lebedeva Date: Tue, 6 Feb 2024 03:20:35 +0300 Subject: [PATCH 2/2] [#905] Make morph client non-panic-prone Morph client returns errors instead. Signed-off-by: Ekaterina Lebedeva --- pkg/morph/client/constructor.go | 4 +--- pkg/morph/client/notary.go | 10 +++++----- pkg/morph/client/notifications.go | 2 +- 3 files changed, 7 insertions(+), 9 deletions(-) diff --git a/pkg/morph/client/constructor.go b/pkg/morph/client/constructor.go index 623f45987..f4fc48b5d 100644 --- a/pkg/morph/client/constructor.go +++ b/pkg/morph/client/constructor.go @@ -76,8 +76,6 @@ func defaultConfig() *cfg { // Notary support should be enabled with EnableNotarySupport client // method separately. // -// If private key is nil, it panics. -// // Other values are set according to provided options, or by default: // - client context: Background; // - dial timeout: 5s; @@ -94,7 +92,7 @@ func defaultConfig() *cfg { // If there are no healthy endpoint - returns ErrNoHealthyEndpoint. func New(ctx context.Context, key *keys.PrivateKey, opts ...Option) (*Client, error) { if key == nil { - panic("empty private key") + return nil, errors.New("empty private key") } acc := wallet.NewAccountFromPrivateKey(key) diff --git a/pkg/morph/client/notary.go b/pkg/morph/client/notary.go index 8f317974f..99119853b 100644 --- a/pkg/morph/client/notary.go +++ b/pkg/morph/client/notary.go @@ -161,7 +161,7 @@ func (c *Client) ProbeNotary() (res bool) { // be called periodically. Notary support should be enabled in client to // use this function. // -// This function must be invoked with notary enabled otherwise it throws panic. +// This function must be invoked with notary enabled. func (c *Client) DepositNotary(amount fixedn.Fixed8, delta uint32) (res util.Uint256, err error) { c.switchLock.RLock() defer c.switchLock.RUnlock() @@ -196,7 +196,7 @@ func (c *Client) DepositNotary(amount fixedn.Fixed8, delta uint32) (res util.Uin // this method sets notary deposit till parameter to a maximum possible value. // This allows to avoid ValidAfterDeposit failures. // -// This function must be invoked with notary enabled otherwise it throws panic. +// This function must be invoked with notary enabled. func (c *Client) DepositEndlessNotary(amount fixedn.Fixed8) (res util.Uint256, err error) { c.switchLock.RLock() defer c.switchLock.RUnlock() @@ -246,7 +246,7 @@ func (c *Client) depositNotary(amount fixedn.Fixed8, till int64) (res util.Uint2 // GetNotaryDeposit returns deposit of client's account in notary contract. // Notary support should be enabled in client to use this function. // -// This function must be invoked with notary enabled otherwise it throws panic. +// This function must be invoked with notary enabled. func (c *Client) GetNotaryDeposit() (res int64, err error) { c.switchLock.RLock() defer c.switchLock.RUnlock() @@ -298,7 +298,7 @@ func (u *UpdateNotaryListPrm) SetHash(hash util.Uint256) { // UpdateNotaryList updates list of notary nodes in designate contract. Requires // committee multi signature. // -// This function must be invoked with notary enabled otherwise it throws panic. +// This function must be invoked with notary enabled. func (c *Client) UpdateNotaryList(prm UpdateNotaryListPrm) error { c.switchLock.RLock() defer c.switchLock.RUnlock() @@ -346,7 +346,7 @@ func (u *UpdateAlphabetListPrm) SetHash(hash util.Uint256) { // As for sidechain list should contain all inner ring nodes. // Requires committee multi signature. // -// This function must be invoked with notary enabled otherwise it throws panic. +// This function must be invoked with notary enabled. func (c *Client) UpdateNeoFSAlphabetList(prm UpdateAlphabetListPrm) error { c.switchLock.RLock() defer c.switchLock.RUnlock() diff --git a/pkg/morph/client/notifications.go b/pkg/morph/client/notifications.go index 4bdd71ca8..f58651cce 100644 --- a/pkg/morph/client/notifications.go +++ b/pkg/morph/client/notifications.go @@ -63,7 +63,7 @@ func (c *Client) ReceiveBlocks(ch chan<- *block.Block) (string, error) { // connection to any of passed RPC endpoints. func (c *Client) ReceiveNotaryRequests(txSigner util.Uint160, ch chan<- *result.NotaryRequestEvent) (string, error) { if c.notary == nil { - panic(notaryDisabledErrMsg) + return "", ErrNotaryDisabled } c.switchLock.Lock() -- 2.45.2