It's slightly less efficient (all comparisons are always made), but for
strings/ints it's negligible performance difference, while the code looks a
tiny bit better.
Signed-off-by: Roman Khimov <roman@nspcc.ru>
"Compare" is almost a standard one now, although math/big uses Cmp for historic
reasons (keys.PublicKey does so too). This also fixes Fixed8 since int64 to int
conversion is lossy.
Signed-off-by: Roman Khimov <roman@nspcc.ru>
* use slices.BinarySearchFunc with its boolean status
* use slices.Insert/slices.Delete, tnis can be a little less efficient, but it
frees the memory faster and this code is more I/O (networking) bound to care
about 1-3%
Signed-off-by: Roman Khimov <roman@nspcc.ru>
Looks like it was wrong since 9592f3e052 because
sort.Search can return an index that is not equal to the target.
slices.BinarySearchFunc can do that too, but it also return a very convenient
boolean status that can be used.
Signed-off-by: Roman Khimov <roman@nspcc.ru>
This also removes bigint.FromBytesUnsigned(), it's not used very often and
it's somewhat misleading in the bigint package (which is supposed to use a
very specific enconding).
Signed-off-by: Roman Khimov <roman@nspcc.ru>
Some clients need more flexible awaiting options (e.g. for short-blocks
networks). The default behaviour is not changed, all exported APIs are
compatible. Ref. https://github.com/nspcc-dev/neofs-node/issues/2864.
Signed-off-by: Anna Shaleva <shaleva.ann@nspcc.ru>
pollTime is never 0 since MillisecondsPerBlock protocol configuration value
is present in `getversion` RPC response since 0.97.3 release. We don't have such
old RPC servers in the network anymore, thus this fallback code may be
safely removed.
Signed-off-by: Anna Shaleva <shaleva.ann@nspcc.ru>
Test coverage is automatically enabled when go test is running with coverage
enabled. It can be disabled for any Executor by using relevant methods.
Coverage is gathered by capturing VM OPs during test contract execution and
mapping them to the contract source code using the DebugInfo information.
Signed-off-by: Slava0135 <super.novalskiy_0135@inbox.ru>
Linter is updated up to v1.60.1, the following issue is fixed:
```
predeclared variable max has same name as predeclared identifier
```
Signed-off-by: Anna Shaleva <shaleva.ann@nspcc.ru>
64 connections is too much for slow GA runners, we have to keep all of
them alive to avoid failures in the test, but it's impossible since
runners are too slow to iterate through all of them in time.
Close#3442.
Signed-off-by: Anna Shaleva <shaleva.ann@nspcc.ru>
1. Bind NewDeployTxBy to Executor to be able to use
(*Executor).AddSystemFee.
2. Replace pre-defined constant deployment fee by calculated one.
This change is needed to be able to properly collect coverage for
_deploy method of a contract via neotest coverage. Ref.
https://github.com/nspcc-dev/neo-go/pull/3462#pullrequestreview-2229601870.
Signed-off-by: Anna Shaleva <shaleva.ann@nspcc.ru>
Starting from b10af1ed31
(*WildPermissionDescs).Add method's call is not enough to construct a
proper restricted permission descriptor, because Wildcard field should
be set properly at the same time. Ref. #3523.
Signed-off-by: Anna Shaleva <shaleva.ann@nspcc.ru>
Metrics should be updated once per action, currently removeInternal is
used by Add and Remove, the first one updates them in the end anyway and
remove should do the same.
Signed-off-by: Roman Khimov <roman@nspcc.ru>
`makeWsRequest` creates a channel for response and waits for it. If between
creating the channel and starting the reading `select` connection is lost
(`writerDone` channel is closed), nothing reads from the channel and a
deadlock appears. Looking at "done" channels when transferring RPC data
solves the issue. Closes#3530.
Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>
This is a bad one.
$ ./bin/neo-go contract testinvokefunction -r https://rpc10.n3.nspcc.ru:10331 0xda65b600f7124ce6c79950c1772a36403104f2be getBlock 5762000
{
"state": "HALT",
"gasconsumed": "202812",
"script": "AtDrVwARwB8MCGdldEJsb2NrDBS+8gQxQDYqd8FQmcfmTBL3ALZl2kFifVtS",
"stack": [
{
"type": "Array",
"value": [
{
"type": "ByteString",
"value": "vq5IPTPEDRhz0JA4cQKIa6/o97pnJt/HfVkDRknd1rg="
},
{
"type": "Integer",
"value": "0"
},
{
"type": "ByteString",
"value": "zFYF3LGaTKdbqVX99shaBUzTq9YjXb0jaPMjk2jdSP4="
},
{
"type": "ByteString",
"value": "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA="
},
{
"type": "Integer",
"value": "1722060076994"
},
{
"type": "Integer",
"value": "5293295626238767595"
},
{
"type": "Integer",
"value": "5762000"
},
{
"type": "ByteString",
"value": "LIt05Fpxhl/kXMX3EAGIASyOSQs="
},
{
"type": "Integer",
"value": "0"
}
]
}
],
"exception": null,
"notifications": []
}
$ ./bin/neo-go contract testinvokefunction -r http://seed3.neo.org:10332 0xda65b600f7124ce6c79950c1772a36403104f2be getBlock 5762000
{
"state": "HALT",
"gasconsumed": "202812",
"script": "AtDrVwARwB8MCGdldEJsb2NrDBS+8gQxQDYqd8FQmcfmTBL3ALZl2kFifVtS",
"stack": [
{
"type": "Array",
"value": [
{
"type": "ByteString",
"value": "vq5IPTPEDRhz0JA4cQKIa6/o97pnJt/HfVkDRknd1rg="
},
{
"type": "Integer",
"value": "0"
},
{
"type": "ByteString",
"value": "zFYF3LGaTKdbqVX99shaBUzTq9YjXb0jaPMjk2jdSP4="
},
{
"type": "ByteString",
"value": "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA="
},
{
"type": "Integer",
"value": "1722060076994"
},
{
"type": "Integer",
"value": "5293295626238767595"
},
{
"type": "Integer",
"value": "5762000"
},
{
"type": "Integer",
"value": "6"
},
{
"type": "ByteString",
"value": "LIt05Fpxhl/kXMX3EAGIASyOSQs="
},
{
"type": "Integer",
"value": "0"
}
]
}
],
"exception": null,
"notifications": []
}
9 fields vs 10, notice the primary index right after the block number.
Back when ac527650eb initially added Ledger I've
used https://github.com/neo-project/neo/pull/2215 as a reference and it was
correct (no primary index). But then https://github.com/neo-project/neo/pull/2296
came into the C# codebase and while it looked like a pure refactoring it
actually did add the primary index as well and this wasn't noticed. It wasn't
noticed even when 3a4e0caeb8 had touched some
nearby code. In short, we had a completely wrong implementation of this call
for more than three years. But looks like it's not a very popular one.
Signed-off-by: Roman Khimov <roman@nspcc.ru>
Every client's (Un)Subscription call does two things: an RPC call and a
subscription map lock (two of maps currently). If we imagine that there is
one routine that tries to subscribe (A) and one routine that tries to
unsubscribe (B), the following sequence can happen:
0. Current number of subscriptions is X
1. B does an RPC and makes number of subscriptions X-1
2. A does an RPC and makes number of subscriptions X again
3. A holds subscription locks and rewrites client's subscription state
(subscription with ID X now points to a different channel; channel that
was registered by B is lost and is not related to any real subscription
but is still included in the `receivers` map)
4. B holds subscription locks and drops subscription X (first, it is an
error and we have just lost a subscription that we think was made
successfully second, we have lost a channel in the `receivers` map, and
no corresponding subscription points to it)
5. X subscription is received by the WS client (in practice it is a new
block, 100ms, quite often to be sure this issue happens every hour), we
range through the receivers, see no corresponding subscription, and
panic.
Closes#3093.
Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>
Refs. #3522. The core problem is the same as for groups/features: we can't
allow empty trusts when they're unmarshalled from JSON. But unlike others we
can't easily differentiate missing any value with other cases because the
default value for WildPermissionDescs is a valid thing. Adding an additional
field makes it invalid and we can build around it. Other options are
implementing custom UnmarshalJSON for Manifest (too much for this) or making
Trusts a pointer (an option, but can fail in too many ways).
Signed-off-by: Roman Khimov <roman@nspcc.ru>
It's needed to give user a hint about what's wrong with the witness
during `calculatenetworkfee` RPC request processing.
Signed-off-by: Anna Shaleva <shaleva.ann@nspcc.ru>
This code was accidentally removed by
https://github.com/nspcc-dev/neo-go/pull/3477, it's important to have
these fields set by default.
Signed-off-by: Anna Shaleva <shaleva.ann@nspcc.ru>
This commit adds a single test that covers the
previously uncovered branch in the mptdata
decoding algorithm.
Signed-off-by: Furetur <furetur@gmail.com>
It should be sufficient to retrieve next block validators once per
updateExtensibleWhitelist call and then reuse this value. `nextVals`
copy intentionally omitted since the only change that
smartcontract.CreateDefaultMultiSigRedeemScript performs over the
`nextVals` list is sorting.
Signed-off-by: Anna Shaleva <shaleva.ann@nspcc.ru>
Refs #3415
This commit introduces a small new change
that implements the Hooks API and more
specifically the OnExecHook. This feature
can be used to implement test coverage
collection, tracing, breakpoints, and etc.
To be more specific, this commit:
1. adds a new `hooks` field to the `VM`
(this field contains the OnExecHook
function)
2. sets the default value of this hook
to be a NOP function
3. adds the `VM.SetOnExecHook` method
Signed-off-by: Furetur <furetur@gmail.com>
If `config-path` is not passed, default configs are used according to
the set network. In VM CLI the default privnet config with InMemory db
is used.
Close#3450Close#3459
Signed-off-by: Ekaterina Pavlova <ekt@morphbits.io>
Signers are very important for notary checks and keeping/passing an additional
copy of them is very inconvenient. Exposing them from invoker makes them
available in actors too.
Signed-off-by: Roman Khimov <roman@nspcc.ru>
Most of the time people are interested in successful executions. Unfortunately,
unwrap package can't help here because of a different result structure (some
interface abstract can help, but it's still mostly stack-oriented and sessions
can be a problem), so this additional interface is needed.
Signed-off-by: Roman Khimov <roman@nspcc.ru>
Add a note about System.Runtime.GetNotifications refcounting to Domovoi
hardfork. Ref. https://github.com/neo-project/neo/pull/3301 and
https://github.com/nspcc-dev/neo-go/pull/3485.
Although NeoGo doesn't have anything to be updated, there's a
behaviour difference between C# and Go nodes before Domovoi hardfork, it
deserves a comment.
Signed-off-by: Anna Shaleva <shaleva.ann@nspcc.ru>
It's not correct to use an updated contract state got from Management to
check for the allowed method call. We need to use manifest from the
currently executing context for that. It may be critical for cases when
executing contract is being updated firstly, and after that calls
another contract. So we need an old (executing) contract manifest for
this check.
This change likely does not affect the mainnet's state since it's hard
to meet the trigger criteria, but I'd put it under the hardfork anyway.
Ref. https://github.com/neo-project/neo/pull/3290.
Signed-off-by: Anna Shaleva <shaleva.ann@nspcc.ru>
This test ensures that NeoGo node doesn't have the DeepCopy problem
described in https://github.com/neo-project/neo/issues/3300 and fixed in
https://github.com/neo-project/neo/pull/3301. This problem leads to the
fact that Notifications items are not being properly refcounted by C#
node which leads to possibility to build an enormously large object on
stack. Go node doesn't have this problem.
The reason (at least, as I understand it) is in the fact that C# node
performs objects refcounting inside the DeepCopy even if the object
itself is not yet on stack. I.e. System.Runtime.Notify handler
immediately adds references to the notification argumetns inside
DeepCopy:
b1d27f0189/src/Neo.VM/Types/Array.cs (L108)b1d27f0189/src/Neo.VM/Types/Array.cs (L75)
Whereas Go node just performs the honest DeepCopy without references counting:
b66cea5ccc/pkg/vm/stackitem/item.go (L1223)
Going further, C# node clears refs for notification arguments (for array
and underlying array items). System.Runtime.GetNotifications pushes the
notificaiton args array back on stack and increments counter only for
the external array, not for its arguments. Which results in negative
refcounter once notificaiton is removed from the stack. The fix itself
(f471c0542d/src/Neo/SmartContract/NotifyEventArgs.cs (L84))
doesn't need to be ported to NeoGo because Go node adds object to the
refcounter only at the moment when it's being pushed to stack by
System.Runtime.GetNotifications handler. This object is treated as new
object since it was deepcopied earlier by System.Runtime.Notify handler:
b66cea5ccc/pkg/vm/stack.go (L178).
Thus, no functoinal changes from the NeoGo side. And we won't
intentionally break our node to follow C# pre-Domovoi invalid behaviour.
Close#3484, close#3482.
Signed-off-by: Anna Shaleva <shaleva.ann@nspcc.ru>
