From 542676471fce549bf04f77cabbbe53a25d2b853c Mon Sep 17 00:00:00 2001 From: Iulian Pascalau Date: Thu, 17 Oct 2024 10:28:12 +0300 Subject: [PATCH 1/3] - fix max gas limit on executor --- executors/multiversx/scCallsExecutor.go | 6 ++ executors/multiversx/scCallsExecutor_test.go | 93 ++++++++++++++++++++ 2 files changed, 99 insertions(+) diff --git a/executors/multiversx/scCallsExecutor.go b/executors/multiversx/scCallsExecutor.go index 1aa64fc8..c17860ac 100644 --- a/executors/multiversx/scCallsExecutor.go +++ b/executors/multiversx/scCallsExecutor.go @@ -27,6 +27,7 @@ const ( scProxyCallFunction = "execute" minCheckValues = 1 transactionNotFoundErrString = "transaction not found" + maxGasLimitToExecute = 249999999 // 250 million - 1 ) // ArgsScCallExecutor represents the DTO struct for creating a new instance of type scCallExecutor @@ -280,6 +281,11 @@ 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 + } + err = executor.nonceTxHandler.ApplyNonceAndGasPrice(ctx, executor.senderAddress, tx) if err != nil { return err diff --git a/executors/multiversx/scCallsExecutor_test.go b/executors/multiversx/scCallsExecutor_test.go index 088b778c..a308b07b 100644 --- a/executors/multiversx/scCallsExecutor_test.go +++ b/executors/multiversx/scCallsExecutor_test.go @@ -687,6 +687,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 maxGasLimitToExecute - 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(maxGasLimitToExecute), 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) From 66be2e940783d27c2fa894394f5cc9bd13149213 Mon Sep 17 00:00:00 2001 From: Iulian Pascalau Date: Thu, 17 Oct 2024 11:35:02 +0300 Subject: [PATCH 2/3] - avoid hardcoding: extracted the value in configs --- cmd/scCallsExecutor/config/config.toml | 1 + cmd/scCallsExecutor/main.go | 5 ++-- config/config.go | 1 + config/tomlConfigs_test.go | 2 ++ executors/multiversx/errors.go | 23 ++++++++++--------- executors/multiversx/module/scCallsModule.go | 1 + .../multiversx/module/scCallsModule_test.go | 3 ++- executors/multiversx/scCallsExecutor.go | 14 +++++++---- executors/multiversx/scCallsExecutor_test.go | 18 +++++++++++++-- 9 files changed, 48 insertions(+), 20 deletions(-) diff --git a/cmd/scCallsExecutor/config/config.toml b/cmd/scCallsExecutor/config/config.toml index 2447dc1b..57956bc7 100644 --- 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 index d2f808d0..01ce3842 100644 --- 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 index bab8947a..6c851ad7 100644 --- 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 index f18762df..7ee85529 100644 --- 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 index 6515d8a4..94e986c0 100644 --- 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 index d5937063..354396d3 100644 --- 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 index d943a960..4e9de029 100644 --- 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 index c17860ac..fa96b2cc 100644 --- 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 index a308b07b..545cbe74 100644 --- 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) From 9fd68c15c979159d0c788102f806752f5d39e2e1 Mon Sep 17 00:00:00 2001 From: Iulian Pascalau Date: Thu, 17 Oct 2024 11:46:25 +0300 Subject: [PATCH 3/3] - fixed integration tests --- integrationTests/relayers/slowTests/framework/testSetup.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/integrationTests/relayers/slowTests/framework/testSetup.go b/integrationTests/relayers/slowTests/framework/testSetup.go index 4a43c994..a7ab3b2f 100644 --- a/integrationTests/relayers/slowTests/framework/testSetup.go +++ b/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,