From 04fc737e2e50aaf0ceeaf134f805affd843baf96 Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Fri, 8 Jul 2022 14:32:29 +0300 Subject: [PATCH 01/11] rpc: drop NewTransactionOutputRaw, move it server-side --- pkg/rpc/response/result/tx_raw_output.go | 21 --------------------- pkg/rpc/server/server.go | 15 ++++++++++++--- 2 files changed, 12 insertions(+), 24 deletions(-) diff --git a/pkg/rpc/response/result/tx_raw_output.go b/pkg/rpc/response/result/tx_raw_output.go index 326923c8a..59405c5a7 100644 --- a/pkg/rpc/response/result/tx_raw_output.go +++ b/pkg/rpc/response/result/tx_raw_output.go @@ -4,8 +4,6 @@ import ( "encoding/json" "errors" - "github.com/nspcc-dev/neo-go/pkg/core/block" - "github.com/nspcc-dev/neo-go/pkg/core/state" "github.com/nspcc-dev/neo-go/pkg/core/transaction" "github.com/nspcc-dev/neo-go/pkg/util" ) @@ -25,25 +23,6 @@ type TransactionMetadata struct { VMState string `json:"vmstate,omitempty"` } -// NewTransactionOutputRaw returns a new ransactionOutputRaw object. -func NewTransactionOutputRaw(tx *transaction.Transaction, header *block.Header, appExecResult *state.AppExecResult, chain LedgerAux) TransactionOutputRaw { - result := TransactionOutputRaw{ - Transaction: *tx, - } - if header == nil { - return result - } - // confirmations formula - confirmations := int(chain.BlockHeight() - header.Index + 1) - result.TransactionMetadata = TransactionMetadata{ - Blockhash: header.Hash(), - Confirmations: confirmations, - Timestamp: header.Timestamp, - VMState: appExecResult.VMState.String(), - } - return result -} - // MarshalJSON implements the json.Marshaler interface. func (t TransactionOutputRaw) MarshalJSON() ([]byte, error) { output, err := json.Marshal(t.TransactionMetadata) diff --git a/pkg/rpc/server/server.go b/pkg/rpc/server/server.go index cd8460e48..0c2d7e9be 100644 --- a/pkg/rpc/server/server.go +++ b/pkg/rpc/server/server.go @@ -1542,8 +1542,11 @@ func (s *Server) getrawtransaction(reqParams params.Params) (interface{}, *respo return nil, response.ErrUnknownTransaction } if v, _ := reqParams.Value(1).GetBoolean(); v { - if height == math.MaxUint32 { - return result.NewTransactionOutputRaw(tx, nil, nil, s.chain), nil + res := result.TransactionOutputRaw{ + Transaction: *tx, + } + if height == math.MaxUint32 { // Mempooled transaction. + return res, nil } _header := s.chain.GetHeaderHash(int(height)) header, err := s.chain.GetHeader(_header) @@ -1557,7 +1560,13 @@ func (s *Server) getrawtransaction(reqParams params.Params) (interface{}, *respo if len(aers) == 0 { return nil, response.NewRPCError("Inconsistent application log", "application log for the transaction is empty") } - return result.NewTransactionOutputRaw(tx, header, &aers[0], s.chain), nil + res.TransactionMetadata = result.TransactionMetadata{ + Blockhash: header.Hash(), + Confirmations: int(s.chain.BlockHeight() - header.Index + 1), + Timestamp: header.Timestamp, + VMState: aers[0].VMState.String(), + } + return res, nil } return tx.Bytes(), nil } From 4333ad49490ce3cb1f5adcf0b5738911e141558c Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Fri, 8 Jul 2022 14:50:00 +0300 Subject: [PATCH 02/11] result: drop NewBlock/NewHeader/LedgerAux Client's don't care about any of these. --- pkg/rpc/response/result/block.go | 24 ------------------------ pkg/rpc/response/result/block_header.go | 19 ------------------- pkg/rpc/server/server.go | 25 +++++++++++++++++++++++-- 3 files changed, 23 insertions(+), 45 deletions(-) diff --git a/pkg/rpc/response/result/block.go b/pkg/rpc/response/result/block.go index 96394776f..f69b5f554 100644 --- a/pkg/rpc/response/result/block.go +++ b/pkg/rpc/response/result/block.go @@ -5,16 +5,10 @@ import ( "errors" "github.com/nspcc-dev/neo-go/pkg/core/block" - "github.com/nspcc-dev/neo-go/pkg/io" "github.com/nspcc-dev/neo-go/pkg/util" ) type ( - // LedgerAux is a set of methods needed to construct some outputs. - LedgerAux interface { - BlockHeight() uint32 - GetHeaderHash(int) util.Uint256 - } // Block wrapper used for the representation of // block.Block / block.Base on the RPC Server. Block struct { @@ -31,24 +25,6 @@ type ( } ) -// NewBlock creates a new Block wrapper. -func NewBlock(b *block.Block, chain LedgerAux) Block { - res := Block{ - Block: *b, - BlockMetadata: BlockMetadata{ - Size: io.GetVarSize(b), - Confirmations: chain.BlockHeight() - b.Index + 1, - }, - } - - hash := chain.GetHeaderHash(int(b.Index) + 1) - if !hash.Equals(util.Uint256{}) { - res.NextBlockHash = &hash - } - - return res -} - // MarshalJSON implements the json.Marshaler interface. func (b Block) MarshalJSON() ([]byte, error) { output, err := json.Marshal(b.BlockMetadata) diff --git a/pkg/rpc/response/result/block_header.go b/pkg/rpc/response/result/block_header.go index 1c23e51c7..421abc536 100644 --- a/pkg/rpc/response/result/block_header.go +++ b/pkg/rpc/response/result/block_header.go @@ -5,8 +5,6 @@ import ( "errors" "github.com/nspcc-dev/neo-go/pkg/core/block" - "github.com/nspcc-dev/neo-go/pkg/io" - "github.com/nspcc-dev/neo-go/pkg/util" ) type ( @@ -18,23 +16,6 @@ type ( } ) -// NewHeader creates a new Header wrapper. -func NewHeader(h *block.Header, chain LedgerAux) Header { - res := Header{ - Header: *h, - BlockMetadata: BlockMetadata{ - Size: io.GetVarSize(h), - Confirmations: chain.BlockHeight() - h.Index + 1, - }, - } - - hash := chain.GetHeaderHash(int(h.Index) + 1) - if !hash.Equals(util.Uint256{}) { - res.NextBlockHash = &hash - } - return res -} - // MarshalJSON implements the json.Marshaler interface. func (h Header) MarshalJSON() ([]byte, error) { output, err := json.Marshal(h.BlockMetadata) diff --git a/pkg/rpc/server/server.go b/pkg/rpc/server/server.go index 0c2d7e9be..8ecd24be4 100644 --- a/pkg/rpc/server/server.go +++ b/pkg/rpc/server/server.go @@ -582,6 +582,19 @@ func (s *Server) blockHashFromParam(param *params.Param) (util.Uint256, *respons return hash, nil } +func (s *Server) fillBlockMetadata(obj io.Serializable, h *block.Header) result.BlockMetadata { + res := result.BlockMetadata{ + Size: io.GetVarSize(obj), // obj can be a Block or a Header. + Confirmations: s.chain.BlockHeight() - h.Index + 1, + } + + hash := s.chain.GetHeaderHash(int(h.Index) + 1) + if !hash.Equals(util.Uint256{}) { + res.NextBlockHash = &hash + } + return res +} + func (s *Server) getBlock(reqParams params.Params) (interface{}, *response.Error) { param := reqParams.Value(0) hash, respErr := s.blockHashFromParam(param) @@ -595,7 +608,11 @@ func (s *Server) getBlock(reqParams params.Params) (interface{}, *response.Error } if v, _ := reqParams.Value(1).GetBoolean(); v { - return result.NewBlock(block, s.chain), nil + res := result.Block{ + Block: *block, + BlockMetadata: s.fillBlockMetadata(block, &block.Header), + } + return res, nil } writer := io.NewBufBinWriter() block.EncodeBinary(writer.BinWriter) @@ -1639,7 +1656,11 @@ func (s *Server) getBlockHeader(reqParams params.Params) (interface{}, *response } if verbose { - return result.NewHeader(h, s.chain), nil + res := result.Header{ + Header: *h, + BlockMetadata: s.fillBlockMetadata(h, h), + } + return res, nil } buf := io.NewBufBinWriter() From fab8dfb9f8630d2ed2ddb9d4ed96e4c506d437ab Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Fri, 8 Jul 2022 17:28:29 +0300 Subject: [PATCH 03/11] vm: move State type into a package of its own It's used a lot in other places that need it, but don't need whole VM at the same time. --- cli/contract_test.go | 14 +-- cli/executor_test.go | 4 +- cli/multisig_test.go | 4 +- cli/query/query.go | 5 +- cli/query_test.go | 5 +- pkg/compiler/native_test.go | 9 +- pkg/core/blockchain.go | 5 +- pkg/core/blockchain_neotest_test.go | 8 +- pkg/core/interop/contract/call.go | 3 +- pkg/core/native/ledger.go | 4 +- pkg/core/native/native_test/ledger_test.go | 10 +- .../native/native_test/management_test.go | 4 +- pkg/core/state/notification_event.go | 8 +- pkg/core/state/notification_event_test.go | 16 +-- pkg/neotest/basic.go | 5 +- pkg/neotest/client.go | 3 +- pkg/rpc/client/rpc_test.go | 4 +- pkg/rpc/server/client_test.go | 8 +- pkg/rpc/server/server_test.go | 13 +-- pkg/vm/json_test.go | 7 +- pkg/vm/state_test.go | 97 ------------------- pkg/vm/vm.go | 71 +++++++------- pkg/vm/{ => vmstate}/state.go | 50 +++++----- pkg/vm/vmstate/state_test.go | 97 +++++++++++++++++++ 24 files changed, 234 insertions(+), 220 deletions(-) delete mode 100644 pkg/vm/state_test.go rename pkg/vm/{ => vmstate}/state.go (51%) create mode 100644 pkg/vm/vmstate/state_test.go diff --git a/cli/contract_test.go b/cli/contract_test.go index a45680a1b..1db9e312d 100644 --- a/cli/contract_test.go +++ b/cli/contract_test.go @@ -23,8 +23,8 @@ import ( "github.com/nspcc-dev/neo-go/pkg/smartcontract/manifest" "github.com/nspcc-dev/neo-go/pkg/smartcontract/nef" "github.com/nspcc-dev/neo-go/pkg/util" - "github.com/nspcc-dev/neo-go/pkg/vm" "github.com/nspcc-dev/neo-go/pkg/vm/stackitem" + "github.com/nspcc-dev/neo-go/pkg/vm/vmstate" "github.com/nspcc-dev/neo-go/pkg/wallet" "github.com/stretchr/testify/require" "gopkg.in/yaml.v3" @@ -354,7 +354,7 @@ func TestContractDeployWithData(t *testing.T) { res := new(result.Invoke) require.NoError(t, json.Unmarshal(e.Out.Bytes(), res)) - require.Equal(t, vm.HaltState.String(), res.State, res.FaultException) + require.Equal(t, vmstate.Halt.String(), res.State, res.FaultException) require.Len(t, res.Stack, 1) require.Equal(t, []byte{12}, res.Stack[0].Value()) @@ -366,7 +366,7 @@ func TestContractDeployWithData(t *testing.T) { res = new(result.Invoke) require.NoError(t, json.Unmarshal(e.Out.Bytes(), res)) - require.Equal(t, vm.HaltState.String(), res.State, res.FaultException) + require.Equal(t, vmstate.Halt.String(), res.State, res.FaultException) require.Len(t, res.Stack, 1) require.Equal(t, []byte("take_me_to_church"), res.Stack[0].Value()) } @@ -672,7 +672,7 @@ func TestComlileAndInvokeFunction(t *testing.T) { res := new(result.Invoke) require.NoError(t, json.Unmarshal(e.Out.Bytes(), res)) - require.Equal(t, vm.HaltState.String(), res.State, res.FaultException) + require.Equal(t, vmstate.Halt.String(), res.State, res.FaultException) require.Len(t, res.Stack, 1) require.Equal(t, []byte("on create|sub create"), res.Stack[0].Value()) @@ -821,7 +821,7 @@ func TestComlileAndInvokeFunction(t *testing.T) { e.Run(t, append(cmd, strconv.FormatInt(storage.FindKeysOnly, 10))...) res := new(result.Invoke) require.NoError(t, json.Unmarshal(e.Out.Bytes(), res)) - require.Equal(t, vm.HaltState.String(), res.State) + require.Equal(t, vmstate.Halt.String(), res.State) require.Len(t, res.Stack, 1) require.Equal(t, []stackitem.Item{ stackitem.Make("findkey1"), @@ -832,7 +832,7 @@ func TestComlileAndInvokeFunction(t *testing.T) { e.Run(t, append(cmd, strconv.FormatInt(storage.FindDefault, 10))...) res := new(result.Invoke) require.NoError(t, json.Unmarshal(e.Out.Bytes(), res)) - require.Equal(t, vm.HaltState.String(), res.State) + require.Equal(t, vmstate.Halt.String(), res.State) require.Len(t, res.Stack, 1) arr, ok := res.Stack[0].Value().([]stackitem.Item) @@ -883,7 +883,7 @@ func TestComlileAndInvokeFunction(t *testing.T) { res := new(result.Invoke) require.NoError(t, json.Unmarshal(e.Out.Bytes(), res)) - require.Equal(t, vm.HaltState.String(), res.State) + require.Equal(t, vmstate.Halt.String(), res.State) require.Len(t, res.Stack, 1) require.Equal(t, []byte("on update|sub update"), res.Stack[0].Value()) }) diff --git a/cli/executor_test.go b/cli/executor_test.go index a61b7f0a2..6f69a6881 100644 --- a/cli/executor_test.go +++ b/cli/executor_test.go @@ -23,7 +23,7 @@ import ( "github.com/nspcc-dev/neo-go/pkg/rpc/server" "github.com/nspcc-dev/neo-go/pkg/smartcontract/trigger" "github.com/nspcc-dev/neo-go/pkg/util" - "github.com/nspcc-dev/neo-go/pkg/vm" + "github.com/nspcc-dev/neo-go/pkg/vm/vmstate" "github.com/stretchr/testify/require" "github.com/urfave/cli" "go.uber.org/zap" @@ -293,7 +293,7 @@ func (e *executor) checkTxPersisted(t *testing.T, prefix ...string) (*transactio aer, err := e.Chain.GetAppExecResults(tx.Hash(), trigger.Application) require.NoError(t, err) require.Equal(t, 1, len(aer)) - require.Equal(t, vm.HaltState, aer[0].VMState) + require.Equal(t, vmstate.Halt, aer[0].VMState) return tx, height } diff --git a/cli/multisig_test.go b/cli/multisig_test.go index 654f131ce..c203a5d4c 100644 --- a/cli/multisig_test.go +++ b/cli/multisig_test.go @@ -15,7 +15,7 @@ import ( "github.com/nspcc-dev/neo-go/pkg/rpc/response/result" "github.com/nspcc-dev/neo-go/pkg/smartcontract" "github.com/nspcc-dev/neo-go/pkg/util" - "github.com/nspcc-dev/neo-go/pkg/vm" + "github.com/nspcc-dev/neo-go/pkg/vm/vmstate" "github.com/stretchr/testify/require" ) @@ -154,7 +154,7 @@ func TestSignMultisigTx(t *testing.T) { e.checkTxTestInvokeOutput(t, 11) res := new(result.Invoke) require.NoError(t, json.Unmarshal(e.Out.Bytes(), res)) - require.Equal(t, vm.HaltState.String(), res.State, res.FaultException) + require.Equal(t, vmstate.Halt.String(), res.State, res.FaultException) }) e.In.WriteString("pass\r") diff --git a/cli/query/query.go b/cli/query/query.go index 2bd29dcc9..6dd387ac6 100644 --- a/cli/query/query.go +++ b/cli/query/query.go @@ -22,6 +22,7 @@ import ( "github.com/nspcc-dev/neo-go/pkg/util" "github.com/nspcc-dev/neo-go/pkg/vm" "github.com/nspcc-dev/neo-go/pkg/vm/stackitem" + "github.com/nspcc-dev/neo-go/pkg/vm/vmstate" "github.com/urfave/cli" ) @@ -129,7 +130,7 @@ func DumpApplicationLog( if len(res.Executions) != 1 { _, _ = tw.Write([]byte("Success:\tunknown (no execution data)\n")) } else { - _, _ = tw.Write([]byte(fmt.Sprintf("Success:\t%t\n", res.Executions[0].VMState == vm.HaltState))) + _, _ = tw.Write([]byte(fmt.Sprintf("Success:\t%t\n", res.Executions[0].VMState == vmstate.Halt))) } } if verbose { @@ -146,7 +147,7 @@ func DumpApplicationLog( v.PrintOps(tw) if res != nil { for _, e := range res.Executions { - if e.VMState != vm.HaltState { + if e.VMState != vmstate.Halt { _, _ = tw.Write([]byte("Exception:\t" + e.FaultException + "\n")) } } diff --git a/cli/query_test.go b/cli/query_test.go index 02a29e36d..850c0a480 100644 --- a/cli/query_test.go +++ b/cli/query_test.go @@ -16,6 +16,7 @@ import ( "github.com/nspcc-dev/neo-go/pkg/smartcontract/trigger" "github.com/nspcc-dev/neo-go/pkg/util" "github.com/nspcc-dev/neo-go/pkg/vm" + "github.com/nspcc-dev/neo-go/pkg/vm/vmstate" "github.com/nspcc-dev/neo-go/pkg/wallet" "github.com/stretchr/testify/require" ) @@ -117,7 +118,7 @@ func (e *executor) compareQueryTxVerbose(t *testing.T, tx *transaction.Transacti e.checkNextLine(t, `BlockHash:\s+`+e.Chain.GetHeaderHash(int(height)).StringLE()) res, _ := e.Chain.GetAppExecResults(tx.Hash(), trigger.Application) - e.checkNextLine(t, fmt.Sprintf(`Success:\s+%t`, res[0].Execution.VMState == vm.HaltState)) + e.checkNextLine(t, fmt.Sprintf(`Success:\s+%t`, res[0].Execution.VMState == vmstate.Halt)) for _, s := range tx.Signers { e.checkNextLine(t, fmt.Sprintf(`Signer:\s+%s\s*\(%s\)`, address.Uint160ToString(s.Account), s.Scopes.String())) } @@ -132,7 +133,7 @@ func (e *executor) compareQueryTxVerbose(t *testing.T, tx *transaction.Transacti } e.checkScriptDump(t, n) - if res[0].Execution.VMState != vm.HaltState { + if res[0].Execution.VMState != vmstate.Halt { e.checkNextLine(t, `Exception:\s+`+regexp.QuoteMeta(res[0].Execution.FaultException)) } e.checkEOF(t) diff --git a/pkg/compiler/native_test.go b/pkg/compiler/native_test.go index a486e49c0..6b8027205 100644 --- a/pkg/compiler/native_test.go +++ b/pkg/compiler/native_test.go @@ -29,6 +29,7 @@ import ( "github.com/nspcc-dev/neo-go/pkg/smartcontract/nef" "github.com/nspcc-dev/neo-go/pkg/vm" "github.com/nspcc-dev/neo-go/pkg/vm/stackitem" + "github.com/nspcc-dev/neo-go/pkg/vm/vmstate" "github.com/stretchr/testify/require" ) @@ -116,10 +117,10 @@ func TestLedgerTransactionWitnessCondition(t *testing.T) { } func TestLedgerVMStates(t *testing.T) { - require.EqualValues(t, ledger.NoneState, vm.NoneState) - require.EqualValues(t, ledger.HaltState, vm.HaltState) - require.EqualValues(t, ledger.FaultState, vm.FaultState) - require.EqualValues(t, ledger.BreakState, vm.BreakState) + require.EqualValues(t, ledger.NoneState, vmstate.None) + require.EqualValues(t, ledger.HaltState, vmstate.Halt) + require.EqualValues(t, ledger.FaultState, vmstate.Fault) + require.EqualValues(t, ledger.BreakState, vmstate.Break) } type nativeTestCase struct { diff --git a/pkg/core/blockchain.go b/pkg/core/blockchain.go index c0e386c8c..e1ddea8ac 100644 --- a/pkg/core/blockchain.go +++ b/pkg/core/blockchain.go @@ -40,6 +40,7 @@ import ( "github.com/nspcc-dev/neo-go/pkg/util" "github.com/nspcc-dev/neo-go/pkg/vm" "github.com/nspcc-dev/neo-go/pkg/vm/stackitem" + "github.com/nspcc-dev/neo-go/pkg/vm/vmstate" "go.uber.org/zap" ) @@ -816,7 +817,7 @@ func (bc *Blockchain) notificationDispatcher() { for ch := range executionFeed { ch <- aer } - if aer.VMState == vm.HaltState { + if aer.VMState == vmstate.Halt { for i := range aer.Events { for ch := range notificationFeed { ch <- &subscriptions.NotificationEvent{ @@ -1065,7 +1066,7 @@ func (bc *Blockchain) storeBlock(block *block.Block, txpool *mempool.Pool) error err = fmt.Errorf("failed to store exec result: %w", err) break } - if aer.Execution.VMState == vm.HaltState { + if aer.Execution.VMState == vmstate.Halt { for j := range aer.Execution.Events { bc.handleNotification(&aer.Execution.Events[j], kvcache, transCache, block, aer.Container) } diff --git a/pkg/core/blockchain_neotest_test.go b/pkg/core/blockchain_neotest_test.go index 50788ed90..e53a12d94 100644 --- a/pkg/core/blockchain_neotest_test.go +++ b/pkg/core/blockchain_neotest_test.go @@ -39,10 +39,10 @@ import ( "github.com/nspcc-dev/neo-go/pkg/smartcontract" "github.com/nspcc-dev/neo-go/pkg/smartcontract/trigger" "github.com/nspcc-dev/neo-go/pkg/util" - "github.com/nspcc-dev/neo-go/pkg/vm" "github.com/nspcc-dev/neo-go/pkg/vm/emit" "github.com/nspcc-dev/neo-go/pkg/vm/opcode" "github.com/nspcc-dev/neo-go/pkg/vm/stackitem" + "github.com/nspcc-dev/neo-go/pkg/vm/vmstate" "github.com/nspcc-dev/neo-go/pkg/wallet" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -826,7 +826,7 @@ func TestBlockchain_Subscriptions(t *testing.T) { exec := <-executionCh require.Equal(t, b.Hash(), exec.Container) - require.Equal(t, exec.VMState, vm.HaltState) + require.Equal(t, exec.VMState, vmstate.Halt) // 3 burn events for every tx and 1 mint for primary node require.True(t, len(notificationCh) >= 4) @@ -841,7 +841,7 @@ func TestBlockchain_Subscriptions(t *testing.T) { require.Equal(t, txExpected, tx) exec := <-executionCh require.Equal(t, tx.Hash(), exec.Container) - if exec.VMState == vm.HaltState { + if exec.VMState == vmstate.Halt { notif := <-notificationCh require.Equal(t, hash.Hash160(tx.Script), notif.ScriptHash) } @@ -855,7 +855,7 @@ func TestBlockchain_Subscriptions(t *testing.T) { exec = <-executionCh require.Equal(t, b.Hash(), exec.Container) - require.Equal(t, exec.VMState, vm.HaltState) + require.Equal(t, exec.VMState, vmstate.Halt) bc.UnsubscribeFromBlocks(blockCh) bc.UnsubscribeFromTransactions(txCh) diff --git a/pkg/core/interop/contract/call.go b/pkg/core/interop/contract/call.go index 7bed15e45..3f47fd5ca 100644 --- a/pkg/core/interop/contract/call.go +++ b/pkg/core/interop/contract/call.go @@ -14,7 +14,6 @@ import ( "github.com/nspcc-dev/neo-go/pkg/smartcontract/callflag" "github.com/nspcc-dev/neo-go/pkg/smartcontract/manifest" "github.com/nspcc-dev/neo-go/pkg/util" - "github.com/nspcc-dev/neo-go/pkg/vm" "github.com/nspcc-dev/neo-go/pkg/vm/stackitem" ) @@ -168,7 +167,7 @@ func CallFromNative(ic *interop.Context, caller util.Uint160, cs *state.Contract return fmt.Errorf("%w: %v", ErrNativeCall, err) } } - if ic.VM.State() == vm.FaultState { + if ic.VM.HasFailed() { return ErrNativeCall } return nil diff --git a/pkg/core/native/ledger.go b/pkg/core/native/ledger.go index e75f476eb..708bc7e6d 100644 --- a/pkg/core/native/ledger.go +++ b/pkg/core/native/ledger.go @@ -13,8 +13,8 @@ import ( "github.com/nspcc-dev/neo-go/pkg/smartcontract/callflag" "github.com/nspcc-dev/neo-go/pkg/smartcontract/manifest" "github.com/nspcc-dev/neo-go/pkg/util" - "github.com/nspcc-dev/neo-go/pkg/vm" "github.com/nspcc-dev/neo-go/pkg/vm/stackitem" + "github.com/nspcc-dev/neo-go/pkg/vm/vmstate" ) // Ledger provides an interface to blocks/transactions storage for smart @@ -169,7 +169,7 @@ func (l *Ledger) getTransactionVMState(ic *interop.Context, params []stackitem.I } h, _, aer, err := ic.DAO.GetTxExecResult(hash) if err != nil || !isTraceableBlock(ic, h) { - return stackitem.Make(vm.NoneState) + return stackitem.Make(vmstate.None) } return stackitem.Make(aer.VMState) } diff --git a/pkg/core/native/native_test/ledger_test.go b/pkg/core/native/native_test/ledger_test.go index dbdd3a7d5..83455f4c5 100644 --- a/pkg/core/native/native_test/ledger_test.go +++ b/pkg/core/native/native_test/ledger_test.go @@ -13,9 +13,9 @@ import ( "github.com/nspcc-dev/neo-go/pkg/neotest" "github.com/nspcc-dev/neo-go/pkg/neotest/chain" "github.com/nspcc-dev/neo-go/pkg/util" - "github.com/nspcc-dev/neo-go/pkg/vm" "github.com/nspcc-dev/neo-go/pkg/vm/opcode" "github.com/nspcc-dev/neo-go/pkg/vm/stackitem" + "github.com/nspcc-dev/neo-go/pkg/vm/vmstate" "github.com/stretchr/testify/require" ) @@ -56,22 +56,22 @@ func TestLedger_GetTransactionState(t *testing.T) { hash := e.InvokeScript(t, []byte{byte(opcode.RET)}, []neotest.Signer{c.Committee}) t.Run("unknown transaction", func(t *testing.T) { - ledgerInvoker.Invoke(t, vm.NoneState, "getTransactionVMState", util.Uint256{1, 2, 3}) + ledgerInvoker.Invoke(t, vmstate.None, "getTransactionVMState", util.Uint256{1, 2, 3}) }) t.Run("not a hash", func(t *testing.T) { ledgerInvoker.InvokeFail(t, "expected []byte of size 32", "getTransactionVMState", []byte{1, 2, 3}) }) t.Run("good: HALT", func(t *testing.T) { - ledgerInvoker.Invoke(t, vm.HaltState, "getTransactionVMState", hash) + ledgerInvoker.Invoke(t, vmstate.Halt, "getTransactionVMState", hash) }) t.Run("isn't traceable", func(t *testing.T) { // Add more blocks so that tx becomes untraceable. e.GenerateNewBlocks(t, int(e.Chain.GetConfig().MaxTraceableBlocks)) - ledgerInvoker.Invoke(t, vm.NoneState, "getTransactionVMState", hash) + ledgerInvoker.Invoke(t, vmstate.None, "getTransactionVMState", hash) }) t.Run("good: FAULT", func(t *testing.T) { faultedH := e.InvokeScript(t, []byte{byte(opcode.ABORT)}, []neotest.Signer{c.Committee}) - ledgerInvoker.Invoke(t, vm.FaultState, "getTransactionVMState", faultedH) + ledgerInvoker.Invoke(t, vmstate.Fault, "getTransactionVMState", faultedH) }) } diff --git a/pkg/core/native/native_test/management_test.go b/pkg/core/native/native_test/management_test.go index ffbc9f37a..50d1b894d 100644 --- a/pkg/core/native/native_test/management_test.go +++ b/pkg/core/native/native_test/management_test.go @@ -21,10 +21,10 @@ import ( "github.com/nspcc-dev/neo-go/pkg/smartcontract/manifest" "github.com/nspcc-dev/neo-go/pkg/smartcontract/nef" "github.com/nspcc-dev/neo-go/pkg/smartcontract/trigger" - "github.com/nspcc-dev/neo-go/pkg/vm" "github.com/nspcc-dev/neo-go/pkg/vm/emit" "github.com/nspcc-dev/neo-go/pkg/vm/opcode" "github.com/nspcc-dev/neo-go/pkg/vm/stackitem" + "github.com/nspcc-dev/neo-go/pkg/vm/vmstate" "github.com/stretchr/testify/require" ) @@ -69,7 +69,7 @@ func TestManagement_ContractCache(t *testing.T) { managementInvoker.CheckHalt(t, tx1.Hash()) aer, err := managementInvoker.Chain.GetAppExecResults(tx2.Hash(), trigger.Application) require.NoError(t, err) - require.Equal(t, vm.HaltState, aer[0].VMState, aer[0].FaultException) + require.Equal(t, vmstate.Halt, aer[0].VMState, aer[0].FaultException) require.NotEqual(t, stackitem.Null{}, aer[0].Stack) } diff --git a/pkg/core/state/notification_event.go b/pkg/core/state/notification_event.go index d87bb40e0..317df0808 100644 --- a/pkg/core/state/notification_event.go +++ b/pkg/core/state/notification_event.go @@ -8,8 +8,8 @@ import ( "github.com/nspcc-dev/neo-go/pkg/io" "github.com/nspcc-dev/neo-go/pkg/smartcontract/trigger" "github.com/nspcc-dev/neo-go/pkg/util" - "github.com/nspcc-dev/neo-go/pkg/vm" "github.com/nspcc-dev/neo-go/pkg/vm/stackitem" + "github.com/nspcc-dev/neo-go/pkg/vm/vmstate" ) // NotificationEvent is a tuple of the scripthash that has emitted the Item as a @@ -94,7 +94,7 @@ func (aer *AppExecResult) EncodeBinaryWithContext(w *io.BinWriter, sc *stackitem func (aer *AppExecResult) DecodeBinary(r *io.BinReader) { r.ReadBytes(aer.Container[:]) aer.Trigger = trigger.Type(r.ReadB()) - aer.VMState = vm.State(r.ReadB()) + aer.VMState = vmstate.State(r.ReadB()) aer.GasConsumed = int64(r.ReadU64LE()) sz := r.ReadVarUint() if stackitem.MaxDeserialized < sz && r.Err == nil { @@ -197,7 +197,7 @@ func (aer *AppExecResult) UnmarshalJSON(data []byte) error { // all resulting notifications, state, stack and other metadata. type Execution struct { Trigger trigger.Type - VMState vm.State + VMState vmstate.State GasConsumed int64 Stack []stackitem.Item Events []NotificationEvent @@ -266,7 +266,7 @@ func (e *Execution) UnmarshalJSON(data []byte) error { return err } e.Trigger = trigger - state, err := vm.StateFromString(aux.VMState) + state, err := vmstate.FromString(aux.VMState) if err != nil { return err } diff --git a/pkg/core/state/notification_event_test.go b/pkg/core/state/notification_event_test.go index 896e2b82a..0a4a11082 100644 --- a/pkg/core/state/notification_event_test.go +++ b/pkg/core/state/notification_event_test.go @@ -8,8 +8,8 @@ import ( "github.com/nspcc-dev/neo-go/internal/testserdes" "github.com/nspcc-dev/neo-go/pkg/io" "github.com/nspcc-dev/neo-go/pkg/smartcontract/trigger" - "github.com/nspcc-dev/neo-go/pkg/vm" "github.com/nspcc-dev/neo-go/pkg/vm/stackitem" + "github.com/nspcc-dev/neo-go/pkg/vm/vmstate" "github.com/stretchr/testify/require" ) @@ -18,7 +18,7 @@ func BenchmarkAppExecResult_EncodeBinary(b *testing.B) { Container: random.Uint256(), Execution: Execution{ Trigger: trigger.Application, - VMState: vm.HaltState, + VMState: vmstate.Halt, GasConsumed: 12345, Stack: []stackitem.Item{}, Events: []NotificationEvent{{ @@ -54,7 +54,7 @@ func TestEncodeDecodeAppExecResult(t *testing.T) { Container: random.Uint256(), Execution: Execution{ Trigger: 1, - VMState: vm.HaltState, + VMState: vmstate.Halt, GasConsumed: 10, Stack: []stackitem.Item{stackitem.NewBool(true)}, Events: []NotificationEvent{}, @@ -63,12 +63,12 @@ func TestEncodeDecodeAppExecResult(t *testing.T) { } t.Run("halt", func(t *testing.T) { appExecResult := newAer() - appExecResult.VMState = vm.HaltState + appExecResult.VMState = vmstate.Halt testserdes.EncodeDecodeBinary(t, appExecResult, new(AppExecResult)) }) t.Run("fault", func(t *testing.T) { appExecResult := newAer() - appExecResult.VMState = vm.FaultState + appExecResult.VMState = vmstate.Fault testserdes.EncodeDecodeBinary(t, appExecResult, new(AppExecResult)) }) t.Run("with interop", func(t *testing.T) { @@ -150,7 +150,7 @@ func TestMarshalUnmarshalJSONAppExecResult(t *testing.T) { Container: random.Uint256(), Execution: Execution{ Trigger: trigger.Application, - VMState: vm.HaltState, + VMState: vmstate.Halt, GasConsumed: 10, Stack: []stackitem.Item{}, Events: []NotificationEvent{}, @@ -164,7 +164,7 @@ func TestMarshalUnmarshalJSONAppExecResult(t *testing.T) { Container: random.Uint256(), Execution: Execution{ Trigger: trigger.Application, - VMState: vm.FaultState, + VMState: vmstate.Fault, GasConsumed: 10, Stack: []stackitem.Item{stackitem.NewBool(true)}, Events: []NotificationEvent{}, @@ -178,7 +178,7 @@ func TestMarshalUnmarshalJSONAppExecResult(t *testing.T) { Container: random.Uint256(), Execution: Execution{ Trigger: trigger.OnPersist, - VMState: vm.HaltState, + VMState: vmstate.Halt, GasConsumed: 10, Stack: []stackitem.Item{}, Events: []NotificationEvent{}, diff --git a/pkg/neotest/basic.go b/pkg/neotest/basic.go index dd9077887..1a5879ceb 100644 --- a/pkg/neotest/basic.go +++ b/pkg/neotest/basic.go @@ -22,6 +22,7 @@ import ( "github.com/nspcc-dev/neo-go/pkg/vm" "github.com/nspcc-dev/neo-go/pkg/vm/emit" "github.com/nspcc-dev/neo-go/pkg/vm/stackitem" + "github.com/nspcc-dev/neo-go/pkg/vm/vmstate" "github.com/nspcc-dev/neo-go/pkg/wallet" "github.com/stretchr/testify/require" ) @@ -210,7 +211,7 @@ func (e *Executor) InvokeScriptCheckFAULT(t testing.TB, script []byte, signers [ func (e *Executor) CheckHalt(t testing.TB, h util.Uint256, stack ...stackitem.Item) *state.AppExecResult { aer, err := e.Chain.GetAppExecResults(h, trigger.Application) require.NoError(t, err) - require.Equal(t, vm.HaltState, aer[0].VMState, aer[0].FaultException) + require.Equal(t, vmstate.Halt, aer[0].VMState, aer[0].FaultException) if len(stack) != 0 { require.Equal(t, stack, aer[0].Stack) } @@ -222,7 +223,7 @@ func (e *Executor) CheckHalt(t testing.TB, h util.Uint256, stack ...stackitem.It func (e *Executor) CheckFault(t testing.TB, h util.Uint256, s string) { aer, err := e.Chain.GetAppExecResults(h, trigger.Application) require.NoError(t, err) - require.Equal(t, vm.FaultState, aer[0].VMState) + require.Equal(t, vmstate.Fault, aer[0].VMState) require.True(t, strings.Contains(aer[0].FaultException, s), "expected: %s, got: %s", s, aer[0].FaultException) } diff --git a/pkg/neotest/client.go b/pkg/neotest/client.go index 513780d7f..b9d676f2b 100644 --- a/pkg/neotest/client.go +++ b/pkg/neotest/client.go @@ -9,6 +9,7 @@ import ( "github.com/nspcc-dev/neo-go/pkg/util" "github.com/nspcc-dev/neo-go/pkg/vm" "github.com/nspcc-dev/neo-go/pkg/vm/stackitem" + "github.com/nspcc-dev/neo-go/pkg/vm/vmstate" "github.com/stretchr/testify/require" ) @@ -91,7 +92,7 @@ func (c *ContractInvoker) InvokeAndCheck(t testing.TB, checkResult func(t testin c.AddNewBlock(t, tx) aer, err := c.Chain.GetAppExecResults(tx.Hash(), trigger.Application) require.NoError(t, err) - require.Equal(t, vm.HaltState, aer[0].VMState, aer[0].FaultException) + require.Equal(t, vmstate.Halt, aer[0].VMState, aer[0].FaultException) if checkResult != nil { checkResult(t, aer[0].Stack) } diff --git a/pkg/rpc/client/rpc_test.go b/pkg/rpc/client/rpc_test.go index 043e7a83f..c50c2b271 100644 --- a/pkg/rpc/client/rpc_test.go +++ b/pkg/rpc/client/rpc_test.go @@ -34,9 +34,9 @@ import ( "github.com/nspcc-dev/neo-go/pkg/smartcontract/nef" "github.com/nspcc-dev/neo-go/pkg/smartcontract/trigger" "github.com/nspcc-dev/neo-go/pkg/util" - "github.com/nspcc-dev/neo-go/pkg/vm" "github.com/nspcc-dev/neo-go/pkg/vm/opcode" "github.com/nspcc-dev/neo-go/pkg/vm/stackitem" + "github.com/nspcc-dev/neo-go/pkg/vm/vmstate" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -129,7 +129,7 @@ var rpcClientTestCases = map[string][]rpcClientTestCase{ Executions: []state.Execution{ { Trigger: trigger.Application, - VMState: vm.HaltState, + VMState: vmstate.Halt, GasConsumed: 1, Stack: []stackitem.Item{stackitem.NewBigInteger(big.NewInt(1))}, Events: []state.NotificationEvent{}, diff --git a/pkg/rpc/server/client_test.go b/pkg/rpc/server/client_test.go index b0be1e62b..40df78ee8 100644 --- a/pkg/rpc/server/client_test.go +++ b/pkg/rpc/server/client_test.go @@ -35,10 +35,10 @@ import ( "github.com/nspcc-dev/neo-go/pkg/smartcontract/manifest" "github.com/nspcc-dev/neo-go/pkg/smartcontract/trigger" "github.com/nspcc-dev/neo-go/pkg/util" - "github.com/nspcc-dev/neo-go/pkg/vm" "github.com/nspcc-dev/neo-go/pkg/vm/emit" "github.com/nspcc-dev/neo-go/pkg/vm/opcode" "github.com/nspcc-dev/neo-go/pkg/vm/stackitem" + "github.com/nspcc-dev/neo-go/pkg/vm/vmstate" "github.com/nspcc-dev/neo-go/pkg/wallet" "github.com/stretchr/testify/require" ) @@ -648,7 +648,7 @@ func TestSignAndPushP2PNotaryRequest(t *testing.T) { require.NoError(t, err) require.Equal(t, 1, len(appLogs)) appLog := appLogs[0] - require.Equal(t, vm.HaltState, appLog.VMState) + require.Equal(t, vmstate.Halt, appLog.VMState) require.Equal(t, appLog.GasConsumed, req.FallbackTransaction.SystemFee) }) } @@ -1282,7 +1282,7 @@ func TestClient_InvokeAndPackIteratorResults(t *testing.T) { t.Run("default max items constraint", func(t *testing.T) { res, err := c.InvokeAndPackIteratorResults(storageHash, "iterateOverValues", []smartcontract.Parameter{}, nil) require.NoError(t, err) - require.Equal(t, vm.HaltState.String(), res.State) + require.Equal(t, vmstate.Halt.String(), res.State) require.Equal(t, 1, len(res.Stack)) require.Equal(t, stackitem.ArrayT, res.Stack[0].Type()) arr, ok := res.Stack[0].Value().([]stackitem.Item) @@ -1298,7 +1298,7 @@ func TestClient_InvokeAndPackIteratorResults(t *testing.T) { max := 123 res, err := c.InvokeAndPackIteratorResults(storageHash, "iterateOverValues", []smartcontract.Parameter{}, nil, max) require.NoError(t, err) - require.Equal(t, vm.HaltState.String(), res.State) + require.Equal(t, vmstate.Halt.String(), res.State) require.Equal(t, 1, len(res.Stack)) require.Equal(t, stackitem.ArrayT, res.Stack[0].Type()) arr, ok := res.Stack[0].Value().([]stackitem.Item) diff --git a/pkg/rpc/server/server_test.go b/pkg/rpc/server/server_test.go index 6e80c8eee..412c1f692 100644 --- a/pkg/rpc/server/server_test.go +++ b/pkg/rpc/server/server_test.go @@ -45,6 +45,7 @@ import ( "github.com/nspcc-dev/neo-go/pkg/vm/emit" "github.com/nspcc-dev/neo-go/pkg/vm/opcode" "github.com/nspcc-dev/neo-go/pkg/vm/stackitem" + "github.com/nspcc-dev/neo-go/pkg/vm/vmstate" "github.com/nspcc-dev/neo-go/pkg/wallet" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -102,7 +103,7 @@ var rpcTestCases = map[string][]rpcTestCase{ assert.Equal(t, 1, len(res.Executions)) assert.Equal(t, expectedTxHash, res.Container) assert.Equal(t, trigger.Application, res.Executions[0].Trigger) - assert.Equal(t, vm.HaltState, res.Executions[0].VMState) + assert.Equal(t, vmstate.Halt, res.Executions[0].VMState) }, }, { @@ -116,7 +117,7 @@ var rpcTestCases = map[string][]rpcTestCase{ assert.Equal(t, 2, len(res.Executions)) assert.Equal(t, trigger.OnPersist, res.Executions[0].Trigger) assert.Equal(t, trigger.PostPersist, res.Executions[1].Trigger) - assert.Equal(t, vm.HaltState, res.Executions[0].VMState) + assert.Equal(t, vmstate.Halt, res.Executions[0].VMState) }, }, { @@ -129,7 +130,7 @@ var rpcTestCases = map[string][]rpcTestCase{ assert.Equal(t, genesisBlockHash, res.Container.StringLE()) assert.Equal(t, 1, len(res.Executions)) assert.Equal(t, trigger.PostPersist, res.Executions[0].Trigger) - assert.Equal(t, vm.HaltState, res.Executions[0].VMState) + assert.Equal(t, vmstate.Halt, res.Executions[0].VMState) }, }, { @@ -142,7 +143,7 @@ var rpcTestCases = map[string][]rpcTestCase{ assert.Equal(t, genesisBlockHash, res.Container.StringLE()) assert.Equal(t, 1, len(res.Executions)) assert.Equal(t, trigger.OnPersist, res.Executions[0].Trigger) - assert.Equal(t, vm.HaltState, res.Executions[0].VMState) + assert.Equal(t, vmstate.Halt, res.Executions[0].VMState) }, }, { @@ -1966,9 +1967,9 @@ func testRPCProtocol(t *testing.T, doRPCCall func(string, string, *testing.T) [] require.NoError(t, json.Unmarshal(data, &res)) require.Equal(t, 2, len(res.Executions)) require.Equal(t, trigger.OnPersist, res.Executions[0].Trigger) - require.Equal(t, vm.HaltState, res.Executions[0].VMState) + require.Equal(t, vmstate.Halt, res.Executions[0].VMState) require.Equal(t, trigger.PostPersist, res.Executions[1].Trigger) - require.Equal(t, vm.HaltState, res.Executions[1].VMState) + require.Equal(t, vmstate.Halt, res.Executions[1].VMState) }) t.Run("submit", func(t *testing.T) { diff --git a/pkg/vm/json_test.go b/pkg/vm/json_test.go index 5d7050dbb..86a68540d 100644 --- a/pkg/vm/json_test.go +++ b/pkg/vm/json_test.go @@ -18,6 +18,7 @@ import ( "github.com/nspcc-dev/neo-go/pkg/smartcontract/callflag" "github.com/nspcc-dev/neo-go/pkg/vm/opcode" "github.com/nspcc-dev/neo-go/pkg/vm/stackitem" + "github.com/nspcc-dev/neo-go/pkg/vm/vmstate" "github.com/stretchr/testify/require" ) @@ -44,7 +45,7 @@ type ( } vmUTExecutionEngineState struct { - State State `json:"state"` + State vmstate.State `json:"state"` ResultStack []vmUTStackItem `json:"resultStack"` InvocationStack []vmUTExecutionContextState `json:"invocationStack"` } @@ -152,14 +153,14 @@ func testFile(t *testing.T, filename string) { t.Run(ut.Tests[i].Name, func(t *testing.T) { prog := []byte(test.Script) vm := load(prog) - vm.state = BreakState + vm.state = vmstate.Break vm.SyscallHandler = testSyscallHandler for i := range test.Steps { execStep(t, vm, test.Steps[i]) result := test.Steps[i].Result require.Equal(t, result.State, vm.state) - if result.State == FaultState { // do not compare stacks on fault + if result.State == vmstate.Fault { // do not compare stacks on fault continue } diff --git a/pkg/vm/state_test.go b/pkg/vm/state_test.go deleted file mode 100644 index 7c4d87015..000000000 --- a/pkg/vm/state_test.go +++ /dev/null @@ -1,97 +0,0 @@ -package vm - -import ( - "encoding/json" - "testing" - - "github.com/stretchr/testify/assert" -) - -func TestStateFromString(t *testing.T) { - var ( - s State - err error - ) - - s, err = StateFromString("HALT") - assert.NoError(t, err) - assert.Equal(t, HaltState, s) - - s, err = StateFromString("BREAK") - assert.NoError(t, err) - assert.Equal(t, BreakState, s) - - s, err = StateFromString("FAULT") - assert.NoError(t, err) - assert.Equal(t, FaultState, s) - - s, err = StateFromString("NONE") - assert.NoError(t, err) - assert.Equal(t, NoneState, s) - - s, err = StateFromString("HALT, BREAK") - assert.NoError(t, err) - assert.Equal(t, HaltState|BreakState, s) - - s, err = StateFromString("FAULT, BREAK") - assert.NoError(t, err) - assert.Equal(t, FaultState|BreakState, s) - - _, err = StateFromString("HALT, KEK") - assert.Error(t, err) -} - -func TestState_HasFlag(t *testing.T) { - assert.True(t, HaltState.HasFlag(HaltState)) - assert.True(t, BreakState.HasFlag(BreakState)) - assert.True(t, FaultState.HasFlag(FaultState)) - assert.True(t, (HaltState | BreakState).HasFlag(HaltState)) - assert.True(t, (HaltState | BreakState).HasFlag(BreakState)) - - assert.False(t, HaltState.HasFlag(BreakState)) - assert.False(t, NoneState.HasFlag(HaltState)) - assert.False(t, (FaultState | BreakState).HasFlag(HaltState)) -} - -func TestState_MarshalJSON(t *testing.T) { - var ( - data []byte - err error - ) - - data, err = json.Marshal(HaltState | BreakState) - assert.NoError(t, err) - assert.Equal(t, data, []byte(`"HALT, BREAK"`)) - - data, err = json.Marshal(FaultState) - assert.NoError(t, err) - assert.Equal(t, data, []byte(`"FAULT"`)) -} - -func TestState_UnmarshalJSON(t *testing.T) { - var ( - s State - err error - ) - - err = json.Unmarshal([]byte(`"HALT, BREAK"`), &s) - assert.NoError(t, err) - assert.Equal(t, HaltState|BreakState, s) - - err = json.Unmarshal([]byte(`"FAULT, BREAK"`), &s) - assert.NoError(t, err) - assert.Equal(t, FaultState|BreakState, s) - - err = json.Unmarshal([]byte(`"NONE"`), &s) - assert.NoError(t, err) - assert.Equal(t, NoneState, s) -} - -// TestState_EnumCompat tests that byte value of State matches the C#'s one got from -// https://github.com/neo-project/neo-vm/blob/0028d862e253bda3c12eb8bb007a2d95822d3922/src/neo-vm/VMState.cs#L16. -func TestState_EnumCompat(t *testing.T) { - assert.Equal(t, byte(0), byte(NoneState)) - assert.Equal(t, byte(1<<0), byte(HaltState)) - assert.Equal(t, byte(1<<1), byte(FaultState)) - assert.Equal(t, byte(1<<2), byte(BreakState)) -} diff --git a/pkg/vm/vm.go b/pkg/vm/vm.go index 95e03a2c1..d29b2f9fc 100644 --- a/pkg/vm/vm.go +++ b/pkg/vm/vm.go @@ -23,6 +23,7 @@ import ( "github.com/nspcc-dev/neo-go/pkg/util/slice" "github.com/nspcc-dev/neo-go/pkg/vm/opcode" "github.com/nspcc-dev/neo-go/pkg/vm/stackitem" + "github.com/nspcc-dev/neo-go/pkg/vm/vmstate" ) type errorAtInstruct struct { @@ -62,7 +63,7 @@ type SyscallHandler = func(*VM, uint32) error // VM represents the virtual machine. type VM struct { - state State + state vmstate.State // callback to get interop price getPrice func(opcode.Opcode, []byte) int64 @@ -104,7 +105,7 @@ func New() *VM { // NewWithTrigger returns a new VM for executions triggered by t. func NewWithTrigger(t trigger.Type) *VM { vm := &VM{ - state: NoneState, + state: vmstate.None, trigger: t, SyscallHandler: defaultSyscallHandler, @@ -126,7 +127,7 @@ func (v *VM) SetPriceGetter(f func(opcode.Opcode, []byte) int64) { // more efficient. It reuses invocation and evaluation stacks as well as VM structure // itself. func (v *VM) Reset(t trigger.Type) { - v.state = NoneState + v.state = vmstate.None v.getPrice = nil v.istack.elems = v.istack.elems[:0] v.estack.elems = v.estack.elems[:0] @@ -288,7 +289,7 @@ func (v *VM) LoadWithFlags(prog []byte, f callflag.CallFlag) { // Clear all stacks and state, it could be a reload. v.istack.Clear() v.estack.Clear() - v.state = NoneState + v.state = vmstate.None v.gasConsumed = 0 v.invTree = nil v.LoadScriptWithFlags(prog, f) @@ -398,7 +399,7 @@ func dumpStack(s *Stack) string { } // State returns the state for the VM. -func (v *VM) State() State { +func (v *VM) State() vmstate.State { return v.state } @@ -413,39 +414,39 @@ func (v *VM) Run() error { var ctx *Context if !v.Ready() { - v.state = FaultState + v.state = vmstate.Fault return errors.New("no program loaded") } - if v.state.HasFlag(FaultState) { + if v.state.HasFlag(vmstate.Fault) { // VM already ran something and failed, in general its state is // undefined in this case so we can't run anything. return errors.New("VM has failed") } - // HaltState (the default) or BreakState are safe to continue. - v.state = NoneState + // vmstate.Halt (the default) or vmstate.Break are safe to continue. + v.state = vmstate.None ctx = v.Context() for { switch { - case v.state.HasFlag(FaultState): + case v.state.HasFlag(vmstate.Fault): // Should be caught and reported already by the v.Step(), // but we're checking here anyway just in case. return errors.New("VM has failed") - case v.state.HasFlag(HaltState), v.state.HasFlag(BreakState): + case v.state.HasFlag(vmstate.Halt), v.state.HasFlag(vmstate.Break): // Normal exit from this loop. return nil - case v.state == NoneState: + case v.state == vmstate.None: if err := v.step(ctx); err != nil { return err } default: - v.state = FaultState + v.state = vmstate.Fault return errors.New("unknown state") } // check for breakpoint before executing the next instruction ctx = v.Context() if ctx != nil && ctx.atBreakPoint() { - v.state = BreakState + v.state = vmstate.Break } } } @@ -460,7 +461,7 @@ func (v *VM) Step() error { func (v *VM) step(ctx *Context) error { op, param, err := ctx.Next() if err != nil { - v.state = FaultState + v.state = vmstate.Fault return newError(ctx.ip, op, err) } return v.execute(ctx, op, param) @@ -472,7 +473,7 @@ func (v *VM) StepInto() error { ctx := v.Context() if ctx == nil { - v.state = HaltState + v.state = vmstate.Halt } if v.HasStopped() { @@ -482,7 +483,7 @@ func (v *VM) StepInto() error { if ctx != nil && ctx.prog != nil { op, param, err := ctx.Next() if err != nil { - v.state = FaultState + v.state = vmstate.Fault return newError(ctx.ip, op, err) } vErr := v.execute(ctx, op, param) @@ -493,7 +494,7 @@ func (v *VM) StepInto() error { cctx := v.Context() if cctx != nil && cctx.atBreakPoint() { - v.state = BreakState + v.state = vmstate.Break } return nil } @@ -501,16 +502,16 @@ func (v *VM) StepInto() error { // StepOut takes the debugger to the line where the current function was called. func (v *VM) StepOut() error { var err error - if v.state == BreakState { - v.state = NoneState + if v.state == vmstate.Break { + v.state = vmstate.None } expSize := v.istack.Len() - for v.state == NoneState && v.istack.Len() >= expSize { + for v.state == vmstate.None && v.istack.Len() >= expSize { err = v.StepInto() } - if v.state == NoneState { - v.state = BreakState + if v.state == vmstate.None { + v.state = vmstate.Break } return err } @@ -523,20 +524,20 @@ func (v *VM) StepOver() error { return err } - if v.state == BreakState { - v.state = NoneState + if v.state == vmstate.Break { + v.state = vmstate.None } expSize := v.istack.Len() for { err = v.StepInto() - if !(v.state == NoneState && v.istack.Len() > expSize) { + if !(v.state == vmstate.None && v.istack.Len() > expSize) { break } } - if v.state == NoneState { - v.state = BreakState + if v.state == vmstate.None { + v.state = vmstate.Break } return err @@ -545,22 +546,22 @@ func (v *VM) StepOver() error { // HasFailed returns whether the VM is in the failed state now. Usually, it's used to // check status after Run. func (v *VM) HasFailed() bool { - return v.state.HasFlag(FaultState) + return v.state.HasFlag(vmstate.Fault) } // HasStopped returns whether the VM is in the Halt or Failed state. func (v *VM) HasStopped() bool { - return v.state.HasFlag(HaltState) || v.state.HasFlag(FaultState) + return v.state.HasFlag(vmstate.Halt) || v.state.HasFlag(vmstate.Fault) } // HasHalted returns whether the VM is in the Halt state. func (v *VM) HasHalted() bool { - return v.state.HasFlag(HaltState) + return v.state.HasFlag(vmstate.Halt) } // AtBreakpoint returns whether the VM is at breakpoint. func (v *VM) AtBreakpoint() bool { - return v.state.HasFlag(BreakState) + return v.state.HasFlag(vmstate.Break) } // GetInteropID converts instruction parameter to an interop ID. @@ -574,10 +575,10 @@ func (v *VM) execute(ctx *Context, op opcode.Opcode, parameter []byte) (err erro // each panic at a central point, putting the VM in a fault state and setting error. defer func() { if errRecover := recover(); errRecover != nil { - v.state = FaultState + v.state = vmstate.Fault err = newError(ctx.ip, op, errRecover) } else if v.refs > MaxStackSize { - v.state = FaultState + v.state = vmstate.Fault err = newError(ctx.ip, op, "stack is too big") } }() @@ -1469,7 +1470,7 @@ func (v *VM) execute(ctx *Context, op opcode.Opcode, parameter []byte) (err erro v.unloadContext(oldCtx) if v.istack.Len() == 0 { - v.state = HaltState + v.state = vmstate.Halt break } diff --git a/pkg/vm/state.go b/pkg/vm/vmstate/state.go similarity index 51% rename from pkg/vm/state.go rename to pkg/vm/vmstate/state.go index 64984b62b..75cf367af 100644 --- a/pkg/vm/state.go +++ b/pkg/vm/vmstate/state.go @@ -1,23 +1,29 @@ -package vm +/* +Package vmstate contains a set of VM state flags along with appropriate type. +It provides a set of conversion/marshaling functions/methods for this type as +well. This package is made to make VM state reusable across all of the other +components that need it without importing whole VM package. +*/ +package vmstate import ( "errors" "strings" ) -// State of the VM. +// State of the VM. It's a set of flags stored in the integer number. type State uint8 // Available States. const ( - // HaltState represents HALT VM state. - HaltState State = 1 << iota - // FaultState represents FAULT VM state. - FaultState - // BreakState represents BREAK VM state. - BreakState - // NoneState represents NONE VM state. - NoneState State = 0 + // Halt represents HALT VM state (finished normally). + Halt State = 1 << iota + // Fault represents FAULT VM state (finished with an error). + Fault + // Break represents BREAK VM state (running, debug mode). + Break + // None represents NONE VM state (not started yet). + None State = 0 ) // HasFlag checks for State flag presence. @@ -25,40 +31,40 @@ func (s State) HasFlag(f State) bool { return s&f != 0 } -// String implements the stringer interface. +// String implements the fmt.Stringer interface. func (s State) String() string { - if s == NoneState { + if s == None { return "NONE" } ss := make([]string, 0, 3) - if s.HasFlag(HaltState) { + if s.HasFlag(Halt) { ss = append(ss, "HALT") } - if s.HasFlag(FaultState) { + if s.HasFlag(Fault) { ss = append(ss, "FAULT") } - if s.HasFlag(BreakState) { + if s.HasFlag(Break) { ss = append(ss, "BREAK") } return strings.Join(ss, ", ") } -// StateFromString converts a string into the VM State. -func StateFromString(s string) (st State, err error) { +// FromString converts a string into the State. +func FromString(s string) (st State, err error) { if s = strings.TrimSpace(s); s == "NONE" { - return NoneState, nil + return None, nil } ss := strings.Split(s, ",") for _, state := range ss { switch state = strings.TrimSpace(state); state { case "HALT": - st |= HaltState + st |= Halt case "FAULT": - st |= FaultState + st |= Fault case "BREAK": - st |= BreakState + st |= Break default: return 0, errors.New("unknown state") } @@ -78,6 +84,6 @@ func (s *State) UnmarshalJSON(data []byte) (err error) { return errors.New("wrong format") } - *s, err = StateFromString(string(data[1 : l-1])) + *s, err = FromString(string(data[1 : l-1])) return } diff --git a/pkg/vm/vmstate/state_test.go b/pkg/vm/vmstate/state_test.go new file mode 100644 index 000000000..e7dcccce8 --- /dev/null +++ b/pkg/vm/vmstate/state_test.go @@ -0,0 +1,97 @@ +package vmstate + +import ( + "encoding/json" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestFromString(t *testing.T) { + var ( + s State + err error + ) + + s, err = FromString("HALT") + assert.NoError(t, err) + assert.Equal(t, Halt, s) + + s, err = FromString("BREAK") + assert.NoError(t, err) + assert.Equal(t, Break, s) + + s, err = FromString("FAULT") + assert.NoError(t, err) + assert.Equal(t, Fault, s) + + s, err = FromString("NONE") + assert.NoError(t, err) + assert.Equal(t, None, s) + + s, err = FromString("HALT, BREAK") + assert.NoError(t, err) + assert.Equal(t, Halt|Break, s) + + s, err = FromString("FAULT, BREAK") + assert.NoError(t, err) + assert.Equal(t, Fault|Break, s) + + _, err = FromString("HALT, KEK") + assert.Error(t, err) +} + +func TestState_HasFlag(t *testing.T) { + assert.True(t, Halt.HasFlag(Halt)) + assert.True(t, Break.HasFlag(Break)) + assert.True(t, Fault.HasFlag(Fault)) + assert.True(t, (Halt | Break).HasFlag(Halt)) + assert.True(t, (Halt | Break).HasFlag(Break)) + + assert.False(t, Halt.HasFlag(Break)) + assert.False(t, None.HasFlag(Halt)) + assert.False(t, (Fault | Break).HasFlag(Halt)) +} + +func TestState_MarshalJSON(t *testing.T) { + var ( + data []byte + err error + ) + + data, err = json.Marshal(Halt | Break) + assert.NoError(t, err) + assert.Equal(t, data, []byte(`"HALT, BREAK"`)) + + data, err = json.Marshal(Fault) + assert.NoError(t, err) + assert.Equal(t, data, []byte(`"FAULT"`)) +} + +func TestState_UnmarshalJSON(t *testing.T) { + var ( + s State + err error + ) + + err = json.Unmarshal([]byte(`"HALT, BREAK"`), &s) + assert.NoError(t, err) + assert.Equal(t, Halt|Break, s) + + err = json.Unmarshal([]byte(`"FAULT, BREAK"`), &s) + assert.NoError(t, err) + assert.Equal(t, Fault|Break, s) + + err = json.Unmarshal([]byte(`"NONE"`), &s) + assert.NoError(t, err) + assert.Equal(t, None, s) +} + +// TestState_EnumCompat tests that byte value of State matches the C#'s one got from +// https://github.com/neo-project/neo-vm/blob/0028d862e253bda3c12eb8bb007a2d95822d3922/src/neo-vm/VMState.cs#L16. +func TestState_EnumCompat(t *testing.T) { + assert.Equal(t, byte(0), byte(None)) + assert.Equal(t, byte(1<<0), byte(Halt)) + assert.Equal(t, byte(1<<1), byte(Fault)) + assert.Equal(t, byte(1<<2), byte(Break)) +} From dc59dc991bd10c908feb160c1dd0ada6649e860b Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Fri, 8 Jul 2022 19:10:46 +0300 Subject: [PATCH 04/11] config: move metrics.Config into config.BasicService Config package should be as lightweight as possible and now it depends on the whole metrics package just to get one structure from it. --- cli/server/server_test.go | 5 ++--- pkg/config/application_config.go | 5 ++--- pkg/config/basic_service.go | 8 ++++++++ pkg/network/metrics/metrics.go | 10 ++-------- pkg/network/metrics/pprof.go | 3 ++- pkg/network/metrics/prometheus.go | 3 ++- 6 files changed, 18 insertions(+), 16 deletions(-) create mode 100644 pkg/config/basic_service.go diff --git a/cli/server/server_test.go b/cli/server/server_test.go index 30d7edb6c..741a0ced6 100644 --- a/cli/server/server_test.go +++ b/cli/server/server_test.go @@ -10,7 +10,6 @@ import ( "github.com/nspcc-dev/neo-go/pkg/config" "github.com/nspcc-dev/neo-go/pkg/config/netmode" "github.com/nspcc-dev/neo-go/pkg/core/storage" - "github.com/nspcc-dev/neo-go/pkg/network/metrics" "github.com/nspcc-dev/neo-go/pkg/rpc" "github.com/stretchr/testify/require" "github.com/urfave/cli" @@ -317,7 +316,7 @@ func TestConfigureAddresses(t *testing.T) { t.Run("custom Pprof address", func(t *testing.T) { cfg := &config.ApplicationConfiguration{ Address: defaultAddress, - Pprof: metrics.Config{ + Pprof: config.BasicService{ Address: customAddress, }, } @@ -330,7 +329,7 @@ func TestConfigureAddresses(t *testing.T) { t.Run("custom Prometheus address", func(t *testing.T) { cfg := &config.ApplicationConfiguration{ Address: defaultAddress, - Prometheus: metrics.Config{ + Prometheus: config.BasicService{ Address: customAddress, }, } diff --git a/pkg/config/application_config.go b/pkg/config/application_config.go index 35dced09f..40322a15a 100644 --- a/pkg/config/application_config.go +++ b/pkg/config/application_config.go @@ -2,7 +2,6 @@ package config import ( "github.com/nspcc-dev/neo-go/pkg/core/storage" - "github.com/nspcc-dev/neo-go/pkg/network/metrics" "github.com/nspcc-dev/neo-go/pkg/rpc" ) @@ -19,8 +18,8 @@ type ApplicationConfiguration struct { NodePort uint16 `yaml:"NodePort"` PingInterval int64 `yaml:"PingInterval"` PingTimeout int64 `yaml:"PingTimeout"` - Pprof metrics.Config `yaml:"Pprof"` - Prometheus metrics.Config `yaml:"Prometheus"` + Pprof BasicService `yaml:"Pprof"` + Prometheus BasicService `yaml:"Prometheus"` ProtoTickInterval int64 `yaml:"ProtoTickInterval"` Relay bool `yaml:"Relay"` RPC rpc.Config `yaml:"RPC"` diff --git a/pkg/config/basic_service.go b/pkg/config/basic_service.go new file mode 100644 index 000000000..3e2137b96 --- /dev/null +++ b/pkg/config/basic_service.go @@ -0,0 +1,8 @@ +package config + +// BasicService is used for simple services like Pprof or Prometheus monitoring. +type BasicService struct { + Enabled bool `yaml:"Enabled"` + Address string `yaml:"Address"` + Port string `yaml:"Port"` +} diff --git a/pkg/network/metrics/metrics.go b/pkg/network/metrics/metrics.go index 088a8477e..071c1fbeb 100644 --- a/pkg/network/metrics/metrics.go +++ b/pkg/network/metrics/metrics.go @@ -4,24 +4,18 @@ import ( "context" "net/http" + "github.com/nspcc-dev/neo-go/pkg/config" "go.uber.org/zap" ) // Service serves metrics. type Service struct { *http.Server - config Config + config config.BasicService log *zap.Logger serviceType string } -// Config config used for monitoring. -type Config struct { - Enabled bool `yaml:"Enabled"` - Address string `yaml:"Address"` - Port string `yaml:"Port"` -} - // Start runs http service with the exposed endpoint on the configured port. func (ms *Service) Start() { if ms.config.Enabled { diff --git a/pkg/network/metrics/pprof.go b/pkg/network/metrics/pprof.go index 887acc0ea..6c3bbb202 100644 --- a/pkg/network/metrics/pprof.go +++ b/pkg/network/metrics/pprof.go @@ -4,6 +4,7 @@ import ( "net/http" "net/http/pprof" + "github.com/nspcc-dev/neo-go/pkg/config" "go.uber.org/zap" ) @@ -11,7 +12,7 @@ import ( type PprofService Service // NewPprofService creates a new service for gathering pprof metrics. -func NewPprofService(cfg Config, log *zap.Logger) *Service { +func NewPprofService(cfg config.BasicService, log *zap.Logger) *Service { if log == nil { return nil } diff --git a/pkg/network/metrics/prometheus.go b/pkg/network/metrics/prometheus.go index c02bc50fb..b00efc8b3 100644 --- a/pkg/network/metrics/prometheus.go +++ b/pkg/network/metrics/prometheus.go @@ -3,6 +3,7 @@ package metrics import ( "net/http" + "github.com/nspcc-dev/neo-go/pkg/config" "github.com/prometheus/client_golang/prometheus/promhttp" "go.uber.org/zap" ) @@ -11,7 +12,7 @@ import ( type PrometheusService Service // NewPrometheusService creates a new service for gathering prometheus metrics. -func NewPrometheusService(cfg Config, log *zap.Logger) *Service { +func NewPrometheusService(cfg config.BasicService, log *zap.Logger) *Service { if log == nil { return nil } From 9987afea4c5fe476f50872ff37a1a0f4eecb1c96 Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Fri, 8 Jul 2022 19:42:06 +0300 Subject: [PATCH 05/11] storage: move DB configuration into a package on its own Lightweight thing to import anywhere, pkg/config should not be dependent on Level/Bolt/anything else. --- cli/server/server_test.go | 4 +- pkg/config/application_config.go | 42 +++++++++---------- pkg/core/bench_test.go | 5 ++- pkg/core/blockchain_neotest_test.go | 3 +- .../native/native_test/management_test.go | 5 ++- pkg/core/storage/boltdb_store.go | 8 +--- pkg/core/storage/boltdb_store_test.go | 3 +- pkg/core/storage/dbconfig/store_config.go | 21 ++++++++++ pkg/core/storage/leveldb_store.go | 8 +--- pkg/core/storage/leveldb_store_test.go | 5 ++- pkg/core/storage/store.go | 3 +- pkg/core/storage/store_config.go | 10 ----- 12 files changed, 63 insertions(+), 54 deletions(-) create mode 100644 pkg/core/storage/dbconfig/store_config.go delete mode 100644 pkg/core/storage/store_config.go diff --git a/cli/server/server_test.go b/cli/server/server_test.go index 741a0ced6..b55d5b600 100644 --- a/cli/server/server_test.go +++ b/cli/server/server_test.go @@ -9,7 +9,7 @@ import ( "github.com/nspcc-dev/neo-go/pkg/config" "github.com/nspcc-dev/neo-go/pkg/config/netmode" - "github.com/nspcc-dev/neo-go/pkg/core/storage" + "github.com/nspcc-dev/neo-go/pkg/core/storage/dbconfig" "github.com/nspcc-dev/neo-go/pkg/rpc" "github.com/stretchr/testify/require" "github.com/urfave/cli" @@ -349,7 +349,7 @@ func TestInitBlockChain(t *testing.T) { t.Run("empty logger", func(t *testing.T) { _, err := initBlockChain(config.Config{ ApplicationConfiguration: config.ApplicationConfiguration{ - DBConfiguration: storage.DBConfiguration{ + DBConfiguration: dbconfig.DBConfiguration{ Type: "inmemory", }, }, diff --git a/pkg/config/application_config.go b/pkg/config/application_config.go index 40322a15a..0a8cb6435 100644 --- a/pkg/config/application_config.go +++ b/pkg/config/application_config.go @@ -1,32 +1,32 @@ package config import ( - "github.com/nspcc-dev/neo-go/pkg/core/storage" + "github.com/nspcc-dev/neo-go/pkg/core/storage/dbconfig" "github.com/nspcc-dev/neo-go/pkg/rpc" ) // ApplicationConfiguration config specific to the node. type ApplicationConfiguration struct { - Address string `yaml:"Address"` - AnnouncedNodePort uint16 `yaml:"AnnouncedPort"` - AttemptConnPeers int `yaml:"AttemptConnPeers"` - DBConfiguration storage.DBConfiguration `yaml:"DBConfiguration"` - DialTimeout int64 `yaml:"DialTimeout"` - LogPath string `yaml:"LogPath"` - MaxPeers int `yaml:"MaxPeers"` - MinPeers int `yaml:"MinPeers"` - NodePort uint16 `yaml:"NodePort"` - PingInterval int64 `yaml:"PingInterval"` - PingTimeout int64 `yaml:"PingTimeout"` - Pprof BasicService `yaml:"Pprof"` - Prometheus BasicService `yaml:"Prometheus"` - ProtoTickInterval int64 `yaml:"ProtoTickInterval"` - Relay bool `yaml:"Relay"` - RPC rpc.Config `yaml:"RPC"` - UnlockWallet Wallet `yaml:"UnlockWallet"` - Oracle OracleConfiguration `yaml:"Oracle"` - P2PNotary P2PNotary `yaml:"P2PNotary"` - StateRoot StateRoot `yaml:"StateRoot"` + Address string `yaml:"Address"` + AnnouncedNodePort uint16 `yaml:"AnnouncedPort"` + AttemptConnPeers int `yaml:"AttemptConnPeers"` + DBConfiguration dbconfig.DBConfiguration `yaml:"DBConfiguration"` + DialTimeout int64 `yaml:"DialTimeout"` + LogPath string `yaml:"LogPath"` + MaxPeers int `yaml:"MaxPeers"` + MinPeers int `yaml:"MinPeers"` + NodePort uint16 `yaml:"NodePort"` + PingInterval int64 `yaml:"PingInterval"` + PingTimeout int64 `yaml:"PingTimeout"` + Pprof BasicService `yaml:"Pprof"` + Prometheus BasicService `yaml:"Prometheus"` + ProtoTickInterval int64 `yaml:"ProtoTickInterval"` + Relay bool `yaml:"Relay"` + RPC rpc.Config `yaml:"RPC"` + UnlockWallet Wallet `yaml:"UnlockWallet"` + Oracle OracleConfiguration `yaml:"Oracle"` + P2PNotary P2PNotary `yaml:"P2PNotary"` + StateRoot StateRoot `yaml:"StateRoot"` // ExtensiblePoolSize is the maximum amount of the extensible payloads from a single sender. ExtensiblePoolSize int `yaml:"ExtensiblePoolSize"` } diff --git a/pkg/core/bench_test.go b/pkg/core/bench_test.go index 8af373b80..2e6415a10 100644 --- a/pkg/core/bench_test.go +++ b/pkg/core/bench_test.go @@ -11,6 +11,7 @@ import ( "github.com/nspcc-dev/neo-go/pkg/core/native/nativenames" "github.com/nspcc-dev/neo-go/pkg/core/state" "github.com/nspcc-dev/neo-go/pkg/core/storage" + "github.com/nspcc-dev/neo-go/pkg/core/storage/dbconfig" "github.com/nspcc-dev/neo-go/pkg/core/transaction" "github.com/nspcc-dev/neo-go/pkg/crypto/keys" "github.com/nspcc-dev/neo-go/pkg/io" @@ -136,7 +137,7 @@ func benchmarkForEachNEP17Transfer(t *testing.B, ps storage.Store, startFromBloc func newLevelDBForTesting(t testing.TB) storage.Store { dbPath := t.TempDir() - dbOptions := storage.LevelDBOptions{ + dbOptions := dbconfig.LevelDBOptions{ DataDirectoryPath: dbPath, } newLevelStore, err := storage.NewLevelDBStore(dbOptions) @@ -147,7 +148,7 @@ func newLevelDBForTesting(t testing.TB) storage.Store { func newBoltStoreForTesting(t testing.TB) storage.Store { d := t.TempDir() dbPath := filepath.Join(d, "test_bolt_db") - boltDBStore, err := storage.NewBoltDBStore(storage.BoltDBOptions{FilePath: dbPath}) + boltDBStore, err := storage.NewBoltDBStore(dbconfig.BoltDBOptions{FilePath: dbPath}) require.NoError(t, err) return boltDBStore } diff --git a/pkg/core/blockchain_neotest_test.go b/pkg/core/blockchain_neotest_test.go index e53a12d94..15e8a56d3 100644 --- a/pkg/core/blockchain_neotest_test.go +++ b/pkg/core/blockchain_neotest_test.go @@ -28,6 +28,7 @@ import ( "github.com/nspcc-dev/neo-go/pkg/core/native/noderoles" "github.com/nspcc-dev/neo-go/pkg/core/state" "github.com/nspcc-dev/neo-go/pkg/core/storage" + "github.com/nspcc-dev/neo-go/pkg/core/storage/dbconfig" "github.com/nspcc-dev/neo-go/pkg/core/transaction" "github.com/nspcc-dev/neo-go/pkg/crypto/hash" "github.com/nspcc-dev/neo-go/pkg/crypto/keys" @@ -52,7 +53,7 @@ func newLevelDBForTestingWithPath(t testing.TB, dbPath string) (storage.Store, s if dbPath == "" { dbPath = t.TempDir() } - dbOptions := storage.LevelDBOptions{ + dbOptions := dbconfig.LevelDBOptions{ DataDirectoryPath: dbPath, } newLevelStore, err := storage.NewLevelDBStore(dbOptions) diff --git a/pkg/core/native/native_test/management_test.go b/pkg/core/native/native_test/management_test.go index 50d1b894d..adadde1fd 100644 --- a/pkg/core/native/native_test/management_test.go +++ b/pkg/core/native/native_test/management_test.go @@ -11,6 +11,7 @@ import ( "github.com/nspcc-dev/neo-go/pkg/core/native/nativenames" "github.com/nspcc-dev/neo-go/pkg/core/state" "github.com/nspcc-dev/neo-go/pkg/core/storage" + "github.com/nspcc-dev/neo-go/pkg/core/storage/dbconfig" "github.com/nspcc-dev/neo-go/pkg/core/transaction" "github.com/nspcc-dev/neo-go/pkg/crypto/keys" "github.com/nspcc-dev/neo-go/pkg/io" @@ -281,9 +282,9 @@ func TestManagement_ContractDeploy(t *testing.T) { func TestManagement_StartFromHeight(t *testing.T) { // Create database to be able to start another chain from the same height later. ldbDir := t.TempDir() - dbConfig := storage.DBConfiguration{ + dbConfig := dbconfig.DBConfiguration{ Type: "leveldb", - LevelDBOptions: storage.LevelDBOptions{ + LevelDBOptions: dbconfig.LevelDBOptions{ DataDirectoryPath: ldbDir, }, } diff --git a/pkg/core/storage/boltdb_store.go b/pkg/core/storage/boltdb_store.go index 4045998b9..7464a9fcb 100644 --- a/pkg/core/storage/boltdb_store.go +++ b/pkg/core/storage/boltdb_store.go @@ -5,16 +5,12 @@ import ( "fmt" "os" + "github.com/nspcc-dev/neo-go/pkg/core/storage/dbconfig" "github.com/nspcc-dev/neo-go/pkg/io" "github.com/nspcc-dev/neo-go/pkg/util/slice" "go.etcd.io/bbolt" ) -// BoltDBOptions configuration for boltdb. -type BoltDBOptions struct { - FilePath string `yaml:"FilePath"` -} - // Bucket represents bucket used in boltdb to store all the data. var Bucket = []byte("DB") @@ -25,7 +21,7 @@ type BoltDBStore struct { } // NewBoltDBStore returns a new ready to use BoltDB storage with created bucket. -func NewBoltDBStore(cfg BoltDBOptions) (*BoltDBStore, error) { +func NewBoltDBStore(cfg dbconfig.BoltDBOptions) (*BoltDBStore, error) { var opts *bbolt.Options // should be exposed via BoltDBOptions if anything needed fileMode := os.FileMode(0600) // should be exposed via BoltDBOptions if anything needed fileName := cfg.FilePath diff --git a/pkg/core/storage/boltdb_store_test.go b/pkg/core/storage/boltdb_store_test.go index dac682318..e46c5b3d4 100644 --- a/pkg/core/storage/boltdb_store_test.go +++ b/pkg/core/storage/boltdb_store_test.go @@ -4,13 +4,14 @@ import ( "path/filepath" "testing" + "github.com/nspcc-dev/neo-go/pkg/core/storage/dbconfig" "github.com/stretchr/testify/require" ) func newBoltStoreForTesting(t testing.TB) Store { d := t.TempDir() testFileName := filepath.Join(d, "test_bolt_db") - boltDBStore, err := NewBoltDBStore(BoltDBOptions{FilePath: testFileName}) + boltDBStore, err := NewBoltDBStore(dbconfig.BoltDBOptions{FilePath: testFileName}) require.NoError(t, err) return boltDBStore } diff --git a/pkg/core/storage/dbconfig/store_config.go b/pkg/core/storage/dbconfig/store_config.go new file mode 100644 index 000000000..c2dde12b7 --- /dev/null +++ b/pkg/core/storage/dbconfig/store_config.go @@ -0,0 +1,21 @@ +/* +Package dbconfig is a micropackage that contains storage DB configuration options. +*/ +package dbconfig + +type ( + // DBConfiguration describes configuration for DB. Supported: 'levelDB', 'boltDB'. + DBConfiguration struct { + Type string `yaml:"Type"` + LevelDBOptions LevelDBOptions `yaml:"LevelDBOptions"` + BoltDBOptions BoltDBOptions `yaml:"BoltDBOptions"` + } + // LevelDBOptions configuration for LevelDB. + LevelDBOptions struct { + DataDirectoryPath string `yaml:"DataDirectoryPath"` + } + // BoltDBOptions configuration for BoltDB. + BoltDBOptions struct { + FilePath string `yaml:"FilePath"` + } +) diff --git a/pkg/core/storage/leveldb_store.go b/pkg/core/storage/leveldb_store.go index 24aa74891..c656f58bb 100644 --- a/pkg/core/storage/leveldb_store.go +++ b/pkg/core/storage/leveldb_store.go @@ -1,17 +1,13 @@ package storage import ( + "github.com/nspcc-dev/neo-go/pkg/core/storage/dbconfig" "github.com/syndtr/goleveldb/leveldb" "github.com/syndtr/goleveldb/leveldb/filter" "github.com/syndtr/goleveldb/leveldb/iterator" "github.com/syndtr/goleveldb/leveldb/opt" ) -// LevelDBOptions configuration for LevelDB. -type LevelDBOptions struct { - DataDirectoryPath string `yaml:"DataDirectoryPath"` -} - // LevelDBStore is the official storage implementation for storing and retrieving // blockchain data. type LevelDBStore struct { @@ -21,7 +17,7 @@ type LevelDBStore struct { // NewLevelDBStore returns a new LevelDBStore object that will // initialize the database found at the given path. -func NewLevelDBStore(cfg LevelDBOptions) (*LevelDBStore, error) { +func NewLevelDBStore(cfg dbconfig.LevelDBOptions) (*LevelDBStore, error) { var opts = new(opt.Options) // should be exposed via LevelDBOptions if anything needed opts.Filter = filter.NewBloomFilter(10) diff --git a/pkg/core/storage/leveldb_store_test.go b/pkg/core/storage/leveldb_store_test.go index 24f21e76b..5bdb31828 100644 --- a/pkg/core/storage/leveldb_store_test.go +++ b/pkg/core/storage/leveldb_store_test.go @@ -3,14 +3,15 @@ package storage import ( "testing" + "github.com/nspcc-dev/neo-go/pkg/core/storage/dbconfig" "github.com/stretchr/testify/require" ) func newLevelDBForTesting(t testing.TB) Store { ldbDir := t.TempDir() - dbConfig := DBConfiguration{ + dbConfig := dbconfig.DBConfiguration{ Type: "leveldb", - LevelDBOptions: LevelDBOptions{ + LevelDBOptions: dbconfig.LevelDBOptions{ DataDirectoryPath: ldbDir, }, } diff --git a/pkg/core/storage/store.go b/pkg/core/storage/store.go index 4d2b1a550..3e2ef4c89 100644 --- a/pkg/core/storage/store.go +++ b/pkg/core/storage/store.go @@ -4,6 +4,7 @@ import ( "errors" "fmt" + "github.com/nspcc-dev/neo-go/pkg/core/storage/dbconfig" "github.com/syndtr/goleveldb/leveldb/util" ) @@ -125,7 +126,7 @@ func seekRangeToPrefixes(sr SeekRange) *util.Range { } // NewStore creates storage with preselected in configuration database type. -func NewStore(cfg DBConfiguration) (Store, error) { +func NewStore(cfg dbconfig.DBConfiguration) (Store, error) { var store Store var err error switch cfg.Type { diff --git a/pkg/core/storage/store_config.go b/pkg/core/storage/store_config.go deleted file mode 100644 index 3521e32a1..000000000 --- a/pkg/core/storage/store_config.go +++ /dev/null @@ -1,10 +0,0 @@ -package storage - -type ( - // DBConfiguration describes configuration for DB. Supported: 'levelDB', 'boltDB'. - DBConfiguration struct { - Type string `yaml:"Type"` - LevelDBOptions LevelDBOptions `yaml:"LevelDBOptions"` - BoltDBOptions BoltDBOptions `yaml:"BoltDBOptions"` - } -) From 8cd7b93208f78815e4d3d323081719f4565bbbd3 Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Fri, 8 Jul 2022 19:51:59 +0300 Subject: [PATCH 06/11] limits: new package with storage limits Packages like core/state or core/mpt shouldn't import whole core/storage just to get some constant value, it's not a good dependency. --- pkg/config/limits/limits.go | 17 +++++++++++++++++ pkg/core/blockchain.go | 3 ++- pkg/core/dao/dao.go | 3 ++- pkg/core/interop/storage/basic.go | 6 +++--- pkg/core/interop/storage/storage_test.go | 8 ++++---- pkg/core/mpt/extension.go | 4 ++-- pkg/core/mpt/leaf.go | 4 ++-- pkg/core/state/tokens.go | 4 ++-- pkg/core/storage/store.go | 8 -------- pkg/rpc/server/server.go | 4 ++-- 10 files changed, 36 insertions(+), 25 deletions(-) create mode 100644 pkg/config/limits/limits.go diff --git a/pkg/config/limits/limits.go b/pkg/config/limits/limits.go new file mode 100644 index 000000000..70c2368e3 --- /dev/null +++ b/pkg/config/limits/limits.go @@ -0,0 +1,17 @@ +/* +Package limits contains a number of system-wide hardcoded constants. +Many of the Neo protocol parameters can be adjusted by the configuration, but +some can not and this package contains hardcoded limits that are relevant for +many applications. +*/ +package limits + +const ( + // MaxStorageKeyLen is the maximum length of a key for storage items. + // Contracts can't use keys longer than that in their requests to the DB. + MaxStorageKeyLen = 64 + // MaxStorageValueLen is the maximum length of a value for storage items. + // It is set to be the maximum value for uint16, contracts can't put + // values longer than that into the DB. + MaxStorageValueLen = 65535 +) diff --git a/pkg/core/blockchain.go b/pkg/core/blockchain.go index e1ddea8ac..366ea38b4 100644 --- a/pkg/core/blockchain.go +++ b/pkg/core/blockchain.go @@ -13,6 +13,7 @@ import ( "time" "github.com/nspcc-dev/neo-go/pkg/config" + "github.com/nspcc-dev/neo-go/pkg/config/limits" "github.com/nspcc-dev/neo-go/pkg/core/block" "github.com/nspcc-dev/neo-go/pkg/core/blockchainer" "github.com/nspcc-dev/neo-go/pkg/core/blockchainer/services" @@ -1328,7 +1329,7 @@ func (bc *Blockchain) handleNotification(note *state.NotificationEvent, d *dao.S var id []byte if len(arr) == 4 { id, err = arr[3].TryBytes() - if err != nil || len(id) > storage.MaxStorageKeyLen { + if err != nil || len(id) > limits.MaxStorageKeyLen { return } } diff --git a/pkg/core/dao/dao.go b/pkg/core/dao/dao.go index c2563f24d..b9f8ced9d 100644 --- a/pkg/core/dao/dao.go +++ b/pkg/core/dao/dao.go @@ -10,6 +10,7 @@ import ( "math/big" "sync" + "github.com/nspcc-dev/neo-go/pkg/config/limits" "github.com/nspcc-dev/neo-go/pkg/core/block" "github.com/nspcc-dev/neo-go/pkg/core/state" "github.com/nspcc-dev/neo-go/pkg/core/storage" @@ -830,7 +831,7 @@ func (dao *Simple) StoreAsTransaction(tx *transaction.Transaction, index uint32, func (dao *Simple) getKeyBuf(len int) []byte { if dao.private { if dao.keyBuf == nil { - dao.keyBuf = make([]byte, 0, 1+4+storage.MaxStorageKeyLen) // Prefix, uint32, key. + dao.keyBuf = make([]byte, 0, 1+4+limits.MaxStorageKeyLen) // Prefix, uint32, key. } return dao.keyBuf[:len] // Should have enough capacity. } diff --git a/pkg/core/interop/storage/basic.go b/pkg/core/interop/storage/basic.go index dcb6771c8..a3cd1db63 100644 --- a/pkg/core/interop/storage/basic.go +++ b/pkg/core/interop/storage/basic.go @@ -4,8 +4,8 @@ import ( "errors" "fmt" + "github.com/nspcc-dev/neo-go/pkg/config/limits" "github.com/nspcc-dev/neo-go/pkg/core/interop" - "github.com/nspcc-dev/neo-go/pkg/core/storage" "github.com/nspcc-dev/neo-go/pkg/vm/stackitem" ) @@ -81,10 +81,10 @@ func getContextInternal(ic *interop.Context, isReadOnly bool) error { } func putWithContext(ic *interop.Context, stc *Context, key []byte, value []byte) error { - if len(key) > storage.MaxStorageKeyLen { + if len(key) > limits.MaxStorageKeyLen { return errors.New("key is too big") } - if len(value) > storage.MaxStorageValueLen { + if len(value) > limits.MaxStorageValueLen { return errors.New("value is too big") } if stc.ReadOnly { diff --git a/pkg/core/interop/storage/storage_test.go b/pkg/core/interop/storage/storage_test.go index 7cfa53a74..7fa0f6f04 100644 --- a/pkg/core/interop/storage/storage_test.go +++ b/pkg/core/interop/storage/storage_test.go @@ -5,6 +5,7 @@ import ( "math/big" "testing" + "github.com/nspcc-dev/neo-go/pkg/config/limits" "github.com/nspcc-dev/neo-go/pkg/core" "github.com/nspcc-dev/neo-go/pkg/core/block" "github.com/nspcc-dev/neo-go/pkg/core/interop" @@ -12,7 +13,6 @@ import ( istorage "github.com/nspcc-dev/neo-go/pkg/core/interop/storage" "github.com/nspcc-dev/neo-go/pkg/core/native" "github.com/nspcc-dev/neo-go/pkg/core/state" - "github.com/nspcc-dev/neo-go/pkg/core/storage" "github.com/nspcc-dev/neo-go/pkg/core/transaction" "github.com/nspcc-dev/neo-go/pkg/crypto/hash" "github.com/nspcc-dev/neo-go/pkg/neotest/chain" @@ -61,7 +61,7 @@ func TestPut(t *testing.T) { }) t.Run("check limits", func(t *testing.T) { - initVM(t, make([]byte, storage.MaxStorageKeyLen), make([]byte, storage.MaxStorageValueLen), -1) + initVM(t, make([]byte, limits.MaxStorageKeyLen), make([]byte, limits.MaxStorageValueLen), -1) require.NoError(t, istorage.Put(ic)) }) @@ -72,11 +72,11 @@ func TestPut(t *testing.T) { require.Error(t, istorage.Put(ic)) }) t.Run("big key", func(t *testing.T) { - initVM(t, make([]byte, storage.MaxStorageKeyLen+1), []byte{1}, -1) + initVM(t, make([]byte, limits.MaxStorageKeyLen+1), []byte{1}, -1) require.Error(t, istorage.Put(ic)) }) t.Run("big value", func(t *testing.T) { - initVM(t, []byte{1}, make([]byte, storage.MaxStorageValueLen+1), -1) + initVM(t, []byte{1}, make([]byte, limits.MaxStorageValueLen+1), -1) require.Error(t, istorage.Put(ic)) }) }) diff --git a/pkg/core/mpt/extension.go b/pkg/core/mpt/extension.go index 518cc2787..49af8558b 100644 --- a/pkg/core/mpt/extension.go +++ b/pkg/core/mpt/extension.go @@ -6,14 +6,14 @@ import ( "errors" "fmt" - "github.com/nspcc-dev/neo-go/pkg/core/storage" + "github.com/nspcc-dev/neo-go/pkg/config/limits" "github.com/nspcc-dev/neo-go/pkg/io" "github.com/nspcc-dev/neo-go/pkg/util" ) const ( // maxPathLength is the max length of the extension node key. - maxPathLength = (storage.MaxStorageKeyLen + 4) * 2 + maxPathLength = (limits.MaxStorageKeyLen + 4) * 2 // MaxKeyLength is the max length of the key to put in the trie // before transforming to nibbles. diff --git a/pkg/core/mpt/leaf.go b/pkg/core/mpt/leaf.go index ce3f36f22..0da66f24f 100644 --- a/pkg/core/mpt/leaf.go +++ b/pkg/core/mpt/leaf.go @@ -5,13 +5,13 @@ import ( "errors" "fmt" - "github.com/nspcc-dev/neo-go/pkg/core/storage" + "github.com/nspcc-dev/neo-go/pkg/config/limits" "github.com/nspcc-dev/neo-go/pkg/io" "github.com/nspcc-dev/neo-go/pkg/util" ) // MaxValueLength is the max length of a leaf node value. -const MaxValueLength = 3 + storage.MaxStorageValueLen + 1 +const MaxValueLength = 3 + limits.MaxStorageValueLen + 1 // LeafNode represents an MPT's leaf node. type LeafNode struct { diff --git a/pkg/core/state/tokens.go b/pkg/core/state/tokens.go index 81739ccdf..63a64f0d2 100644 --- a/pkg/core/state/tokens.go +++ b/pkg/core/state/tokens.go @@ -4,7 +4,7 @@ import ( "bytes" "math/big" - "github.com/nspcc-dev/neo-go/pkg/core/storage" + "github.com/nspcc-dev/neo-go/pkg/config/limits" "github.com/nspcc-dev/neo-go/pkg/encoding/bigint" "github.com/nspcc-dev/neo-go/pkg/io" "github.com/nspcc-dev/neo-go/pkg/util" @@ -224,5 +224,5 @@ func (t *NEP11Transfer) EncodeBinary(w *io.BinWriter) { // DecodeBinary implements the io.Serializable interface. func (t *NEP11Transfer) DecodeBinary(r *io.BinReader) { t.NEP17Transfer.DecodeBinary(r) - t.ID = r.ReadVarBytes(storage.MaxStorageKeyLen) + t.ID = r.ReadVarBytes(limits.MaxStorageKeyLen) } diff --git a/pkg/core/storage/store.go b/pkg/core/storage/store.go index 3e2ef4c89..785e47886 100644 --- a/pkg/core/storage/store.go +++ b/pkg/core/storage/store.go @@ -41,14 +41,6 @@ const ( ExecTransaction byte = 2 ) -const ( - // MaxStorageKeyLen is the maximum length of a key for storage items. - MaxStorageKeyLen = 64 - // MaxStorageValueLen is the maximum length of a value for storage items. - // It is set to be the maximum value for uint16. - MaxStorageValueLen = 65535 -) - // Operation represents a single KV operation (add/del/change) performed // in the DB. type Operation struct { diff --git a/pkg/rpc/server/server.go b/pkg/rpc/server/server.go index 8ecd24be4..0e0b86e47 100644 --- a/pkg/rpc/server/server.go +++ b/pkg/rpc/server/server.go @@ -20,6 +20,7 @@ import ( "github.com/google/uuid" "github.com/gorilla/websocket" + "github.com/nspcc-dev/neo-go/pkg/config/limits" "github.com/nspcc-dev/neo-go/pkg/config/netmode" "github.com/nspcc-dev/neo-go/pkg/core" "github.com/nspcc-dev/neo-go/pkg/core/block" @@ -31,7 +32,6 @@ import ( "github.com/nspcc-dev/neo-go/pkg/core/mpt" "github.com/nspcc-dev/neo-go/pkg/core/native" "github.com/nspcc-dev/neo-go/pkg/core/state" - "github.com/nspcc-dev/neo-go/pkg/core/storage" "github.com/nspcc-dev/neo-go/pkg/core/transaction" "github.com/nspcc-dev/neo-go/pkg/crypto/hash" "github.com/nspcc-dev/neo-go/pkg/crypto/keys" @@ -846,7 +846,7 @@ contract_loop: curAsset := &bs.Balances[len(bs.Balances)-1] for i := range toks { id, err := toks[i].TryBytes() - if err != nil || len(id) > storage.MaxStorageKeyLen { + if err != nil || len(id) > limits.MaxStorageKeyLen { continue } var amount = "1" From 1e624745142e0b2a39a499cb7ff11343f220886e Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Fri, 8 Jul 2022 20:49:21 +0300 Subject: [PATCH 07/11] vm: move InvocationTree into a package of its own result.Invoke shouldn't depend on vm. --- pkg/rpc/response/result/invoke.go | 6 +++--- pkg/rpc/server/server_test.go | 18 +++++++++--------- pkg/vm/context.go | 3 ++- pkg/vm/invocation_tree.go | 12 ------------ pkg/vm/invocation_tree_test.go | 11 ++++++----- pkg/vm/invocations/invocation_tree.go | 12 ++++++++++++ pkg/vm/vm.go | 9 +++++---- 7 files changed, 37 insertions(+), 34 deletions(-) delete mode 100644 pkg/vm/invocation_tree.go create mode 100644 pkg/vm/invocations/invocation_tree.go diff --git a/pkg/rpc/response/result/invoke.go b/pkg/rpc/response/result/invoke.go index 5f84ba5cc..e1073954c 100644 --- a/pkg/rpc/response/result/invoke.go +++ b/pkg/rpc/response/result/invoke.go @@ -10,7 +10,7 @@ import ( "github.com/nspcc-dev/neo-go/pkg/core/state" "github.com/nspcc-dev/neo-go/pkg/core/storage" "github.com/nspcc-dev/neo-go/pkg/core/transaction" - "github.com/nspcc-dev/neo-go/pkg/vm" + "github.com/nspcc-dev/neo-go/pkg/vm/invocations" "github.com/nspcc-dev/neo-go/pkg/vm/stackitem" ) @@ -36,8 +36,8 @@ type RegisterIterator func(sessionID string, item stackitem.Item, id int, finali // InvokeDiag is an additional diagnostic data for invocation. type InvokeDiag struct { - Changes []storage.Operation `json:"storagechanges"` - Invocations []*vm.InvocationTree `json:"invokedcontracts"` + Changes []storage.Operation `json:"storagechanges"` + Invocations []*invocations.Tree `json:"invokedcontracts"` } // NewInvoke returns a new Invoke structure with the given fields set. diff --git a/pkg/rpc/server/server_test.go b/pkg/rpc/server/server_test.go index 412c1f692..b36c10f62 100644 --- a/pkg/rpc/server/server_test.go +++ b/pkg/rpc/server/server_test.go @@ -41,8 +41,8 @@ import ( rpc2 "github.com/nspcc-dev/neo-go/pkg/services/oracle/broadcaster" "github.com/nspcc-dev/neo-go/pkg/smartcontract/trigger" "github.com/nspcc-dev/neo-go/pkg/util" - "github.com/nspcc-dev/neo-go/pkg/vm" "github.com/nspcc-dev/neo-go/pkg/vm/emit" + "github.com/nspcc-dev/neo-go/pkg/vm/invocations" "github.com/nspcc-dev/neo-go/pkg/vm/opcode" "github.com/nspcc-dev/neo-go/pkg/vm/stackitem" "github.com/nspcc-dev/neo-go/pkg/vm/vmstate" @@ -962,12 +962,12 @@ var rpcTestCases = map[string][]rpcTestCase{ Notifications: []state.NotificationEvent{}, Diagnostics: &result.InvokeDiag{ Changes: []storage.Operation{}, - Invocations: []*vm.InvocationTree{{ + Invocations: []*invocations.Tree{{ Current: hash.Hash160(script), - Calls: []*vm.InvocationTree{ + Calls: []*invocations.Tree{ { Current: nnsHash, - Calls: []*vm.InvocationTree{ + Calls: []*invocations.Tree{ { Current: stdHash, }, @@ -1075,12 +1075,12 @@ var rpcTestCases = map[string][]rpcTestCase{ Notifications: []state.NotificationEvent{}, Diagnostics: &result.InvokeDiag{ Changes: []storage.Operation{}, - Invocations: []*vm.InvocationTree{{ + Invocations: []*invocations.Tree{{ Current: hash.Hash160(script), - Calls: []*vm.InvocationTree{ + Calls: []*invocations.Tree{ { Current: nnsHash, - Calls: []*vm.InvocationTree{ + Calls: []*invocations.Tree{ { Current: stdHash, }, @@ -1167,7 +1167,7 @@ var rpcTestCases = map[string][]rpcTestCase{ Notifications: []state.NotificationEvent{}, Diagnostics: &result.InvokeDiag{ Changes: []storage.Operation{}, - Invocations: []*vm.InvocationTree{{ + Invocations: []*invocations.Tree{{ Current: hash.Hash160(script), }}, }, @@ -1278,7 +1278,7 @@ var rpcTestCases = map[string][]rpcTestCase{ Notifications: []state.NotificationEvent{}, Diagnostics: &result.InvokeDiag{ Changes: []storage.Operation{}, - Invocations: []*vm.InvocationTree{{ + Invocations: []*invocations.Tree{{ Current: hash.Hash160(script), }}, }, diff --git a/pkg/vm/context.go b/pkg/vm/context.go index 5cdbf9a4c..955554482 100644 --- a/pkg/vm/context.go +++ b/pkg/vm/context.go @@ -11,6 +11,7 @@ import ( "github.com/nspcc-dev/neo-go/pkg/smartcontract/callflag" "github.com/nspcc-dev/neo-go/pkg/smartcontract/nef" "github.com/nspcc-dev/neo-go/pkg/util" + "github.com/nspcc-dev/neo-go/pkg/vm/invocations" "github.com/nspcc-dev/neo-go/pkg/vm/opcode" "github.com/nspcc-dev/neo-go/pkg/vm/stackitem" ) @@ -53,7 +54,7 @@ type Context struct { // NEF represents a NEF file for the current contract. NEF *nef.File // invTree is an invocation tree (or branch of it) for this context. - invTree *InvocationTree + invTree *invocations.Tree // onUnload is a callback that should be called after current context unloading // if no exception occurs. onUnload ContextUnloadCallback diff --git a/pkg/vm/invocation_tree.go b/pkg/vm/invocation_tree.go deleted file mode 100644 index c743310be..000000000 --- a/pkg/vm/invocation_tree.go +++ /dev/null @@ -1,12 +0,0 @@ -package vm - -import ( - "github.com/nspcc-dev/neo-go/pkg/util" -) - -// InvocationTree represents a tree with script hashes; when traversing it, -// you can see how contracts called each other. -type InvocationTree struct { - Current util.Uint160 `json:"hash"` - Calls []*InvocationTree `json:"call,omitempty"` -} diff --git a/pkg/vm/invocation_tree_test.go b/pkg/vm/invocation_tree_test.go index dc60ba677..b3f3896c7 100644 --- a/pkg/vm/invocation_tree_test.go +++ b/pkg/vm/invocation_tree_test.go @@ -4,6 +4,7 @@ import ( "testing" "github.com/nspcc-dev/neo-go/pkg/util" + "github.com/nspcc-dev/neo-go/pkg/vm/invocations" "github.com/nspcc-dev/neo-go/pkg/vm/opcode" "github.com/stretchr/testify/require" ) @@ -36,13 +37,13 @@ func TestInvocationTree(t *testing.T) { topHash := v.Context().ScriptHash() require.NoError(t, v.Run()) - res := &InvocationTree{ - Calls: []*InvocationTree{{ + res := &invocations.Tree{ + Calls: []*invocations.Tree{{ Current: topHash, - Calls: []*InvocationTree{ + Calls: []*invocations.Tree{ { Current: util.Uint160{1}, - Calls: []*InvocationTree{ + Calls: []*invocations.Tree{ { Current: util.Uint160{2}, }, @@ -53,7 +54,7 @@ func TestInvocationTree(t *testing.T) { }, { Current: util.Uint160{4}, - Calls: []*InvocationTree{ + Calls: []*invocations.Tree{ { Current: util.Uint160{5}, }, diff --git a/pkg/vm/invocations/invocation_tree.go b/pkg/vm/invocations/invocation_tree.go new file mode 100644 index 000000000..9b5f0be0e --- /dev/null +++ b/pkg/vm/invocations/invocation_tree.go @@ -0,0 +1,12 @@ +package invocations + +import ( + "github.com/nspcc-dev/neo-go/pkg/util" +) + +// Tree represents a tree with script hashes; when traversing it, +// you can see how contracts called each other. +type Tree struct { + Current util.Uint160 `json:"hash"` + Calls []*Tree `json:"call,omitempty"` +} diff --git a/pkg/vm/vm.go b/pkg/vm/vm.go index d29b2f9fc..aea077e74 100644 --- a/pkg/vm/vm.go +++ b/pkg/vm/vm.go @@ -21,6 +21,7 @@ import ( "github.com/nspcc-dev/neo-go/pkg/smartcontract/trigger" "github.com/nspcc-dev/neo-go/pkg/util" "github.com/nspcc-dev/neo-go/pkg/util/slice" + "github.com/nspcc-dev/neo-go/pkg/vm/invocations" "github.com/nspcc-dev/neo-go/pkg/vm/opcode" "github.com/nspcc-dev/neo-go/pkg/vm/stackitem" "github.com/nspcc-dev/neo-go/pkg/vm/vmstate" @@ -87,7 +88,7 @@ type VM struct { trigger trigger.Type // invTree is a top-level invocation tree (if enabled). - invTree *InvocationTree + invTree *invocations.Tree } var ( @@ -271,11 +272,11 @@ func (v *VM) LoadFileWithFlags(path string, f callflag.CallFlag) error { // CollectInvocationTree enables collecting invocation tree data. func (v *VM) EnableInvocationTree() { - v.invTree = &InvocationTree{} + v.invTree = &invocations.Tree{} } // GetInvocationTree returns the current invocation tree structure. -func (v *VM) GetInvocationTree() *InvocationTree { +func (v *VM) GetInvocationTree() *invocations.Tree { return v.invTree } @@ -356,7 +357,7 @@ func (v *VM) loadScriptWithCallingHash(b []byte, exe *nef.File, caller util.Uint if parent != nil { curTree = parent.invTree } - newTree := &InvocationTree{Current: ctx.ScriptHash()} + newTree := &invocations.Tree{Current: ctx.ScriptHash()} curTree.Calls = append(curTree.Calls, newTree) ctx.invTree = newTree } From 96c4e61063fe467cf0f8f80721ba5ae4aadae778 Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Fri, 8 Jul 2022 21:17:52 +0300 Subject: [PATCH 08/11] storage: move Operation into package of its own Don't use storage.* types in rpc/response/result. --- cli/server/dump.go | 7 ++++--- pkg/core/storage/dboper/operation.go | 13 +++++++++++++ pkg/core/storage/store.go | 20 ++++++-------------- pkg/core/storage/store_test.go | 3 ++- pkg/rpc/response/result/invoke.go | 3 ++- pkg/rpc/server/server_test.go | 12 ++++++------ 6 files changed, 33 insertions(+), 25 deletions(-) create mode 100644 pkg/core/storage/dboper/operation.go diff --git a/cli/server/dump.go b/cli/server/dump.go index 762b2e636..d7b6a1803 100644 --- a/cli/server/dump.go +++ b/cli/server/dump.go @@ -7,14 +7,15 @@ import ( "path/filepath" "github.com/nspcc-dev/neo-go/pkg/core/storage" + "github.com/nspcc-dev/neo-go/pkg/core/storage/dboper" ) type dump []blockDump type blockDump struct { - Block uint32 `json:"block"` - Size int `json:"size"` - Storage []storage.Operation `json:"storage"` + Block uint32 `json:"block"` + Size int `json:"size"` + Storage []dboper.Operation `json:"storage"` } func newDump() *dump { diff --git a/pkg/core/storage/dboper/operation.go b/pkg/core/storage/dboper/operation.go new file mode 100644 index 000000000..8207b2b9e --- /dev/null +++ b/pkg/core/storage/dboper/operation.go @@ -0,0 +1,13 @@ +/* +Package dboper contains a type used to represent single DB operation. +*/ +package dboper + +// Operation represents a single KV operation (add/del/change) performed +// in the DB. +type Operation struct { + // State can be Added, Changed or Deleted. + State string `json:"state"` + Key []byte `json:"key"` + Value []byte `json:"value,omitempty"` +} diff --git a/pkg/core/storage/store.go b/pkg/core/storage/store.go index 785e47886..931d153ff 100644 --- a/pkg/core/storage/store.go +++ b/pkg/core/storage/store.go @@ -5,6 +5,7 @@ import ( "fmt" "github.com/nspcc-dev/neo-go/pkg/core/storage/dbconfig" + "github.com/nspcc-dev/neo-go/pkg/core/storage/dboper" "github.com/syndtr/goleveldb/leveldb/util" ) @@ -41,15 +42,6 @@ const ( ExecTransaction byte = 2 ) -// Operation represents a single KV operation (add/del/change) performed -// in the DB. -type Operation struct { - // State can be Added, Changed or Deleted. - State string `json:"state"` - Key []byte `json:"key"` - Value []byte `json:"value,omitempty"` -} - // SeekRange represents options for Store.Seek operation. type SeekRange struct { // Prefix denotes the Seek's lookup key. @@ -134,10 +126,10 @@ func NewStore(cfg dbconfig.DBConfiguration) (Store, error) { return store, err } -// BatchToOperations converts a batch of changes into array of Operations. -func BatchToOperations(batch *MemBatch) []Operation { +// BatchToOperations converts a batch of changes into array of dboper.Operation. +func BatchToOperations(batch *MemBatch) []dboper.Operation { size := len(batch.Put) + len(batch.Deleted) - ops := make([]Operation, 0, size) + ops := make([]dboper.Operation, 0, size) for i := range batch.Put { key := batch.Put[i].Key if len(key) == 0 || key[0] != byte(STStorage) && key[0] != byte(STTempStorage) { @@ -149,7 +141,7 @@ func BatchToOperations(batch *MemBatch) []Operation { op = "Changed" } - ops = append(ops, Operation{ + ops = append(ops, dboper.Operation{ State: op, Key: key[1:], Value: batch.Put[i].Value, @@ -163,7 +155,7 @@ func BatchToOperations(batch *MemBatch) []Operation { continue } - ops = append(ops, Operation{ + ops = append(ops, dboper.Operation{ State: "Deleted", Key: key[1:], }) diff --git a/pkg/core/storage/store_test.go b/pkg/core/storage/store_test.go index 7191e46ad..125ebb1ed 100644 --- a/pkg/core/storage/store_test.go +++ b/pkg/core/storage/store_test.go @@ -3,6 +3,7 @@ package storage import ( "testing" + "github.com/nspcc-dev/neo-go/pkg/core/storage/dboper" "github.com/stretchr/testify/require" ) @@ -19,7 +20,7 @@ func TestBatchToOperations(t *testing.T) { {KeyValue: KeyValue{Key: []byte{byte(STStorage), 0x06}, Value: []byte{0x06}}, Exists: true}, }, } - o := []Operation{ + o := []dboper.Operation{ {State: "Added", Key: []byte{0x01}, Value: []byte{0x01}}, {State: "Changed", Key: []byte{0x03}, Value: []byte{0x03}}, {State: "Deleted", Key: []byte{0x06}}, diff --git a/pkg/rpc/response/result/invoke.go b/pkg/rpc/response/result/invoke.go index e1073954c..e51cb4042 100644 --- a/pkg/rpc/response/result/invoke.go +++ b/pkg/rpc/response/result/invoke.go @@ -9,6 +9,7 @@ import ( "github.com/nspcc-dev/neo-go/pkg/core/interop/iterator" "github.com/nspcc-dev/neo-go/pkg/core/state" "github.com/nspcc-dev/neo-go/pkg/core/storage" + "github.com/nspcc-dev/neo-go/pkg/core/storage/dboper" "github.com/nspcc-dev/neo-go/pkg/core/transaction" "github.com/nspcc-dev/neo-go/pkg/vm/invocations" "github.com/nspcc-dev/neo-go/pkg/vm/stackitem" @@ -36,7 +37,7 @@ type RegisterIterator func(sessionID string, item stackitem.Item, id int, finali // InvokeDiag is an additional diagnostic data for invocation. type InvokeDiag struct { - Changes []storage.Operation `json:"storagechanges"` + Changes []dboper.Operation `json:"storagechanges"` Invocations []*invocations.Tree `json:"invokedcontracts"` } diff --git a/pkg/rpc/server/server_test.go b/pkg/rpc/server/server_test.go index b36c10f62..271d4688f 100644 --- a/pkg/rpc/server/server_test.go +++ b/pkg/rpc/server/server_test.go @@ -27,7 +27,7 @@ import ( "github.com/nspcc-dev/neo-go/pkg/core/fee" "github.com/nspcc-dev/neo-go/pkg/core/native/nativenames" "github.com/nspcc-dev/neo-go/pkg/core/state" - "github.com/nspcc-dev/neo-go/pkg/core/storage" + "github.com/nspcc-dev/neo-go/pkg/core/storage/dboper" "github.com/nspcc-dev/neo-go/pkg/core/transaction" "github.com/nspcc-dev/neo-go/pkg/crypto/hash" "github.com/nspcc-dev/neo-go/pkg/crypto/keys" @@ -926,7 +926,7 @@ var rpcTestCases = map[string][]rpcTestCase{ assert.Equal(t, "HALT", res.State) assert.Equal(t, []stackitem.Item{stackitem.Make(true)}, res.Stack) assert.NotEqual(t, 0, res.GasConsumed) - chg := []storage.Operation{{ + chg := []dboper.Operation{{ State: "Changed", Key: []byte{0xfa, 0xff, 0xff, 0xff, 0xb}, Value: []byte{0x70, 0xd9, 0x59, 0x9d, 0x51, 0x79, 0x12}, @@ -961,7 +961,7 @@ var rpcTestCases = map[string][]rpcTestCase{ Stack: []stackitem.Item{stackitem.Make("1.2.3.4")}, Notifications: []state.NotificationEvent{}, Diagnostics: &result.InvokeDiag{ - Changes: []storage.Operation{}, + Changes: []dboper.Operation{}, Invocations: []*invocations.Tree{{ Current: hash.Hash160(script), Calls: []*invocations.Tree{ @@ -1074,7 +1074,7 @@ var rpcTestCases = map[string][]rpcTestCase{ Stack: []stackitem.Item{stackitem.Make("1.2.3.4")}, Notifications: []state.NotificationEvent{}, Diagnostics: &result.InvokeDiag{ - Changes: []storage.Operation{}, + Changes: []dboper.Operation{}, Invocations: []*invocations.Tree{{ Current: hash.Hash160(script), Calls: []*invocations.Tree{ @@ -1166,7 +1166,7 @@ var rpcTestCases = map[string][]rpcTestCase{ FaultException: "at instruction 0 (ROT): too big index", Notifications: []state.NotificationEvent{}, Diagnostics: &result.InvokeDiag{ - Changes: []storage.Operation{}, + Changes: []dboper.Operation{}, Invocations: []*invocations.Tree{{ Current: hash.Hash160(script), }}, @@ -1277,7 +1277,7 @@ var rpcTestCases = map[string][]rpcTestCase{ FaultException: "at instruction 0 (ROT): too big index", Notifications: []state.NotificationEvent{}, Diagnostics: &result.InvokeDiag{ - Changes: []storage.Operation{}, + Changes: []dboper.Operation{}, Invocations: []*invocations.Tree{{ Current: hash.Hash160(script), }}, From 0c45ff8f51d4b07e04b9b51f398c5c813ef7e1e2 Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Fri, 8 Jul 2022 23:25:22 +0300 Subject: [PATCH 09/11] rpc: simplify result.Invoke creation, remove needless deps Change stack items before marshaling them which makes code in result package much simpler and not requiring interop, iterator and storage dependencies that clients shouldn't care about. This also changes SessionBackedByMPT behavior, now instead of waiting for traverseiterator call it'll rerun the script immediately if a new session is created. --- pkg/rpc/response/result/invoke.go | 151 ++++++-------------- pkg/rpc/server/server.go | 223 +++++++++++++++--------------- 2 files changed, 157 insertions(+), 217 deletions(-) diff --git a/pkg/rpc/response/result/invoke.go b/pkg/rpc/response/result/invoke.go index e51cb4042..2cd78bb13 100644 --- a/pkg/rpc/response/result/invoke.go +++ b/pkg/rpc/response/result/invoke.go @@ -5,10 +5,7 @@ import ( "fmt" "github.com/google/uuid" - "github.com/nspcc-dev/neo-go/pkg/core/interop" - "github.com/nspcc-dev/neo-go/pkg/core/interop/iterator" "github.com/nspcc-dev/neo-go/pkg/core/state" - "github.com/nspcc-dev/neo-go/pkg/core/storage" "github.com/nspcc-dev/neo-go/pkg/core/storage/dboper" "github.com/nspcc-dev/neo-go/pkg/core/transaction" "github.com/nspcc-dev/neo-go/pkg/vm/invocations" @@ -18,57 +15,23 @@ import ( // Invoke represents a code invocation result and is used by several RPC calls // that invoke functions, scripts and generic bytecode. type Invoke struct { - State string - GasConsumed int64 - Script []byte - Stack []stackitem.Item - FaultException string - Notifications []state.NotificationEvent - Transaction *transaction.Transaction - Diagnostics *InvokeDiag - maxIteratorResultItems int - Session uuid.UUID - finalize func() - registerIterator RegisterIterator + State string + GasConsumed int64 + Script []byte + Stack []stackitem.Item + FaultException string + Notifications []state.NotificationEvent + Transaction *transaction.Transaction + Diagnostics *InvokeDiag + Session uuid.UUID } -// RegisterIterator is a callback used to register new iterator on the server side. -type RegisterIterator func(sessionID string, item stackitem.Item, id int, finalize func()) (uuid.UUID, error) - // InvokeDiag is an additional diagnostic data for invocation. type InvokeDiag struct { Changes []dboper.Operation `json:"storagechanges"` Invocations []*invocations.Tree `json:"invokedcontracts"` } -// NewInvoke returns a new Invoke structure with the given fields set. -func NewInvoke(ic *interop.Context, script []byte, faultException string, registerIterator RegisterIterator, maxIteratorResultItems int) *Invoke { - var diag *InvokeDiag - tree := ic.VM.GetInvocationTree() - if tree != nil { - diag = &InvokeDiag{ - Invocations: tree.Calls, - Changes: storage.BatchToOperations(ic.DAO.GetBatch()), - } - } - notifications := ic.Notifications - if notifications == nil { - notifications = make([]state.NotificationEvent, 0) - } - return &Invoke{ - State: ic.VM.State().String(), - GasConsumed: ic.VM.GasConsumed(), - Script: script, - Stack: ic.VM.Estack().ToArray(), - FaultException: faultException, - Notifications: notifications, - Diagnostics: diag, - finalize: ic.Finalize, - maxIteratorResultItems: maxIteratorResultItems, - registerIterator: registerIterator, - } -} - type invokeAux struct { State string `json:"state"` GasConsumed int64 `json:"gasconsumed,string"` @@ -107,85 +70,55 @@ type Iterator struct { Truncated bool } -// Finalize releases resources occupied by Iterators created at the script invocation. -// This method will be called automatically on Invoke marshalling or by the Server's -// sessions handler. -func (r *Invoke) Finalize() { - if r.finalize != nil { - r.finalize() +// MarshalJSON implements the json.Marshaler. +func (r Iterator) MarshalJSON() ([]byte, error) { + var iaux iteratorAux + iaux.Type = stackitem.InteropT.String() + if r.ID != nil { + iaux.Interface = iteratorInterfaceName + iaux.ID = r.ID.String() + } else { + value := make([]json.RawMessage, len(r.Values)) + for i := range r.Values { + var err error + value[i], err = stackitem.ToJSONWithTypes(r.Values[i]) + if err != nil { + return nil, err + } + } + iaux.Value = value + iaux.Truncated = r.Truncated } + return json.Marshal(iaux) } // MarshalJSON implements the json.Marshaler. func (r Invoke) MarshalJSON() ([]byte, error) { var ( - st json.RawMessage - err error - faultSep string - arr = make([]json.RawMessage, len(r.Stack)) - sessionsEnabled = r.registerIterator != nil - sessionID string + st json.RawMessage + err error + faultSep string + arr = make([]json.RawMessage, len(r.Stack)) ) if len(r.FaultException) != 0 { faultSep = " / " } -arrloop: for i := range arr { var data []byte - if (r.Stack[i].Type() == stackitem.InteropT) && iterator.IsIterator(r.Stack[i]) { - if sessionsEnabled { - if sessionID == "" { - sessionID = uuid.NewString() - } - iteratorID, err := r.registerIterator(sessionID, r.Stack[i], i, r.finalize) - if err != nil { - // Call finalizer immediately, there can't be race between server and marshaller because session wasn't added to server's session pool. - r.Finalize() - return nil, fmt.Errorf("failed to register iterator session: %w", err) - } - data, err = json.Marshal(iteratorAux{ - Type: stackitem.InteropT.String(), - Interface: iteratorInterfaceName, - ID: iteratorID.String(), - }) - if err != nil { - r.FaultException += fmt.Sprintf("%sjson error: failed to marshal iterator: %v", faultSep, err) - break - } - } else { - iteratorValues, truncated := iterator.ValuesTruncated(r.Stack[i], r.maxIteratorResultItems) - value := make([]json.RawMessage, len(iteratorValues)) - for j := range iteratorValues { - value[j], err = stackitem.ToJSONWithTypes(iteratorValues[j]) - if err != nil { - r.FaultException += fmt.Sprintf("%sjson error: %v", faultSep, err) - break arrloop - } - } - data, err = json.Marshal(iteratorAux{ - Type: stackitem.InteropT.String(), - Value: value, - Truncated: truncated, - }) - if err != nil { - r.FaultException += fmt.Sprintf("%sjson error: %v", faultSep, err) - break - } - } + + iter, ok := r.Stack[i].Value().(Iterator) + if (r.Stack[i].Type() == stackitem.InteropT) && ok { + data, err = json.Marshal(iter) } else { data, err = stackitem.ToJSONWithTypes(r.Stack[i]) - if err != nil { - r.FaultException += fmt.Sprintf("%sjson error: %v", faultSep, err) - break - } + } + if err != nil { + r.FaultException += fmt.Sprintf("%sjson error: %v", faultSep, err) + break } arr[i] = data } - if !sessionsEnabled || sessionID == "" { - // Call finalizer manually if iterators are disabled or there's no unnested iterators on estack. - defer r.Finalize() - } if err == nil { st, err = json.Marshal(arr) if err != nil { @@ -196,6 +129,10 @@ arrloop: if r.Transaction != nil { txbytes = r.Transaction.Bytes() } + var sessionID string + if r.Session != (uuid.UUID{}) { + sessionID = r.Session.String() + } aux := &invokeAux{ GasConsumed: r.GasConsumed, Script: r.Script, diff --git a/pkg/rpc/server/server.go b/pkg/rpc/server/server.go index 0e0b86e47..28ccfa246 100644 --- a/pkg/rpc/server/server.go +++ b/pkg/rpc/server/server.go @@ -32,6 +32,7 @@ import ( "github.com/nspcc-dev/neo-go/pkg/core/mpt" "github.com/nspcc-dev/neo-go/pkg/core/native" "github.com/nspcc-dev/neo-go/pkg/core/state" + "github.com/nspcc-dev/neo-go/pkg/core/storage" "github.com/nspcc-dev/neo-go/pkg/core/transaction" "github.com/nspcc-dev/neo-go/pkg/crypto/hash" "github.com/nspcc-dev/neo-go/pkg/crypto/keys" @@ -101,29 +102,14 @@ type ( // or from historic MPT-based invocation. In the second case, iteratorIdentifiers are supposed // to be filled during the first `traverseiterator` call using corresponding params. iteratorIdentifiers []*iteratorIdentifier - // params stores invocation params for historic MPT-based iterator traversing. It is nil in case - // of default non-MPT-based sessions mechanism enabled. - params *invocationParams - timer *time.Timer - finalize func() + timer *time.Timer + finalize func() } - // iteratorIdentifier represents Iterator on the server side, holding iterator ID, Iterator stackitem - // and iterator index on stack. + // iteratorIdentifier represents Iterator on the server side, holding iterator ID and Iterator stackitem. iteratorIdentifier struct { ID string - // Item represents Iterator stackitem. It is nil if SessionBackedByMPT is set to true and no `traverseiterator` - // call was called for the corresponding session. + // Item represents Iterator stackitem. Item stackitem.Item - // StackIndex represents Iterator stackitem index on the stack. It can be used only for SessionBackedByMPT configuration. - StackIndex int - } - // invocationParams is a set of parameters used for invoke* calls. - invocationParams struct { - Trigger trigger.Type - Script []byte - ContractScriptHash util.Uint160 - Transaction *transaction.Transaction - NextBlockHeight uint32 } ) @@ -350,9 +336,7 @@ func (s *Server) Shutdown() { for _, session := range s.sessions { // Concurrent iterator traversal may still be in process, thus need to protect iteratorIdentifiers access. session.iteratorsLock.Lock() - if session.finalize != nil { - session.finalize() - } + session.finalize() if !session.timer.Stop() { <-session.timer.C } @@ -2036,64 +2020,117 @@ func (s *Server) runScriptInVM(t trigger.Type, script []byte, contractScriptHash if err != nil { faultException = err.Error() } - var registerIterator result.RegisterIterator - if s.config.SessionEnabled { - registerIterator = func(sessionID string, item stackitem.Item, stackIndex int, finalize func()) (uuid.UUID, error) { - iterID := uuid.New() + items := ic.VM.Estack().ToArray() + sess := s.postProcessExecStack(items) + var id uuid.UUID + + if sess != nil { + // b == nil only when we're not using MPT-backed storage, therefore + // the second attempt won't stop here. + if s.config.SessionBackedByMPT && b == nil { + ic.Finalize() + b, err = s.getFakeNextBlock(ic.Block.Index) + if err != nil { + return nil, response.NewInternalServerError(fmt.Sprintf("unable to prepare block for historic call: %s", err)) + } + // Rerun with MPT-backed storage. + return s.runScriptInVM(t, script, contractScriptHash, tx, b, verbose) + } + id = uuid.New() + sessionID := id.String() + sess.finalize = ic.Finalize + sess.timer = time.AfterFunc(time.Second*time.Duration(s.config.SessionExpirationTime), func() { s.sessionsLock.Lock() + defer s.sessionsLock.Unlock() + if len(s.sessions) == 0 { + return + } sess, ok := s.sessions[sessionID] if !ok { - if len(s.sessions) >= s.config.SessionPoolSize { - return uuid.UUID{}, errors.New("max capacity reached") - } - timer := time.AfterFunc(time.Second*time.Duration(s.config.SessionExpirationTime), func() { - s.sessionsLock.Lock() - defer s.sessionsLock.Unlock() - if len(s.sessions) == 0 { - return - } - sess, ok := s.sessions[sessionID] - if !ok { - return - } - sess.iteratorsLock.Lock() - if sess.finalize != nil { - sess.finalize() - } - delete(s.sessions, sessionID) - sess.iteratorsLock.Unlock() - }) - sess = &session{ - finalize: finalize, - timer: timer, - } - if s.config.SessionBackedByMPT { - sess.params = &invocationParams{ - Trigger: t, - Script: script, - ContractScriptHash: contractScriptHash, - Transaction: tx, - NextBlockHeight: ic.Block.Index, - } - // Call finalizer manually if MPT-based iterator sessions are enabled. If disabled, then register finalizator. - if finalize != nil { - finalize() - sess.finalize = nil - } - item = nil - } + return } - sess.iteratorIdentifiers = append(sess.iteratorIdentifiers, &iteratorIdentifier{ - ID: iterID.String(), - Item: item, - StackIndex: stackIndex, - }) - s.sessions[sessionID] = sess + sess.iteratorsLock.Lock() + sess.finalize() + delete(s.sessions, sessionID) + sess.iteratorsLock.Unlock() + }) + s.sessionsLock.Lock() + if len(s.sessions) >= s.config.SessionPoolSize { + ic.Finalize() s.sessionsLock.Unlock() - return iterID, nil + return nil, response.NewInternalServerError("max session capacity reached") + } + s.sessions[sessionID] = sess + s.sessionsLock.Unlock() + } else { + ic.Finalize() + } + var diag *result.InvokeDiag + tree := ic.VM.GetInvocationTree() + if tree != nil { + diag = &result.InvokeDiag{ + Invocations: tree.Calls, + Changes: storage.BatchToOperations(ic.DAO.GetBatch()), } } - return result.NewInvoke(ic, script, faultException, registerIterator, s.config.MaxIteratorResultItems), nil + notifications := ic.Notifications + if notifications == nil { + notifications = make([]state.NotificationEvent, 0) + } + res := &result.Invoke{ + State: ic.VM.State().String(), + GasConsumed: ic.VM.GasConsumed(), + Script: script, + Stack: items, + FaultException: faultException, + Notifications: notifications, + Diagnostics: diag, + Session: id, + } + + return res, nil +} + +// postProcessExecStack changes iterator interop items according to the server configuration. +// It does modifications in-place, but it returns a session if any iterator was registered. +func (s *Server) postProcessExecStack(stack []stackitem.Item) *session { + var sess session + + for i, v := range stack { + var id uuid.UUID + + stack[i], id = s.registerOrDumpIterator(v) + if id != (uuid.UUID{}) { + sess.iteratorIdentifiers = append(sess.iteratorIdentifiers, &iteratorIdentifier{ + ID: id.String(), + Item: v, + }) + } + } + if len(sess.iteratorIdentifiers) != 0 { + return &sess + } + return nil +} + +// registerOrDumpIterator changes iterator interop stack items into result.Iterator +// interop stack items and returns a uuid for it if sessions are enabled. All the other stack +// items are not changed. +func (s *Server) registerOrDumpIterator(item stackitem.Item) (stackitem.Item, uuid.UUID) { + var iterID uuid.UUID + + if (item.Type() != stackitem.InteropT) || !iterator.IsIterator(item) { + return item, iterID + } + var resIterator result.Iterator + + if s.config.SessionEnabled { + iterID = uuid.New() + resIterator.ID = &iterID + } else { + resIterator.Values, resIterator.Truncated = iterator.ValuesTruncated(item, s.config.MaxIteratorResultItems) + } + return stackitem.NewInterop(resIterator), iterID } func (s *Server) traverseIterator(reqParams params.Params) (interface{}, *response.Error) { @@ -2132,43 +2169,11 @@ func (s *Server) traverseIterator(reqParams params.Params) (interface{}, *respon s.sessionsLock.Unlock() var ( - iIDStr = iID.String() - iVals []stackitem.Item - respErr *response.Error + iIDStr = iID.String() + iVals []stackitem.Item ) for _, it := range session.iteratorIdentifiers { if iIDStr == it.ID { - // If SessionBackedByMPT is enabled, then use MPT-backed historic call to retrieve and traverse iterator. - // Otherwise, iterator stackitem is ready and can be used. - if s.config.SessionBackedByMPT && it.Item == nil { - var ( - b *block.Block - ic *interop.Context - ) - b, err = s.getFakeNextBlock(session.params.NextBlockHeight) - if err != nil { - session.iteratorsLock.Unlock() - return nil, response.NewInternalServerError(fmt.Sprintf("unable to prepare block for historic call: %s", err)) - } - ic, respErr = s.prepareInvocationContext(session.params.Trigger, session.params.Script, session.params.ContractScriptHash, session.params.Transaction, b, false) - if respErr != nil { - session.iteratorsLock.Unlock() - return nil, respErr - } - _ = ic.VM.Run() // No error check because FAULTed invocations could also contain iterator on stack. - stack := ic.VM.Estack().ToArray() - - // Fill in the whole set of iterators for the current session in order not to repeat test invocation one more time for other session iterators. - for _, itID := range session.iteratorIdentifiers { - j := itID.StackIndex - if (stack[j].Type() != stackitem.InteropT) || !iterator.IsIterator(stack[j]) { - session.iteratorsLock.Unlock() - return nil, response.NewInternalServerError(fmt.Sprintf("inconsistent historic call result: expected %s, got %s at stack position #%d", stackitem.InteropT, stack[j].Type(), j)) - } - session.iteratorIdentifiers[j].Item = stack[j] - } - session.finalize = ic.Finalize - } iVals = iterator.Values(it.Item, count) break } @@ -2201,9 +2206,7 @@ func (s *Server) terminateSession(reqParams params.Params) (interface{}, *respon // Iterators access Seek channel under the hood; finalizer closes this channel, thus, // we need to perform finalisation under iteratorsLock. session.iteratorsLock.Lock() - if session.finalize != nil { - session.finalize() - } + session.finalize() if !session.timer.Stop() { <-session.timer.C } From 07f58abe3dd4fbc5de3134966fa37ef4a5f3d4d0 Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Sat, 9 Jul 2022 09:48:15 +0300 Subject: [PATCH 10/11] result: provide (*Iterator).UnmarshalJSON It makes Iterator more symmetric and simplifies (*Invoke).UnmarshalJSON code. No functional changes. --- pkg/rpc/response/result/invoke.go | 69 ++++++++++++++++--------------- 1 file changed, 35 insertions(+), 34 deletions(-) diff --git a/pkg/rpc/response/result/invoke.go b/pkg/rpc/response/result/invoke.go index 2cd78bb13..ef579f600 100644 --- a/pkg/rpc/response/result/invoke.go +++ b/pkg/rpc/response/result/invoke.go @@ -92,6 +92,36 @@ func (r Iterator) MarshalJSON() ([]byte, error) { return json.Marshal(iaux) } +// UnmarshalJSON implements the json.Unmarshaler. +func (r *Iterator) UnmarshalJSON(data []byte) error { + iteratorAux := new(iteratorAux) + err := json.Unmarshal(data, iteratorAux) + if err != nil { + return err + } + if len(iteratorAux.Interface) != 0 { + if iteratorAux.Interface != iteratorInterfaceName { + return fmt.Errorf("unknown InteropInterface: %s", iteratorAux.Interface) + } + var iID uuid.UUID + iID, err = uuid.Parse(iteratorAux.ID) + if err != nil { + return fmt.Errorf("failed to unmarshal iterator ID: %w", err) + } + r.ID = &iID + } else { + r.Values = make([]stackitem.Item, len(iteratorAux.Value)) + for j := range r.Values { + r.Values[j], err = stackitem.FromJSONWithTypes(iteratorAux.Value[j]) + if err != nil { + return fmt.Errorf("failed to unmarshal iterator values: %w", err) + } + } + r.Truncated = iteratorAux.Truncated + } + return nil +} + // MarshalJSON implements the json.Marshaler. func (r Invoke) MarshalJSON() ([]byte, error) { var ( @@ -171,41 +201,12 @@ func (r *Invoke) UnmarshalJSON(data []byte) error { break } if st[i].Type() == stackitem.InteropT { - iteratorAux := new(iteratorAux) - if json.Unmarshal(arr[i], iteratorAux) == nil { - if len(iteratorAux.Interface) != 0 { - if iteratorAux.Interface != iteratorInterfaceName { - err = fmt.Errorf("unknown InteropInterface: %s", iteratorAux.Interface) - break - } - var iID uuid.UUID - iID, err = uuid.Parse(iteratorAux.ID) // iteratorAux.ID is always non-empty, see https://github.com/neo-project/neo-modules/pull/715#discussion_r897635424. - if err != nil { - err = fmt.Errorf("failed to unmarshal iterator ID: %w", err) - break - } - // It's impossible to restore initial iterator type; also iterator is almost - // useless outside the VM, thus let's replace it with a special structure. - st[i] = stackitem.NewInterop(Iterator{ - ID: &iID, - }) - } else { - iteratorValues := make([]stackitem.Item, len(iteratorAux.Value)) - for j := range iteratorValues { - iteratorValues[j], err = stackitem.FromJSONWithTypes(iteratorAux.Value[j]) - if err != nil { - err = fmt.Errorf("failed to unmarshal iterator values: %w", err) - break - } - } - // It's impossible to restore initial iterator type; also iterator is almost - // useless outside the VM, thus let's replace it with a special structure. - st[i] = stackitem.NewInterop(Iterator{ - Values: iteratorValues, - Truncated: iteratorAux.Truncated, - }) - } + var iter = Iterator{} + err = json.Unmarshal(arr[i], &iter) + if err != nil { + break } + st[i] = stackitem.NewInterop(iter) } } if err != nil { From 125c2805d399c33d2279638259f7cd4047ce66d0 Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Mon, 11 Jul 2022 16:13:19 +0300 Subject: [PATCH 11/11] storage: reduce lock time in (*MemoryStore).Seek It makes a copy of the resulting set, so the lock can be released earlier. This helps a lot with iterators that keep Seek() unfinished for a long time, --- pkg/core/storage/memory_store.go | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/pkg/core/storage/memory_store.go b/pkg/core/storage/memory_store.go index 5462b937a..84a49e366 100644 --- a/pkg/core/storage/memory_store.go +++ b/pkg/core/storage/memory_store.go @@ -68,13 +68,14 @@ func (s *MemoryStore) putChangeSet(puts map[string][]byte, stores map[string][]b // Seek implements the Store interface. func (s *MemoryStore) Seek(rng SeekRange, f func(k, v []byte) bool) { - s.mut.RLock() - s.seek(rng, f) - s.mut.RUnlock() + s.seek(rng, f, s.mut.RLock, s.mut.RUnlock) } // SeekGC implements the Store interface. func (s *MemoryStore) SeekGC(rng SeekRange, keep func(k, v []byte) bool) error { + noop := func() {} + // Keep RW lock for the whole Seek time, state must be consistent across whole + // operation and we call delete in the handler. s.mut.Lock() // We still need to perform normal seek, some GC operations can be // sensitive to the order of KV pairs. @@ -83,7 +84,7 @@ func (s *MemoryStore) SeekGC(rng SeekRange, keep func(k, v []byte) bool) error { delete(s.chooseMap(k), string(k)) } return true - }) + }, noop, noop) s.mut.Unlock() return nil } @@ -91,7 +92,7 @@ func (s *MemoryStore) SeekGC(rng SeekRange, keep func(k, v []byte) bool) error { // seek is an internal unlocked implementation of Seek. `start` denotes whether // seeking starting from the provided prefix should be performed. Backwards // seeking from some point is supported with corresponding SeekRange field set. -func (s *MemoryStore) seek(rng SeekRange, f func(k, v []byte) bool) { +func (s *MemoryStore) seek(rng SeekRange, f func(k, v []byte) bool, lock func(), unlock func()) { sPrefix := string(rng.Prefix) lPrefix := len(sPrefix) sStart := string(rng.Start) @@ -111,6 +112,7 @@ func (s *MemoryStore) seek(rng SeekRange, f func(k, v []byte) bool) { return res != 0 && rng.Backwards == (res > 0) } + lock() m := s.chooseMap(rng.Prefix) for k, v := range m { if v != nil && isKeyOK(k) { @@ -120,6 +122,7 @@ func (s *MemoryStore) seek(rng SeekRange, f func(k, v []byte) bool) { }) } } + unlock() sort.Slice(memList, func(i, j int) bool { return less(memList[i].Key, memList[j].Key) })