From 8467196a7aba43324cd3651988c1fcdf169e574e Mon Sep 17 00:00:00 2001 From: Iulian Pascalau Date: Fri, 3 Jan 2025 15:29:50 +0200 Subject: [PATCH 1/3] - added TTL for failed executed operations based on id --- cmd/scCallsExecutor/config/config.toml | 6 +- cmd/scCallsExecutor/main.go | 12 +- config/config.go | 9 +- config/tomlConfigs_test.go | 16 +-- executors/multiversx/common.go | 51 ++++++- executors/multiversx/module/scCallsModule.go | 9 +- .../multiversx/module/scCallsModule_test.go | 65 +++------ executors/multiversx/refundExecutor.go | 21 ++- executors/multiversx/refundExecutor_test.go | 105 ++++++++++++-- executors/multiversx/scCallsExecutor.go | 12 +- executors/multiversx/scCallsExecutor_test.go | 83 ++++++++++- executors/multiversx/transactionExecutor.go | 110 ++++---------- .../multiversx/transactionExecutor_test.go | 134 ++---------------- .../relayers/slowTests/framework/testSetup.go | 10 +- 14 files changed, 336 insertions(+), 307 deletions(-) diff --git a/cmd/scCallsExecutor/config/config.toml b/cmd/scCallsExecutor/config/config.toml index c9304182..c2f78f62 100644 --- a/cmd/scCallsExecutor/config/config.toml +++ b/cmd/scCallsExecutor/config/config.toml @@ -15,10 +15,12 @@ MaxGasLimitToUse = 249999999 # this is a safe max gas limit to use both intra-shard & cross-shard GasLimitForOutOfGasTransactions = 30000000 # this value will be used when a transaction specified a gas limit > 249999999 PollingIntervalInMillis = 6000 + TTLForFailedRefundIdInSeconds = 3600 [RefundExecutor] GasToExecute = 30000000 PollingIntervalInMillis = 6000 + TTLForFailedRefundIdInSeconds = 86400 [Filter] AllowedEthAddresses = ["*"] # execute SC calls from all ETH addresses @@ -30,9 +32,5 @@ LogFileLifeSpanInMB = 1024 # 1GB [TransactionChecks] - CheckTransactionResults = true # enable or disable the transaction execution checking TimeInSecondsBetweenChecks = 6 # the number of seconds to recheck the status of the transaction ExecutionTimeoutInSeconds = 120 # the number of seconds reserved for each execution to complete - CloseAppOnError = false # enable or disable if the executor should automatically close on a transaction execution error - ExtraDelayInSecondsOnError = 300 # extra delay in seconds if the transaction execution errored - diff --git a/cmd/scCallsExecutor/main.go b/cmd/scCallsExecutor/main.go index 7b13a3da..b73d3102 100644 --- a/cmd/scCallsExecutor/main.go +++ b/cmd/scCallsExecutor/main.go @@ -106,8 +106,7 @@ func startExecutor(ctx *cli.Context, version string) error { return fmt.Errorf("empty NetworkAddress in config file") } - chCloseApp := make(chan struct{}, 1) - scCallsExecutor, err := module.NewScCallsModule(cfg, log, chCloseApp) + scCallsExecutor, err := module.NewScCallsModule(cfg, log) if err != nil { return err } @@ -115,12 +114,9 @@ func startExecutor(ctx *cli.Context, version string) error { sigs := make(chan os.Signal, 1) signal.Notify(sigs, syscall.SIGINT, syscall.SIGTERM) - select { - case <-sigs: - log.Info("application closing by user error input, calling Close on all subcomponents...") - case <-chCloseApp: - log.Info("application closing, requested internally, calling Close on all subcomponents...") - } + <-sigs + + log.Info("application closing by user error input, calling Close on all subcomponents...") return scCallsExecutor.Close() } diff --git a/config/config.go b/config/config.go index 7eddf0e9..f138e92a 100644 --- a/config/config.go +++ b/config/config.go @@ -225,21 +225,20 @@ type ScCallsExecutorConfig struct { MaxGasLimitToUse uint64 GasLimitForOutOfGasTransactions uint64 PollingIntervalInMillis uint64 + TTLForFailedRefundIdInSeconds uint64 } // RefundExecutorConfig will hold the settings for the refund executor type RefundExecutorConfig struct { - GasToExecute uint64 - PollingIntervalInMillis uint64 + GasToExecute uint64 + PollingIntervalInMillis uint64 + TTLForFailedRefundIdInSeconds uint64 } // TransactionChecksConfig will hold the setting for how to handle the transaction execution type TransactionChecksConfig struct { - CheckTransactionResults bool TimeInSecondsBetweenChecks uint64 ExecutionTimeoutInSeconds uint64 - CloseAppOnError bool - ExtraDelayInSecondsOnError uint64 } // MigrationToolConfig is the migration tool config struct diff --git a/config/tomlConfigs_test.go b/config/tomlConfigs_test.go index 011d394d..1dae48a7 100644 --- a/config/tomlConfigs_test.go +++ b/config/tomlConfigs_test.go @@ -438,10 +438,12 @@ func TestScCallsExecutorConfigs(t *testing.T) { MaxGasLimitToUse: 249999999, GasLimitForOutOfGasTransactions: 30000000, PollingIntervalInMillis: 6000, + TTLForFailedRefundIdInSeconds: 3600, }, RefundExecutor: RefundExecutorConfig{ - GasToExecute: 20000000, - PollingIntervalInMillis: 6000, + GasToExecute: 20000000, + PollingIntervalInMillis: 6000, + TTLForFailedRefundIdInSeconds: 86400, }, Filter: PendingOperationsFilterConfig{ AllowedEthAddresses: []string{"*"}, @@ -453,11 +455,8 @@ func TestScCallsExecutorConfigs(t *testing.T) { LogFileLifeSpanInMB: 1024, }, TransactionChecks: TransactionChecksConfig{ - CheckTransactionResults: true, TimeInSecondsBetweenChecks: 6, ExecutionTimeoutInSeconds: 120, - CloseAppOnError: false, - ExtraDelayInSecondsOnError: 120, }, } @@ -480,10 +479,12 @@ func TestScCallsExecutorConfigs(t *testing.T) { MaxGasLimitToUse = 249999999 # this is a safe max gas limit to use both intra-shard & cross-shard GasLimitForOutOfGasTransactions = 30000000 # this value will be used when a transaction specified a gas limit > 249999999 PollingIntervalInMillis = 6000 + TTLForFailedRefundIdInSeconds = 3600 [RefundExecutor] GasToExecute = 20000000 PollingIntervalInMillis = 6000 + TTLForFailedRefundIdInSeconds = 86400 [Filter] AllowedEthAddresses = ["*"] # execute SC calls from all ETH addresses @@ -495,11 +496,8 @@ func TestScCallsExecutorConfigs(t *testing.T) { LogFileLifeSpanInMB = 1024 # 1GB [TransactionChecks] - CheckTransactionResults = true # enable or disable the transaction execution checking TimeInSecondsBetweenChecks = 6 # the number of seconds to recheck the status of the transaction - ExecutionTimeoutInSeconds = 120 # the number of seconds after the transaction is considered failed if it was not seen by the blockchain - CloseAppOnError = false # enable or disable if the executor should automatically close on a transaction execution error - ExtraDelayInSecondsOnError = 120 # extra delay in seconds if the transaction execution errored + ExecutionTimeoutInSeconds = 120 # the number of seconds after the transaction is considered failed if it was not seen by the blockchain ` cfg := ScCallsModuleConfig{} diff --git a/executors/multiversx/common.go b/executors/multiversx/common.go index e59d398a..6e10a88a 100644 --- a/executors/multiversx/common.go +++ b/executors/multiversx/common.go @@ -4,6 +4,8 @@ import ( "context" "fmt" "strings" + "sync" + "time" bridgeCore "github.com/multiversx/mx-bridge-eth-go/core" "github.com/multiversx/mx-bridge-eth-go/errors" @@ -12,6 +14,10 @@ import ( "github.com/multiversx/mx-sdk-go/data" ) +const ( + minTTLForFailedRefundID = time.Second +) + type baseExecutor struct { scProxyBech32Addresses []string proxy Proxy @@ -19,6 +25,9 @@ type baseExecutor struct { codec Codec filter ScCallsExecuteFilter log logger.Logger + ttlForFailedRefundID time.Duration + mutFailedRefundMap sync.RWMutex + failedRefundMap map[uint64]time.Time } func (executor *baseExecutor) checkBaseComponents() error { @@ -37,6 +46,9 @@ func (executor *baseExecutor) checkBaseComponents() error { if check.IfNil(executor.log) { return errNilLogger } + if executor.ttlForFailedRefundID < minTTLForFailedRefundID { + return fmt.Errorf("%w for TTLForFailedRefundID: provided: %v, absolute minimum required: %v", errInvalidValue, executor.ttlForFailedRefundID, minTTLForFailedRefundID) + } if len(executor.scProxyBech32Addresses) == 0 { return errEmptyListOfBridgeSCProxy @@ -71,9 +83,14 @@ func (executor *baseExecutor) executeOnAllScProxyAddress(ctx context.Context, ha func (executor *baseExecutor) filterOperations(component string, pendingOperations map[uint64]bridgeCore.ProxySCCompleteCallData) map[uint64]bridgeCore.ProxySCCompleteCallData { result := make(map[uint64]bridgeCore.ProxySCCompleteCallData) for id, callData := range pendingOperations { - if executor.filter.ShouldExecute(callData) { - result[id] = callData + if !executor.filter.ShouldExecute(callData) { + continue + } + if executor.isFailed(id) { + continue } + + result[id] = callData } executor.log.Debug(component, "input pending ops", len(pendingOperations), "result pending ops", len(result)) @@ -81,6 +98,36 @@ func (executor *baseExecutor) filterOperations(component string, pendingOperatio return result } +func (executor *baseExecutor) isFailed(id uint64) bool { + executor.mutFailedRefundMap.RLock() + defer executor.mutFailedRefundMap.RUnlock() + + _, found := executor.failedRefundMap[id] + return found +} + +func (executor *baseExecutor) addFailed(id uint64) { + executor.mutFailedRefundMap.Lock() + defer executor.mutFailedRefundMap.Unlock() + + executor.failedRefundMap[id] = time.Now() +} + +func (executor *baseExecutor) cleanupTTLCache(source string) { + executor.mutFailedRefundMap.Lock() + defer executor.mutFailedRefundMap.Unlock() + + for id, insertedTime := range executor.failedRefundMap { + if insertedTime.Add(executor.ttlForFailedRefundID).Unix() < time.Now().Unix() { + executor.log.Debug("TTL expired, remove from cache", + "ID", id, + "executor", source, + "TTL", executor.ttlForFailedRefundID) + delete(executor.failedRefundMap, id) + } + } +} + func (executor *baseExecutor) executeVmQuery(ctx context.Context, scProxyAddress string, function string) (*data.VmValuesResponseData, error) { request := &data.VmValueRequest{ Address: scProxyAddress, diff --git a/executors/multiversx/module/scCallsModule.go b/executors/multiversx/module/scCallsModule.go index f0edb3c8..69917367 100644 --- a/executors/multiversx/module/scCallsModule.go +++ b/executors/multiversx/module/scCallsModule.go @@ -35,7 +35,7 @@ type scCallsModule struct { } // NewScCallsModule creates a starts a new scCallsModule instance -func NewScCallsModule(cfg config.ScCallsModuleConfig, log logger.Logger, chCloseApp chan struct{}) (*scCallsModule, error) { +func NewScCallsModule(cfg config.ScCallsModuleConfig, log logger.Logger) (*scCallsModule, error) { module := &scCallsModule{ cfg: cfg, log: log, @@ -56,7 +56,7 @@ func NewScCallsModule(cfg config.ScCallsModuleConfig, log logger.Logger, chClose return nil, err } - err = module.createTransactionExecutor(chCloseApp) + err = module.createTransactionExecutor() if err != nil { return nil, err } @@ -110,7 +110,7 @@ func (module *scCallsModule) createNonceTxHandler() error { return err } -func (module *scCallsModule) createTransactionExecutor(chCloseApp chan struct{}) error { +func (module *scCallsModule) createTransactionExecutor() error { wallet := interactors.NewWallet() multiversXPrivateKeyBytes, err := wallet.LoadPrivateKeyFromPemFile(module.cfg.General.PrivateKeyFile) if err != nil { @@ -129,7 +129,6 @@ func (module *scCallsModule) createTransactionExecutor(chCloseApp chan struct{}) PrivateKey: privateKey, SingleSigner: singleSigner, TransactionChecks: module.cfg.TransactionChecks, - CloseAppChan: chCloseApp, } module.txExecutor, err = multiversx.NewTransactionExecutor(argsTxExecutor) @@ -184,7 +183,7 @@ func (module *scCallsModule) createRefundExecutor() error { Codec: &parsers.MultiversxCodec{}, Filter: module.filter, Log: module.log, - GasToExecute: module.cfg.RefundExecutor.GasToExecute, + RefundConfig: module.cfg.RefundExecutor, } executorInstance, err := multiversx.NewRefundExecutor(argsExecutor) diff --git a/executors/multiversx/module/scCallsModule_test.go b/executors/multiversx/module/scCallsModule_test.go index 282a01ae..9c9f2a26 100644 --- a/executors/multiversx/module/scCallsModule_test.go +++ b/executors/multiversx/module/scCallsModule_test.go @@ -28,10 +28,12 @@ func createTestConfigs() config.ScCallsModuleConfig { MaxGasLimitToUse: 249999999, GasLimitForOutOfGasTransactions: 30000000, PollingIntervalInMillis: 10000, + TTLForFailedRefundIdInSeconds: 3600, }, RefundExecutor: config.RefundExecutorConfig{ - GasToExecute: 30000000, - PollingIntervalInMillis: 10000, + GasToExecute: 30000000, + PollingIntervalInMillis: 10000, + TTLForFailedRefundIdInSeconds: 86400, }, Filter: config.PendingOperationsFilterConfig{ DeniedEthAddresses: nil, @@ -41,6 +43,10 @@ func createTestConfigs() config.ScCallsModuleConfig { DeniedTokens: nil, AllowedTokens: []string{"*"}, }, + TransactionChecks: config.TransactionChecksConfig{ + TimeInSecondsBetweenChecks: 6, + ExecutionTimeoutInSeconds: 120, + }, } } @@ -53,7 +59,7 @@ func TestNewScCallsModule(t *testing.T) { cfg := createTestConfigs() cfg.Filter.DeniedTokens = []string{"*"} - module, err := NewScCallsModule(cfg, &testsCommon.LoggerStub{}, nil) + module, err := NewScCallsModule(cfg, &testsCommon.LoggerStub{}) assert.NotNil(t, err) assert.Contains(t, err.Error(), "unsupported marker * on item at index 0 in list DeniedTokens") assert.Nil(t, module) @@ -64,7 +70,7 @@ func TestNewScCallsModule(t *testing.T) { cfg := createTestConfigs() cfg.General.ProxyCacherExpirationSeconds = 0 - module, err := NewScCallsModule(cfg, &testsCommon.LoggerStub{}, nil) + module, err := NewScCallsModule(cfg, &testsCommon.LoggerStub{}) assert.NotNil(t, err) assert.Contains(t, err.Error(), "invalid caching duration, provided: 0s, minimum: 1s") assert.Nil(t, module) @@ -75,7 +81,7 @@ func TestNewScCallsModule(t *testing.T) { cfg := createTestConfigs() cfg.General.IntervalToResendTxsInSeconds = 0 - module, err := NewScCallsModule(cfg, &testsCommon.LoggerStub{}, nil) + module, err := NewScCallsModule(cfg, &testsCommon.LoggerStub{}) assert.NotNil(t, err) assert.Contains(t, err.Error(), "invalid value for intervalToResend in NewNonceTransactionHandlerV2") assert.Nil(t, module) @@ -86,7 +92,7 @@ func TestNewScCallsModule(t *testing.T) { cfg := createTestConfigs() cfg.General.PrivateKeyFile = "" - module, err := NewScCallsModule(cfg, &testsCommon.LoggerStub{}, nil) + module, err := NewScCallsModule(cfg, &testsCommon.LoggerStub{}) assert.NotNil(t, err) assert.Nil(t, module) }) @@ -96,7 +102,7 @@ func TestNewScCallsModule(t *testing.T) { cfg := createTestConfigs() cfg.ScCallsExecutor.PollingIntervalInMillis = 0 - module, err := NewScCallsModule(cfg, &testsCommon.LoggerStub{}, nil) + module, err := NewScCallsModule(cfg, &testsCommon.LoggerStub{}) assert.NotNil(t, err) assert.Contains(t, err.Error(), "invalid value for PollingInterval") assert.Nil(t, module) @@ -107,7 +113,7 @@ func TestNewScCallsModule(t *testing.T) { cfg := createTestConfigs() cfg.ScCallsExecutor.MaxGasLimitToUse = 1 - module, err := NewScCallsModule(cfg, &testsCommon.LoggerStub{}, nil) + module, err := NewScCallsModule(cfg, &testsCommon.LoggerStub{}) assert.NotNil(t, err) assert.Contains(t, err.Error(), "provided gas limit is less than absolute minimum required for MaxGasLimitToUse") assert.Nil(t, module) @@ -118,7 +124,7 @@ func TestNewScCallsModule(t *testing.T) { cfg := createTestConfigs() cfg.RefundExecutor.PollingIntervalInMillis = 0 - module, err := NewScCallsModule(cfg, &testsCommon.LoggerStub{}, nil) + module, err := NewScCallsModule(cfg, &testsCommon.LoggerStub{}) assert.NotNil(t, err) assert.Contains(t, err.Error(), "invalid value for PollingInterval") assert.Nil(t, module) @@ -129,51 +135,16 @@ func TestNewScCallsModule(t *testing.T) { cfg := createTestConfigs() cfg.RefundExecutor.GasToExecute = 0 - module, err := NewScCallsModule(cfg, &testsCommon.LoggerStub{}, nil) + module, err := NewScCallsModule(cfg, &testsCommon.LoggerStub{}) assert.NotNil(t, err) assert.Contains(t, err.Error(), "provided gas limit is less than absolute minimum required for GasToExecute") assert.Nil(t, module) }) - t.Run("should work with nil close app chan", func(t *testing.T) { - t.Parallel() - - cfg := createTestConfigs() - module, err := NewScCallsModule(cfg, &testsCommon.LoggerStub{}, nil) - assert.Nil(t, err) - assert.NotNil(t, module) - - assert.Zero(t, module.GetNumSentTransaction()) - - err = module.Close() - assert.Nil(t, err) - }) - t.Run("should work with not nil close app chan", func(t *testing.T) { - t.Parallel() - - cfg := createTestConfigs() - cfg.TransactionChecks.CheckTransactionResults = true - cfg.TransactionChecks.TimeInSecondsBetweenChecks = 1 - cfg.TransactionChecks.ExecutionTimeoutInSeconds = 1 - cfg.TransactionChecks.CloseAppOnError = true - module, err := NewScCallsModule(cfg, &testsCommon.LoggerStub{}, make(chan struct{}, 1)) - assert.Nil(t, err) - assert.NotNil(t, module) - - assert.Zero(t, module.GetNumSentTransaction()) - - err = module.Close() - assert.Nil(t, err) - }) - t.Run("should work with not nil close app chan and 2 sc proxy addresses", func(t *testing.T) { + t.Run("should work", func(t *testing.T) { t.Parallel() cfg := createTestConfigs() - cfg.General.ScProxyBech32Addresses = append(cfg.General.ScProxyBech32Addresses, "erd1qqqqqqqqqqqqqpgqzyuaqg3dl7rqlkudrsnm5ek0j3a97qevd8sszj0glf") - cfg.TransactionChecks.CheckTransactionResults = true - cfg.TransactionChecks.TimeInSecondsBetweenChecks = 1 - cfg.TransactionChecks.ExecutionTimeoutInSeconds = 1 - cfg.TransactionChecks.CloseAppOnError = true - module, err := NewScCallsModule(cfg, &testsCommon.LoggerStub{}, make(chan struct{}, 1)) + module, err := NewScCallsModule(cfg, &testsCommon.LoggerStub{}) assert.Nil(t, err) assert.NotNil(t, module) diff --git a/executors/multiversx/refundExecutor.go b/executors/multiversx/refundExecutor.go index 1289d6f2..2faef190 100644 --- a/executors/multiversx/refundExecutor.go +++ b/executors/multiversx/refundExecutor.go @@ -4,7 +4,9 @@ import ( "context" "fmt" "math/big" + "time" + "github.com/multiversx/mx-bridge-eth-go/config" bridgeCore "github.com/multiversx/mx-bridge-eth-go/core" logger "github.com/multiversx/mx-chain-logger-go" "github.com/multiversx/mx-sdk-go/builders" @@ -27,7 +29,7 @@ type ArgsRefundExecutor struct { Codec Codec Filter ScCallsExecuteFilter Log logger.Logger - GasToExecute uint64 + RefundConfig config.RefundExecutorConfig } type refundExecutor struct { @@ -37,8 +39,8 @@ type refundExecutor struct { // NewRefundExecutor creates a new instance of type refundExecutor func NewRefundExecutor(args ArgsRefundExecutor) (*refundExecutor, error) { - if args.GasToExecute < minGasToExecuteSCCalls { - return nil, fmt.Errorf("%w for GasToExecute: provided: %d, absolute minimum required: %d", errGasLimitIsLessThanAbsoluteMinimum, args.GasToExecute, minGasToExecuteSCCalls) + if args.RefundConfig.GasToExecute < minGasToExecuteSCCalls { + return nil, fmt.Errorf("%w for GasToExecute: provided: %d, absolute minimum required: %d", errGasLimitIsLessThanAbsoluteMinimum, args.RefundConfig.GasToExecute, minGasToExecuteSCCalls) } executor := &refundExecutor{ @@ -49,8 +51,10 @@ func NewRefundExecutor(args ArgsRefundExecutor) (*refundExecutor, error) { codec: args.Codec, filter: args.Filter, log: args.Log, + ttlForFailedRefundID: time.Duration(args.RefundConfig.TTLForFailedRefundIdInSeconds) * time.Second, + failedRefundMap: make(map[uint64]time.Time), }, - gasToExecute: args.GasToExecute, + gasToExecute: args.RefundConfig.GasToExecute, } err := executor.checkBaseComponents() @@ -63,6 +67,8 @@ func NewRefundExecutor(args ArgsRefundExecutor) (*refundExecutor, error) { // Execute will execute one step: get all pending operations, call the filter and send execution transactions func (executor *refundExecutor) Execute(ctx context.Context) error { + executor.cleanupTTLCache(refundTxType) + return executor.executeOnAllScProxyAddress(ctx, executor.executeRefundForScProxyAddress) } @@ -141,7 +147,12 @@ func (executor *refundExecutor) executeOperation( return err } - return executor.transactionExecutor.ExecuteTransaction(ctx, networkConfig, scProxyAddress, refundTxType, executor.gasToExecute, dataBytes) + err = executor.transactionExecutor.ExecuteTransaction(ctx, networkConfig, scProxyAddress, refundTxType, executor.gasToExecute, dataBytes) + if err != nil { + executor.addFailed(id) + } + + return err } // IsInterfaceNil returns true if there is no value under the interface diff --git a/executors/multiversx/refundExecutor_test.go b/executors/multiversx/refundExecutor_test.go index bd67ba8f..9d655d78 100644 --- a/executors/multiversx/refundExecutor_test.go +++ b/executors/multiversx/refundExecutor_test.go @@ -5,7 +5,9 @@ import ( "context" "errors" "testing" + "time" + "github.com/multiversx/mx-bridge-eth-go/config" bridgeCore "github.com/multiversx/mx-bridge-eth-go/core" "github.com/multiversx/mx-bridge-eth-go/testsCommon" "github.com/multiversx/mx-bridge-eth-go/testsCommon/interactors" @@ -24,7 +26,10 @@ func createMockArgsRefundExecutor() ArgsRefundExecutor { Codec: &testsCommon.MultiversxCodecStub{}, Filter: &testsCommon.ScCallsExecuteFilterStub{}, Log: &testsCommon.LoggerStub{}, - GasToExecute: minGasToExecuteSCCalls, + RefundConfig: config.RefundExecutorConfig{ + GasToExecute: minGasToExecuteSCCalls, + TTLForFailedRefundIdInSeconds: 1, + }, } } @@ -105,7 +110,7 @@ func TestNewRefundExecutor(t *testing.T) { t.Parallel() args := createMockArgsRefundExecutor() - args.GasToExecute = minGasToExecuteSCCalls - 1 + args.RefundConfig.GasToExecute = minGasToExecuteSCCalls - 1 executor, err := NewRefundExecutor(args) assert.Nil(t, executor) @@ -113,6 +118,17 @@ func TestNewRefundExecutor(t *testing.T) { assert.Contains(t, err.Error(), "provided: 2009999, absolute minimum required: 2010000") assert.Contains(t, err.Error(), "GasToExecute") }) + t.Run("invalid TTLForFailedRefundID should error", func(t *testing.T) { + t.Parallel() + + args := createMockArgsRefundExecutor() + args.RefundConfig.TTLForFailedRefundIdInSeconds = 0 + + executor, err := NewRefundExecutor(args) + assert.Nil(t, executor) + assert.ErrorIs(t, err, errInvalidValue) + assert.Contains(t, err.Error(), "provided: 0s, absolute minimum required: 1s") + }) t.Run("should work", func(t *testing.T) { t.Parallel() @@ -306,12 +322,15 @@ func TestRefundExecutor_Execute(t *testing.T) { assert.Contains(t, err.Error(), expectedError.Error()) assert.Contains(t, err.Error(), "errors found during execution") }) - t.Run("SendTransaction errors, should error", func(t *testing.T) { + t.Run("SendTransaction errors, should error and register the failed id", func(t *testing.T) { t.Parallel() args := createMockArgsRefundExecutor() + args.RefundConfig.TTLForFailedRefundIdInSeconds = 60 + numExecuted := 0 args.TransactionExecutor = &testsCommon.TransactionExecutorStub{ ExecuteTransactionCalled: func(ctx context.Context, networkConfig *data.NetworkConfig, receiver string, transactionType string, gasLimit uint64, dataBytes []byte) error { + numExecuted++ return expectedError }, } @@ -346,12 +365,78 @@ func TestRefundExecutor_Execute(t *testing.T) { assert.NotNil(t, err) assert.Contains(t, err.Error(), expectedError.Error()) assert.Contains(t, err.Error(), "errors found during execution") + + //re-run the same call, no errors should be found + err = executor.Execute(context.Background()) + assert.Nil(t, err) + + //only one sent transaction should be + assert.Equal(t, 1, numExecuted) + assert.True(t, executor.isFailed(1)) + }) + t.Run("SendTransaction errors, should error and register the failed id, after TTL expires should call again", func(t *testing.T) { + t.Parallel() + + args := createMockArgsRefundExecutor() + args.RefundConfig.TTLForFailedRefundIdInSeconds = 2 + numExecuted := 0 + args.TransactionExecutor = &testsCommon.TransactionExecutorStub{ + ExecuteTransactionCalled: func(ctx context.Context, networkConfig *data.NetworkConfig, receiver string, transactionType string, gasLimit uint64, dataBytes []byte) error { + numExecuted++ + return expectedError + }, + } + args.Proxy = &interactors.ProxyStub{ + ExecuteVMQueryCalled: func(ctx context.Context, vmRequest *data.VmValueRequest) (*data.VmValuesResponseData, error) { + return &data.VmValuesResponseData{ + Data: &vm.VMOutputApi{ + ReturnCode: okCodeAfterExecution, + ReturnData: [][]byte{ + {0x01}, + {0x03, 0x04}, + }, + }, + }, nil + }, + GetNetworkConfigCalled: func(ctx context.Context) (*data.NetworkConfig, error) { + return &data.NetworkConfig{}, nil + }, + } + args.Codec = &testsCommon.MultiversxCodecStub{ + DecodeProxySCCompleteCallDataCalled: func(buff []byte) (bridgeCore.ProxySCCompleteCallData, error) { + assert.Equal(t, []byte{0x03, 0x04}, buff) + + return bridgeCore.ProxySCCompleteCallData{ + To: data.NewAddressFromBytes(bytes.Repeat([]byte{1}, 32)), + }, nil + }, + } + + executor, _ := NewRefundExecutor(args) + err := executor.Execute(context.Background()) + assert.NotNil(t, err) + assert.Contains(t, err.Error(), expectedError.Error()) + assert.Contains(t, err.Error(), "errors found during execution") + assert.True(t, executor.isFailed(1)) + + //wait for TTL to expire + time.Sleep(time.Second * 3) + + //re-run the same call, same error should be found + err = executor.Execute(context.Background()) + assert.NotNil(t, err) + assert.Contains(t, err.Error(), expectedError.Error()) + assert.Contains(t, err.Error(), "errors found during execution") + + //only one sent transaction should be + assert.Equal(t, 2, numExecuted) + assert.True(t, executor.isFailed(1)) }) t.Run("should work", func(t *testing.T) { t.Parallel() args := createMockArgsRefundExecutor() - args.GasToExecute = 250000000 + args.RefundConfig.GasToExecute = 250000000 sendWasCalled := false args.Proxy = &interactors.ProxyStub{ @@ -404,7 +489,7 @@ func TestRefundExecutor_Execute(t *testing.T) { ExecuteTransactionCalled: func(ctx context.Context, networkConfig *data.NetworkConfig, receiver string, transactionType string, gasLimit uint64, dataBytes []byte) error { assert.Equal(t, "TEST", networkConfig.ChainID) assert.Equal(t, uint32(111), networkConfig.MinTransactionVersion) - assert.Equal(t, args.GasToExecute, gasLimit) + assert.Equal(t, args.RefundConfig.GasToExecute, gasLimit) assert.Equal(t, "erd1qqqqqqqqqqqqqpgqk839entmk46ykukvhpn90g6knskju3dtanaq20f66e", receiver) assert.Equal(t, refundTxType, transactionType) @@ -422,13 +507,15 @@ func TestRefundExecutor_Execute(t *testing.T) { err := executor.Execute(context.Background()) assert.Nil(t, err) assert.True(t, sendWasCalled) + assert.False(t, executor.isFailed(1)) + assert.False(t, executor.isFailed(2)) }) - t.Run("should work with one two proxy address", func(t *testing.T) { + t.Run("should work with two proxy addresses", func(t *testing.T) { t.Parallel() args := createMockArgsRefundExecutor() args.ScProxyBech32Addresses = append(args.ScProxyBech32Addresses, "erd1qqqqqqqqqqqqqpgqzyuaqg3dl7rqlkudrsnm5ek0j3a97qevd8sszj0glf") - args.GasToExecute = 250000000 + args.RefundConfig.GasToExecute = 250000000 args.Proxy = &interactors.ProxyStub{ ExecuteVMQueryCalled: func(ctx context.Context, vmRequest *data.VmValueRequest) (*data.VmValuesResponseData, error) { @@ -515,13 +602,13 @@ func TestRefundExecutor_Execute(t *testing.T) { { receiver: "erd1qqqqqqqqqqqqqpgqk839entmk46ykukvhpn90g6knskju3dtanaq20f66e", transactionType: refundTxType, - gasLimit: args.GasToExecute, + gasLimit: args.RefundConfig.GasToExecute, dataBytes: []byte(executeRefundTransactionFunction + "@02"), }, { receiver: "erd1qqqqqqqqqqqqqpgqzyuaqg3dl7rqlkudrsnm5ek0j3a97qevd8sszj0glf", transactionType: refundTxType, - gasLimit: args.GasToExecute, + gasLimit: args.RefundConfig.GasToExecute, dataBytes: []byte(executeRefundTransactionFunction + "@04"), }, } diff --git a/executors/multiversx/scCallsExecutor.go b/executors/multiversx/scCallsExecutor.go index 5fef012b..4667d2d0 100644 --- a/executors/multiversx/scCallsExecutor.go +++ b/executors/multiversx/scCallsExecutor.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "math/big" + "time" "github.com/multiversx/mx-bridge-eth-go/config" bridgeCore "github.com/multiversx/mx-bridge-eth-go/core" @@ -53,6 +54,8 @@ func NewScCallExecutor(args ArgsScCallExecutor) (*scCallExecutor, error) { codec: args.Codec, filter: args.Filter, log: args.Log, + ttlForFailedRefundID: time.Duration(args.ExecutorConfig.TTLForFailedRefundIdInSeconds) * time.Second, + failedRefundMap: make(map[uint64]time.Time), }, extraGasToExecute: args.ExecutorConfig.ExtraGasToExecute, maxGasLimitToUse: args.ExecutorConfig.MaxGasLimitToUse, @@ -80,6 +83,8 @@ func checkScCallExecutorArgs(args ArgsScCallExecutor) error { // Execute will execute one step: get all pending operations, call the filter and send execution transactions func (executor *scCallExecutor) Execute(ctx context.Context) error { + executor.cleanupTTLCache(scCallTxType) + return executor.executeOnAllScProxyAddress(ctx, executor.executeScCallForScProxyAddress) } @@ -201,7 +206,12 @@ func (executor *scCallExecutor) executeOperation( return nil } - return executor.transactionExecutor.ExecuteTransaction(ctx, networkConfig, scProxyAddress, scCallTxType, txGasLimit, dataBytes) + err = executor.transactionExecutor.ExecuteTransaction(ctx, networkConfig, scProxyAddress, scCallTxType, txGasLimit, dataBytes) + if err != nil { + executor.addFailed(id) + } + + return err } // IsInterfaceNil returns true if there is no value under the interface diff --git a/executors/multiversx/scCallsExecutor_test.go b/executors/multiversx/scCallsExecutor_test.go index 91499424..2404b2de 100644 --- a/executors/multiversx/scCallsExecutor_test.go +++ b/executors/multiversx/scCallsExecutor_test.go @@ -6,6 +6,7 @@ import ( "errors" "math/big" "testing" + "time" "github.com/ethereum/go-ethereum/common" "github.com/multiversx/mx-bridge-eth-go/config" @@ -34,6 +35,7 @@ func createMockArgsScCallExecutor() ArgsScCallExecutor { ExtraGasToExecute: 100, MaxGasLimitToUse: minGasToExecuteSCCalls, GasLimitForOutOfGasTransactions: minGasToExecuteSCCalls, + TTLForFailedRefundIdInSeconds: 1, }, } } @@ -154,6 +156,17 @@ func TestNewScCallExecutor(t *testing.T) { assert.Contains(t, err.Error(), "provided: 2009999, absolute minimum required: 2010000") assert.Contains(t, err.Error(), "GasLimitForOutOfGasTransactions") }) + t.Run("invalid TTLForFailedRefundID should error", func(t *testing.T) { + t.Parallel() + + args := createMockArgsScCallExecutor() + args.ExecutorConfig.TTLForFailedRefundIdInSeconds = 0 + + executor, err := NewScCallExecutor(args) + assert.Nil(t, executor) + assert.ErrorIs(t, err, errInvalidValue) + assert.Contains(t, err.Error(), "provided: 0s, absolute minimum required: 1s") + }) t.Run("should work", func(t *testing.T) { t.Parallel() @@ -347,12 +360,67 @@ func TestScCallExecutor_Execute(t *testing.T) { assert.Contains(t, err.Error(), expectedError.Error()) assert.Contains(t, err.Error(), "errors found during execution") }) - t.Run("SendTransaction errors, should error", func(t *testing.T) { + t.Run("SendTransaction errors, should error and register the failed id", func(t *testing.T) { + t.Parallel() + + args := createMockArgsScCallExecutor() + args.ExecutorConfig.TTLForFailedRefundIdInSeconds = 60 + numExecuted := 0 + args.TransactionExecutor = &testsCommon.TransactionExecutorStub{ + ExecuteTransactionCalled: func(ctx context.Context, networkConfig *data.NetworkConfig, receiver string, transactionType string, gasLimit uint64, dataBytes []byte) error { + numExecuted++ + return expectedError + }, + } + args.Proxy = &interactors.ProxyStub{ + ExecuteVMQueryCalled: func(ctx context.Context, vmRequest *data.VmValueRequest) (*data.VmValuesResponseData, error) { + return &data.VmValuesResponseData{ + Data: &vm.VMOutputApi{ + ReturnCode: okCodeAfterExecution, + ReturnData: [][]byte{ + {0x01}, + {0x03, 0x04}, + }, + }, + }, nil + }, + GetNetworkConfigCalled: func(ctx context.Context) (*data.NetworkConfig, error) { + return &data.NetworkConfig{}, nil + }, + } + args.Codec = &testsCommon.MultiversxCodecStub{ + DecodeProxySCCompleteCallDataCalled: func(buff []byte) (bridgeCore.ProxySCCompleteCallData, error) { + assert.Equal(t, []byte{0x03, 0x04}, buff) + + return bridgeCore.ProxySCCompleteCallData{ + To: data.NewAddressFromBytes(bytes.Repeat([]byte{1}, 32)), + }, nil + }, + } + + executor, _ := NewScCallExecutor(args) + err := executor.Execute(context.Background()) + assert.NotNil(t, err) + assert.Contains(t, err.Error(), expectedError.Error()) + assert.Contains(t, err.Error(), "errors found during execution") + + //re-run the same call, no errors should be found + err = executor.Execute(context.Background()) + assert.Nil(t, err) + + //only one sent transaction should be + assert.Equal(t, 1, numExecuted) + assert.True(t, executor.isFailed(1)) + }) + t.Run("SendTransaction errors, should error and register the failed id, after TTL expires should call again", func(t *testing.T) { t.Parallel() args := createMockArgsScCallExecutor() + args.ExecutorConfig.TTLForFailedRefundIdInSeconds = 2 + numExecuted := 0 args.TransactionExecutor = &testsCommon.TransactionExecutorStub{ ExecuteTransactionCalled: func(ctx context.Context, networkConfig *data.NetworkConfig, receiver string, transactionType string, gasLimit uint64, dataBytes []byte) error { + numExecuted++ return expectedError }, } @@ -387,6 +455,19 @@ func TestScCallExecutor_Execute(t *testing.T) { assert.NotNil(t, err) assert.Contains(t, err.Error(), expectedError.Error()) assert.Contains(t, err.Error(), "errors found during execution") + + //wait for TTL to expire + time.Sleep(time.Second * 3) + + //re-run the same call, same error should be found + err = executor.Execute(context.Background()) + assert.NotNil(t, err) + assert.Contains(t, err.Error(), expectedError.Error()) + assert.Contains(t, err.Error(), "errors found during execution") + + //only one sent transaction should be + assert.Equal(t, 2, numExecuted) + assert.True(t, executor.isFailed(1)) }) t.Run("should not execute transactions with high gas limit usage", func(t *testing.T) { t.Parallel() diff --git a/executors/multiversx/transactionExecutor.go b/executors/multiversx/transactionExecutor.go index 58a2e258..d3ba94fa 100644 --- a/executors/multiversx/transactionExecutor.go +++ b/executors/multiversx/transactionExecutor.go @@ -33,24 +33,20 @@ type ArgsTransactionExecutor struct { PrivateKey crypto.PrivateKey SingleSigner crypto.SingleSigner TransactionChecks config.TransactionChecksConfig - CloseAppChan chan struct{} } type transactionExecutor struct { - proxy Proxy - nonceTxHandler NonceTransactionsHandler - numSentTransactions uint32 - privateKey crypto.PrivateKey - singleSigner crypto.SingleSigner - senderAddress core.AddressHandler - log logger.Logger - timeBetweenChecks time.Duration - closeAppOnError bool - extraDelayOnError time.Duration - executionTimeout time.Duration - closeAppChan chan struct{} - checkTransactionResults bool - mutCriticalSection sync.Mutex + proxy Proxy + nonceTxHandler NonceTransactionsHandler + numSentTransactions uint32 + privateKey crypto.PrivateKey + singleSigner crypto.SingleSigner + senderAddress core.AddressHandler + log logger.Logger + timeBetweenChecks time.Duration + closeAppOnError bool + executionTimeout time.Duration + mutCriticalSection sync.Mutex } // NewTransactionExecutor creates a new executor instance that is able to send transactions & handle results @@ -68,18 +64,14 @@ func NewTransactionExecutor(args ArgsTransactionExecutor) (*transactionExecutor, senderAddress := data.NewAddressFromBytes(publicKeyBytes) return &transactionExecutor{ - proxy: args.Proxy, - log: args.Log, - nonceTxHandler: args.NonceTxHandler, - privateKey: args.PrivateKey, - singleSigner: args.SingleSigner, - senderAddress: senderAddress, - checkTransactionResults: args.TransactionChecks.CheckTransactionResults, - timeBetweenChecks: time.Second * time.Duration(args.TransactionChecks.TimeInSecondsBetweenChecks), - closeAppOnError: args.TransactionChecks.CloseAppOnError, - extraDelayOnError: time.Second * time.Duration(args.TransactionChecks.ExtraDelayInSecondsOnError), - executionTimeout: time.Second * time.Duration(args.TransactionChecks.ExecutionTimeoutInSeconds), - closeAppChan: args.CloseAppChan, + proxy: args.Proxy, + log: args.Log, + nonceTxHandler: args.NonceTxHandler, + privateKey: args.PrivateKey, + singleSigner: args.SingleSigner, + senderAddress: senderAddress, + timeBetweenChecks: time.Second * time.Duration(args.TransactionChecks.TimeInSecondsBetweenChecks), + executionTimeout: time.Second * time.Duration(args.TransactionChecks.ExecutionTimeoutInSeconds), }, nil } @@ -100,24 +92,15 @@ func checkTransactionExecutorArgs(args ArgsTransactionExecutor) error { if check.IfNil(args.SingleSigner) { return errNilSingleSigner } - err := checkTransactionChecksConfig(args.TransactionChecks, args.Log) + err := checkTransactionChecksConfig(args.TransactionChecks) if err != nil { return err } - if args.CloseAppChan == nil && args.TransactionChecks.CloseAppOnError { - return fmt.Errorf("%w while the TransactionChecks.CloseAppOnError is set to true", errNilCloseAppChannel) - } - return nil } -func checkTransactionChecksConfig(args config.TransactionChecksConfig, log logger.Logger) error { - if !args.CheckTransactionResults { - log.Warn("transaction checks are disabled! This can lead to funds being drained in case of a repetitive error") - return nil - } - +func checkTransactionChecksConfig(args config.TransactionChecksConfig) error { if args.TimeInSecondsBetweenChecks < minCheckValues { return fmt.Errorf("%w for TransactionChecks.TimeInSecondsBetweenChecks, minimum: %d, got: %d", errInvalidValue, minCheckValues, args.TimeInSecondsBetweenChecks) @@ -163,12 +146,8 @@ func (executor *transactionExecutor) ExecuteTransaction( Value: "0", } - workingCtx := ctx - if executor.checkTransactionResults { - var cancel func() - workingCtx, cancel = context.WithTimeout(ctx, executor.executionTimeout) - defer cancel() - } + workingCtx, cancel := context.WithTimeout(ctx, executor.executionTimeout) + defer cancel() hash, err := executor.executeAsCriticalSection(workingCtx, tx) if err != nil { @@ -185,7 +164,7 @@ func (executor *transactionExecutor) ExecuteTransaction( atomic.AddUint32(&executor.numSentTransactions, 1) - return executor.handleResults(workingCtx, hash) + return executor.checkResultsUntilDone(workingCtx, hash) } func (executor *transactionExecutor) executeAsCriticalSection(ctx context.Context, tx *transaction.FrontendTransaction) (string, error) { @@ -223,16 +202,6 @@ func (executor *transactionExecutor) signTransactionWithPrivateKey(tx *transacti return nil } -func (executor *transactionExecutor) handleResults(ctx context.Context, hash string) error { - if !executor.checkTransactionResults { - return nil - } - - err := executor.checkResultsUntilDone(ctx, hash) - executor.waitForExtraDelay(ctx, err) - return err -} - func (executor *transactionExecutor) checkResultsUntilDone(ctx context.Context, hash string) error { timer := time.NewTimer(executor.timeBetweenChecks) defer timer.Stop() @@ -246,7 +215,6 @@ func (executor *transactionExecutor) checkResultsUntilDone(ctx context.Context, case <-timer.C: err, shouldStop := executor.checkResults(ctx, hash) if shouldStop { - executor.handleError(ctx, err) return err } } @@ -290,36 +258,6 @@ func (executor *transactionExecutor) logFullTransaction(ctx context.Context, has executor.log.Error("transaction failed", "hash", hash, "full transaction details", string(txDataString)) } -func (executor *transactionExecutor) handleError(ctx context.Context, err error) { - if err == nil { - return - } - if !executor.closeAppOnError { - return - } - - go func() { - // wait here until we could write in the close app chan - // ... or the context expired (application might close) - select { - case <-ctx.Done(): - case executor.closeAppChan <- struct{}{}: - } - }() -} - -func (executor *transactionExecutor) waitForExtraDelay(ctx context.Context, err error) { - if err == nil { - return - } - - timer := time.NewTimer(executor.extraDelayOnError) - select { - case <-ctx.Done(): - case <-timer.C: - } -} - // GetNumSentTransaction returns the total sent transactions func (executor *transactionExecutor) GetNumSentTransaction() uint32 { return atomic.LoadUint32(&executor.numSentTransactions) diff --git a/executors/multiversx/transactionExecutor_test.go b/executors/multiversx/transactionExecutor_test.go index 97b3dd89..57a85674 100644 --- a/executors/multiversx/transactionExecutor_test.go +++ b/executors/multiversx/transactionExecutor_test.go @@ -31,17 +31,10 @@ func createMockArgsTransactionExecutor() ArgsTransactionExecutor { NonceTxHandler: &testsCommon.TxNonceHandlerV2Stub{}, PrivateKey: testCrypto.NewPrivateKeyMock(), SingleSigner: &testCrypto.SingleSignerStub{}, - CloseAppChan: make(chan struct{}), - } -} - -func createMockCheckConfigs() config.TransactionChecksConfig { - return config.TransactionChecksConfig{ - CheckTransactionResults: true, - TimeInSecondsBetweenChecks: 6, - ExecutionTimeoutInSeconds: 120, - CloseAppOnError: true, - ExtraDelayInSecondsOnError: 120, + TransactionChecks: config.TransactionChecksConfig{ + TimeInSecondsBetweenChecks: 6, + ExecutionTimeoutInSeconds: 120, + }, } } @@ -102,7 +95,6 @@ func TestNewExecutor(t *testing.T) { t.Parallel() args := createMockArgsTransactionExecutor() - args.TransactionChecks = createMockCheckConfigs() args.TransactionChecks.TimeInSecondsBetweenChecks = 0 executor, err := NewTransactionExecutor(args) @@ -114,7 +106,6 @@ func TestNewExecutor(t *testing.T) { t.Parallel() args := createMockArgsTransactionExecutor() - args.TransactionChecks = createMockCheckConfigs() args.TransactionChecks.ExecutionTimeoutInSeconds = 0 executor, err := NewTransactionExecutor(args) @@ -122,32 +113,10 @@ func TestNewExecutor(t *testing.T) { assert.ErrorIs(t, err, errInvalidValue) assert.Contains(t, err.Error(), "for TransactionChecks.ExecutionTimeoutInSeconds, minimum: 1, got: 0") }) - t.Run("nil close app chan should error", func(t *testing.T) { - t.Parallel() - - args := createMockArgsTransactionExecutor() - args.TransactionChecks = createMockCheckConfigs() - args.CloseAppChan = nil - - executor, err := NewTransactionExecutor(args) - assert.Nil(t, executor) - assert.ErrorIs(t, err, errNilCloseAppChannel) - assert.Contains(t, err.Error(), "while the TransactionChecks.CloseAppOnError is set to true") - }) - t.Run("should work without transaction checks", func(t *testing.T) { - t.Parallel() - - args := createMockArgsTransactionExecutor() - - executor, err := NewTransactionExecutor(args) - assert.NotNil(t, executor) - assert.Nil(t, err) - }) - t.Run("should work with transaction checks", func(t *testing.T) { + t.Run("should work", func(t *testing.T) { t.Parallel() args := createMockArgsTransactionExecutor() - args.TransactionChecks = createMockCheckConfigs() executor, err := NewTransactionExecutor(args) assert.NotNil(t, executor) @@ -268,7 +237,6 @@ func TestTransactionExecutor_ExecuteTransaction(t *testing.T) { t.Parallel() args := createMockArgsTransactionExecutor() - args.TransactionChecks = createMockCheckConfigs() args.TransactionChecks.TimeInSecondsBetweenChecks = 1 txHash := "tx hash" processTransactionStatusCalled := false @@ -331,27 +299,10 @@ func TestTransactionExecutor_ExecuteTransaction(t *testing.T) { }) } -func TestTransactionExecutor_handleResults(t *testing.T) { +func TestTransactionExecutor_checkResultsUntilDone(t *testing.T) { t.Parallel() testHash := "test hash" - t.Run("checkTransactionResults false should not check and return nil", func(t *testing.T) { - t.Parallel() - - args := createMockArgsTransactionExecutor() - args.Proxy = &interactors.ProxyStub{ - ProcessTransactionStatusCalled: func(ctx context.Context, hexTxHash string) (transaction.TxStatus, error) { - assert.Fail(t, "should have not called ProcessTransactionStatusCalled") - - return transaction.TxStatusFail, nil - }, - } - - executor, _ := NewTransactionExecutor(args) - - err := executor.handleResults(context.Background(), testHash) - assert.Nil(t, err) - }) t.Run("timeout before process transaction called", func(t *testing.T) { t.Parallel() @@ -363,14 +314,13 @@ func TestTransactionExecutor_handleResults(t *testing.T) { return transaction.TxStatusFail, nil }, } - args.TransactionChecks = createMockCheckConfigs() executor, _ := NewTransactionExecutor(args) workingCtx, cancel := context.WithTimeout(context.Background(), time.Second) defer cancel() - err := executor.handleResults(workingCtx, testHash) + err := executor.checkResultsUntilDone(workingCtx, testHash) assert.ErrorIs(t, err, context.DeadlineExceeded) }) t.Run("transaction not found should continuously request the status", func(t *testing.T) { @@ -389,13 +339,12 @@ func TestTransactionExecutor_handleResults(t *testing.T) { return transaction.TxStatusInvalid, errors.New("transaction not found") }, } - args.TransactionChecks = createMockCheckConfigs() args.TransactionChecks.TimeInSecondsBetweenChecks = 1 executor, _ := NewTransactionExecutor(args) go func() { - err := executor.handleResults(context.Background(), testHash) + err := executor.checkResultsUntilDone(context.Background(), testHash) assert.ErrorIs(t, err, context.DeadlineExceeded) // this will be the actual error when the function finishes }() @@ -422,13 +371,12 @@ func TestTransactionExecutor_handleResults(t *testing.T) { return transaction.TxStatusPending, nil }, } - args.TransactionChecks = createMockCheckConfigs() args.TransactionChecks.TimeInSecondsBetweenChecks = 1 executor, _ := NewTransactionExecutor(args) go func() { - err := executor.handleResults(context.Background(), testHash) + err := executor.checkResultsUntilDone(context.Background(), testHash) assert.ErrorIs(t, err, context.DeadlineExceeded) // this will be the actual error when the function finishes }() @@ -439,67 +387,31 @@ func TestTransactionExecutor_handleResults(t *testing.T) { assert.Fail(t, "timeout") } }) - t.Run("error while requesting the status should return the error and wait", func(t *testing.T) { + t.Run("error while requesting the status should return the error", func(t *testing.T) { t.Parallel() expectedErr := errors.New("expected error") args := createMockArgsTransactionExecutor() - args.CloseAppChan = make(chan struct{}, 1) args.Proxy = &interactors.ProxyStub{ ProcessTransactionStatusCalled: func(ctx context.Context, hexTxHash string) (transaction.TxStatus, error) { return transaction.TxStatusInvalid, expectedErr }, } - args.TransactionChecks = createMockCheckConfigs() args.TransactionChecks.TimeInSecondsBetweenChecks = 1 - args.TransactionChecks.ExtraDelayInSecondsOnError = 6 executor, _ := NewTransactionExecutor(args) start := time.Now() - err := executor.handleResults(context.Background(), testHash) + err := executor.checkResultsUntilDone(context.Background(), testHash) assert.Equal(t, expectedErr, err) end := time.Now() - assert.GreaterOrEqual(t, end.Sub(start), time.Second*6) - select { - case <-args.CloseAppChan: - default: - assert.Fail(t, "failed to write on the close app chan") - } - }) - t.Run("error while requesting the status should not write on the close app chan, if not enabled", func(t *testing.T) { - t.Parallel() - - expectedErr := errors.New("expected error") - args := createMockArgsTransactionExecutor() - args.CloseAppChan = make(chan struct{}, 1) - args.Proxy = &interactors.ProxyStub{ - ProcessTransactionStatusCalled: func(ctx context.Context, hexTxHash string) (transaction.TxStatus, error) { - return transaction.TxStatusInvalid, expectedErr - }, - } - args.TransactionChecks = createMockCheckConfigs() - args.TransactionChecks.TimeInSecondsBetweenChecks = 1 - args.TransactionChecks.ExtraDelayInSecondsOnError = 1 - args.TransactionChecks.CloseAppOnError = false - - executor, _ := NewTransactionExecutor(args) - - err := executor.handleResults(context.Background(), testHash) - assert.Equal(t, expectedErr, err) - - select { - case <-args.CloseAppChan: - assert.Fail(t, "should have not written on the close chan") - default: - } + assert.GreaterOrEqual(t, end.Sub(start), time.Second) }) t.Run("transaction failed, should get more info and signal error", func(t *testing.T) { t.Parallel() args := createMockArgsTransactionExecutor() - args.CloseAppChan = make(chan struct{}, 1) args.Proxy = &interactors.ProxyStub{ ProcessTransactionStatusCalled: func(ctx context.Context, hexTxHash string) (transaction.TxStatus, error) { return transaction.TxStatusFail, nil @@ -508,20 +420,12 @@ func TestTransactionExecutor_handleResults(t *testing.T) { return &data.TransactionInfo{}, nil }, } - args.TransactionChecks = createMockCheckConfigs() args.TransactionChecks.TimeInSecondsBetweenChecks = 1 - args.TransactionChecks.ExtraDelayInSecondsOnError = 1 executor, _ := NewTransactionExecutor(args) - err := executor.handleResults(context.Background(), testHash) + err := executor.checkResultsUntilDone(context.Background(), testHash) assert.ErrorIs(t, err, errTransactionFailed) - - select { - case <-args.CloseAppChan: - default: - assert.Fail(t, "failed to write on the close app chan") - } }) t.Run("transaction failed, get more info fails, should signal error and not panic", func(t *testing.T) { t.Parallel() @@ -534,7 +438,6 @@ func TestTransactionExecutor_handleResults(t *testing.T) { }() args := createMockArgsTransactionExecutor() - args.CloseAppChan = make(chan struct{}, 1) args.Proxy = &interactors.ProxyStub{ ProcessTransactionStatusCalled: func(ctx context.Context, hexTxHash string) (transaction.TxStatus, error) { return transaction.TxStatusFail, nil @@ -543,20 +446,12 @@ func TestTransactionExecutor_handleResults(t *testing.T) { return nil, fmt.Errorf("random error") }, } - args.TransactionChecks = createMockCheckConfigs() args.TransactionChecks.TimeInSecondsBetweenChecks = 1 - args.TransactionChecks.ExtraDelayInSecondsOnError = 1 executor, _ := NewTransactionExecutor(args) - err := executor.handleResults(context.Background(), testHash) + err := executor.checkResultsUntilDone(context.Background(), testHash) assert.ErrorIs(t, err, errTransactionFailed) - - select { - case <-args.CloseAppChan: - default: - assert.Fail(t, "failed to write on the close app chan") - } }) } @@ -564,7 +459,6 @@ func TestTransactionExecutor_ExecuteTransactionInParallelShouldWork(t *testing.T t.Parallel() args := createMockArgsTransactionExecutor() - args.TransactionChecks = createMockCheckConfigs() args.TransactionChecks.TimeInSecondsBetweenChecks = 1 nonceCounter := uint64(100) diff --git a/integrationTests/relayers/slowTests/framework/testSetup.go b/integrationTests/relayers/slowTests/framework/testSetup.go index 2daf9f6e..f52af100 100644 --- a/integrationTests/relayers/slowTests/framework/testSetup.go +++ b/integrationTests/relayers/slowTests/framework/testSetup.go @@ -133,10 +133,12 @@ func (setup *TestSetup) startScCallerModule() { MaxGasLimitToUse: 249_999_999, // max cross shard limit GasLimitForOutOfGasTransactions: 30_000_000, // gas to use when a higher than max allowed is encountered PollingIntervalInMillis: 1000, // 1 second + TTLForFailedRefundIdInSeconds: 1, // 1 second }, RefundExecutor: config.RefundExecutorConfig{ - GasToExecute: 30_000_000, - PollingIntervalInMillis: 1000, + GasToExecute: 30_000_000, + PollingIntervalInMillis: 1000, + TTLForFailedRefundIdInSeconds: 1, }, Filter: config.PendingOperationsFilterConfig{ AllowedEthAddresses: []string{"*"}, @@ -145,15 +147,13 @@ func (setup *TestSetup) startScCallerModule() { }, Logs: config.LogsConfig{}, TransactionChecks: config.TransactionChecksConfig{ - CheckTransactionResults: true, - CloseAppOnError: false, ExecutionTimeoutInSeconds: 2, TimeInSecondsBetweenChecks: 1, }, } var err error - setup.ScCallerModuleInstance, err = module.NewScCallsModule(cfg, log, nil) + setup.ScCallerModuleInstance, err = module.NewScCallsModule(cfg, log) require.Nil(setup, err) log.Info("started SC calls module", "monitoring SC proxy address", setup.MultiversxHandler.ScProxyAddress) } From 10645160548094863f22cf87e987e2781091428c Mon Sep 17 00:00:00 2001 From: Iulian Pascalau Date: Fri, 3 Jan 2025 15:48:15 +0200 Subject: [PATCH 2/3] - linter fix --- executors/multiversx/transactionExecutor.go | 1 - 1 file changed, 1 deletion(-) diff --git a/executors/multiversx/transactionExecutor.go b/executors/multiversx/transactionExecutor.go index d3ba94fa..b74f019c 100644 --- a/executors/multiversx/transactionExecutor.go +++ b/executors/multiversx/transactionExecutor.go @@ -44,7 +44,6 @@ type transactionExecutor struct { senderAddress core.AddressHandler log logger.Logger timeBetweenChecks time.Duration - closeAppOnError bool executionTimeout time.Duration mutCriticalSection sync.Mutex } From a941f5945c27dd81c217d2ce54af0ed40e39fa63 Mon Sep 17 00:00:00 2001 From: Iulian Pascalau Date: Fri, 3 Jan 2025 16:02:22 +0200 Subject: [PATCH 3/3] - fix after review --- config/tomlConfigs_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/tomlConfigs_test.go b/config/tomlConfigs_test.go index 1dae48a7..d9204eef 100644 --- a/config/tomlConfigs_test.go +++ b/config/tomlConfigs_test.go @@ -479,7 +479,7 @@ func TestScCallsExecutorConfigs(t *testing.T) { MaxGasLimitToUse = 249999999 # this is a safe max gas limit to use both intra-shard & cross-shard GasLimitForOutOfGasTransactions = 30000000 # this value will be used when a transaction specified a gas limit > 249999999 PollingIntervalInMillis = 6000 - TTLForFailedRefundIdInSeconds = 3600 + TTLForFailedRefundIdInSeconds = 3600 [RefundExecutor] GasToExecute = 20000000