- avoid hardcoding: extracted the value in configs

multiversx · Oct 17, 2024 · 66be2e9 · 66be2e9
1 parent 5426764
commit 66be2e9
Show file tree

Hide file tree

Showing 9 changed files with 48 additions and 20 deletions.
diff --git a/cmd/scCallsExecutor/config/config.toml b/cmd/scCallsExecutor/config/config.toml
@@ -1,5 +1,6 @@
 ScProxyBech32Address = "erd1qqqqqqqqqqqqqpgqnef5f5aq32d63kljld8w5vnvz4gk5sy9hrrq2ld08s"
 ExtraGasToExecute = 60000000 #this value allow the SC calls without provided gas limit to be refunded
+MaxGasLimitToUse = 249999999 # this is a safe max gas limit to use both intra-shard & cross-shard
 NetworkAddress = "http://127.0.0.1:8085"
 ProxyMaxNoncesDelta = 7
 ProxyFinalityCheck = true

diff --git a/cmd/scCallsExecutor/main.go b/cmd/scCallsExecutor/main.go
@@ -54,7 +54,7 @@ func main() {
 	}
 
 	app.Action = func(c *cli.Context) error {
-		return startRelay(c, app.Version)
+		return startExecutor(c, app.Version)
 	}
 
 	err := app.Run(os.Args)
@@ -64,7 +64,7 @@ func main() {
 	}
 }
 
-func startRelay(ctx *cli.Context, version string) error {
+func startExecutor(ctx *cli.Context, version string) error {
 	flagsConfig := getFlagsConfig(ctx)
 
 	fileLogging, errLogger := attachFileLogger(log, flagsConfig)
@@ -113,6 +113,7 @@ func startRelay(ctx *cli.Context, version string) error {
 	args := config.ScCallsModuleConfig{
 		ScProxyBech32Address:         cfg.ScProxyBech32Address,
 		ExtraGasToExecute:            cfg.ExtraGasToExecute,
+		MaxGasLimitToUse:             cfg.MaxGasLimitToUse,
 		NetworkAddress:               cfg.NetworkAddress,
 		ProxyMaxNoncesDelta:          cfg.ProxyMaxNoncesDelta,
 		ProxyFinalityCheck:           cfg.ProxyFinalityCheck,

diff --git a/config/config.go b/config/config.go
@@ -194,6 +194,7 @@ type PendingOperationsFilterConfig struct {
 type ScCallsModuleConfig struct {
 	ScProxyBech32Address         string
 	ExtraGasToExecute            uint64
+	MaxGasLimitToUse             uint64
 	NetworkAddress               string
 	ProxyMaxNoncesDelta          int
 	ProxyFinalityCheck           bool

diff --git a/config/tomlConfigs_test.go b/config/tomlConfigs_test.go
@@ -407,6 +407,7 @@ func TestScCallsExecutorConfigs(t *testing.T) {
 	expectedConfig := ScCallsModuleConfig{
 		ScProxyBech32Address:         "erd1qqqqqqqqqqqqqpgqnef5f5aq32d63kljld8w5vnvz4gk5sy9hrrq2ld08s",
 		ExtraGasToExecute:            50000000,
+		MaxGasLimitToUse:             249999999,
 		NetworkAddress:               "127.0.0.1:8085",
 		ProxyMaxNoncesDelta:          7,
 		ProxyFinalityCheck:           true,
@@ -436,6 +437,7 @@ func TestScCallsExecutorConfigs(t *testing.T) {
 	testString := `
 ScProxyBech32Address = "erd1qqqqqqqqqqqqqpgqnef5f5aq32d63kljld8w5vnvz4gk5sy9hrrq2ld08s"
 ExtraGasToExecute = 50000000
+MaxGasLimitToUse = 249999999 # this is a safe max gas limit to use both intra-shard & cross-shard
 NetworkAddress = "127.0.0.1:8085"
 ProxyMaxNoncesDelta = 7
 ProxyFinalityCheck = true

diff --git a/executors/multiversx/errors.go b/executors/multiversx/errors.go
@@ -3,15 +3,16 @@ package multiversx
 import "errors"
 
 var (
-	errInvalidNumberOfResponseLines = errors.New("invalid number of responses")
-	errNilProxy                     = errors.New("nil proxy")
-	errNilCodec                     = errors.New("nil codec")
-	errNilFilter                    = errors.New("nil filter")
-	errNilLogger                    = errors.New("nil logger")
-	errNilNonceTxHandler            = errors.New("nil nonce transaction handler")
-	errNilPrivateKey                = errors.New("nil private key")
-	errNilSingleSigner              = errors.New("nil single signer")
-	errInvalidValue                 = errors.New("invalid value")
-	errNilCloseAppChannel           = errors.New("nil close application channel")
-	errTransactionFailed            = errors.New("transaction failed")
+	errInvalidNumberOfResponseLines  = errors.New("invalid number of responses")
+	errNilProxy                      = errors.New("nil proxy")
+	errNilCodec                      = errors.New("nil codec")
+	errNilFilter                     = errors.New("nil filter")
+	errNilLogger                     = errors.New("nil logger")
+	errNilNonceTxHandler             = errors.New("nil nonce transaction handler")
+	errNilPrivateKey                 = errors.New("nil private key")
+	errNilSingleSigner               = errors.New("nil single signer")
+	errInvalidValue                  = errors.New("invalid value")
+	errNilCloseAppChannel            = errors.New("nil close application channel")
+	errTransactionFailed             = errors.New("transaction failed")
+	errMaxGasLimitIsLessThanRequired = errors.New("max gas limit to execute a SC call is less than the minimum required")
 )
diff --git a/executors/multiversx/module/scCallsModule.go b/executors/multiversx/module/scCallsModule.go
@@ -79,6 +79,7 @@ func NewScCallsModule(cfg config.ScCallsModuleConfig, log logger.Logger, chClose
 		Filter:               filter,
 		Log:                  log,
 		ExtraGasToExecute:    cfg.ExtraGasToExecute,
+		MaxGasLimitToUse:     cfg.MaxGasLimitToUse,
 		NonceTxHandler:       module.nonceTxsHandler,
 		PrivateKey:           privateKey,
 		SingleSigner:         singleSigner,

diff --git a/executors/multiversx/module/scCallsModule_test.go b/executors/multiversx/module/scCallsModule_test.go
@@ -12,7 +12,8 @@ import (
 func createTestConfigs() config.ScCallsModuleConfig {
 	return config.ScCallsModuleConfig{
 		ScProxyBech32Address:         "erd1qqqqqqqqqqqqqpgqgftcwj09u0nhmskrw7xxqcqh8qmzwyexd8ss7ftcxx",
-		ExtraGasToExecute:            1000,
+		ExtraGasToExecute:            6000000,
+		MaxGasLimitToUse:             249999999,
 		NetworkAddress:               "http://127.0.0.1:8079",
 		ProxyMaxNoncesDelta:          5,
 		ProxyFinalityCheck:           false,

diff --git a/executors/multiversx/scCallsExecutor.go b/executors/multiversx/scCallsExecutor.go
@@ -27,7 +27,7 @@ const (
 	scProxyCallFunction            = "execute"
 	minCheckValues                 = 1
 	transactionNotFoundErrString   = "transaction not found"
-	maxGasLimitToExecute           = 249999999 // 250 million - 1
+	minGasToExecuteSCCalls         = 2010000 // the absolut minimum gas limit to do a SC call
 )
 
 // ArgsScCallExecutor represents the DTO struct for creating a new instance of type scCallExecutor
@@ -38,6 +38,7 @@ type ArgsScCallExecutor struct {
 	Filter               ScCallsExecuteFilter
 	Log                  logger.Logger
 	ExtraGasToExecute    uint64
+	MaxGasLimitToUse     uint64
 	NonceTxHandler       NonceTransactionsHandler
 	PrivateKey           crypto.PrivateKey
 	SingleSigner         crypto.SingleSigner
@@ -52,6 +53,7 @@ type scCallExecutor struct {
 	filter                  ScCallsExecuteFilter
 	log                     logger.Logger
 	extraGasToExecute       uint64
+	maxGasLimitToUse        uint64
 	nonceTxHandler          NonceTransactionsHandler
 	privateKey              crypto.PrivateKey
 	singleSigner            crypto.SingleSigner
@@ -86,6 +88,7 @@ func NewScCallExecutor(args ArgsScCallExecutor) (*scCallExecutor, error) {
 		filter:                  args.Filter,
 		log:                     args.Log,
 		extraGasToExecute:       args.ExtraGasToExecute,
+		maxGasLimitToUse:        args.MaxGasLimitToUse,
 		nonceTxHandler:          args.NonceTxHandler,
 		privateKey:              args.PrivateKey,
 		singleSigner:            args.SingleSigner,
@@ -121,6 +124,9 @@ func checkArgs(args ArgsScCallExecutor) error {
 	if check.IfNil(args.SingleSigner) {
 		return errNilSingleSigner
 	}
+	if args.MaxGasLimitToUse < minGasToExecuteSCCalls {
+		return fmt.Errorf("%w: provided: %d, absolute minimum required: %d", errMaxGasLimitIsLessThanRequired, args.MaxGasLimitToUse, minGasToExecuteSCCalls)
+	}
 	err := checkTransactionChecksConfig(args)
 	if err != nil {
 		return err
@@ -281,9 +287,9 @@ func (executor *scCallExecutor) executeOperation(
 		Value:    "0",
 	}
 
-	if tx.GasLimit > maxGasLimitToExecute {
-		executor.log.Warn("capped the gas limit", "computed gas limit", tx.GasLimit, "capped to", maxGasLimitToExecute)
-		tx.GasLimit = maxGasLimitToExecute
+	if tx.GasLimit > executor.maxGasLimitToUse {
+		executor.log.Warn("capped the gas limit", "computed gas limit", tx.GasLimit, "capped to", executor.maxGasLimitToUse)
+		tx.GasLimit = executor.maxGasLimitToUse
 	}
 
 	err = executor.nonceTxHandler.ApplyNonceAndGasPrice(ctx, executor.senderAddress, tx)

diff --git a/executors/multiversx/scCallsExecutor_test.go b/executors/multiversx/scCallsExecutor_test.go
@@ -34,6 +34,7 @@ func createMockArgsScCallExecutor() ArgsScCallExecutor {
 		Filter:               &testsCommon.ScCallsExecuteFilterStub{},
 		Log:                  &testsCommon.LoggerStub{},
 		ExtraGasToExecute:    100,
+		MaxGasLimitToUse:     minGasToExecuteSCCalls,
 		NonceTxHandler:       &testsCommon.TxNonceHandlerV2Stub{},
 		PrivateKey:           testCrypto.NewPrivateKeyMock(),
 		SingleSigner:         &testCrypto.SingleSignerStub{},
@@ -189,6 +190,18 @@ func TestNewScCallExecutor(t *testing.T) {
 		assert.ErrorIs(t, err, errNilCloseAppChannel)
 		assert.Contains(t, err.Error(), "while the TransactionChecks.CloseAppOnError is set to true")
 	})
+	t.Run("invalid MaxGasLimitToUse should error", func(t *testing.T) {
+		t.Parallel()
+
+		args := createMockArgsScCallExecutor()
+		args.TransactionChecks = createMockCheckConfigs()
+		args.MaxGasLimitToUse = minGasToExecuteSCCalls - 1
+
+		executor, err := NewScCallExecutor(args)
+		assert.Nil(t, executor)
+		assert.ErrorIs(t, err, errMaxGasLimitIsLessThanRequired)
+		assert.Contains(t, err.Error(), "provided: 2009999, absolute minimum required: 2010000")
+	})
 	t.Run("should work without transaction checks", func(t *testing.T) {
 		t.Parallel()
 
@@ -499,6 +512,7 @@ func TestScCallExecutor_Execute(t *testing.T) {
 		t.Parallel()
 
 		args := createMockArgsScCallExecutor()
+		args.MaxGasLimitToUse = 250000000
 		args.TransactionChecks = createMockCheckConfigs()
 		args.TransactionChecks.TimeInSecondsBetweenChecks = 1
 		txHash := "tx hash"
@@ -736,7 +750,7 @@ func TestScCallExecutor_Execute(t *testing.T) {
 				return parsers.ProxySCCompleteCallData{}, errors.New("wrong buffer")
 			},
 			ExtractGasLimitFromRawCallDataCalled: func(buff []byte) (uint64, error) {
-				return maxGasLimitToExecute - args.ExtraGasToExecute + 1, nil
+				return args.MaxGasLimitToUse - args.ExtraGasToExecute + 1, nil
 			},
 		}
 		args.Filter = &testsCommon.ScCallsExecuteFilterStub{
@@ -754,7 +768,7 @@ func TestScCallExecutor_Execute(t *testing.T) {
 			SendTransactionCalled: func(ctx context.Context, tx *transaction.FrontendTransaction) (string, error) {
 				assert.Equal(t, "TEST", tx.ChainID)
 				assert.Equal(t, uint32(111), tx.Version)
-				assert.Equal(t, uint64(maxGasLimitToExecute), tx.GasLimit) // the cap was done
+				assert.Equal(t, uint64(args.MaxGasLimitToUse), tx.GasLimit) // the cap was done
 				assert.Equal(t, nonceCounter-1, tx.Nonce)
 				assert.Equal(t, uint64(101010), tx.GasPrice)
 				assert.Equal(t, hex.EncodeToString([]byte("sig")), tx.Signature)