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>
It's dangerous to change `Max*` ProtocolConfiguration settings:
* Changes in MaxBlockSize, MaxBlockSystemFee and MaxTransactionsPerBlock
may lead to the fact that accepted block or transaction becomes invalid.
I agree that these settings are not written in the DB, but at the same
time it's not correct to compare databases that have these settings
mismatched.
* Changes in MaxTraceableBlocks may lead to the fact that some
transaction will be processed differently, it's a possible contract
state mismatch.
* Changes in MaxValidUntilBlockIncrement may lead to the fact that
`setMaxNotValidBeforeDelta` method of native Notary contract may be
processed in a different way which is also a possible contract state
mismatch.
Ref. 5d29a3fdab.
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>
We want to cut the tail more aggressively, what matters is state, but not
old blocks and transactions that contracts don't use in any way. #3493
suggests that it's safe to limit the tail to one day, but let's be a bit
more conservative for now.
As a sidenote, EVM only allows to fetch things from the recent 256 blocks.
Signed-off-by: Roman Khimov <roman@nspcc.ru>
