Create ScillaIPCServer #1725
Create ScillaIPCServer #1725
Conversation
Codecov Report
@@ Coverage Diff @@
## feature/contractmap #1725 +/- ##
======================================================
Coverage ? 30.91%
======================================================
Files ? 267
Lines ? 31865
Branches ? 0
======================================================
Hits ? 9852
Misses ? 22013
Partials ? 0
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Look good to me so far. Please wait for @kaikawaliu 's approval before merging (to his branch).
|
You may need to run |
of the server in https://github.com/Zilliqa/ipc-experiment. The unit tests have been disabled and need to be added thoroughly later.
|
I have cleaned up and updated the code to mimic the standalone tested toy implementation of the server (and also the client) in https://github.com/Zilliqa/ipc-experiment. @kaikawaliu This PR is now ready to be reviewed and merged to your branch. Most of the white-space changes are due to running @advaypal if you have the option, please make this "ready to review" rather than WIP. |
src/libServer/ScillaIPCServer.cpp
Outdated
| } | ||
|
|
||
| void ScillaIPCServer::setContractAddress(dev::h160 &address) { |
There was a problem hiding this comment.
You might want to check with @kaikawaliu whether it is ok to remove this method, because it was after discussing with him that I had put it there.
There was a problem hiding this comment.
m_contrAddr is a const private member. I can't imagine what advantage having an accessor for it would provide. If @kaikawaliu insists, I'll add it.
There was a problem hiding this comment.
Not an accessor, the ability to set it.
There was a problem hiding this comment.
Would there be a situation where it needs to be set after the server object is instantiated?
There was a problem hiding this comment.
That's what needs to be checked with @kaikawaliu
There was a problem hiding this comment.
Yes that's needed, as long as the scilla query doesn't include the address itself and we need the address to compose the index (you can see the param is needed to be fed to the ContractStorage2 functions), we have to provide the address to the ipc somehow.
There was a problem hiding this comment.
the addr needed to be changed frequently, so the setter is needed here
There was a problem hiding this comment.
Ah that makes sense. I thought once the server is initialized, it will be for the same address. I'll make the change. Thank you.
There was a problem hiding this comment.
@kaikawaliu this is now done.
|
@advaypal your review comments have been addressed. |
Description
To create a jsonrpc server that will be used for communication with the Scilla executable
This is part of the larger IPC project for more efficient state communication
See branch feature/contractmap
Backward Compatibility
Review Suggestion
Status
Implementation
Integration Test (Core Team)