Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Split up RPC into separate packages #676

Merged
merged 22 commits into from
Feb 21, 2020
Merged

Split up RPC into separate packages #676

merged 22 commits into from
Feb 21, 2020

Conversation

fyrchik
Copy link
Contributor

@fyrchik fyrchik commented Feb 17, 2020

Closes #510 .

@codecov
Copy link

codecov bot commented Feb 17, 2020

Codecov Report

Merging #676 into master will increase coverage by 0.51%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #676      +/-   ##
==========================================
+ Coverage   65.21%   65.73%   +0.51%     
==========================================
  Files         125      128       +3     
  Lines       10920    10827      -93     
==========================================
- Hits         7122     7117       -5     
+ Misses       3516     3432      -84     
+ Partials      282      278       -4     
Impacted Files Coverage Δ
pkg/rpc/request/params.go 0.00% <0.00%> (ø)
pkg/rpc/request/txBuilder.go 45.73% <0.00%> (ø)
pkg/rpc/response/result/asset_state.go 0.00% <0.00%> (ø)
pkg/rpc/response/result/tx_output.go 0.00% <0.00%> (ø)
pkg/rpc/response/result/account_state.go 0.00% <0.00%> (ø)
pkg/rpc/response/result/contract_state.go 0.00% <0.00%> (ø)
pkg/rpc/response/result/unspents.go 0.00% <0.00%> (ø)
pkg/rpc/response/result/block.go 0.00% <0.00%> (ø)
pkg/rpc/response/result/tx_raw_output.go 0.00% <0.00%> (ø)
pkg/rpc/response/result/peers.go 100.00% <0.00%> (ø)
... and 1 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 f345db5...8da2005. Read the comment docs.

roman-khimov and others added 19 commits February 21, 2020 15:12
Makes no sense copying Server here.
Deduplicate and simplify code.
Which allows node to respond to `getpeers` RPC request correctly.
It's a server implementation detail, it has nothing to do with the data format
itself. It also makes no sense exporing it.
Because it is a result of the RPC call.
These are all RPC call results, `wrappers` package doesn't make much sense to
me.
`response` is to be populated soon.
Mostly as is, no real effort done yet to optimize them, so there are still a
lot of duplicates there, but at least we sort them out into different smaller
packages.
This includes Client struct with RPC methods and
BalanceGetter implementation with NeoSCAN.
sendrawtransaction just returns a bool, sendtoaddress returns a proper
transaction and that should be the same as the one we have in
TransactionOutputRaw.
These might be undefined for mempool transactions, thus they should be defined
as omitempty.
In case of error our own server responds with an HTTP error and proper
JSON-RPC error in the body, so look there first as it has more specific data.
@roman-khimov
Copy link
Member

@fyrchik: take a look at the updated branch, please. I think it's time to merge it.

pkg/rpc/client/rpc.go Outdated Show resolved Hide resolved
Adjust structures accordingly and throw away most of them, they're useless.
We actually do emit it ourselves in String()
We have proper results now, so use those. The only left is Invoke, but that
depends on another issue at the moment.
@fyrchik
Copy link
Contributor Author

fyrchik commented Feb 21, 2020

@roman-khimov LGTM

@roman-khimov roman-khimov merged commit 56f87cd into master Feb 21, 2020
@roman-khimov roman-khimov deleted the dedup-rpc-types branch February 21, 2020 13:22
@roman-khimov roman-khimov mentioned this pull request Apr 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rpc RPC server and client
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Deduplicate RPC types
3 participants