Add NEO SDK RPC module by chenquanyu · Pull Request #850 · neo-project/neo

chenquanyu · 2019-06-21T07:45:42Z

#849 NEO SDK based on RPC interfaces
Add NEO SDK RPC module
This PR contains the rpc client module for neo3 and the unit tests for all the methods

Changed for neo 3.0, not final yet.

sync with neo-project

codecov-io · 2019-06-21T07:54:28Z

Codecov Report

Merging #850 into master will increase coverage by 1.03%.
The diff coverage is 76.19%.

@@            Coverage Diff             @@
##           master     #850      +/-   ##
==========================================
+ Coverage   52.59%   53.62%   +1.03%     
==========================================
  Files         179      193      +14     
  Lines       12707    13211     +504     
==========================================
+ Hits         6683     7085     +402     
- Misses       6024     6126     +102

Impacted Files	Coverage Δ
neo/Network/RPC/Models/RpcNep5Balances.cs	`0% <0%> (ø)`
neo/Network/P2P/Payloads/TransactionAttribute.cs	`21.73% <0%> (-7.68%)`	⬇️
neo/Network/P2P/Payloads/Header.cs	`58.97% <100%> (+7.45%)`	⬆️
neo/Network/P2P/Payloads/Witness.cs	`100% <100%> (ø)`	⬆️
neo/Network/P2P/Payloads/BlockBase.cs	`75% <100%> (+3%)`	⬆️
neo/Network/P2P/Payloads/Block.cs	`89.77% <100%> (+1.02%)`	⬆️
neo/Network/RPC/Models/RpcPlugin.cs	`100% <100%> (ø)`
neo/Network/RPC/Models/RpcTransaction.cs	`100% <100%> (ø)`
neo/Network/RPC/Models/RpcValidateAddressResult.cs	`100% <100%> (ø)`
neo/Network/RPC/Models/RpcInvokeResult.cs	`100% <100%> (ø)`
... and 30 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9e2faf5...eee837a. Read the comment docs.

neo/SDK/RPC/Model/GetBlockHeader.cs

erikzhang · 2019-06-21T08:48:39Z

We can move most of the classes to the Neo.Network.RPC namespace. And, do we really need the models? Such as Model/GetBlock.cs, Model/GetBlockHeader.cs. Can we just use Block and Header?

chenquanyu · 2019-06-21T09:06:27Z

We can move most of the classes to the Neo.Network.RPC namespace. And, do we really need the models? Such as Model/GetBlock.cs, Model/GetBlockHeader.cs. Can we just use Block and Header?

Yes, we can inherit from 'Block' to add property "confirmations" and "nextblockhash".
I will make up FromJson method for the existing models.

vncoelho

It looks clear, nice design.

I think @jsolman will like.
What do you think @igormcoelho?

neo/SDK/RPC/RpcClient.cs

igormcoelho

Nice work being done here... SDK is fundamental for any dev platform.
I recommend renaming quite a few things... GetBlock for example could be ModelBlock or SDKBlock, or in the worst case, Block using different namespace.. but it's confusing. This applies to all other classes.

erikzhang · 2019-07-25T08:02:34Z

neo.UnitTests/SDK/UT_RpcClient.cs

+using System.Threading;
+using System.Threading.Tasks;
+
+namespace Neo.UnitTests.SDK


The RpcClient has been moved to Neo.Network.RPC. So this namespace should be changed too.

Put it the same place with others

erikzhang · 2019-07-25T08:04:31Z

neo/Network/RPC/HttpService.cs

+
+namespace Neo.Network.RPC
+{
+    public class HttpService : IDisposable


I think we can remove this class, and move HttpClient into RpcClient.

erikzhang · 2019-07-25T08:04:52Z

neo/Network/RPC/IRpcClient.cs

+    /// <summary>
+    /// Wrappar of NEO RPC APIs
+    /// </summary>
+    public interface IRpcClient


Why do we need this interface?

erikzhang · 2019-07-25T08:05:11Z

neo/Network/RPC/Model/RpcBlock.cs

+using Neo.IO.Json;
+using Neo.Network.P2P.Payloads;
+
+namespace Neo.Network.RPC.Model


Suggested change

namespace Neo.Network.RPC.Model

namespace Neo.Network.RPC.Models

And many other classes are the same.

erikzhang · 2019-07-25T08:06:38Z

neo/Network/RPC/RpcServer.cs

        }

-        private static JObject CreateErrorResponse(JObject id, int code, string message, JObject data = null)
+        internal static JObject CreateErrorResponse(JObject id, int code, string message, JObject data = null)


Why internal?

This method is covered in unit test.

#943 (comment)

According to the discussion here, we don't need to test private methods. Testing for public methods can completely cover private methods.

shargon · 2019-07-25T08:14:47Z

neo.UnitTests/SDK/UT_RpcClient.cs

+        {
+            // create block
+            var block = new Block();
+            TestUtils.SetupBlockWithValues(block, UInt256.Zero, out UInt256 merkRootVal, out UInt160 val160, out uint timestampVal, out uint indexVal, out Witness scriptVal, out Transaction[] transactionsVal, 0);


should be ulong now, timestamp is in ms

changed👍

Dev

longfeiWan9 · 2019-07-25T09:00:27Z

@erikzhang should we open a new branch for all NEO3 SDK functions?

RPC module is only the first module for SDK. We will also need to add wallet, transaction and smart contract invocation supports which gets data via RPC request instead of data from full node.
With a new branch, we can avoid adding unfinished/partial SDK function into master branch.
We can keep merging master into this new branch constantly, so we can avoid conflicts when we finally merge mature SDK branch back to master.

erikzhang · 2019-07-25T09:04:42Z

should we open a new branch for all NEO3 SDK functions?

No. I don't think it's necessary.

PR correction

chenquanyu · 2019-07-25T09:54:30Z

We won't have REST in this PR, right?

No, the REST is more about RpcServer reconstruction, we won't have it in this PR.

longfeiWan9 · 2019-07-30T01:51:59Z

hi, all.
Any outstanding issues or updates for this pr ? We are waiting this PR to be merged, so we can move forward with other modules for SDK functions.

erikzhang · 2019-07-30T05:28:07Z

neo.UnitTests/UT_RpcClient.cs

+namespace Neo.UnitTests
+{
+    [TestClass]
+    public class UT_RpcClient


Should this file be moved into a sub folder?

Yes, I have moved the file to Network/RPC folder

codecov-io · 2019-07-30T05:42:03Z

Codecov Report

Merging #850 into master will increase coverage by 1.02%.
The diff coverage is 76.19%.

@@            Coverage Diff             @@
##           master     #850      +/-   ##
==========================================
+ Coverage   52.74%   53.76%   +1.02%     
==========================================
  Files         180      194      +14     
  Lines       12749    13253     +504     
==========================================
+ Hits         6724     7126     +402     
- Misses       6025     6127     +102

Impacted Files	Coverage Δ
neo/Network/RPC/Models/RpcNep5Balances.cs	`0% <0%> (ø)`
neo/Network/P2P/Payloads/TransactionAttribute.cs	`21.73% <0%> (-7.68%)`	⬇️
neo/Network/P2P/Payloads/Header.cs	`58.97% <100%> (+7.45%)`	⬆️
neo/Network/P2P/Payloads/Witness.cs	`100% <100%> (ø)`	⬆️
neo/Network/P2P/Payloads/BlockBase.cs	`75% <100%> (+3%)`	⬆️
neo/Network/P2P/Payloads/Block.cs	`89.77% <100%> (+1.02%)`	⬆️
neo/Network/P2P/Payloads/ConsensusData.cs	`100% <100%> (ø)`	⬆️
neo/Network/RPC/Models/RpcTransaction.cs	`100% <100%> (ø)`
neo/Network/RPC/Models/RpcValidateAddressResult.cs	`100% <100%> (ø)`
neo/Network/RPC/Models/RpcInvokeResult.cs	`100% <100%> (ø)`
... and 30 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 547234a...c5c0de1. Read the comment docs.

vncoelho

For maintenance it looks better. A great job!
Hope it also give all the flexibility you want for the SDK RPC module.

* Add NEO SDK based on RPC client * add rpc interface methods for neo3 * update unit test * add unit test * Update TransactionHelper.cs Changed for neo 3.0, not final yet. * implement sdk rpc client methods * backup files * change class name * remove uncompleted modules for pull request * change json deserialize method with Neo JObject * modified JSON implementation, added FromJson() * more RPC change * PR correction * RPC module fix, remove newton.json * fix * fix getblock issue * PR correction * PR Correction * PR Correction: rename RPC models * PR Correction * resolve conflicts * Clean code * Clean code * Clean code * Clean code * Update RpcValidateAddressResult.cs * Clean code * PR correction * Move test file to the right place

chenquanyu and others added 10 commits June 17, 2019 14:26

Add NEO SDK based on RPC client

03c9a20

add rpc interface methods for neo3

4f84a63

update unit test

16c1471

add unit test

79523dd

Update TransactionHelper.cs

fe06992

Changed for neo 3.0, not final yet.

implement sdk rpc client methods

b5b2cd6

backup files

efd27a8

change class name

c78e965

Merge remote-tracking branch 'erik/master' into dev

cf7a184

sync with neo-project

remove uncompleted modules for pull request

66b269c

shargon reviewed Jun 21, 2019

View reviewed changes

neo/SDK/RPC/Model/GetBlockHeader.cs Outdated Show resolved Hide resolved

joeqian10 and others added 2 commits June 21, 2019 17:15

Merge branch 'master' of https://github.com/chenquanyu/neo

26b4334

Merge branch 'master' into dev

8c0c37f

vncoelho reviewed Jun 21, 2019

View reviewed changes

neo/SDK/RPC/RpcClient.cs Outdated Show resolved Hide resolved

igormcoelho reviewed Jun 21, 2019

View reviewed changes

chenquanyu and others added 11 commits June 24, 2019 10:13

Merge branch 'master' into dev

b2705be

change json deserialize method with Neo JObject

3ae6480

Merge remote-tracking branch 'erik/master'

5de69a0

modified JSON implementation, added FromJson()

ca1048e

Merge remote-tracking branch 'erik/master'

af6d7e0

more RPC change

2f58594

PR correction

d5fa4a7

RPC module fix, remove newton.json

90b225c

fix

a08d5e9

Merge remote-tracking branch 'erik/master'

238abd5

Merge branch 'dev' into master

7c193b5

shargon added 3 commits July 25, 2019 09:50

Update RpcValidateAddressResult.cs

4bc742d

Clean code

b958970

Merge branch 'master' into dev

ea0b6b4

shargon requested a review from erikzhang July 25, 2019 07:59

erikzhang reviewed Jul 25, 2019

View reviewed changes

shargon reviewed Jul 25, 2019

View reviewed changes

Merge pull request #8 from chenquanyu/dev

b01b353

Dev

chenquanyu added 2 commits July 25, 2019 17:22

PR correction

0d9b99a

Merge pull request #9 from chenquanyu/master

4d3e3a2

PR correction

Merge branch 'master' into dev

73a53ac

chenquanyu added 3 commits July 30, 2019 11:01

Merge branch 'master' into dev

eee837a

Merge remote-tracking branch 'erik/master' into dev

7da09eb

Merge branch 'dev'

2214846

erikzhang previously approved these changes Jul 30, 2019

View reviewed changes

Merge branch 'master' into dev

51e30f3

erikzhang reviewed Jul 30, 2019

View reviewed changes

chenquanyu added 2 commits July 30, 2019 13:52

Merge branch 'dev' of https://github.com/chenquanyu/neo into dev

881af0b

Move test file to the right place

b7403ec

chenquanyu dismissed erikzhang’s stale review via b7403ec July 30, 2019 05:55

Merge branch 'master' into dev

c5c0de1

erikzhang approved these changes Jul 30, 2019

View reviewed changes

vncoelho approved these changes Jul 30, 2019

View reviewed changes

vncoelho merged commit cd99489 into neo-project:master Jul 30, 2019

Thacryba pushed a commit to simplitech/neo that referenced this pull request Feb 17, 2020

Fix typo & improve title (neo-project#850)

64fddde

	namespace Neo.Network.RPC.Model
	namespace Neo.Network.RPC.Models

Conversation

chenquanyu commented Jun 21, 2019

Uh oh!

codecov-io commented Jun 21, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

erikzhang commented Jun 21, 2019

Uh oh!

chenquanyu commented Jun 21, 2019 • edited by erikzhang Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vncoelho left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

igormcoelho left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shargon Jul 25, 2019 • 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!

Choose a reason for hiding this comment

Uh oh!

longfeiWan9 commented Jul 25, 2019

Uh oh!

erikzhang commented Jul 25, 2019

Uh oh!

chenquanyu commented Jul 25, 2019 • edited by erikzhang Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

longfeiWan9 commented Jul 30, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov-io commented Jul 30, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

vncoelho left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

codecov-io commented Jun 21, 2019 •

edited

Loading

chenquanyu commented Jun 21, 2019 •

edited by erikzhang

Loading

shargon Jul 25, 2019 •

edited

Loading

chenquanyu commented Jul 25, 2019 •

edited by erikzhang

Loading

codecov-io commented Jul 30, 2019 •

edited

Loading