Refactor asyncComplete

scenario/gasSchedules/gasSchedules.go

@@ -1,6 +1,5 @@
 package gasschedules
 
-// TODO: go:embed can be used after we upgrade to go 1.16
 // import _ "embed"
 
 // //go:embed gasScheduleV1.toml

test/contracts/erc20/erc20.c

@@ -44,7 +44,6 @@ void computeAllowanceKey(byte *destination, byte *from, byte* to) {
   // Note: in smart contract addresses, the first 10 bytes are all 0
   // therefore we read from byte 10 onwards to provide more significant bytes
   // and to minimize the chance for collisions
-  // TODO: switching to a hash instead of a concatenation of addresses might make it safer
   for (int i = 0; i < 15; i++) {
     destination[1+i] = from[10+i];
   }

vmhost/contexts/async.go

@@ -183,7 +183,7 @@ func (context *asyncContext) PushState() {
 		callbackData:       context.callbackData,
 		gasAccumulated:     context.gasAccumulated,
 		returnData:         context.returnData,
-		asyncCallGroups:    context.asyncCallGroups, // TODO matei-p use cloneCallGroups()?
+		asyncCallGroups:    context.cloneCallGroups(),
 
 		callType:                     context.callType,
 		callbackAsyncInitiatorCallID: context.callbackAsyncInitiatorCallID,
@@ -864,7 +864,7 @@ func (context *asyncContext) callCallback(callID []byte, vmOutput *vmcommon.VMOu
 	}
 
 	context.host.Metering().DisableRestoreGas()
-	isComplete, callbackVMOutput := loadedContext.ExecuteSyncCallbackAndFinishOutput(asyncCall, vmOutput, nil, gasAccumulated, err)
+	isComplete, callbackVMOutput := loadedContext.ExecuteLocalCallbackAndFinishOutput(asyncCall, vmOutput, nil, gasAccumulated, err)
 	context.host.Metering().EnableRestoreGas()
 	return isComplete, callbackVMOutput, nil
 }

vmhost/contexts/asyncComposability.go

@@ -63,31 +63,34 @@ func (context *asyncContext) complete() error {
 		return nil
 	}
 
+	gasToAccumulate := context.gasAccumulated
+	notifyChildComplete := true
 	currentCallID := context.GetCallID()
-	if context.callType == vm.AsynchronousCall {
+	switch context.callType {
+	case vm.AsynchronousCall:
 		vmOutput := context.childResults
-		isCallbackComplete, _, err := context.callCallback(currentCallID, vmOutput, nil)
+		notifyChildComplete, _, err = context.callCallback(currentCallID, vmOutput, nil)
 		if err != nil {
 			return err
 		}
-		if isCallbackComplete {
-			return context.NotifyChildIsComplete(currentCallID, 0)
-		}
-	} else if context.callType == vm.AsynchronousCallBack {
+		gasToAccumulate = 0
+	case vm.AsynchronousCallBack:
 		err = context.LoadParentContext()
 		if err != nil {
 			return err
 		}
 
-		currentCallID := context.GetCallerCallID()
-		return context.NotifyChildIsComplete(currentCallID, context.gasAccumulated)
-	} else if context.callType == vm.DirectCall {
+		currentCallID = context.GetCallerCallID()
+	case vm.DirectCall:
 		err = context.LoadParentContext()
 		if err != nil {
 			return err
 		}
+		currentCallID = nil
+	}
 
-		return context.NotifyChildIsComplete(nil, context.gasAccumulated)
+	if notifyChildComplete {
+		return context.NotifyChildIsComplete(currentCallID, gasToAccumulate)
 	}
 
 	return nil

vmhost/contexts/asyncLocal.go

@@ -35,7 +35,6 @@ func (context *asyncContext) executeAsyncLocalCalls() error {
 	return nil
 }
 
-// TODO split this method into smaller ones
 func (context *asyncContext) executeAsyncLocalCall(asyncCall *vmhost.AsyncCall) error {
 	destinationCallInput, err := context.createContractCallInput(asyncCall)
 	if err != nil {
@@ -79,56 +78,54 @@ func (context *asyncContext) executeAsyncLocalCall(asyncCall *vmhost.AsyncCall)
 	asyncCall.UpdateStatus(vmOutput.ReturnCode)
 
 	if isComplete {
+		callbackGasRemaining := uint64(0)
 		if asyncCall.HasCallback() {
 			// Restore gas locked while still on the caller instance; otherwise, the
 			// locked gas will appear to have been used twice by the caller instance.
-			isCallbackComplete, callbackVMOutput := context.ExecuteSyncCallbackAndFinishOutput(asyncCall, vmOutput, destinationCallInput, 0, err)
+			isCallbackComplete, callbackVMOutput := context.ExecuteLocalCallbackAndFinishOutput(asyncCall, vmOutput, destinationCallInput, 0, err)
 			if callbackVMOutput == nil {
 				return vmhost.ErrAsyncNoOutputFromCallback
 			}
 
 			context.host.CompleteLogEntriesWithCallType(callbackVMOutput, vmhost.AsyncCallbackString)
 
 			if isCallbackComplete {
-				callbackGasRemaining := callbackVMOutput.GasRemaining
+				callbackGasRemaining = callbackVMOutput.GasRemaining
 				callbackVMOutput.GasRemaining = 0
-				return context.completeChild(asyncCall.CallID, callbackGasRemaining)
 			}
-		} else {
-			return context.completeChild(asyncCall.CallID, 0)
 		}
+
+		return context.completeChild(asyncCall.CallID, callbackGasRemaining)
 	}
 
 	return nil
 }
 
-// ExecuteSyncCallbackAndFinishOutput executes the callback and finishes the output
-// TODO rename to executeLocalCallbackAndFinishOutput
-func (context *asyncContext) ExecuteSyncCallbackAndFinishOutput(
+// ExecuteLocalCallbackAndFinishOutput executes the callback and finishes the output
+func (context *asyncContext) ExecuteLocalCallbackAndFinishOutput(
 	asyncCall *vmhost.AsyncCall,
 	vmOutput *vmcommon.VMOutput,
 	_ *vmcommon.ContractCallInput,
 	gasAccumulated uint64,
 	err error) (bool, *vmcommon.VMOutput) {
-	callbackVMOutput, isComplete, _ := context.executeSyncCallback(asyncCall, vmOutput, gasAccumulated, err)
+	callbackVMOutput, isComplete, _ := context.executeLocalCallback(asyncCall, vmOutput, gasAccumulated, err)
 	context.finishAsyncLocalCallbackExecution()
 	return isComplete, callbackVMOutput
 }
 
-// TODO rename to executeLocalCallback
-func (context *asyncContext) executeSyncCallback(
+func (context *asyncContext) executeLocalCallback(
 	asyncCall *vmhost.AsyncCall,
 	destinationVMOutput *vmcommon.VMOutput,
 	gasAccumulated uint64,
 	destinationErr error,
 ) (*vmcommon.VMOutput, bool, error) {
 	callbackInput, err := context.createCallbackInput(asyncCall, destinationVMOutput, gasAccumulated, destinationErr)
 	if err != nil {
-		logAsync.Trace("executeSyncCallback", "error", err)
+		logAsync.Trace("executeLocalCallback", "error", err)
 		return nil, true, err
 	}
 
-	logAsync.Trace("executeSyncCallback",
+	logAsync.Trace("executeLocalCallback",
 		"caller", callbackInput.CallerAddr,
 		"dest", callbackInput.RecipientAddr,
 		"func", callbackInput.Function,
@@ -183,7 +180,7 @@ func (context *asyncContext) executeSyncHalfOfBuiltinFunction(asyncCall *vmhost.
 	if vmOutput.ReturnCode != vmcommon.Ok {
 		asyncCall.Reject()
 		if asyncCall.HasCallback() {
-			_, _, _ = context.executeSyncCallback(asyncCall, vmOutput, 0, err)
+			_, _, _ = context.executeLocalCallback(asyncCall, vmOutput, 0, err)
 			context.finishAsyncLocalCallbackExecution()
 		}
 	}
@@ -240,7 +237,6 @@ func (context *asyncContext) createContractCallInput(asyncCall *vmhost.AsyncCall
 	return contractCallInput, nil
 }
 
-// TODO function too large; refactor needed
 func (context *asyncContext) createCallbackInput(
 	asyncCall *vmhost.AsyncCall,
 	vmOutput *vmcommon.VMOutput,
@@ -255,32 +251,29 @@ func (context *asyncContext) createCallbackInput(
 	}
 
 	arguments := context.getArgumentsForCallback(vmOutput, destinationErr)
-
 	returnWithError := false
 	if destinationErr != nil || vmOutput.ReturnCode != vmcommon.Ok {
 		returnWithError = true
 	}
 
 	callbackFunction := asyncCall.GetCallbackName()
-
 	dataLength := computeDataLengthFromArguments(callbackFunction, arguments)
 	gasLimit, err := context.computeGasLimitForCallback(asyncCall, vmOutput, dataLength)
 	if err != nil {
 		return nil, err
 	}
 
 	originalCaller := runtime.GetOriginalCallerAddress()
-
 	caller := context.address
-	lastTransferInfo := context.extractLastTransferWithoutData(caller, vmOutput)
+	lastTransferData := context.extractLastTransferWithoutData(caller, vmOutput)
 
 	// Return to the sender SC, calling its specified callback method.
 	contractCallInput := &vmcommon.ContractCallInput{
 		VMInput: vmcommon.VMInput{
 			OriginalCallerAddr:   originalCaller,
 			CallerAddr:           actualCallbackInitiator,
 			Arguments:            arguments,
-			CallValue:            lastTransferInfo.callValue,
+			CallValue:            lastTransferData.callValue,
 			CallType:             vm.AsynchronousCallBack,
 			GasPrice:             runtime.GetVMInput().GasPrice,
 			GasProvided:          gasLimit,
@@ -289,7 +282,7 @@ func (context *asyncContext) createCallbackInput(
 			OriginalTxHash:       runtime.GetOriginalTxHash(),
 			PrevTxHash:           runtime.GetPrevTxHash(),
 			ReturnCallAfterError: returnWithError,
-			ESDTTransfers:        lastTransferInfo.lastESDTTransfers,
+			ESDTTransfers:        lastTransferData.lastESDTTransfers,
 		},
 		RecipientAddr: caller,
 		Function:      callbackFunction,

vmhost/contexts/asyncParams.go

@@ -8,31 +8,23 @@ import (
 	vmcommon "github.com/multiversx/mx-chain-vm-common-go"
 	"github.com/multiversx/mx-chain-vm-common-go/txDataBuilder"
 	"github.com/multiversx/mx-chain-vm-go/crypto"
-	"github.com/multiversx/mx-chain-vm-go/vmhost"
 )
 
-/*
-	Called to process OutputTransfers created by a
-	direct call (on dest) builtin function call by the VM
-*/
+// AddAsyncArgumentsToOutputTransfers
+// Called to process OutputTransfers created by a
+// direct call (on dest) builtin function call by the VM
 func AddAsyncArgumentsToOutputTransfers(
-	output vmhost.OutputContext,
-	address []byte,
 	asyncParams *vmcommon.AsyncArguments,
 	callType vm.CallType,
-	vmOutput *vmcommon.VMOutput) error {
+	vmOutput *vmcommon.VMOutput,
+) error {
 	if asyncParams == nil {
 		return nil
 	}
+
 	for _, outAcc := range vmOutput.OutputAccounts {
-		// if !bytes.Equal(address, outAcc.Address) {
-		// 	continue
-		// }
 
 		for t, outTransfer := range outAcc.OutputTransfers {
-			// if !bytes.Equal(address, outTransfer.SenderAddress) {
-			// 	continue
-			// }
 			if outTransfer.CallType != callType {
 				continue
 			}
@@ -84,122 +76,6 @@ func createDataFromAsyncParams(
 	return callData.ToBytes(), nil
 }
 
-/*
-	Called when a SCR for a callback is created outside the VM
-	(by createAsyncCallBackSCRFromVMOutput())
-	This is the case
-	A)	after an async call executed following a builtin function call,
-	B)	other cases where processing the output trasnfers of a VMOutput did
-		not produce a SCR of type AsynchronousCallBack
-    TODO(check): function not used?
-*/
-func AppendAsyncArgumentsToCallbackCallData(
-	hasher crypto.Hasher,
-	data []byte,
-	asyncArguments *vmcommon.AsyncArguments,
-	parseArgumentsFunc func(data string) ([][]byte, error)) ([]byte, error) {
-
-	return appendAsyncParamsToCallData(
-		CreateCallbackAsyncParams(hasher, asyncArguments),
-		data,
-		false,
-		parseArgumentsFunc)
-}
-
-/*
-	Called when a SCR is created from VMOutput in order to recompose
-	async data and call data into a transfer data ready for the SCR
-	(by preprocessOutTransferToSCR())
-    TODO(check): function not used?
-*/
-func AppendTransferAsyncDataToCallData(
-	callData []byte,
-	asyncData []byte,
-	parseArgumentsFunc func(data string) ([][]byte, error)) ([]byte, error) {
-
-	var asyncParams [][]byte
-	if asyncData != nil {
-		asyncParams, _ = parseArgumentsFunc(string(asyncData))
-		// string start with a @ so first parsed argument will be empty always
-		asyncParams = asyncParams[1:]
-	} else {
-		return callData, nil
-	}
-
-	return appendAsyncParamsToCallData(
-		asyncParams,
-		callData,
-		true,
-		parseArgumentsFunc)
-}
-
-func appendAsyncParamsToCallData(
-	asyncParams [][]byte,
-	data []byte,
-	hasFunction bool,
-	parseArgumentsFunc func(data string) ([][]byte, error)) ([]byte, error) {
-
-	if data == nil {
-		return nil, nil
-	}
-
-	args, err := parseArgumentsFunc(string(data))
-	if err != nil {
-		return nil, err
-	}
-
-	var functionName string
-	if hasFunction {
-		functionName = string(args[0])
-	}
-
-	// check if there is only one argument and that is 0
-	if len(args) != 0 {
-		args = args[1:]
-	}
-
-	callData := txDataBuilder.NewBuilder()
-
-	if functionName != "" {
-		callData.Func(functionName)
-	}
-
-	if len(args) != 0 {
-		for _, arg := range args {
-			callData.Bytes(arg)
-		}
-	} else {
-		if !hasFunction {
-			callData.Bytes([]byte{})
-		}
-	}
-
-	for _, asyncParam := range asyncParams {
-		callData.Bytes(asyncParam)
-	}
-
-	return callData.ToBytes(), nil
-}
-
-/*
-	Used by when a callback SCR is created
-	1)	after a failure of an async call
-		Async data is extracted (by extractAsyncCallParamsFromTxData()) and then
-		reappended to the new SCR's callback data (by reapendAsyncParamsToTxData())
-	2)	from the last transfer (see useLastTransferAsAsyncCallBackWhenNeeded())
-*/
-func CreateCallbackAsyncParams(hasher crypto.Hasher, asyncParams *vmcommon.AsyncArguments) [][]byte {
-	if asyncParams == nil {
-		return nil
-	}
-	newAsyncParams := make([][]byte, 4)
-	newAsyncParams[0] = GenerateNewCallID(hasher, asyncParams.CallID, []byte{0})
-	newAsyncParams[1] = asyncParams.CallID
-	newAsyncParams[2] = asyncParams.CallerCallID
-	newAsyncParams[3] = []byte{0}
-	return newAsyncParams
-}
-
 // GenerateNewCallID will generate a new call ID as byte slice
 func GenerateNewCallID(hasher crypto.Hasher, parentCallID []byte, suffix []byte) []byte {
 	newCallID := append(parentCallID, suffix...)