Merge pull request #3463 from ElrondNetwork/backward-on-saveKeyVal-error

Backward on save key val error

cmd/node/config/enableEpochs.toml

@@ -114,6 +114,9 @@
     # SCRSizeInvariantCheckEnableEpoch represents the epoch when the scr size invariant check is enabled
     SCRSizeInvariantCheckEnableEpoch = 5
 
+    # BackwardCompSaveKeyValueEnableEpoch represents the epoch when the backward compatibility for save key value error is enabled
+    BackwardCompSaveKeyValueEnableEpoch = 5
+
     # MaxNodesChangeEnableEpoch holds configuration for changing the maximum number of nodes and the enabling epoch
     MaxNodesChangeEnableEpoch = [
         { EpochEnable = 0, MaxNumNodes = 36, NodesToShufflePerShard = 4 },

config/epochConfig.go

@@ -51,6 +51,7 @@ type EnableEpochs struct {
 	BuiltInFunctionOnMetaEnableEpoch            uint32
 	ComputeRewardCheckpointEnableEpoch          uint32
 	SCRSizeInvariantCheckEnableEpoch            uint32
+	BackwardCompSaveKeyValueEnableEpoch         uint32
 }
 
 // GasScheduleByEpochs represents a gas schedule toml entry that will be applied from the provided epoch

config/tomlConfig_test.go

@@ -587,6 +587,9 @@ func TestEnableEpochConfig(t *testing.T) {
     # SCRSizeInvariantCheckEnableEpoch represents the epoch when the scr size invariant check is enabled
     SCRSizeInvariantCheckEnableEpoch = 37
 
+    # BackwardCompSaveKeyValueEnableEpoch represents the epoch when backward compatibility save key value is enabled
+    BackwardCompSaveKeyValueEnableEpoch = 38
+
     # MaxNodesChangeEnableEpoch holds configuration for changing the maximum number of nodes and the enabling epoch
     MaxNodesChangeEnableEpoch = [
         { EpochEnable = 36, MaxNumNodes = 37, NodesToShufflePerShard = 38 },
@@ -651,6 +654,7 @@ func TestEnableEpochConfig(t *testing.T) {
 			BuiltInFunctionOnMetaEnableEpoch:            35,
 			ComputeRewardCheckpointEnableEpoch:          36,
 			SCRSizeInvariantCheckEnableEpoch:            37,
+			BackwardCompSaveKeyValueEnableEpoch:         38,
 		},
 		GasSchedule: GasScheduleConfig{
 			GasScheduleByEpochs: []GasScheduleByEpochs{

factory/blockProcessorCreator.go

@@ -258,6 +258,7 @@ func (pcf *processComponentsFactory) newShardBlockProcessor(
 		StakingV2EnableEpoch:                  pcf.epochConfig.EnableEpochs.StakingV2EnableEpoch,
 		BuiltInFunctionOnMetachainEnableEpoch: pcf.epochConfig.EnableEpochs.BuiltInFunctionOnMetaEnableEpoch,
 		SCRSizeInvariantCheckEnableEpoch:      pcf.epochConfig.EnableEpochs.SCRSizeInvariantCheckEnableEpoch,
+		BackwardCompSaveKeyValueEnableEpoch:   pcf.epochConfig.EnableEpochs.BackwardCompSaveKeyValueEnableEpoch,
 		VMOutputCacher:                        txcache.NewDisabledCache(),
 		ArwenChangeLocker:                     arwenChangeLocker,
 
@@ -584,6 +585,7 @@ func (pcf *processComponentsFactory) newMetaBlockProcessor(
 		StakingV2EnableEpoch:                  pcf.epochConfig.EnableEpochs.StakingV2EnableEpoch,
 		BuiltInFunctionOnMetachainEnableEpoch: pcf.epochConfig.EnableEpochs.BuiltInFunctionOnMetaEnableEpoch,
 		SCRSizeInvariantCheckEnableEpoch:      pcf.epochConfig.EnableEpochs.SCRSizeInvariantCheckEnableEpoch,
+		BackwardCompSaveKeyValueEnableEpoch:   pcf.epochConfig.EnableEpochs.BackwardCompSaveKeyValueEnableEpoch,
 		VMOutputCacher:                        txcache.NewDisabledCache(),
 		ArwenChangeLocker:                     arwenChangeLocker,
 

genesis/process/metaGenesisBlockCreator.go

@@ -370,6 +370,7 @@ func createProcessorsForMetaGenesisBlock(arg ArgsGenesisBlockCreator, enableEpoc
 		SenderInOutTransferEnableEpoch:        enableEpochs.SenderInOutTransferEnableEpoch,
 		BuiltInFunctionOnMetachainEnableEpoch: enableEpochs.BuiltInFunctionOnMetaEnableEpoch,
 		SCRSizeInvariantCheckEnableEpoch:      enableEpochs.SCRSizeInvariantCheckEnableEpoch,
+		BackwardCompSaveKeyValueEnableEpoch:   enableEpochs.BackwardCompSaveKeyValueEnableEpoch,
 		IsGenesisProcessing:                   true,
 		StakingV2EnableEpoch:                  arg.EpochConfig.EnableEpochs.StakingV2EnableEpoch,
 		ArwenChangeLocker:                     &sync.RWMutex{}, // local Locker as to not interfere with the rest of the components

genesis/process/shardGenesisBlockCreator.go

@@ -62,6 +62,7 @@ func createGenesisConfig() config.EnableEpochs {
 		ComputeRewardCheckpointEnableEpoch:          unreachableEpoch,
 		IncrementSCRNonceInMultiTransferEnableEpoch: unreachableEpoch,
 		SCRSizeInvariantCheckEnableEpoch:            unreachableEpoch,
+		BackwardCompSaveKeyValueEnableEpoch:         unreachableEpoch,
 	}
 }
 
@@ -422,6 +423,7 @@ func createProcessorsForShardGenesisBlock(arg ArgsGenesisBlockCreator, enableEpo
 		SenderInOutTransferEnableEpoch:        enableEpochs.SenderInOutTransferEnableEpoch,
 		BuiltInFunctionOnMetachainEnableEpoch: enableEpochs.BuiltInFunctionOnMetaEnableEpoch,
 		SCRSizeInvariantCheckEnableEpoch:      enableEpochs.SCRSizeInvariantCheckEnableEpoch,
+		BackwardCompSaveKeyValueEnableEpoch:   enableEpochs.BackwardCompSaveKeyValueEnableEpoch,
 		IsGenesisProcessing:                   true,
 		StakingV2EnableEpoch:                  arg.EpochConfig.EnableEpochs.StakingV2EnableEpoch,
 		VMOutputCacher:                        txcache.NewDisabledCache(),

integrationTests/testProcessorNode.go

@@ -1451,6 +1451,7 @@ func (tpn *TestProcessorNode) initInnerProcessors(gasMap map[string]map[string]u
 		ArwenChangeLocker:                     tpn.ArwenChangeLocker,
 		BuiltInFunctionOnMetachainEnableEpoch: tpn.EnableEpochs.BuiltInFunctionOnMetaEnableEpoch,
 		SCRSizeInvariantCheckEnableEpoch:      tpn.EnableEpochs.SCRSizeInvariantCheckEnableEpoch,
+		BackwardCompSaveKeyValueEnableEpoch:   tpn.EnableEpochs.BackwardCompSaveKeyValueEnableEpoch,
 	}
 	sc, _ := smartContract.NewSmartContractProcessor(argsNewScProcessor)
 	tpn.ScProcessor = smartContract.NewTestScProcessor(sc)
@@ -1681,6 +1682,7 @@ func (tpn *TestProcessorNode) initMetaInnerProcessors() {
 		ArwenChangeLocker:                     tpn.ArwenChangeLocker,
 		BuiltInFunctionOnMetachainEnableEpoch: tpn.EnableEpochs.BuiltInFunctionOnMetaEnableEpoch,
 		SCRSizeInvariantCheckEnableEpoch:      tpn.EnableEpochs.SCRSizeInvariantCheckEnableEpoch,
+		BackwardCompSaveKeyValueEnableEpoch:   tpn.EnableEpochs.BackwardCompSaveKeyValueEnableEpoch,
 	}
 	scProcessor, _ := smartContract.NewSmartContractProcessor(argsNewScProcessor)
 	tpn.ScProcessor = smartContract.NewTestScProcessor(scProcessor)

integrationTests/vm/testInitializer.go

@@ -80,12 +80,13 @@ const maxTrieLevelInMemory = uint(5)
 
 // ArgEnableEpoch will specify the enable epoch values for certain flags
 type ArgEnableEpoch struct {
-	PenalizedTooMuchGasEnableEpoch uint32
-	BuiltinEnableEpoch             uint32
-	DeployEnableEpoch              uint32
-	MetaProtectionEnableEpoch      uint32
-	RelayedTxEnableEpoch           uint32
-	UnbondTokensV2EnableEpoch      uint32
+	PenalizedTooMuchGasEnableEpoch      uint32
+	BuiltinEnableEpoch                  uint32
+	DeployEnableEpoch                   uint32
+	MetaProtectionEnableEpoch           uint32
+	RelayedTxEnableEpoch                uint32
+	UnbondTokensV2EnableEpoch           uint32
+	BackwardCompSaveKeyValueEnableEpoch uint32
 }
 
 // VMTestAccount -
@@ -786,28 +787,29 @@ func CreateTxProcessorWithOneSCExecutorWithVMs(
 
 	intermediateTxHandler := &mock.IntermediateTransactionHandlerMock{}
 	argsNewSCProcessor := smartContract.ArgsNewSmartContractProcessor{
-		VmContainer:                    vmContainer,
-		ArgsParser:                     smartContract.NewArgumentParser(),
-		Hasher:                         testHasher,
-		Marshalizer:                    testMarshalizer,
-		AccountsDB:                     accnts,
-		BlockChainHook:                 blockChainHook,
-		PubkeyConv:                     pubkeyConv,
-		ShardCoordinator:               shardCoordinator,
-		ScrForwarder:                   intermediateTxHandler,
-		BadTxForwarder:                 intermediateTxHandler,
-		TxFeeHandler:                   feeAccumulator,
-		EconomicsFee:                   economicsData,
-		TxTypeHandler:                  txTypeHandler,
-		GasHandler:                     gasComp,
-		GasSchedule:                    mock.NewGasScheduleNotifierMock(gasSchedule),
-		TxLogsProcessor:                &mock.TxLogsProcessorStub{},
-		EpochNotifier:                  forking.NewGenericEpochNotifier(),
-		PenalizedTooMuchGasEnableEpoch: argEnableEpoch.PenalizedTooMuchGasEnableEpoch,
-		DeployEnableEpoch:              argEnableEpoch.DeployEnableEpoch,
-		BuiltinEnableEpoch:             argEnableEpoch.BuiltinEnableEpoch,
-		ArwenChangeLocker:              arwenChangeLocker,
-		VMOutputCacher:                 txcache.NewDisabledCache(),
+		VmContainer:                         vmContainer,
+		ArgsParser:                          smartContract.NewArgumentParser(),
+		Hasher:                              testHasher,
+		Marshalizer:                         testMarshalizer,
+		AccountsDB:                          accnts,
+		BlockChainHook:                      blockChainHook,
+		PubkeyConv:                          pubkeyConv,
+		ShardCoordinator:                    shardCoordinator,
+		ScrForwarder:                        intermediateTxHandler,
+		BadTxForwarder:                      intermediateTxHandler,
+		TxFeeHandler:                        feeAccumulator,
+		EconomicsFee:                        economicsData,
+		TxTypeHandler:                       txTypeHandler,
+		GasHandler:                          gasComp,
+		GasSchedule:                         mock.NewGasScheduleNotifierMock(gasSchedule),
+		TxLogsProcessor:                     &mock.TxLogsProcessorStub{},
+		EpochNotifier:                       forking.NewGenericEpochNotifier(),
+		PenalizedTooMuchGasEnableEpoch:      argEnableEpoch.PenalizedTooMuchGasEnableEpoch,
+		DeployEnableEpoch:                   argEnableEpoch.DeployEnableEpoch,
+		BuiltinEnableEpoch:                  argEnableEpoch.BuiltinEnableEpoch,
+		ArwenChangeLocker:                   arwenChangeLocker,
+		VMOutputCacher:                      txcache.NewDisabledCache(),
+		BackwardCompSaveKeyValueEnableEpoch: argEnableEpoch.BackwardCompSaveKeyValueEnableEpoch,
 	}
 
 	scProcessor, err := smartContract.NewSmartContractProcessor(argsNewSCProcessor)

integrationTests/vm/txsFee/builtInFunctions_test.go

@@ -5,6 +5,7 @@
 package txsFee
 
 import (
+	"bytes"
 	"encoding/hex"
 	"math/big"
 	"testing"
@@ -15,6 +16,7 @@ import (
 	"github.com/ElrondNetwork/elrond-go/integrationTests/vm/txsFee/utils"
 	"github.com/ElrondNetwork/elrond-go/process"
 	"github.com/ElrondNetwork/elrond-go/sharding"
+	"github.com/ElrondNetwork/elrond-go/state"
 	vmcommon "github.com/ElrondNetwork/elrond-vm-common"
 	"github.com/stretchr/testify/require"
 )
@@ -266,3 +268,32 @@ func TestBuildInFunctionSaveKeyValue_WrongDestination(t *testing.T) {
 	requiredData := hex.EncodeToString([]byte("nil destination SC account"))
 	require.Equal(t, "@"+requiredData, string(intermediateTxs[0].GetData()))
 }
+
+func TestBuildInFunctionSaveKeyValue_NotEnoughGasFor3rdSave(t *testing.T) {
+	shardCoord, _ := sharding.NewMultiShardCoordinator(2, 0)
+
+	testContext, err := vm.CreatePreparedTxProcessorWithVMsWithShardCoordinator(vm.ArgEnableEpoch{BackwardCompSaveKeyValueEnableEpoch: 5}, shardCoord)
+	require.Nil(t, err)
+	defer testContext.Close()
+
+	sndAddr := []byte("12345678901234567890123456789112")
+
+	senderBalance := big.NewInt(100000)
+	_, _ = vm.CreateAccount(testContext.Accounts, sndAddr, 0, senderBalance)
+
+	txData := []byte(core.BuiltInFunctionSaveKeyValue + "@01000000@02000000@03000000@04000000@05000000@06000000")
+	gasLimit := uint64(len(txData) + 20)
+	gasPrice := uint64(10)
+
+	tx := vm.CreateTransaction(0, big.NewInt(0), sndAddr, sndAddr, gasPrice, gasLimit, txData)
+	retCode, err := testContext.TxProcessor.ProcessTransaction(tx)
+	require.Equal(t, vmcommon.UserError, retCode)
+	require.Equal(t, process.ErrFailedTransaction, err)
+
+	intermediateTxs := testContext.GetIntermediateTransactions(t)
+	require.True(t, len(intermediateTxs) > 1)
+
+	account, _ := testContext.Accounts.LoadAccount(sndAddr)
+	userAcc, _ := account.(state.UserAccountHandler)
+	require.True(t, bytes.Equal(make([]byte, 32), userAcc.GetRootHash()))
+}

node/nodeRunner.go

@@ -148,6 +148,7 @@ func printEnableEpochs(configs *config.Configs) {
 	log.Debug(readEpochFor("built in functions on metachain"), "epoch", enableEpochs.BuiltInFunctionOnMetaEnableEpoch)
 	log.Debug(readEpochFor("compute rewards checkpoint on delegation"), "epoch", enableEpochs.ComputeRewardCheckpointEnableEpoch)
 	log.Debug(readEpochFor("SCR size invariant check"), "epoch", enableEpochs.SCRSizeInvariantCheckEnableEpoch)
+	log.Debug(readEpochFor("backward compatibility flag for save key value"), "epoch", enableEpochs.BackwardCompSaveKeyValueEnableEpoch)
 
 	gasSchedule := configs.EpochConfig.GasSchedule
 

process/smartContract/process.go

@@ -68,6 +68,7 @@ type scProcessor struct {
 	incrementSCRNonceInMultiTransferEnableEpoch uint32
 	builtInFunctionOnMetachainEnableEpoch       uint32
 	scrSizeInvariantCheckEnableEpoch            uint32
+	backwardCompSaveKeyValueEnableEpoch         uint32
 	flagStakingV2                               atomic.Flag
 	flagDeploy                                  atomic.Flag
 	flagBuiltin                                 atomic.Flag
@@ -78,6 +79,7 @@ type scProcessor struct {
 	flagIncrementSCRNonceInMultiTransfer        atomic.Flag
 	flagBuiltInFunctionOnMetachain              atomic.Flag
 	flagSCRSizeInvariantCheck                   atomic.Flag
+	flagBackwardCompOnSaveKeyValue              atomic.Flag
 	arwenChangeLocker                           process.Locker
 
 	badTxForwarder process.IntermediateTransactionHandler
@@ -88,6 +90,8 @@ type scProcessor struct {
 	gasHandler     process.GasHandler
 
 	builtInGasCosts     map[string]uint64
+	persistPerByte      uint64
+	storePerByte        uint64
 	mutGasLock          sync.RWMutex
 	txLogsProcessor     process.TransactionLogProcessor
 	vmOutputCacher      storage.Cacher
@@ -122,6 +126,7 @@ type ArgsNewSmartContractProcessor struct {
 	IncrementSCRNonceInMultiTransferEnableEpoch uint32
 	BuiltInFunctionOnMetachainEnableEpoch       uint32
 	SCRSizeInvariantCheckEnableEpoch            uint32
+	BackwardCompSaveKeyValueEnableEpoch         uint32
 	EpochNotifier                               process.EpochNotifier
 	VMOutputCacher                              storage.Cacher
 	ArwenChangeLocker                           process.Locker
@@ -189,6 +194,7 @@ func NewSmartContractProcessor(args ArgsNewSmartContractProcessor) (*scProcessor
 	}
 
 	builtInFuncCost := args.GasSchedule.LatestGasSchedule()[common.BuiltInCost]
+	baseOperationCost := args.GasSchedule.LatestGasSchedule()[common.BaseOperationCost]
 	sc := &scProcessor{
 		vmContainer:                           args.VmContainer,
 		argsParser:                            args.ArgsParser,
@@ -216,9 +222,11 @@ func NewSmartContractProcessor(args ArgsNewSmartContractProcessor) (*scProcessor
 		senderInOutTransferEnableEpoch:        args.SenderInOutTransferEnableEpoch,
 		builtInFunctionOnMetachainEnableEpoch: args.BuiltInFunctionOnMetachainEnableEpoch,
 		scrSizeInvariantCheckEnableEpoch:      args.SCRSizeInvariantCheckEnableEpoch,
+		backwardCompSaveKeyValueEnableEpoch:   args.BackwardCompSaveKeyValueEnableEpoch,
 		arwenChangeLocker:                     args.ArwenChangeLocker,
 		vmOutputCacher:                        args.VMOutputCacher,
-
+		storePerByte:                          baseOperationCost["StorePerByte"],
+		persistPerByte:                        baseOperationCost["PersistPerByte"],
 		incrementSCRNonceInMultiTransferEnableEpoch: args.IncrementSCRNonceInMultiTransferEnableEpoch,
 	}
 
@@ -237,6 +245,7 @@ func NewSmartContractProcessor(args ArgsNewSmartContractProcessor) (*scProcessor
 		"epoch", sc.incrementSCRNonceInMultiTransferEnableEpoch)
 	log.Debug("smartContract/process: enable epoch for built in functions on metachain", "epoch", sc.builtInFunctionOnMetachainEnableEpoch)
 	log.Debug("smartContract/process: enable epoch for scr size invariant check", "epoch", sc.scrSizeInvariantCheckEnableEpoch)
+	log.Debug("smartContract/process: disable epoch for backward compatibility check on save key value error", "epoch", sc.scrSizeInvariantCheckEnableEpoch)
 
 	args.EpochNotifier.RegisterNotifyHandler(sc)
 	args.GasSchedule.RegisterNotifyHandler(sc)
@@ -255,6 +264,8 @@ func (sc *scProcessor) GasScheduleChange(gasSchedule map[string]map[string]uint6
 	}
 
 	sc.builtInGasCosts = builtInFuncCost
+	sc.storePerByte = gasSchedule[common.BaseOperationCost]["StorePerByte"]
+	sc.persistPerByte = gasSchedule[common.BaseOperationCost]["PersistPerByte"]
 }
 
 func (sc *scProcessor) checkTxValidity(tx data.TransactionHandler) error {
@@ -1231,6 +1242,8 @@ func (sc *scProcessor) ProcessIfError(
 		return err
 	}
 
+	sc.setEmptyRoothashOnErrorIfSaveKeyValue(tx, acntSnd)
+
 	scrIfError, consumedFee := sc.createSCRsWhenError(acntSnd, txHash, tx, returnCode, returnMessage, gasLocked)
 	err = sc.addBackTxValues(acntSnd, scrIfError, tx)
 	if err != nil {
@@ -1259,6 +1272,49 @@ func (sc *scProcessor) ProcessIfError(
 	return nil
 }
 
+func (sc *scProcessor) setEmptyRoothashOnErrorIfSaveKeyValue(tx data.TransactionHandler, account state.UserAccountHandler) {
+	if !sc.flagBackwardCompOnSaveKeyValue.IsSet() {
+		return
+	}
+	if sc.shardCoordinator.SelfId() == core.MetachainShardId {
+		return
+	}
+	if check.IfNil(account) {
+		return
+	}
+	if !bytes.Equal(tx.GetSndAddr(), tx.GetRcvAddr()) {
+		return
+	}
+	if account.GetRootHash() != nil {
+		return
+	}
+
+	function, args, err := sc.argsParser.ParseCallData(string(tx.GetData()))
+	if err != nil {
+		return
+	}
+	if function != core.BuiltInFunctionSaveKeyValue {
+		return
+	}
+	if len(args) < 3 {
+		return
+	}
+
+	txGasProvided, err := sc.prepareGasProvided(tx)
+	if err != nil {
+		return
+	}
+
+	lenVal := len(args[1])
+	lenKeyVal := len(args[0]) + len(args[1])
+	gasToUseForOneSave := sc.builtInGasCosts[function] + sc.persistPerByte*uint64(lenKeyVal) + sc.storePerByte*uint64(lenVal)
+	if txGasProvided < gasToUseForOneSave {
+		return
+	}
+
+	account.SetRootHash(make([]byte, 32))
+}
+
 func (sc *scProcessor) processForRelayerWhenError(
 	originalTx data.TransactionHandler,
 	txHash []byte,
@@ -2471,6 +2527,9 @@ func (sc *scProcessor) EpochConfirmed(epoch uint32, _ uint64) {
 
 	sc.flagSCRSizeInvariantCheck.Toggle(epoch >= sc.scrSizeInvariantCheckEnableEpoch)
 	log.Debug("scProcessor: scr size invariant check", "enabled", sc.flagSCRSizeInvariantCheck.IsSet())
+
+	sc.flagBackwardCompOnSaveKeyValue.Toggle(epoch < sc.backwardCompSaveKeyValueEnableEpoch)
+	log.Debug("scProcessor: backward compatibility on save key value", "enabled", sc.flagBackwardCompOnSaveKeyValue.IsSet())
 }
 
 // IsInterfaceNil returns true if there is no value under the interface

vm/systemSmartContracts/esdt.go

@@ -228,6 +228,10 @@ func (e *esdt) checkBasicCreateArguments(args *vmcommon.ContractCallInput) vmcom
 		e.eei.AddReturnMessage(err.Error())
 		return vmcommon.UserError
 	}
+	if len(args.Arguments) < 2 {
+		e.eei.AddReturnMessage("not enough arguments")
+		return vmcommon.UserError
+	}
 	if len(args.Arguments[0]) < minLengthForTokenName ||
 		len(args.Arguments[0]) > int(esdtConfig.MaxTokenNameLength) {
 		e.eei.AddReturnMessage("token name length not in parameters")

vm/systemSmartContracts/esdt_test.go

@@ -175,6 +175,10 @@ func TestEsdt_ExecuteIssueAlways6charactersForRandom(t *testing.T) {
 	assert.Equal(t, vmcommon.Ok, output)
 	lastOutput = eei.output[len(eei.output)-1]
 	assert.Equal(t, len(lastOutput), len(ticker)+1+6)
+
+	vmInput.Arguments = nil
+	output = e.Execute(vmInput)
+	assert.Equal(t, vmcommon.UserError, output)
 }
 
 func TestEsdt_ExecuteIssue(t *testing.T) {