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>
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>
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>
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>
Do not use the updated contract state from native Management to perform
permissions checks. We need to use the currently executing state
instead got from the currently executing VM context until context is
unloaded.
Close#3471.
Signed-off-by: Anna Shaleva <shaleva.ann@nspcc.ru>
Account is blocked when it's in the Policy's storage, not when it's
missing from the Policy storage. Introduced in
bbbc6805a8.
This bug leads to the fact that during native Neo cache initialization
at the last block in the dBFT epoch, all candidates accounts are
"blocked", and thus, stand-by committee and validators are used in the
subsequent new epoch. Close#3424.
This bug may lead to the consequences described in #3273, but it needs
to be confirmed.
Signed-off-by: Anna Shaleva <shaleva.ann@nspcc.ru>
It's possible for transaction to include block hash into Conflicts
attribure. If so, then we must not remove block executable record while
cleaning transation's conflict records.
This commit is a direct consequence of
e6ceee0f230a21c87006a9297636be29c0d8ea47. Ref. #3427.
Signed-off-by: Anna Shaleva <shaleva.ann@nspcc.ru>
Conflicts-related code contains more and more these magic numbers, and
there's no good in it even if all the usages are commented. This
approach produces bugs like #3426.
No functional changes, just a refactoring.
Signed-off-by: Anna Shaleva <shaleva.ann@nspcc.ru>
Conflict record stub has value of 5 bytes length: 1 byte for
storage.ExecTransaction prefix and 4 bytes for the block index LE. This
scheme was implemented in #3138, and this commit should be a part of
this PR.
Also, transaction.DummyVersion is removed since it's unused anymore.
Close#3426. The reason of `failed to locate application log: EOF` error
during genesis AER request is in the following: genesis executable was
overwritten by conflict record stub produced by transaction
0x289c235dcdab8be7426d05f0fbb5e86c619f81481ea136493fa95deee5dbb7cc (ref.
#3427). As a consequence, an attempt to decode transaction AER was
initited, but conflict record scheme was changed in #3138.
Signed-off-by: Anna Shaleva <shaleva.ann@nspcc.ru>
Transaction
0x289c235dcdab8be7426d05f0fbb5e86c619f81481ea136493fa95deee5dbb7cc is
already on mainnet at block 5272006 and we can't do anything with it.
This transaction has genesis block hash in Conflicts attribute. It leads
to the following consequences:
1. Genesis block executable record is overwritten by conflict record
stub. Genesis block can't be retrieved anymore. This bug is described
in #3427.
2. Somehow this transaction has passed verification on NeoGo CN without
any warnings:
```
Apr 24 16:12:30 kangra neo-go[2453907]: 2024-04-24T16:12:30.865+0300 INFO initializing dbft {"height": 5272006, "view": 0, "index": 6, "role": "Backup"}
Apr 24 16:12:31 kangra neo-go[2453907]: 2024-04-24T16:12:31.245+0300 INFO persisted to disk {"blocks": 1, "keys": 37, "headerHeight": 5272005, "blockHeight": 5272005, "took": "14.548903ms"}
Apr 24 16:12:34 kangra neo-go[2453907]: 2024-04-24T16:12:34.977+0300 ERROR can't add SV-signed state root {"error": "stateroot mismatch at block 5272005: 9d5f95784f26c862d6f889f213aad1e3330611880c02330e88db8802c750aa46 vs d25304d518645df725014897d13bbf023919928e79074abcea48f31cf9f32a25"}
Apr 24 16:12:45 kangra neo-go[2453907]: 2024-04-24T16:12:45.820+0300 INFO received PrepareRequest {"validator": 5, "tx": 1}
Apr 24 16:12:45 kangra neo-go[2453907]: 2024-04-24T16:12:45.821+0300 INFO sending PrepareResponse {"height": 5272006, "view": 0}
Apr 24 16:12:45 kangra neo-go[2453907]: 2024-04-24T16:12:45.827+0300 INFO received PrepareResponse {"validator": 4}
Apr 24 16:12:45 kangra neo-go[2453907]: 2024-04-24T16:12:45.830+0300 INFO received PrepareResponse {"validator": 3}
Apr 24 16:12:45 kangra neo-go[2453907]: 2024-04-24T16:12:45.875+0300 INFO received PrepareResponse {"validator": 2}
Apr 24 16:12:45 kangra neo-go[2453907]: 2024-04-24T16:12:45.878+0300 INFO sending Commit {"height": 5272006, "view": 0}
Apr 24 16:12:45 kangra neo-go[2453907]: 2024-04-24T16:12:45.879+0300 INFO received Commit {"validator": 4}
Apr 24 16:12:45 kangra neo-go[2453907]: 2024-04-24T16:12:45.881+0300 INFO received PrepareResponse {"validator": 0}
Apr 24 16:12:45 kangra neo-go[2453907]: 2024-04-24T16:12:45.881+0300 INFO received Commit {"validator": 3}
Apr 24 16:12:45 kangra neo-go[2453907]: 2024-04-24T16:12:45.906+0300 INFO received Commit {"validator": 0}
Apr 24 16:12:45 kangra neo-go[2453907]: 2024-04-24T16:12:45.907+0300 INFO received PrepareResponse {"validator": 1}
Apr 24 16:12:45 kangra neo-go[2453907]: 2024-04-24T16:12:45.915+0300 INFO received Commit {"validator": 1}
Apr 24 16:12:45 kangra neo-go[2453907]: 2024-04-24T16:12:45.915+0300 INFO approving block {"height": 5272006, "hash": "6b111519537343ce579d04ccad71c43318b12c680d0f374dfcd466aa22643fb6", "tx_count": 1, "merkle": "ccb7dbe5ee5da93f4936a11e48819f616ce8b5fbf0056d42e78babcd5d239c28", "prev": "12ad6cc5d0cd357b9fc9fb0c1a016ba8014d3cdd5a96818598e6a40a1a4a2a21"}
Apr 24 16:12:45 kangra neo-go[2453907]: 2024-04-24T16:12:45.917+0300 WARN contract invocation failed {"tx": "289c235dcdab8be7426d05f0fbb5e86c619f81481ea136493fa95deee5dbb7cc", "block": 5272006, "error": "at instruction 86 (ASSERT): ASSERT failed"}
Apr 24 16:12:45 kangra neo-go[2453907]: 2024-04-24T16:12:45.950+0300 INFO initializing dbft {"height": 5272007, "view": 0, "index": 6, "role": "Primary"}
Apr 24 16:12:46 kangra neo-go[2453907]: 2024-04-24T16:12:46.256+0300 INFO persisted to disk {"blocks": 1, "keys": 67, "headerHeight": 5272006, "blockHeight": 5272006, "took": "16.576594ms"}
```
And thus, we must treat this transaction as valid for this behaviour
to be reproducable.
This commit contains two fixes:
1. Do not overwrite block executable records by conflict record stubs.
If some transaction conflicts with block, then just skip the conflict
record stub for this attribute since it's impossible to create
transaction with the same hash.
2. Do not fail verification for those transactions that have Conflicts
attribute with block hash inside. This one is controversial, but we
have to adjust this code to treat already accepted transaction as
valid.
Close#3427.
The transaction itself:
```
{
"id" : 1,
"jsonrpc" : "2.0",
"result" : {
"attributes" : [
{
"height" : 0,
"type" : "NotValidBefore"
},
{
"hash" : "0x1f4d1defa46faa5e7b9b8d3f79a06bec777d7c26c4aa5f6f5899a291daa87c15",
"type" : "Conflicts"
}
],
"blockhash" : "0xb63f6422aa66d4fc4d370f0d682cb11833c471adcc049d57ce4373531915116b",
"blocktime" : 1713964365700,
"confirmations" : 108335,
"hash" : "0x289c235dcdab8be7426d05f0fbb5e86c619f81481ea136493fa95deee5dbb7cc",
"netfee" : "237904",
"nonce" : 0,
"script" : "CxAMFIPvkoyXujYCRmgq9qEfMJQ4wNveDBSD75KMl7o2AkZoKvahHzCUOMDb3hTAHwwIdHJhbnNmZXIMFPVj6kC8KD1NDgXEjqMFs/Kgc0DvQWJ9W1I5",
"sender" : "NbcGB1tBEGM5MfhNbDAimvpJKzvVjLQ3jW",
"signers" : [
{
"account" : "0x649ca095e38a790d6c15ff78e0c6175099b428ac",
"scopes" : "None"
},
{
"account" : "0xdedbc03894301fa1f62a68460236ba978c92ef83",
"scopes" : "None"
}
],
"size" : 412,
"sysfee" : "997778",
"validuntilblock" : 5277629,
"version" : 0,
"vmstate" : "FAULT",
"witnesses" : [
{
"invocation" : "DECw8XNuyRg5vPeHxisQXlZ7VYNDxxK4xEm8zwpPyWJSSu+JaRKQxdrlPkXxXj34wc4ZSrZvKICGgPFE0ZHXhLPo",
"verification" : "DCEC+PI2tRSlp0wGwnjRuQdWdI0tBXNS7SlzSBBHFsaKUsdBVuezJw=="
},
{
"invocation" : "DEAxwi97t+rg9RsccOUzdJTJK7idbR7uUqQp0/0/ob9FbuW/tFius3/FOi82PDZtwdhk7s7KiNM/pU7vZLsgIbM0",
"verification" : "DCEDbInkzF5llzmgljE4HSMvtrNgPaz73XO5wgVJXLHNLXRBVuezJw=="
}
]
}
}
```
Signed-off-by: Anna Shaleva <shaleva.ann@nspcc.ru>
These warnings must be monitored by developers since it might be a sign
of behaviour difference between Go and C# nodes.
Signed-off-by: Anna Shaleva <shaleva.ann@nspcc.ru>
Make the script a bit shorter. ABORTMSG would cost a bit more.
Signed-off-by: Roman Khimov <roman@nspcc.ru>
Signed-off-by: Anna Shaleva <shaleva.ann@nspcc.ru>
1. Make prologue be exactly the same as regular CheckMultisig.
2. But instead of "SYSCALL System.Crypto.CheckMultisig" do INITSLOT and K check.
3. This makes all of the code from INITSLOT below be independent of N/M, so
one can parse the script beginning in the same way CheckMultisig is parsed and
then just compare the rest of it with some known-good blob.
4. The script becomes a tiny bit larger now, but properties above are too good.
Signed-off-by: Roman Khimov <roman@nspcc.ru>
Signed-off-by: Anna Shaleva <shaleva.ann@nspcc.ru>
Value calculated by calculatenetworkfee is enough to pass the real
tx verification. However, network fee may be decreased, so calculations
are not quite accurate. Need to investigate, why.
Signed-off-by: Anna Shaleva <shaleva.ann@nspcc.ru>
Replace native CryptoLib's verifyWithECDsa `curve` parameter by
`curveHash` parameter which is a enum over supported pairs of named
curves and hash functions.
Even though this change is a compatible extension of the protocol, it
changes the genesis state due to parameter renaming. But we're going to
resync chain in 3.7 release anyway, so it's not a big deal.
Also, we need to check mainnet and testnet compatibility in case if
anyone has ever called verifyWithECDsa with 24 or 25 `curve` value.
Signed-off-by: Anna Shaleva <shaleva.ann@nspcc.ru>
