Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug-fix: Fix /transaction/cost api route #5878

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion facade/interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"math/big"

"github.com/multiversx/mx-chain-core-go/core"
coreData "github.com/multiversx/mx-chain-core-go/data"
"github.com/multiversx/mx-chain-core-go/data/alteredAccount"
"github.com/multiversx/mx-chain-core-go/data/api"
"github.com/multiversx/mx-chain-core-go/data/esdt"
Expand Down Expand Up @@ -106,7 +107,7 @@ type NodeHandler interface {

// TransactionSimulatorProcessor defines the actions which a transaction simulator processor has to implement
type TransactionSimulatorProcessor interface {
ProcessTx(tx *transaction.Transaction) (*txSimData.SimulationResultsWithVMOutput, error)
ProcessTx(tx *transaction.Transaction, currentHeader coreData.HeaderHandler) (*txSimData.SimulationResultsWithVMOutput, error)
IsInterfaceNil() bool
}

Expand Down
5 changes: 5 additions & 0 deletions factory/processing/txSimulatorProcessComponents.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ func (pcf *processComponentsFactory) createAPITransactionEvaluator() (factory.Tr
Accounts: simulationAccountsDB,
ShardCoordinator: pcf.bootstrapComponents.ShardCoordinator(),
EnableEpochsHandler: pcf.coreData.EnableEpochsHandler(),
BlockChain: pcf.data.Blockchain(),
})

return apiTransactionEvaluator, vmContainerFactory, err
Expand Down Expand Up @@ -141,6 +142,8 @@ func (pcf *processComponentsFactory) createArgsTxSimulatorProcessorForMeta(
return args, nil, nil, err
}

args.BlockChainHook = vmContainerFactory.BlockChainHookImpl()

vmContainer, err := vmContainerFactory.Create()
if err != nil {
return args, nil, nil, err
Expand Down Expand Up @@ -301,6 +304,8 @@ func (pcf *processComponentsFactory) createArgsTxSimulatorProcessorShard(
return args, nil, nil, err
}

args.BlockChainHook = vmContainerFactory.BlockChainHookImpl()

err = builtInFuncFactory.SetPayableHandler(vmContainerFactory.BlockChainHookImpl())
if err != nil {
return args, nil, nil, err
Expand Down
2 changes: 2 additions & 0 deletions integrationTests/testProcessorNodeWithTestWebServer.go
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,7 @@ func createFacadeComponents(tpn *TestProcessorNode) nodeFacade.ApiResolver {
Hasher: TestHasher,
VMOutputCacher: &testscommon.CacherMock{},
DataFieldParser: dataFieldParser,
BlockChainHook: tpn.BlockchainHook,
}

txSimulator, err := transactionEvaluator.NewTransactionSimulator(argSimulator)
Expand All @@ -194,6 +195,7 @@ func createFacadeComponents(tpn *TestProcessorNode) nodeFacade.ApiResolver {
Accounts: wrappedAccounts,
ShardCoordinator: tpn.ShardCoordinator,
EnableEpochsHandler: tpn.EnableEpochsHandler,
BlockChain: tpn.BlockChain,
}
apiTransactionEvaluator, err := transactionEvaluator.NewAPITransactionEvaluator(argsTransactionEvaluator)
log.LogIfError(err)
Expand Down
8 changes: 8 additions & 0 deletions integrationTests/vm/testInitializer.go
Original file line number Diff line number Diff line change
Expand Up @@ -818,6 +818,7 @@ func CreateTxProcessorWithOneSCExecutorWithVMs(
epochNotifierInstance process.EpochNotifier,
guardianChecker process.GuardianChecker,
roundNotifierInstance process.RoundNotifier,
chainHandler data.ChainHandler,
) (*ResultsCreateTxProcessor, error) {
if check.IfNil(poolsHolder) {
poolsHolder = dataRetrieverMock.NewPoolsHolderMock()
Expand Down Expand Up @@ -980,6 +981,7 @@ func CreateTxProcessorWithOneSCExecutorWithVMs(
Marshalizer: integrationtests.TestMarshalizer,
Hasher: integrationtests.TestHasher,
DataFieldParser: dataFieldParser,
BlockChainHook: blockChainHook,
}

argsNewSCProcessor.VMOutputCacher = txSimulatorProcessorArgs.VMOutputCacher
Expand All @@ -1006,6 +1008,7 @@ func CreateTxProcessorWithOneSCExecutorWithVMs(
Accounts: simulationAccountsDB,
ShardCoordinator: shardCoordinator,
EnableEpochsHandler: argsNewSCProcessor.EnableEpochsHandler,
BlockChain: chainHandler,
}
apiTransactionEvaluator, err := transactionEvaluator.NewAPITransactionEvaluator(argsTransactionEvaluator)
if err != nil {
Expand Down Expand Up @@ -1128,6 +1131,7 @@ func CreatePreparedTxProcessorAndAccountsWithVMsWithRoundsConfig(
epochNotifierInstance,
guardedAccountHandler,
roundNotifierInstance,
chainHandler,
)
if err != nil {
return nil, err
Expand Down Expand Up @@ -1279,6 +1283,7 @@ func CreatePreparedTxProcessorWithVMConfigWithShardCoordinatorDBAndGasAndRoundCo
epochNotifierInstance,
guardedAccountHandler,
roundNotifierInstance,
chainHandler,
)
if err != nil {
return nil, err
Expand Down Expand Up @@ -1374,6 +1379,7 @@ func CreateTxProcessorArwenVMWithGasScheduleAndRoundConfig(
epochNotifierInstance,
guardedAccountHandler,
roundNotifierInstance,
chainHandler,
)
if err != nil {
return nil, err
Expand Down Expand Up @@ -1455,6 +1461,7 @@ func CreateTxProcessorArwenWithVMConfigAndRoundConfig(
epochNotifierInstance,
guardedAccountHandler,
roundNotifierInstance,
chainHandler,
)
if err != nil {
return nil, err
Expand Down Expand Up @@ -1885,6 +1892,7 @@ func CreatePreparedTxProcessorWithVMsMultiShardRoundVMConfig(
epochNotifierInstance,
guardedAccountHandler,
roundNotifierInstance,
chainHandler,
)
if err != nil {
return nil, err
Expand Down
7 changes: 4 additions & 3 deletions process/mock/transactionSimulatorStub.go
Original file line number Diff line number Diff line change
@@ -1,19 +1,20 @@
package mock

import (
"github.com/multiversx/mx-chain-core-go/data"
"github.com/multiversx/mx-chain-core-go/data/transaction"
txSimData "github.com/multiversx/mx-chain-go/process/transactionEvaluator/data"
)

// TransactionSimulatorStub -
type TransactionSimulatorStub struct {
ProcessTxCalled func(tx *transaction.Transaction) (*txSimData.SimulationResultsWithVMOutput, error)
ProcessTxCalled func(tx *transaction.Transaction, currentHeader data.HeaderHandler) (*txSimData.SimulationResultsWithVMOutput, error)
}

// ProcessTx -
func (tss *TransactionSimulatorStub) ProcessTx(tx *transaction.Transaction) (*txSimData.SimulationResultsWithVMOutput, error) {
func (tss *TransactionSimulatorStub) ProcessTx(tx *transaction.Transaction, currentHeader data.HeaderHandler) (*txSimData.SimulationResultsWithVMOutput, error) {
if tss.ProcessTxCalled != nil {
return tss.ProcessTxCalled(tx)
return tss.ProcessTxCalled(tx, currentHeader)
}

return nil, nil
Expand Down
14 changes: 12 additions & 2 deletions process/transactionEvaluator/transactionEvaluator.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
"sync"

"github.com/multiversx/mx-chain-core-go/core/check"
"github.com/multiversx/mx-chain-core-go/data"
"github.com/multiversx/mx-chain-core-go/data/transaction"
"github.com/multiversx/mx-chain-go/common"
"github.com/multiversx/mx-chain-go/facade"
Expand All @@ -31,6 +32,7 @@
Accounts state.AccountsAdapterWithClean
ShardCoordinator sharding.Coordinator
EnableEpochsHandler common.EnableEpochsHandler
BlockChain data.ChainHandler
}

type apiTransactionEvaluator struct {
Expand All @@ -40,6 +42,7 @@
feeHandler process.FeeHandler
txSimulator facade.TransactionSimulatorProcessor
enableEpochsHandler common.EnableEpochsHandler
blockChain data.ChainHandler
mutExecution sync.RWMutex
}

Expand All @@ -63,6 +66,9 @@
if check.IfNil(args.EnableEpochsHandler) {
return nil, process.ErrNilEnableEpochsHandler
}
if check.IfNil(args.BlockChain) {
return nil, process.ErrNilBlockChain

Check warning on line 70 in process/transactionEvaluator/transactionEvaluator.go

View check run for this annotation

Codecov / codecov/patch

process/transactionEvaluator/transactionEvaluator.go#L70

Added line #L70 was not covered by tests
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add unit test?

}

tce := &apiTransactionEvaluator{
txTypeHandler: args.TxTypeHandler,
Expand All @@ -71,6 +77,7 @@
accounts: args.Accounts,
shardCoordinator: args.ShardCoordinator,
enableEpochsHandler: args.EnableEpochsHandler,
blockChain: args.BlockChain,
}

return tce, nil
Expand All @@ -84,7 +91,9 @@
ate.mutExecution.Unlock()
}()

return ate.txSimulator.ProcessTx(tx)
currentHeader := ate.blockChain.GetCurrentBlockHeader()

Check warning on line 94 in process/transactionEvaluator/transactionEvaluator.go

View check run for this annotation

Codecov / codecov/patch

process/transactionEvaluator/transactionEvaluator.go#L94

Added line #L94 was not covered by tests
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unit tests?


return ate.txSimulator.ProcessTx(tx, currentHeader)

Check warning on line 96 in process/transactionEvaluator/transactionEvaluator.go

View check run for this annotation

Codecov / codecov/patch

process/transactionEvaluator/transactionEvaluator.go#L96

Added line #L96 was not covered by tests
}

// ComputeTransactionGasLimit will calculate how many gas units a transaction will consume
Expand Down Expand Up @@ -133,8 +142,9 @@
}

costResponse := &transaction.CostResponse{}
currentHeader := ate.blockChain.GetCurrentBlockHeader()

res, err := ate.txSimulator.ProcessTx(tx)
res, err := ate.txSimulator.ProcessTx(tx, currentHeader)
if err != nil {
costResponse.ReturnMessage = err.Error()
return costResponse, nil
Expand Down
16 changes: 9 additions & 7 deletions process/transactionEvaluator/transactionEvaluator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ func createArgs() ArgsApiTransactionEvaluator {
Accounts: &stateMock.AccountsStub{},
ShardCoordinator: &mock.ShardCoordinatorStub{},
EnableEpochsHandler: &enableEpochsHandlerMock.EnableEpochsHandlerStub{},
BlockChain: &testscommon.ChainHandlerMock{},
}
}

Expand Down Expand Up @@ -103,7 +104,7 @@ func TestComputeTransactionGasLimit_MoveBalance(t *testing.T) {
},
}
args.TxSimulator = &mock.TransactionSimulatorStub{
ProcessTxCalled: func(tx *transaction.Transaction) (*txSimData.SimulationResultsWithVMOutput, error) {
ProcessTxCalled: func(tx *transaction.Transaction, currentHeader data.HeaderHandler) (*txSimData.SimulationResultsWithVMOutput, error) {
return &txSimData.SimulationResultsWithVMOutput{}, nil
},
}
Expand Down Expand Up @@ -142,7 +143,7 @@ func TestComputeTransactionGasLimit_MoveBalanceInvalidNonceShouldStillComputeCos
},
}
args.TxSimulator = &mock.TransactionSimulatorStub{
ProcessTxCalled: func(tx *transaction.Transaction) (*txSimData.SimulationResultsWithVMOutput, error) {
ProcessTxCalled: func(tx *transaction.Transaction, currentHeader data.HeaderHandler) (*txSimData.SimulationResultsWithVMOutput, error) {
return nil, simulationErr
},
}
Expand Down Expand Up @@ -173,7 +174,7 @@ func TestComputeTransactionGasLimit_BuiltInFunction(t *testing.T) {
},
}
args.TxSimulator = &mock.TransactionSimulatorStub{
ProcessTxCalled: func(tx *transaction.Transaction) (*txSimData.SimulationResultsWithVMOutput, error) {
ProcessTxCalled: func(tx *transaction.Transaction, currentHeader data.HeaderHandler) (*txSimData.SimulationResultsWithVMOutput, error) {
return &txSimData.SimulationResultsWithVMOutput{
VMOutput: &vmcommon.VMOutput{
ReturnCode: vmcommon.Ok,
Expand Down Expand Up @@ -209,7 +210,7 @@ func TestComputeTransactionGasLimit_BuiltInFunctionShouldErr(t *testing.T) {
},
}
args.TxSimulator = &mock.TransactionSimulatorStub{
ProcessTxCalled: func(tx *transaction.Transaction) (*txSimData.SimulationResultsWithVMOutput, error) {
ProcessTxCalled: func(tx *transaction.Transaction, currentHeader data.HeaderHandler) (*txSimData.SimulationResultsWithVMOutput, error) {
return nil, localErr
},
}
Expand Down Expand Up @@ -239,7 +240,7 @@ func TestComputeTransactionGasLimit_NilVMOutput(t *testing.T) {
},
}
args.TxSimulator = &mock.TransactionSimulatorStub{
ProcessTxCalled: func(tx *transaction.Transaction) (*txSimData.SimulationResultsWithVMOutput, error) {
ProcessTxCalled: func(tx *transaction.Transaction, currentHeader data.HeaderHandler) (*txSimData.SimulationResultsWithVMOutput, error) {
return &txSimData.SimulationResultsWithVMOutput{}, nil
},
}
Expand All @@ -248,7 +249,8 @@ func TestComputeTransactionGasLimit_NilVMOutput(t *testing.T) {
return &stateMock.UserAccountStub{Balance: big.NewInt(100000)}, nil
},
}
tce, _ := NewAPITransactionEvaluator(args)
tce, err := NewAPITransactionEvaluator(args)
require.Nil(t, err)

tx := &transaction.Transaction{}
cost, err := tce.ComputeTransactionGasLimit(tx)
Expand All @@ -269,7 +271,7 @@ func TestComputeTransactionGasLimit_RetCodeNotOk(t *testing.T) {
},
}
args.TxSimulator = &mock.TransactionSimulatorStub{
ProcessTxCalled: func(tx *transaction.Transaction) (*txSimData.SimulationResultsWithVMOutput, error) {
ProcessTxCalled: func(tx *transaction.Transaction, _ data.HeaderHandler) (*txSimData.SimulationResultsWithVMOutput, error) {
return &txSimData.SimulationResultsWithVMOutput{
VMOutput: &vmcommon.VMOutput{
ReturnCode: vmcommon.UserError,
Expand Down
11 changes: 10 additions & 1 deletion process/transactionEvaluator/transactionSimulator.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

"github.com/multiversx/mx-chain-core-go/core"
"github.com/multiversx/mx-chain-core-go/core/check"
"github.com/multiversx/mx-chain-core-go/data"
"github.com/multiversx/mx-chain-core-go/data/block"
"github.com/multiversx/mx-chain-core-go/data/receipt"
"github.com/multiversx/mx-chain-core-go/data/smartContractResult"
Expand Down Expand Up @@ -33,6 +34,7 @@
Hasher hashing.Hasher
Marshalizer marshal.Marshalizer
DataFieldParser DataFieldParser
BlockChainHook process.BlockChainHookHandler
}

type refundHandler interface {
Expand All @@ -50,6 +52,7 @@
marshalizer marshal.Marshalizer
refundDetector refundHandler
dataFieldParser DataFieldParser
blockChainHook process.BlockChainHookHandler
}

// NewTransactionSimulator returns a new instance of a transactionSimulator
Expand Down Expand Up @@ -78,6 +81,9 @@
if check.IfNilReflect(args.DataFieldParser) {
return nil, ErrNilDataFieldParser
}
if check.IfNil(args.BlockChainHook) {
return nil, process.ErrNilBlockChainHook

Check warning on line 85 in process/transactionEvaluator/transactionSimulator.go

View check run for this annotation

Codecov / codecov/patch

process/transactionEvaluator/transactionSimulator.go#L85

Added line #L85 was not covered by tests
}

return &transactionSimulator{
txProcessor: args.TransactionProcessor,
Expand All @@ -89,17 +95,20 @@
hasher: args.Hasher,
refundDetector: transactionAPI.NewRefundDetector(),
dataFieldParser: args.DataFieldParser,
blockChainHook: args.BlockChainHook,
}, nil
}

// ProcessTx will process the transaction in a special environment, where state-writing is not allowed
func (ts *transactionSimulator) ProcessTx(tx *transaction.Transaction) (*txSimData.SimulationResultsWithVMOutput, error) {
func (ts *transactionSimulator) ProcessTx(tx *transaction.Transaction, currentHeader data.HeaderHandler) (*txSimData.SimulationResultsWithVMOutput, error) {
ts.mutOperation.Lock()
defer ts.mutOperation.Unlock()

txStatus := transaction.TxStatusPending
failReason := ""

ts.blockChainHook.SetCurrentHeader(currentHeader)

retCode, err := ts.txProcessor.ProcessTransaction(tx)
if err != nil {
failReason = err.Error()
Expand Down
7 changes: 4 additions & 3 deletions process/transactionEvaluator/transactionSimulator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ func TestTransactionSimulator_ProcessTxProcessingErrShouldSignal(t *testing.T) {
}
ts, _ := NewTransactionSimulator(args)

results, err := ts.ProcessTx(&transaction.Transaction{Nonce: 37})
results, err := ts.ProcessTx(&transaction.Transaction{Nonce: 37}, &block.Header{})
require.NoError(t, err)
require.Equal(t, expErr.Error(), results.FailReason)
}
Expand Down Expand Up @@ -207,7 +207,7 @@ func TestTransactionSimulator_ProcessTxShouldIncludeScrsAndReceipts(t *testing.T
txHash, _ := core.CalculateHash(args.Marshalizer, args.Hasher, tx)
args.VMOutputCacher.Put(txHash, &vmcommon.VMOutput{}, 0)

results, err := ts.ProcessTx(tx)
results, err := ts.ProcessTx(tx, &block.Header{})
require.NoError(t, err)
require.Equal(
t,
Expand Down Expand Up @@ -236,6 +236,7 @@ func getTxSimulatorArgs() ArgsTxSimulator {
Marshalizer: &mock.MarshalizerMock{},
Hasher: &hashingMocks.HasherMock{},
DataFieldParser: dataFieldParser,
BlockChainHook: &testscommon.BlockChainHookStub{},
}
}

Expand All @@ -261,7 +262,7 @@ func TestTransactionSimulator_ProcessTxConcurrentCalls(t *testing.T) {
for i := 0; i < numCalls; i++ {
go func(idx int) {
time.Sleep(time.Millisecond * 10)
_, _ = txSimulator.ProcessTx(tx)
_, _ = txSimulator.ProcessTx(tx, &block.Header{})
wg.Done()
}(i)
}
Expand Down
Loading