Validate serialization during Contract deploy and Update

[shargon]

Close #2899
Close #2829

[Jim8y]

Can you put 0 before others?

[shargon]

Cloud you check it @cschuchardt88 ? I'm not sure if this solve the problem, because it seems to be serialized as NULL

[cschuchardt88]

{
    "code": -2147467262,
    "name": "InvalidCastException",
    "message": "Specified cast is not valid."
}

if (skip < 1 || take < 1 || take > RestServerSettings.Current.MaxPageSize)
    throw new InvalidParameterRangeException();
var contracts = NativeContract.ContractManagement.ListContracts(_neosystem.StoreView);
if (contracts.Any() == false)
    return NoContent();
var contractRequestList = contracts.OrderBy(o => o.Manifest.Name).Skip((skip - 1) * take).Take(take); // crashes with Manifest.Name
if (contractRequestList.Any() == false)
    return NoContent();
return Ok(contractRequestList);

But ill resync to make sure.

[shargon]

@cschuchardt88 is this your request? to which server?

{
    "id": 1,
    "jsonrpc": "2.0",
    "method": "getstorage",
    "params": ["0x12fbec62fb765ce3f55845106ccf3717716723ad"]
}

[cschuchardt88]

My localhost with your changes and http://seed1t5.neo.org:20332/

But im resyncing to see if still erroring... so be sometime.

[cschuchardt88]

I get

{
    "jsonrpc": "2.0",
    "id": 1,
    "error": {
        "code": -2146233086,
        "message": "Index was out of range. Must be non-negative and less than the size of the collection. (Parameter 'index')",
        "data": "   at Neo.Json.JArray.get_Item(Int32 index)\r\n   at Neo.Plugins.RpcServer.GetStorage(JArray _params)\r\n   at Neo.Plugins.RpcServer.ProcessRequestAsync(HttpContext context, JObject request)"
    }
}

But its not crashing now for var contractRequestList = contracts.OrderBy(o => o.Manifest.Name).Skip((skip - 1) * take).Take(take);

I would say fixed

[shargon]

@cschuchardt88 mmm looks like there are a manifest that it can't deserialize...

[Jim8y]

@shargon

[shargon]

@shargon

We need to find why it fault, I tried to detect it with UT, but @cschuchardt88 still receiving an error.

"error": {
        "code": -2146233086,
        "message": "Index was out of range. Must be non-negative and less than the size of the collection. (Parameter 'index')",
        "data": "   at Neo.Json.JArray.get_Item(Int32 index)\r\n   at Neo.Plugins.RpcServer.GetStorage(JArray _params)\r\n   at Neo.Plugins.RpcServer.ProcessRequestAsync(HttpContext context, JObject request)"
    }

[cschuchardt88]

sorry was an error on my part with the RPC method. Its working now.

[shargon]

{
    "id": 1,
    "jsonrpc": "2.0",
    "method": "getstorage",
    "params": ["0x12fbec62fb765ce3f55845106ccf3717716723ad"]
}

@cschuchardt88 is this your request?

I receive {"jsonrpc":"2.0","id":1,"error":{"code":-2147467262,"message":"Specified cast is not valid."}}

[cschuchardt88]

You have to call it like this: You have to provide the storage key. But storage key is invalid which is fine. Because its calling ListContracts before it returns error.

{
    "id": 1,
    "jsonrpc": "2.0",
    "method": "getstorage",
    "params": ["0x12fbec62fb765ce3f55845106ccf3717716723ad", "MHgxMmZiZWM2MmZiNzY1Y2UzZjU1ODQ1MTA2Y2NmMzcxNzcxNjcyM2Fk"]
}

[Jim8y]

You have to call it like this: You have to provide the storage key. But storage key is invalid which is fine. Because its calling ListContracts before it returns error.

{
    "id": 1,
    "jsonrpc": "2.0",
    "method": "getstorage",
    "params": ["0x12fbec62fb765ce3f55845106ccf3717716723ad", "MHgxMmZiZWM2MmZiNzY1Y2UzZjU1ODQ1MTA2Y2NmMzcxNzcxNjcyM2Fk"]
}

Then i guess we should update the execption messgae then.

[shargon]

getstorage

using https://sync.ngd.network/ testnet data I get:

{
"jsonrpc":"2.0","id":1,"error":{"code":-100,
"message":"Unknown storage",
"data":"   at Neo.Plugins.RpcServer.GetStorage(JArray _params) in C:\\Red4Sec\\Neo\\neo-modules\\src\\RpcServer\\RpcServer.Blockchain.cs:line 204\r\n   at Neo.Plugins.RpcServer.ProcessRequestAsync(HttpContext context, JObject request) in C:\\Red4Sec\\Neo\\neo-modules\\src\\RpcServer\\RpcServer.cs:line 245"}
}

[shargon]

You have to call it like this: You have to provide the storage key. But storage key is invalid which is fine. Because its calling ListContracts before it returns error.

{
    "id": 1,
    "jsonrpc": "2.0",
    "method": "getstorage",
    "params": ["0x12fbec62fb765ce3f55845106ccf3717716723ad", "MHgxMmZiZWM2MmZiNzY1Y2UzZjU1ODQ1MTA2Y2NmMzcxNzcxNjcyM2Fk"]
}

where it calls ListContracts for getStorage? I can't find it...

[shargon]

I can read all of them manually

[cschuchardt88]

https://github.com/neo-project/neo-modules/blob/2d7fb2778ca7ad11815e438534916103859e36ac/src/RpcServer/RpcServer.Blockchain.cs#L188-L206

Looks like it calls GetContract.

But I use ListContracts, In RestServer, that's where i found the problem to begin with. It crashed on ContractState manifest.name. So it's working now.

[shargon]

@superboyiii could you check that tx:0x154f30d743c97d22fb17c5d363040ac4c9d351a1068b77b3ca6a71f91dfdcfc1 fail after this update, the contract is not stored, and no balances are affected?

[AnnaShaleva]

The PR LGTM, and #2829 should be finally fixed with these changes. However, it may affect not only the 0x154f30d743c97d22fb17c5d363040ac4c9d351a1068b77b3ca6a71f91dfdcfc1, but also some other transactions in mainnet, because serialization checks were added everywhere including RuntimeNotify, and we need to check how it affects mainnet.

I'm a little bit worried about 3.6.2. This release supposed to be a quick stabilizing release without significant changes. I would suggest to postpone this PR until 3.7.0 and release 3.6.2 as it is now.

[roman-khimov]

@AnnaShaleva, if we can test it in a few days (for example, before #2829 could celebrate its first year) and finally deal with #2829, it's worth doing that in 3.6.2.

[Jim8y]

@AnnaShaleva, if we can test it in a few days (for example, before #2829 could celebrate its first year) and finally deal with #2829, it's worth doing that in 3.6.2.

Took me a few seconds to understand the first year.

[shargon]

This could affect to other txs

[superboyiii]

@superboyiii could you check that tx:0x154f30d743c97d22fb17c5d363040ac4c9d351a1068b77b3ca6a71f91dfdcfc1 fail after this update, the contract is not stored, and no balances are affected?

Sure

[superboyiii]

@shargon Only 1 storage differed by this PR on Block 2506873
Seems the contract updated by txid 0x154f30d743c97d22fb17c5d363040ac4c9d351a1068b77b3ca6a71f91dfdcfc1 is failed by this PR.

contract: 0xccf264e2e0e3772945146a6f7af268377237186f
But yes, other storage before or after has not been influenced

[shargon]

@shargon Only 1 storage differed by this PR on Block 2506873 Seems the contract updated by txid 0x154f30d743c97d22fb17c5d363040ac4c9d351a1068b77b3ca6a71f91dfdcfc1 is failed by this PR. contract: 0xccf264e2e0e3772945146a6f7af268377237186f But yes, other storage before or after has not been influenced

That is the expected one, isn't it?
For me we can merge and release 3.6.2 @neo-project/core ?