Merge pull request #359 from multiversx/mvx-executor-fix-max-gas-limit

Fix max gas limit on executor

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

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,

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

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

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

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,

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,

executors/multiversx/scCallsExecutor.go

@@ -27,6 +27,7 @@ const (
 	scProxyCallFunction            = "execute"
 	minCheckValues                 = 1
 	transactionNotFoundErrString   = "transaction not found"
+	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
@@ -37,6 +38,7 @@ type ArgsScCallExecutor struct {
 	Filter               ScCallsExecuteFilter
 	Log                  logger.Logger
 	ExtraGasToExecute    uint64
+	MaxGasLimitToUse     uint64
 	NonceTxHandler       NonceTransactionsHandler
 	PrivateKey           crypto.PrivateKey
 	SingleSigner         crypto.SingleSigner
@@ -51,6 +53,7 @@ type scCallExecutor struct {
 	filter                  ScCallsExecuteFilter
 	log                     logger.Logger
 	extraGasToExecute       uint64
+	maxGasLimitToUse        uint64
 	nonceTxHandler          NonceTransactionsHandler
 	privateKey              crypto.PrivateKey
 	singleSigner            crypto.SingleSigner
@@ -85,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,
@@ -120,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
@@ -280,6 +287,11 @@ func (executor *scCallExecutor) executeOperation(
 		Value:    "0",
 	}
 
+	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)
 	if err != nil {
 		return err

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"
@@ -687,6 +701,99 @@ func TestScCallExecutor_Execute(t *testing.T) {
 
 		executor, _ := NewScCallExecutor(args)
 
+		err := executor.Execute(context.Background())
+		assert.Nil(t, err)
+		assert.True(t, sendWasCalled)
+		assert.Equal(t, uint32(1), executor.GetNumSentTransaction())
+	})
+	t.Run("should work even if the gas limit exceeds the maximum allowed", func(t *testing.T) {
+		t.Parallel()
+
+		args := createMockArgsScCallExecutor()
+
+		nonceCounter := uint64(100)
+		sendWasCalled := false
+
+		args.Proxy = &interactors.ProxyStub{
+			ExecuteVMQueryCalled: func(ctx context.Context, vmRequest *data.VmValueRequest) (*data.VmValuesResponseData, error) {
+				assert.Equal(t, args.ScProxyBech32Address, vmRequest.Address)
+				assert.Equal(t, getPendingTransactionsFunction, vmRequest.FuncName)
+
+				return &data.VmValuesResponseData{
+					Data: &vm.VMOutputApi{
+						ReturnCode: okCodeAfterExecution,
+						ReturnData: [][]byte{
+							{0x01},
+							[]byte("ProxySCCompleteCallData 1"),
+							{0x02},
+							[]byte("ProxySCCompleteCallData 2"),
+						},
+					},
+				}, nil
+			},
+			GetNetworkConfigCalled: func(ctx context.Context) (*data.NetworkConfig, error) {
+				return &data.NetworkConfig{
+					ChainID:               "TEST",
+					MinTransactionVersion: 111,
+				}, nil
+			},
+		}
+		args.Codec = &testsCommon.MultiversxCodecStub{
+			DecodeProxySCCompleteCallDataCalled: func(buff []byte) (parsers.ProxySCCompleteCallData, error) {
+				if string(buff) == "ProxySCCompleteCallData 1" {
+					return createTestProxySCCompleteCallData("tkn1"), nil
+				}
+				if string(buff) == "ProxySCCompleteCallData 2" {
+					return createTestProxySCCompleteCallData("tkn2"), nil
+				}
+
+				return parsers.ProxySCCompleteCallData{}, errors.New("wrong buffer")
+			},
+			ExtractGasLimitFromRawCallDataCalled: func(buff []byte) (uint64, error) {
+				return args.MaxGasLimitToUse - args.ExtraGasToExecute + 1, nil
+			},
+		}
+		args.Filter = &testsCommon.ScCallsExecuteFilterStub{
+			ShouldExecuteCalled: func(callData parsers.ProxySCCompleteCallData) bool {
+				return callData.Token == "tkn2"
+			},
+		}
+		args.NonceTxHandler = &testsCommon.TxNonceHandlerV2Stub{
+			ApplyNonceAndGasPriceCalled: func(ctx context.Context, address core.AddressHandler, tx *transaction.FrontendTransaction) error {
+				tx.Nonce = nonceCounter
+				tx.GasPrice = 101010
+				nonceCounter++
+				return nil
+			},
+			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(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)
+				_, err := data.NewAddressFromBech32String(tx.Sender)
+				assert.Nil(t, err)
+				assert.Equal(t, "erd1qqqqqqqqqqqqqpgqk839entmk46ykukvhpn90g6knskju3dtanaq20f66e", tx.Receiver)
+				assert.Equal(t, "0", tx.Value)
+
+				// only the second pending operation gor through the filter
+				expectedData := scProxyCallFunction + "@02"
+				assert.Equal(t, expectedData, string(tx.Data))
+
+				sendWasCalled = true
+
+				return "", nil
+			},
+		}
+		args.SingleSigner = &testCrypto.SingleSignerStub{
+			SignCalled: func(private crypto.PrivateKey, msg []byte) ([]byte, error) {
+				return []byte("sig"), nil
+			},
+		}
+
+		executor, _ := NewScCallExecutor(args)
+
 		err := executor.Execute(context.Background())
 		assert.Nil(t, err)
 		assert.True(t, sendWasCalled)

integrationTests/relayers/slowTests/framework/testSetup.go

@@ -113,7 +113,8 @@ func (setup *TestSetup) StartRelayersAndScModule() {
 func (setup *TestSetup) startScCallerModule() {
 	cfg := config.ScCallsModuleConfig{
 		ScProxyBech32Address:         setup.MultiversxHandler.ScProxyAddress.Bech32(),
-		ExtraGasToExecute:            60_000_000, // 60 million: this ensures that a SC call with 0 gas limit is refunded
+		ExtraGasToExecute:            60_000_000,  // 60 million: this ensures that a SC call with 0 gas limit is refunded
+		MaxGasLimitToUse:             249_999_999, // max cross shard limit
 		NetworkAddress:               setup.ChainSimulator.GetNetworkAddress(),
 		ProxyMaxNoncesDelta:          5,
 		ProxyFinalityCheck:           false,