The tests are still there, coverage should counted fine, but it improves things:
? github.com/nspcc-dev/neo-go/cli [no test files]
ok github.com/nspcc-dev/neo-go/cli/app 0.058s coverage: 100.0% of statements
ok github.com/nspcc-dev/neo-go/cli/cmdargs 0.005s coverage: 60.8% of statements
ok github.com/nspcc-dev/neo-go/cli/flags 0.027s coverage: 97.7% of statements
? github.com/nspcc-dev/neo-go/cli/input [no test files]
ok github.com/nspcc-dev/neo-go/cli/nep_test 30.443s coverage: [no statements]
ok github.com/nspcc-dev/neo-go/cli/options 0.054s coverage: 50.0% of statements
? github.com/nspcc-dev/neo-go/cli/paramcontext [no test files]
ok github.com/nspcc-dev/neo-go/cli/query 2.089s coverage: 45.3% of statements
ok github.com/nspcc-dev/neo-go/cli/server 1.510s coverage: 67.8% of statements
ok github.com/nspcc-dev/neo-go/cli/smartcontract 8.433s coverage: 94.3% of statements
ok github.com/nspcc-dev/neo-go/cli/util 0.013s coverage: 10.9% of statements
? github.com/nspcc-dev/neo-go/cli/vm [no test files]
ok github.com/nspcc-dev/neo-go/cli/wallet 47.252s coverage: 63.0% of statements
Refs. #2379, but not completely solves it, one package seriously outweights
others:
? github.com/nspcc-dev/neo-go/cli [no test files]
ok github.com/nspcc-dev/neo-go/cli/app 0.036s coverage: 100.0% of statements
ok github.com/nspcc-dev/neo-go/cli/cmdargs 0.011s coverage: 60.8% of statements
ok github.com/nspcc-dev/neo-go/cli/flags 0.009s coverage: 97.7% of statements
? github.com/nspcc-dev/neo-go/cli/input [no test files]
ok github.com/nspcc-dev/neo-go/cli/options 0.033s coverage: 50.0% of statements
? github.com/nspcc-dev/neo-go/cli/paramcontext [no test files]
ok github.com/nspcc-dev/neo-go/cli/query 2.155s coverage: 45.3% of statements
ok github.com/nspcc-dev/neo-go/cli/server 1.373s coverage: 67.8% of statements
ok github.com/nspcc-dev/neo-go/cli/smartcontract 8.819s coverage: 94.3% of statements
ok github.com/nspcc-dev/neo-go/cli/util 0.006s coverage: 10.9% of statements
? github.com/nspcc-dev/neo-go/cli/vm [no test files]
ok github.com/nspcc-dev/neo-go/cli/wallet 72.103s coverage: 88.2% of statements
Still a nice thing to have.
Make NEP-11 code use getnep11balances the same way NEP-17 code uses
getnep17balances. This command was introduced well before getnep11balances
appeared, so it required always specifying contract explicitly. Now this
constraint can be relaxed somewhat in most cases.
1. In the single token mode compare known hashes instead of names, names can
be misleading.
2. Hardcode NEO/GAS, they are special (if not overrided by the wallet data).
We have this data available since 0.99.1 while all public networks require at
least 0.99.2 for compatibility and NeoFS setups use 0.99.2+ too. This data can
simplify account handling considerably making additional requests unneccessary
in many cases.
In the same way we do for NEP-17 tokens. This code predates "getnep11balances"
call, so this wasn't possible back then, but now we can improve the situation
(allow specifying names/symbols instead of hashes only).
Unfortunately Go doesn't allow to easily reuse readers in full packages, still
we can have this wrapper with a little overhead (the alternative is to move
specific methods into types of their own, but I'm not sure how it's going to
be accepted user-side).
Notice that int64 types are used for gas per block or registration price
because the price has to fit into the system fee limitation and gas per block
value can't be more than 10 GAS. We use int64 for votes as well in other types
since NEO is limited to 100M.
See neo-project/neo#2390. Can't see it there? No wonder, that's why we have
this bug for a year and a half. Not critical, we don't care about versions,
but _very_ annoying.
Saving into a file can't be successful without signAndPush flag (wallet
present). This situation can't happen in CLI invocations since
testinvokefunction doesn't have `--out` flag, but still it's a logic
error. Everything else can be simplified a bit taking that into account.
Some commands don't accept arguments, but users try giving them and don't
notice a mistake. It's a bit more user-friendly to tell the user that there is
something wrong with the way he tries to use the command.
It has a stub for SIGHUP, but doesn't have anything for USR1 and USR2:
Error: cli\server\server.go:520:31: undefined: syscall.SIGUSR1
Error: cli\server\server.go:521:31: undefined: syscall.SIGUSR2
Error: cli\server\server.go:565:17: undefined: syscall.SIGUSR1
Error: cli\server\server.go:608:17: undefined: syscall.SIGUSR2
Which allows to enable/disable the service, change nodes, keys and other
settings. Unfortunately, atomic.Value doesn't allow Store(nil), so we have to
store a pointer there that can point to nil interface.
Turns out, our getnextvalidators implementation already works the way
getcandidates is supposed to work, but original getnextvalidators works a bit
differently. It only returns validators, it doesn't return Active flag (all
of them are active) and it represents votes as a number. So for the maximum
compatibility:
* drop non-validator keys from getnextvalidators server-side
* drop Active flag client-side (sorry, it doesn't exist)
* allow unmarshalling old answers along with the new one
This technically breaks `query candidates` CLI command, but it'll be fixed
when getcandidates are to be introduced.
https://github.com/nspcc-dev/neo-go/pull/2435 breaks compatibility
between newer RPC clients and older RPC servers with the following
error:
```
failed to get network magic: json: cannot unmarshal string into Go struct field Protocol.protocol.initialgasdistribution of type int64
```
This behaviour is expected, but we can't allow this radical change.
Thus, the following solution is implemented:
1. RPC server responds with proper non-stringified
InitialGasDistribution value. The value represents an integral
of fixed8 multiplied by the decimals.
2. RPC client is able to distinguish older and newer responses. For
older one the stringified value without decimals part is
expected. For newer responses the int64 value with decimal part
is expected.
The cludge will be present in the code for a while until nodes of
version <=0.98.3 become completely absolete.
Old help message is misleading a bit:
```
OPTIONS:
--verbose, -v Output full tx info and execution logs
--rpc-endpoint value, -r value RPC node address
--timeout value, -s value Timeout for the operation (10 seconds by default) (default: 0s)
```
The new one:
```
OPTIONS:
--verbose, -v Output full tx info and execution logs
--rpc-endpoint value, -r value RPC node address
--timeout value, -s value Timeout for the operation (default: 10s)
```
Note explicitly that transaction hash should be specified, so instead of
an old help text:
```
$ ./bin/neo-go query tx --help
NAME:
neo-go query tx - Query transaction status
USAGE:
neo-go query tx [command options] [arguments...]
OPTIONS:
--verbose, -v Output full tx info and execution logs
--rpc-endpoint value, -r value RPC node address
--timeout value, -s value Timeout for the operation (10 seconds by default) (default: 0s)
```
now we got the more informative one:
```
$ ./bin/neo-go query tx --help
NAME:
neo-go query tx - Query transaction status
USAGE:
neo-go query tx <hash> -r endpoint [-v]
OPTIONS:
--verbose, -v Output full tx info and execution logs
--rpc-endpoint value, -r value RPC node address
--timeout value, -s value Timeout for the operation (10 seconds by default) (default: 0s)
```
1. Keep initDone check only for the places where cache is directly accessed.
We don't need to check it in other places, otherwise we have a mess of
duplicating checks.
2. Fix bug in code related to block deserialisation. There's no magic, so
checking that initialisation is done is not enough for proper block
deserialisation. We need to manually fill StateRootEnabled field.
3. Since transaction doesn't need network magic to compute its hash, we don't
need to perform Client initialisation before transaction-related requests.
4. Check that cache is initialised before accessing network magic.
5. Refactor the way Policy contract hash is fetched for Client requests.
We don't really need Client initialisation for that, it's OK to fetch Policy
hash on-the-fly.
Currently we can't properly stop running server on Windows and SIGHUP
is also not supported. This leads to occupied resources and failed
test cleanup:
```
--- FAIL: TestServerStart (0.35s)
--- FAIL: TestServerStart/good (0.10s)
testing.go:894: TempDir RemoveAll cleanup: remove C:\Users\Anna\AppData\Local\Temp\TestServerStart_good337747932\001\neogotestchain\000001.log:
The process cannot access the file because it is being used by another process.
2022-02-08T14:11:20.959+0300 INFO persisted to disk {"blocks": 0, "keys": 112, "headerHeight": 0, "blockHeight": 0, "took": "10.0049ms"}
```
Blockchain occupies resources (e.g. it opens log files for DB, etc.)
on creation and running. We need to release these resources if something
goes wrong during execution chain-related commands.
This commit solves the following problem on Windows:
```
--- FAIL: TestServerStart (0.32s)
--- FAIL: TestServerStart/stateroot_service_is_on_&&_StateRootInHeader=true (0.04s)
testing.go:894: TempDir RemoveAll cleanup: remove C:\Users\Anna\AppData\Local\Temp\TestServerStart_stateroot_service_is_on_&&_StateRootInHeader=true460557297\001\neogotestchain\000001.log: The process cannot access the file because it is being used by another process.
--- FAIL: TestServerStart/invalid_Oracle_config (0.03s)
testing.go:894: TempDir RemoveAll cleanup: remove C:\Users\Anna\AppData\Local\Temp\TestServerStart_invalid_Oracle_config810064028\001\neogotestchain\000001.log: The process cannot access the file because it is being used by another process.
--- FAIL: TestServerStart/invalid_consensus_config (0.04s)
testing.go:894: TempDir RemoveAll cleanup: remove C:\Users\Anna\AppData\Local\Temp\TestServerStart_invalid_consensus_config217270091\001\neogotestchain\000001.log: The process cannot access the file because it is being used by another process.
--- FAIL: TestServerStart/invalid_Notary_config (0.07s)
--- FAIL: TestServerStart/invalid_Notary_config/malformed_config (0.04s)
testing.go:894: TempDir RemoveAll cleanup: remove C:\Users\Anna\AppData\Local\Temp\TestServerStart_invalid_Notary_config_malformed_config754934830\001\neogotestchain\000001.log: The process cannot access the file because it is being used by another process.
--- FAIL: TestServerStart/invalid_Notary_config/invalid_wallet (0.03s)
testing.go:894: TempDir RemoveAll cleanup: remove C:\Users\Anna\AppData\Local\Temp\TestServerStart_invalid_Notary_config_invalid_wallet934249397\001\neogotestchain\000001.log: The process cannot access the file because it is being used by another process.
--- FAIL: TestServerStart/good (0.11s)
testing.go:894: TempDir RemoveAll cleanup: remove C:\Users\Anna\AppData\Local\Temp\TestServerStart_good596150160\001\neogotestchain\000001.log: The process cannot access the file because it is being used by another process.
```
This commit also unifies blockchain and services releasing code.
zap never closes open sinks except its own tests. This behaviour
prevents TestHandleLoggingParams from successful cleanup because
temp log output file can't be closed due to the following error:
```
TempDir RemoveAll cleanup: remove C:\\Users\\Anna\\AppData\\Local\\Temp\\TestHandleLoggingParams_debug5796883
33\\001\\file.log: The process cannot access the file because it is being used by another process.
```
So this tremendous cludge is made mosetly for our testing code.
It is not for concurrent usage (we don't have cases of
multithreaded access to logger output sink).
Notice that it makes the node accept Extensible payloads with any category
which is the same way C# node works. We're trusting Extensible senders,
improper payloads are harmless until they DoS the network, but we have some
protections against that too (and spamming with proper category doesn't differ
a lot).
Unfortunately, testing this code is not possible without an additional wrapper
in `input`, but adding it just to test this seems to be too excessive. Fixes
Problem:
```
--- FAIL: TestDumpDB (0.08s)
--- FAIL: TestDumpDB/too_low_chain
testing.go:894: TempDir RemoveAll cleanup: remove C:\Users\Anna\AppData\Local\Temp\TestDumpDB_too_low_chain357310492\001\chains\privnet\000001.log: The process cannot access the file because it is being used by another process.
```
Solution:
Release resources occupied by the chain even on non-error command exit.
Problem:
```
--- FAIL: TestHandleLoggingParams (0.02s)
--- FAIL: TestHandleLoggingParams/default (0.00s)
server_test.go:51:
Error Trace: server_test.go:51
Error: Received unexpected error:
couldn't open sink "C:\\Users\\Anna\\AppData\\Local\\Temp\\TestHandleLoggingParams226652490\\001/file.log": no sink found for scheme "c"
Test: TestHandleLoggingParams/default
--- FAIL: TestHandleLoggingParams/debug (0.00s)
server_test.go:64:
Error Trace: server_test.go:64
Error: Received unexpected error:
couldn't open sink "C:\\Users\\Anna\\AppData\\Local\\Temp\\TestHandleLoggingParams226652490\\001/file.log": no sink found for scheme "c"
Test: TestHandleLoggingParams/debug
```
Solution:
Currently no solution is implemented, but we can use relative paths instead of absolute.
We use 2 prefixes for storing items because of state synchronization.
This commit allows to parametrize dao with the default prefix.
Signed-off-by: Evgeniy Stratonikov <evgeniy@nspcc.ru>
It doesn't trigger the problem fixed in the previous commit because here
CreateTxFromScript is used with pre-calculated GAS value from InvokeFunction
and thus it's not performing invocation. Still, it's a nice test extension.
Standards are NEP-11 and NEP-17, not NEP11, not NEP17, not anything
else. Variable/function names of course can use whatever fits, but documents
and comments should be consistent wrt this.
