Convert InteropInterface<IEnumerator> to Array for RpcClient to digest by joeqian10 · Pull Request #245 · neo-project/neo-modules

joeqian10 · 2020-05-19T06:17:36Z

Close #230

Any suggestions and modifications are welcome.

shargon · 2020-05-19T13:10:27Z

src/RpcServer/RpcServer.SmartContract.cs

            return json;
        }

+        public static StackItem[] ConvertIEnumeratorToArray(StackItem[] stackItems)


If we change the items in the same array, we should not return an array, because it's the same.

Tommo-L · 2020-05-20T02:22:06Z

tests/Neo.Network.RPC.Tests/UT_RpcServer.cs

+                12345,
+                "hello",
+                new InteropInterface(new int[] { 1, 2, 3 }.GetEnumerator()),
+                new InteropInterface(new Nep5AccountState[] {new Nep5AccountState()}.GetEnumerator())


Could add a StorageMap type here, which used in nep11, see neo-project/neo-devpack-dotnet#268

Tommo-L · 2020-05-20T02:22:51Z

src/RpcServer/RpcServer.SmartContract.cs

+                        while (enumerator.MoveNext())
+                        {
+                            var current = enumerator.Current;
+                            array.Add(current is StackItem stackItem ? stackItem : current is IInteroperable interoperable ? interoperable.ToStackItem(null) : new InteropInterface(current));


This line is too line to read, it's better to split into multiple lines.

ProDog · 2020-05-26T08:32:02Z

If the contract contains return Enumerator<byte[]> , an exception will be thrown when call invokefunction.

Contract like:

        public static Enumerator<byte[]> GetEnumerator()
        {
            var a = new byte[] { 0x22, 0x32, 0x24 };
            var b = new byte[] { 0x10, 0x11, 0x22 };
            var c = new byte[] { 0x33, 0x44, 0x55 };
            var enumerator = Enumerator<byte[]>.Create(new byte[3][] { a, b, c });
            
            return enumerator;
        }

invokefunction:

{
  "jsonrpc": "2.0",
  "id": 1,
  "method": "invokefunction",
  "params": [
    "0xa02a4bb9dbadb03b41507efe1910ecdfb7d3d146",
    "getEnumerator"
  ]
}

result:

{
    "jsonrpc": "2.0",
    "id": 1,
    "error": {
        "code": -2147024809,
        "message": "StackItemType(Buffer) is not supported to ContractParameter.",
        "data": "   at Neo.VM.Helper.ToParameter(StackItem item, List`1 context) in D:\\Work\\neo_code\\neo\\src\\neo\\VM\\Helper.cs:line 340\r\n   at Neo.VM.Helper.<>c__DisplayClass16_0.<ToParameter>b__0(StackItem p) in D:\\Work\\neo_code\\neo\\src\\neo\\VM\\Helper.cs:line 289\r\n   at System.Linq.Enumerable.SelectEnumerableIterator`2.ToList()\r\n   at System.Linq.Enumerable.ToList[TSource](IEnumerable`1 source)\r\n   at Neo.VM.Helper.ToParameter(StackItem item, List`1 context) in D:\\Work\\neo_code\\neo\\src\\neo\\VM\\Helper.cs:line 289\r\n   at Neo.VM.Helper.ToParameter(StackItem item) in D:\\Work\\neo_code\\neo\\src\\neo\\VM\\Helper.cs:line 271\r\n   at Neo.Plugins.RpcServer.<>c.<GetInvokeResult>b__31_0(StackItem p)\r\n   at System.Linq.Enumerable.SelectArrayIterator`2.MoveNext()\r\n   at System.Collections.Generic.List`1.InsertRange(Int32 index, IEnumerable`1 collection)\r\n   at Neo.IO.Json.JArray..ctor(IEnumerable`1 items) in D:\\Work\\neo_code\\neo\\src\\neo\\IO\\Json\\JArray.cs:line 18\r\n   at Neo.Plugins.RpcServer.GetInvokeResult(Byte[] script, IVerifiable checkWitnessHashes)\r\n   at Neo.Plugins.RpcServer.InvokeFunction(JArray _params)\r\n   at Neo.Plugins.RpcServer.ProcessRequest(HttpContext context, JObject request)"
    }
}

src/RpcServer/RpcServer.SmartContract.cs

Co-authored-by: HaoqiangZhang <gripzhang@outlook.com>

gsmachado · 2020-05-28T10:56:39Z

just to mention that this is pretty important to us since many of our tests are failing at the moment (we had to ignore them) due to this. 😄

joeqian10 · 2020-06-05T08:09:37Z

This PR also closes #254

Now the GetInvokeResult method directly converts a StackItem to json, so there will no longer be ByteArray type in the response.

ProDog · 2020-06-12T07:03:21Z

Please review this PR.@erikzhang

erikzhang

If the enumerator returns too many items, it may cause out of memory exception. That's why we need enumerator instead of using array directly.

erikzhang · 2020-06-14T08:02:14Z

Paging does not solve all problems. Because if the user requests the last page, you have to traverse all the items.

joeqian10 · 2020-06-17T06:14:54Z

@erikzhang @shargon @gsmachado @Tommo-L Got any other ideas? 🤔😂

erikzhang · 2020-06-17T06:29:50Z

Currently the only way I can think of is to modify the API of the contract so that the results returned by the contract support paging natively. But one problem I can't solve this way is that Storage.Find() itself cannot support paging. So the contract should also be powerless.

erikzhang · 2020-06-17T06:31:56Z

So I don't think RPC should do the conversion anymore.

shargon · 2020-06-17T09:12:00Z

Maybe we should limit to the X first registers, and allow more by config. Then you can allow to your own node to return whole data

Tommo-L · 2020-07-07T02:07:26Z

Maybe we should limit to the X first registers, and allow more by config. Then you can allow to your own node to return whole data

I think it's okay.

But one problem I can't solve this way is that Storage.Find() itself cannot support paging.

We can create a StorageOrderedMap, the key of map is prefix + number + key => value, when we want to delete one key:value, we just move the last element to the current position. But the implementation is a little complicated.

shargon · 2020-07-07T07:45:56Z

src/RpcServer/RpcServer.SmartContract.cs

+                        int low = page * ResultsPerPage; // 0, 50, 100...
+                        int high = (page + 1) * ResultsPerPage - 1; // 49, 99, 149...
+                        while (sysEnum.MoveNext() && index <= high)
+                        {


maxPage in config?

Add ConvertIEnumeratorToArray().

ae0c96b

joeqian10 requested review from Tommo-L, erikzhang, shargon and superboyiii May 19, 2020 06:18

Fix format.

bda45f1

shargon reviewed May 19, 2020

View reviewed changes

Tommo-L reviewed May 20, 2020

View reviewed changes

joeqian10 and others added 3 commits May 25, 2020 18:48

Add support for Neo.SmartContract.Enumerators.IEnumerator and UT.

cb16508

Merge branch 'master' into close-230

5cf9eb9

Update RpcServer.SmartContract.cs

7882501

shargon previously approved these changes May 26, 2020

View reviewed changes

ProDog reviewed May 26, 2020

View reviewed changes

src/RpcServer/RpcServer.SmartContract.cs Outdated Show resolved Hide resolved

Convert StackItem directly to JSON.

80fc63e

Co-authored-by: HaoqiangZhang <gripzhang@outlook.com>

joeqian10 dismissed shargon’s stale review via 80fc63e May 26, 2020 09:19

ProDog requested review from Tommo-L and shargon May 28, 2020 14:15

Luchuan added 3 commits May 29, 2020 13:37

optimze

19fb08a

fix conflicts

bd96cec

fix

a376e4d

Tommo-L previously approved these changes May 29, 2020

View reviewed changes

joeqian10 mentioned this pull request Jun 5, 2020

Invokefunction return stackitem isn't in the same type as applicationlogs #254

Closed

Merge branch 'master' into close-230

82ab08a

shargon previously approved these changes Jun 5, 2020

View reviewed changes

Merge branch 'master' into close-230

2ef4f8c

joeqian10 dismissed stale reviews from shargon and Tommo-L via 2ef4f8c June 8, 2020 02:28

shargon previously approved these changes Jun 8, 2020

View reviewed changes

erikzhang suggested changes Jun 12, 2020

View reviewed changes

Add paging when converting

fc7595a

joeqian10 dismissed shargon’s stale review via fc7595a June 13, 2020 10:11

joeqian10 requested review from ProDog, Tommo-L, erikzhang and shargon June 13, 2020 10:16

Fix format

3797021

joeqian10 mentioned this pull request Jun 18, 2020

Convert StackItem directly to json #264

Merged

shargon reviewed Jul 7, 2020

View reviewed changes

erikzhang closed this Apr 17, 2021

Conversation

joeqian10 commented May 19, 2020

Uh oh!

shargon May 19, 2020

Choose a reason for hiding this comment

Uh oh!

Tommo-L May 20, 2020

Choose a reason for hiding this comment

Uh oh!

Tommo-L May 20, 2020

Choose a reason for hiding this comment

Uh oh!

ProDog commented May 26, 2020

Uh oh!

Uh oh!

gsmachado commented May 28, 2020

Uh oh!

joeqian10 commented Jun 5, 2020

Uh oh!

ProDog commented Jun 12, 2020

Uh oh!

erikzhang left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

erikzhang commented Jun 14, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

joeqian10 commented Jun 17, 2020

Uh oh!

erikzhang commented Jun 17, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

erikzhang commented Jun 17, 2020

Uh oh!

shargon commented Jun 17, 2020

Uh oh!

Tommo-L commented Jul 7, 2020

Uh oh!

shargon Jul 7, 2020

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

erikzhang left a comment •

edited

Loading

erikzhang commented Jun 14, 2020 •

edited

Loading

erikzhang commented Jun 17, 2020 •

edited

Loading