From 3e3991cef8e397557eee2c55a8fee10df5bbbcc1 Mon Sep 17 00:00:00 2001 From: Anna Shaleva Date: Tue, 13 Aug 2024 18:57:31 +0300 Subject: [PATCH 1/5] rpcclient: remove default PolingBased poll interval pollTime is never 0 since MillisecondsPerBlock protocol configuration value is present in `getversion` RPC response since 0.97.3 release. We don't have such old RPC servers in the network anymore, thus this fallback code may be safely removed. Signed-off-by: Anna Shaleva --- pkg/rpcclient/waiter/waiter.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/pkg/rpcclient/waiter/waiter.go b/pkg/rpcclient/waiter/waiter.go index 830fe20cf..e118dfe3b 100644 --- a/pkg/rpcclient/waiter/waiter.go +++ b/pkg/rpcclient/waiter/waiter.go @@ -167,9 +167,6 @@ func (w *PollingBased) WaitAny(ctx context.Context, vub uint32, hashes ...util.U failedAttempt int pollTime = time.Millisecond * time.Duration(w.version.Protocol.MillisecondsPerBlock) / 2 ) - if pollTime == 0 { - pollTime = time.Second - } timer := time.NewTicker(pollTime) defer timer.Stop() for { From 027d726b656101b834f66c1c78328d9d28c5d46c Mon Sep 17 00:00:00 2001 From: Anna Shaleva Date: Tue, 13 Aug 2024 18:10:15 +0300 Subject: [PATCH 2/5] rpcclient: allow to tune PollingBased waiter Some clients need more flexible awaiting options (e.g. for short-blocks networks). The default behaviour is not changed, all exported APIs are compatible. Ref. https://github.com/nspcc-dev/neofs-node/issues/2864. Signed-off-by: Anna Shaleva --- pkg/rpcclient/waiter/waiter.go | 65 +++++++++++++++++++++++++--------- 1 file changed, 48 insertions(+), 17 deletions(-) diff --git a/pkg/rpcclient/waiter/waiter.go b/pkg/rpcclient/waiter/waiter.go index e118dfe3b..95d11fdd7 100644 --- a/pkg/rpcclient/waiter/waiter.go +++ b/pkg/rpcclient/waiter/waiter.go @@ -15,11 +15,11 @@ import ( "github.com/nspcc-dev/neo-go/pkg/util" ) -// PollingBasedRetryCount is a threshold for a number of subsequent failed +// DefaultPollRetryCount is a threshold for a number of subsequent failed // attempts to get block count from the RPC server for PollingBased. If it fails -// to retrieve block count PollingBasedRetryCount times in a raw then transaction +// to retrieve block count DefaultPollRetryCount times in a raw then transaction // awaiting attempt considered to be failed and an error is returned. -const PollingBasedRetryCount = 3 +const DefaultPollRetryCount = 3 var ( // ErrTxNotAccepted is returned when transaction wasn't accepted to the chain @@ -87,6 +87,17 @@ type Null struct{} type PollingBased struct { polling RPCPollingBased version *result.Version + config PollConfig +} + +// PollConfig is a configuration for PollingBased waiter. +type PollConfig struct { + // PollInterval is a time interval between subsequent polls. If not set, then + // default value is a half of configured block time (in milliseconds). + PollInterval time.Duration + // RetryCount is the number of retry attempts while fetching a subsequent block + // count before an error is returned from Wait or WaitAny. + RetryCount int } // EventBased is a websocket-based Waiter. @@ -109,18 +120,12 @@ func errIsAlreadyExists(err error) bool { func New(base any, v *result.Version) Waiter { if eventW, ok := base.(RPCEventBased); ok { return &EventBased{ - ws: eventW, - polling: &PollingBased{ - polling: eventW, - version: v, - }, + ws: eventW, + polling: newCustomPollingBased(eventW, v, PollConfig{}), } } if pollW, ok := base.(RPCPollingBased); ok { - return &PollingBased{ - polling: pollW, - version: v, - } + return newCustomPollingBased(pollW, v, PollConfig{}) } return NewNull() } @@ -142,14 +147,33 @@ func (Null) WaitAny(ctx context.Context, vub uint32, hashes ...util.Uint256) (*s // NewPollingBased creates an instance of Waiter supporting poll-based transaction awaiting. func NewPollingBased(waiter RPCPollingBased) (*PollingBased, error) { + return NewCustomPollingBased(waiter, PollConfig{}) +} + +// NewCustomPollingBased creates an instance of Waiter supporting poll-based transaction awaiting. +// Poll options may be specified via config parameter. +func NewCustomPollingBased(waiter RPCPollingBased, config PollConfig) (*PollingBased, error) { v, err := waiter.GetVersion() if err != nil { return nil, err } + return newCustomPollingBased(waiter, v, config), nil +} + +// newCustomPollingBased is an internal constructor of PollingBased waiter that sets +// default configuration values if needed. +func newCustomPollingBased(waiter RPCPollingBased, v *result.Version, config PollConfig) *PollingBased { + if config.PollInterval <= 0 { + config.PollInterval = time.Millisecond * time.Duration(v.Protocol.MillisecondsPerBlock) / 2 + } + if config.RetryCount <= 0 { + config.RetryCount = DefaultPollRetryCount + } return &PollingBased{ polling: waiter, version: v, - }, nil + config: config, + } } // Wait implements Waiter interface. @@ -165,9 +189,8 @@ func (w *PollingBased) WaitAny(ctx context.Context, vub uint32, hashes ...util.U var ( currentHeight uint32 failedAttempt int - pollTime = time.Millisecond * time.Duration(w.version.Protocol.MillisecondsPerBlock) / 2 ) - timer := time.NewTicker(pollTime) + timer := time.NewTicker(w.config.PollInterval) defer timer.Stop() for { select { @@ -175,7 +198,7 @@ func (w *PollingBased) WaitAny(ctx context.Context, vub uint32, hashes ...util.U blockCount, err := w.polling.GetBlockCount() if err != nil { failedAttempt++ - if failedAttempt > PollingBasedRetryCount { + if failedAttempt > w.config.RetryCount { return nil, fmt.Errorf("failed to retrieve block count: %w", err) } continue @@ -209,7 +232,15 @@ func (w *PollingBased) WaitAny(ctx context.Context, vub uint32, hashes ...util.U // EventBased contains PollingBased under the hood and falls back to polling when subscription-based // awaiting fails. func NewEventBased(waiter RPCEventBased) (*EventBased, error) { - polling, err := NewPollingBased(waiter) + return NewCustomEventBased(waiter, PollConfig{}) +} + +// NewCustomEventBased creates an instance of Waiter supporting websocket event-based transaction awaiting. +// EventBased contains PollingBased under the hood and falls back to polling when subscription-based +// awaiting fails. PollingBased configuration options may be specified via pollConfig parameter +// (defaults are used if not specified). +func NewCustomEventBased(waiter RPCEventBased, pollConfig PollConfig) (*EventBased, error) { + polling, err := NewCustomPollingBased(waiter, pollConfig) if err != nil { return nil, err } From afbb51e78c596c42f26a36944d9296f951396bc5 Mon Sep 17 00:00:00 2001 From: Anna Shaleva Date: Wed, 14 Aug 2024 10:26:29 +0300 Subject: [PATCH 3/5] rpcsrv: add test for Waiter constructor Ensure that WSClient-based Actor is able to create EventBased waiter. Signed-off-by: Anna Shaleva --- pkg/services/rpcsrv/client_test.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/pkg/services/rpcsrv/client_test.go b/pkg/services/rpcsrv/client_test.go index 3938f44a4..410be2544 100644 --- a/pkg/services/rpcsrv/client_test.go +++ b/pkg/services/rpcsrv/client_test.go @@ -51,6 +51,7 @@ import ( "github.com/nspcc-dev/neo-go/pkg/rpcclient/oracle" "github.com/nspcc-dev/neo-go/pkg/rpcclient/policy" "github.com/nspcc-dev/neo-go/pkg/rpcclient/rolemgmt" + "github.com/nspcc-dev/neo-go/pkg/rpcclient/waiter" "github.com/nspcc-dev/neo-go/pkg/smartcontract" "github.com/nspcc-dev/neo-go/pkg/smartcontract/callflag" "github.com/nspcc-dev/neo-go/pkg/smartcontract/manifest" @@ -1809,6 +1810,15 @@ func TestClient_Wait(t *testing.T) { require.NoError(t, err) require.True(t, len(b.Transactions) > 0) + // Ensure Waiter constructor works properly. + if ws { + _, ok := act.Waiter.(*waiter.EventBased) + require.True(t, ok) + } else { + _, ok := act.Waiter.(*waiter.PollingBased) + require.True(t, ok) + } + check := func(t *testing.T, h util.Uint256, vub uint32, errExpected bool) { rcvr := make(chan struct{}) go func() { From 92c6361be8c211a33d2efdd812012a2ff3a52995 Mon Sep 17 00:00:00 2001 From: Anna Shaleva Date: Wed, 14 Aug 2024 10:54:11 +0300 Subject: [PATCH 4/5] rpcclient: integrate customizable Waiter with Actor Signed-off-by: Anna Shaleva --- pkg/rpcclient/actor/actor.go | 5 +++++ pkg/rpcclient/waiter/waiter.go | 16 ++++++++++++++-- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/pkg/rpcclient/actor/actor.go b/pkg/rpcclient/actor/actor.go index bbaec24a8..b4b6a71e7 100644 --- a/pkg/rpcclient/actor/actor.go +++ b/pkg/rpcclient/actor/actor.go @@ -106,6 +106,10 @@ type Options struct { // before it's signed (other methods that perform test invocations // use CheckerModifier). MakeUnsigned* methods do not run it. Modifier TransactionModifier + // waiter.PollConfig is used by [waiter.Waiter] constructor to customize + // [waiter.PollingBased] behaviour. This option may be kept empty for default + // polling behaviour. + waiter.PollConfig } // New creates an Actor instance using the specified RPC interface and the set of @@ -183,6 +187,7 @@ func NewTuned(ra RPCActor, signers []SignerAccount, opts Options) (*Actor, error if opts.Modifier != nil { a.opts.Modifier = opts.Modifier } + a.Waiter = waiter.NewCustom(ra, a.version, opts.PollConfig) return a, err } diff --git a/pkg/rpcclient/waiter/waiter.go b/pkg/rpcclient/waiter/waiter.go index 95d11fdd7..d97ed3ab5 100644 --- a/pkg/rpcclient/waiter/waiter.go +++ b/pkg/rpcclient/waiter/waiter.go @@ -118,14 +118,26 @@ func errIsAlreadyExists(err error) bool { // or not an implementation of these two interfaces. It returns websocket-based // waiter, polling-based waiter or a stub correspondingly. func New(base any, v *result.Version) Waiter { + return NewCustom(base, v, PollConfig{}) +} + +// NewCustom creates Waiter instance. It can be either websocket-based or +// polling-base, otherwise Waiter stub is returned. As a first argument +// it accepts RPCEventBased implementation, RPCPollingBased implementation +// or not an implementation of these two interfaces. It returns websocket-based +// waiter, polling-based waiter or a stub correspondingly. As the second +// argument it accepts the RPC node version necessary for awaiting behaviour +// customisation. As a third argument it accepts the configuration of +// [PollingBased] [Waiter]. +func NewCustom(base any, v *result.Version, pollConfig PollConfig) Waiter { if eventW, ok := base.(RPCEventBased); ok { return &EventBased{ ws: eventW, - polling: newCustomPollingBased(eventW, v, PollConfig{}), + polling: newCustomPollingBased(eventW, v, pollConfig), } } if pollW, ok := base.(RPCPollingBased); ok { - return newCustomPollingBased(pollW, v, PollConfig{}) + return newCustomPollingBased(pollW, v, pollConfig) } return NewNull() } From 42555668da71837ada3573d3cf484fb865cb1aba Mon Sep 17 00:00:00 2001 From: Anna Shaleva Date: Wed, 14 Aug 2024 11:25:24 +0300 Subject: [PATCH 5/5] rpcclient: add Waiter.Config Include Waiter.PollConfig into Waiter.Config and use extended Waiter configuration where needed. Signed-off-by: Anna Shaleva --- pkg/rpcclient/actor/actor.go | 10 +++++----- pkg/rpcclient/waiter/waiter.go | 24 +++++++++++++++--------- 2 files changed, 20 insertions(+), 14 deletions(-) diff --git a/pkg/rpcclient/actor/actor.go b/pkg/rpcclient/actor/actor.go index b4b6a71e7..727dbda23 100644 --- a/pkg/rpcclient/actor/actor.go +++ b/pkg/rpcclient/actor/actor.go @@ -106,10 +106,10 @@ type Options struct { // before it's signed (other methods that perform test invocations // use CheckerModifier). MakeUnsigned* methods do not run it. Modifier TransactionModifier - // waiter.PollConfig is used by [waiter.Waiter] constructor to customize - // [waiter.PollingBased] behaviour. This option may be kept empty for default - // polling behaviour. - waiter.PollConfig + // WaiterConfig is used by [waiter.Waiter] constructor to customize + // awaiting behaviour. This option may be kept empty for default + // awaiting behaviour. + WaiterConfig waiter.Config } // New creates an Actor instance using the specified RPC interface and the set of @@ -187,7 +187,7 @@ func NewTuned(ra RPCActor, signers []SignerAccount, opts Options) (*Actor, error if opts.Modifier != nil { a.opts.Modifier = opts.Modifier } - a.Waiter = waiter.NewCustom(ra, a.version, opts.PollConfig) + a.Waiter = waiter.NewCustom(ra, a.version, opts.WaiterConfig) return a, err } diff --git a/pkg/rpcclient/waiter/waiter.go b/pkg/rpcclient/waiter/waiter.go index d97ed3ab5..a62dc0260 100644 --- a/pkg/rpcclient/waiter/waiter.go +++ b/pkg/rpcclient/waiter/waiter.go @@ -90,6 +90,12 @@ type PollingBased struct { config PollConfig } +// Config is a unified configuration for [Waiter] implementations that allows to +// customize awaiting behaviour. +type Config struct { + PollConfig +} + // PollConfig is a configuration for PollingBased waiter. type PollConfig struct { // PollInterval is a time interval between subsequent polls. If not set, then @@ -118,7 +124,7 @@ func errIsAlreadyExists(err error) bool { // or not an implementation of these two interfaces. It returns websocket-based // waiter, polling-based waiter or a stub correspondingly. func New(base any, v *result.Version) Waiter { - return NewCustom(base, v, PollConfig{}) + return NewCustom(base, v, Config{}) } // NewCustom creates Waiter instance. It can be either websocket-based or @@ -128,16 +134,16 @@ func New(base any, v *result.Version) Waiter { // waiter, polling-based waiter or a stub correspondingly. As the second // argument it accepts the RPC node version necessary for awaiting behaviour // customisation. As a third argument it accepts the configuration of -// [PollingBased] [Waiter]. -func NewCustom(base any, v *result.Version, pollConfig PollConfig) Waiter { +// [Waiter]. +func NewCustom(base any, v *result.Version, config Config) Waiter { if eventW, ok := base.(RPCEventBased); ok { return &EventBased{ ws: eventW, - polling: newCustomPollingBased(eventW, v, pollConfig), + polling: newCustomPollingBased(eventW, v, config.PollConfig), } } if pollW, ok := base.(RPCPollingBased); ok { - return newCustomPollingBased(pollW, v, pollConfig) + return newCustomPollingBased(pollW, v, config.PollConfig) } return NewNull() } @@ -244,15 +250,15 @@ func (w *PollingBased) WaitAny(ctx context.Context, vub uint32, hashes ...util.U // EventBased contains PollingBased under the hood and falls back to polling when subscription-based // awaiting fails. func NewEventBased(waiter RPCEventBased) (*EventBased, error) { - return NewCustomEventBased(waiter, PollConfig{}) + return NewCustomEventBased(waiter, Config{}) } // NewCustomEventBased creates an instance of Waiter supporting websocket event-based transaction awaiting. // EventBased contains PollingBased under the hood and falls back to polling when subscription-based -// awaiting fails. PollingBased configuration options may be specified via pollConfig parameter +// awaiting fails. Waiter configuration options may be specified via config parameter // (defaults are used if not specified). -func NewCustomEventBased(waiter RPCEventBased, pollConfig PollConfig) (*EventBased, error) { - polling, err := NewCustomPollingBased(waiter, pollConfig) +func NewCustomEventBased(waiter RPCEventBased, config Config) (*EventBased, error) { + polling, err := NewCustomPollingBased(waiter, config.PollConfig) if err != nil { return nil, err }