diff --git a/test/features/composability/scenarios/forw_raw_transf_exec_fallible_egld_reject.scen.json b/test/features/composability/scenarios/forw_raw_transf_exec_fallible_egld_reject.scen.json index 040244a5c..4cf571fce 100644 --- a/test/features/composability/scenarios/forw_raw_transf_exec_fallible_egld_reject.scen.json +++ b/test/features/composability/scenarios/forw_raw_transf_exec_fallible_egld_reject.scen.json @@ -43,26 +43,6 @@ "status": "0", "message": "", "logs": [ - { - "address": "sc:forwarder", - "endpoint": "str:MultiESDTNFTTransfer", - "topics": [ - "str:EGLD-000000", - "", - "100", - "sc:vault" - ], - "data": [ - "str:TransferAndExecute", - "str:MultiESDTNFTTransfer", - "sc:vault", - "1", - "str:EGLD-000000", - "", - "100", - "str:reject_funds" - ] - }, { "address": "address:a_user", "endpoint": "str:internalVMErrors", diff --git a/test/features/composability/scenarios/forw_raw_transf_exec_fallible_multi_egld_reject.scen.json b/test/features/composability/scenarios/forw_raw_transf_exec_fallible_multi_egld_reject.scen.json index 02b47c096..a3ecb05c6 100644 --- a/test/features/composability/scenarios/forw_raw_transf_exec_fallible_multi_egld_reject.scen.json +++ b/test/features/composability/scenarios/forw_raw_transf_exec_fallible_multi_egld_reject.scen.json @@ -71,44 +71,6 @@ "status": "0", "message": "", "logs": [ - { - "address": "sc:forwarder", - "endpoint": "str:MultiESDTNFTTransfer", - "topics": [ - "str:FWD-TOKEN", - "0", - "800", - "str:NFT-123456", - "1", - "1", - "str:EGLD-000000", - "0", - "100", - "str:SFT-456789", - "3", - "6", - "sc:vault" - ], - "data": [ - "str:TransferAndExecute", - "str:MultiESDTNFTTransfer", - "sc:vault", - "4", - "str:FWD-TOKEN", - "0", - "800", - "str:NFT-123456", - "1", - "1", - "str:EGLD-000000", - "0", - "100", - "str:SFT-456789", - "3", - "6", - "str:reject_funds" - ] - }, { "address": "address:a_user", "endpoint": "str:internalVMErrors", diff --git a/test/features/composability/scenarios/forw_raw_transf_exec_fallible_multi_esdt_reject.scen.json b/test/features/composability/scenarios/forw_raw_transf_exec_fallible_multi_esdt_reject.scen.json index ca580acdf..b0b135b71 100644 --- a/test/features/composability/scenarios/forw_raw_transf_exec_fallible_multi_esdt_reject.scen.json +++ b/test/features/composability/scenarios/forw_raw_transf_exec_fallible_multi_esdt_reject.scen.json @@ -49,32 +49,6 @@ "status": "0", "message": "", "logs": [ - { - "address": "sc:forwarder", - "endpoint": "str:MultiESDTNFTTransfer", - "topics": [ - "str:FWD-TOKEN", - "0", - "100", - "str:FWD-TOKEN", - "0", - "200", - "sc:vault" - ], - "data": [ - "str:TransferAndExecute", - "str:MultiESDTNFTTransfer", - "sc:vault", - "2", - "str:FWD-TOKEN", - "0", - "100", - "str:FWD-TOKEN", - "0", - "200", - "str:reject_funds" - ] - }, { "address": "address:a_user", "endpoint": "str:internalVMErrors", diff --git a/test/features/composability/scenarios/forwarder_call_async_reject_esdt.scen.json b/test/features/composability/scenarios/forwarder_call_async_reject_esdt.scen.json index 05f5463ac..ded636dc3 100644 --- a/test/features/composability/scenarios/forwarder_call_async_reject_esdt.scen.json +++ b/test/features/composability/scenarios/forwarder_call_async_reject_esdt.scen.json @@ -47,26 +47,6 @@ "gas": "*", "refund": "*", "logs": [ - { - "address": "sc:forwarder", - "endpoint": "str:MultiESDTNFTTransfer", - "topics": [ - "str:FWD-TOKEN", - "0", - "1000", - "sc:vault" - ], - "data": [ - "str:AsyncCall", - "str:MultiESDTNFTTransfer", - "sc:vault", - "1", - "str:FWD-TOKEN", - "0", - "1000", - "str:reject_funds" - ] - }, { "address": "sc:vault", "endpoint": "str:transferValueOnly", diff --git a/test/features/composability/scenarios/forwarder_call_sync_fallible_multi_transfer_egld_reject.scen.json b/test/features/composability/scenarios/forwarder_call_sync_fallible_multi_transfer_egld_reject.scen.json index a8a34fda5..07085f931 100644 --- a/test/features/composability/scenarios/forwarder_call_sync_fallible_multi_transfer_egld_reject.scen.json +++ b/test/features/composability/scenarios/forwarder_call_sync_fallible_multi_transfer_egld_reject.scen.json @@ -71,44 +71,6 @@ "status": "0", "message": "", "logs": [ - { - "address": "sc:forwarder", - "endpoint": "str:MultiESDTNFTTransfer", - "topics": [ - "str:FWD-TOKEN", - "0", - "100", - "str:NFT-123456", - "1", - "1", - "str:EGLD-000000", - "0", - "100", - "str:SFT-456789", - "3", - "6", - "sc:vault" - ], - "data": [ - "str:ExecuteOnDestContext", - "str:MultiESDTNFTTransfer", - "sc:vault", - "4", - "str:FWD-TOKEN", - "0", - "100", - "str:NFT-123456", - "1", - "1", - "str:EGLD-000000", - "0", - "100", - "str:SFT-456789", - "3", - "6", - "str:reject_funds" - ] - }, { "address": "sc:forwarder", "endpoint": "str:forward_sync_reject_funds_multi_transfer", diff --git a/test/features/composability/scenarios/promises_multi_transfer_err.scen.json b/test/features/composability/scenarios/promises_multi_transfer_err.scen.json index 11f7de6c4..cabdf04cc 100644 --- a/test/features/composability/scenarios/promises_multi_transfer_err.scen.json +++ b/test/features/composability/scenarios/promises_multi_transfer_err.scen.json @@ -67,38 +67,6 @@ "status": "0", "message": "", "logs": [ - { - "address": "sc:promises", - "endpoint": "str:MultiESDTNFTTransfer", - "topics": [ - "str:FWD-TOKEN", - "0", - "500", - "str:NFT-123456", - "1", - "1", - "str:SFT-456789", - "3", - "6", - "sc:vault" - ], - "data": [ - "str:AsyncCall", - "str:MultiESDTNFTTransfer", - "sc:vault", - "3", - "str:FWD-TOKEN", - "0", - "500", - "str:NFT-123456", - "1", - "1", - "str:SFT-456789", - "3", - "6", - "str:explicit_panic" - ] - }, { "address": "sc:vault", "endpoint": "str:transferValueOnly", diff --git a/vmhost/contexts/blockchain.go b/vmhost/contexts/blockchain.go index 680e2a791..3d8ec8462 100644 --- a/vmhost/contexts/blockchain.go +++ b/vmhost/contexts/blockchain.go @@ -375,8 +375,8 @@ func (context *blockchainContext) GetSnapshot() int { } // RevertToSnapshot - reverts to the specified snapshot via blockchain hook -func (context *blockchainContext) RevertToSnapshot(snapshot int) { - _ = context.blockChainHook.RevertToSnapshot(snapshot) +func (context *blockchainContext) RevertToSnapshot(snapshot int) error { + return context.blockChainHook.RevertToSnapshot(snapshot) } // IsLimitedTransfer returns true if token transfers are limited diff --git a/vmhost/contexts/output.go b/vmhost/contexts/output.go index bb12736e4..6fbc30d32 100644 --- a/vmhost/contexts/output.go +++ b/vmhost/contexts/output.go @@ -498,7 +498,6 @@ func (context *outputContext) TransferESDT( destAcc, _ := context.GetOutputAccount(transfersArgs.Destination) outputAcc, ok := vmOutput.OutputAccounts[string(transfersArgs.Destination)] - if ok && len(outputAcc.OutputTransfers) == 1 { esdtOutTransfer := outputAcc.OutputTransfers[0] esdtOutTransfer.GasLimit = gasRemaining @@ -507,7 +506,6 @@ func (context *outputContext) TransferESDT( if sameShard { esdtOutTransfer.GasLimit = 0 } - AppendOutputTransfers(destAcc, destAcc.OutputTransfers, esdtOutTransfer) } diff --git a/vmhost/hostCore/execution.go b/vmhost/hostCore/execution.go index c67b31f84..02879914d 100644 --- a/vmhost/hostCore/execution.go +++ b/vmhost/hostCore/execution.go @@ -290,13 +290,16 @@ func (host *vmHost) ExecuteOnDestContext(input *vmcommon.ContractCallInput) (vmO scExecutionInput := input blockchain := host.Blockchain() + output := host.Output() blockchain.PushState() + output.PushState() if host.IsBuiltinFunctionName(input.Function) { scExecutionInput, vmOutput, err = host.handleBuiltinFunctionCall(input) if err != nil { blockchain.PopSetActiveState() + output.PopSetActiveState() host.Runtime().AddError(err, input.Function) vmOutput = host.Output().CreateVMOutputInCaseOfError(err) isChildComplete = true @@ -313,8 +316,10 @@ func (host *vmHost) ExecuteOnDestContext(input *vmcommon.ContractCallInput) (vmO if err != nil { blockchain.PopSetActiveState() + output.PopSetActiveState() } else { blockchain.PopDiscard() + output.PopDiscard() } return @@ -598,7 +603,7 @@ func (host *vmHost) ExecuteOnSameContext(input *vmcommon.ContractCallInput) erro var err error - defer host.finishExecuteOnSameContext(err) + defer func() { host.finishExecuteOnSameContext(err) }() // Perform a value transfer to the called SC. If the execution fails, this // transfer will not persist. @@ -1267,8 +1272,6 @@ func (host *vmHost) callFunctionAndExecuteAsync() (bool, error) { runtime := host.Runtime() async := host.Async() - // TODO refactor this, and apply this condition in other places where a - // function is called if runtime.FunctionName() != "" { err := host.verifyAllowedFunctionCall() if err != nil { @@ -1306,11 +1309,11 @@ func (host *vmHost) callFunctionAndExecuteAsync() (bool, error) { err = async.Save() return false, err } - } else { - return false, executor.ErrInvalidFunction + + return true, nil } - return true, nil + return false, executor.ErrInvalidFunction } func (host *vmHost) verifyAllowedFunctionCall() error { diff --git a/vmhost/hosttest/execution_gas_test.go b/vmhost/hosttest/execution_gas_test.go index 4f0a55cc0..f19c6095d 100644 --- a/vmhost/hosttest/execution_gas_test.go +++ b/vmhost/hosttest/execution_gas_test.go @@ -1653,8 +1653,7 @@ func testGasUsedESDTTransferThenExecuteAsyncCallChildFails(t *testing.T, isLegac verify.Ok(). GasRemaining(expectedGasRemaining). GasUsed(test.ParentAddress, expectedGasUsedByParent). - GasUsed(test.ChildAddress, 0). - Transfers(expectedTransfers...) + GasUsed(test.ChildAddress, 0) parentESDTBalance, _ := parentAccount.GetTokenBalanceUint64(test.ESDTTestTokenName, 0) require.Equal(t, initialESDTTokenBalance, parentESDTBalance) diff --git a/vmhost/interface.go b/vmhost/interface.go index 9c1a4694f..3c20e41df 100644 --- a/vmhost/interface.go +++ b/vmhost/interface.go @@ -108,7 +108,7 @@ type BlockchainContext interface { GetUserAccount(address []byte) (vmcommon.UserAccountHandler, error) ProcessBuiltInFunction(input *vmcommon.ContractCallInput) (*vmcommon.VMOutput, error) GetSnapshot() int - RevertToSnapshot(snapshot int) + RevertToSnapshot(snapshot int) error ClearCompiledCodes() ExecuteSmartContractCallOnOtherVM(input *vmcommon.ContractCallInput) (*vmcommon.VMOutput, error) } diff --git a/vmhost/mock/blockchainContextMock.go b/vmhost/mock/blockchainContextMock.go index 7310c83e4..e827320eb 100644 --- a/vmhost/mock/blockchainContextMock.go +++ b/vmhost/mock/blockchainContextMock.go @@ -219,7 +219,8 @@ func (b *BlockchainContextMock) GetSnapshot() int { } // RevertToSnapshot - -func (b *BlockchainContextMock) RevertToSnapshot(_ int) { +func (b *BlockchainContextMock) RevertToSnapshot(_ int) error { + return nil } // IsLimitedTransfer - @@ -237,6 +238,6 @@ func (b *BlockchainContextMock) ClearCompiledCodes() { } // ExecuteSmartContractCallOnOtherVM - -func (b *BlockchainContextMock) ExecuteSmartContractCallOnOtherVM(input *vmcommon.ContractCallInput) (*vmcommon.VMOutput, error) { +func (b *BlockchainContextMock) ExecuteSmartContractCallOnOtherVM(_ *vmcommon.ContractCallInput) (*vmcommon.VMOutput, error) { return nil, nil } diff --git a/vmhost/vmhooks/baseOps.go b/vmhost/vmhooks/baseOps.go index f15044721..b17f520d9 100644 --- a/vmhost/vmhooks/baseOps.go +++ b/vmhost/vmhooks/baseOps.go @@ -1171,6 +1171,7 @@ func TransferESDTNFTExecuteWithTypedArgsWithFailure( } snapshotBeforeTransfer := host.Blockchain().GetSnapshot() + output.PushState() originalCaller := host.Runtime().GetOriginalCallerAddress() transfersArgs := &vmhost.ESDTTransfersArgs{ @@ -1182,6 +1183,11 @@ func TransferESDTNFTExecuteWithTypedArgsWithFailure( } gasLimitForExec, executeErr := output.TransferESDT(transfersArgs, contractCallInput) if executeErr != nil { + output.PopSetActiveState() + errRevert := host.Blockchain().RevertToSnapshot(snapshotBeforeTransfer) + if errRevert != nil { + FailExecution(host, errRevert) + } if withFailure { FailExecution(host, executeErr) } @@ -1195,17 +1201,22 @@ func TransferESDTNFTExecuteWithTypedArgsWithFailure( vmOutput, executeErr := executeOnDestContextFromAPI(host, contractCallInput) if executeErr != nil { logEEI.Trace("ESDT post-transfer execution failed", "error", executeErr) - host.Blockchain().RevertToSnapshot(snapshotBeforeTransfer) + output.PopSetActiveState() + errRevert := host.Blockchain().RevertToSnapshot(snapshotBeforeTransfer) + if errRevert != nil { + FailExecution(host, errRevert) + } if vmOutput == nil || withFailure { FailExecution(host, executeErr) } - return 1 } + output.PopDiscard() return 0 } + output.PopDiscard() return 0 } @@ -1255,6 +1266,8 @@ func TransferESDTNFTExecuteByUserWithTypedArgs( contractCallInput.ESDTTransfers = transfers } + snapshotBeforeTransfer := host.Blockchain().GetSnapshot() + output.PushState() originalCaller := host.Runtime().GetOriginalCallerAddress() transfersArgs := &vmhost.ESDTTransfersArgs{ @@ -1266,6 +1279,11 @@ func TransferESDTNFTExecuteByUserWithTypedArgs( } gasLimitForExec, executeErr := output.TransferESDT(transfersArgs, contractCallInput) if executeErr != nil { + output.PopSetActiveState() + errRevert := host.Blockchain().RevertToSnapshot(snapshotBeforeTransfer) + if errRevert != nil { + FailExecution(host, errRevert) + } // no fail execution is needed here - transfer was not successful, returning error which can be treated at SC level return 1 } @@ -1277,7 +1295,6 @@ func TransferESDTNFTExecuteByUserWithTypedArgs( _, executeErr = executeOnDestContextFromAPI(host, contractCallInput) if executeErr != nil { logEEI.Trace("ESDT post-transfer execution failed, started transfer to user", "error", executeErr) - // in case of failed execution, the funds have to be moved to the user returnTransferArgs := &vmhost.ESDTTransfersArgs{ Destination: callerForExecution, @@ -1289,17 +1306,25 @@ func TransferESDTNFTExecuteByUserWithTypedArgs( } _, executeErr = output.TransferESDT(returnTransferArgs, nil) if executeErr != nil { + output.PopSetActiveState() + errRevert := host.Blockchain().RevertToSnapshot(snapshotBeforeTransfer) + if errRevert != nil { + FailExecution(host, errRevert) + } // fail execution is needed here - tokens are at destination contract, so fail is needed to revert everything FailExecution(host, executeErr) return 1 } + output.PopDiscard() return 1 } + output.PopDiscard() return 0 } + output.PopDiscard() return 0 }