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

Enabling RPCserver call from plugins #713

Closed
wants to merge 1 commit into from

Conversation

vncoelho
Copy link
Member

We are currently export/import plugin with TX's state (Fault or Halt - #712).
It works in parallel with AppLog Plugin.

@erikzhang
Copy link
Member

Why do you need to expose RpcServer.ProcessRequest?

@vncoelho
Copy link
Member Author

vncoelho commented Apr 29, 2019

neo-project/neo-modules#90

Inside ImportBlocks plugin we make a GetApplicationLog log call in order to export TX state.
This looks like to be the easiest and safest way to interact with AppLog Plugin without exposing plugins to suffer from the behavior of other plugins.

What do you think?

@shargon
Copy link
Member

shargon commented Apr 29, 2019

How do you obtain the HttpContext parameter?

@vncoelho
Copy link
Member Author

vncoelho commented Apr 29, 2019

@shargon
Copy link
Member

shargon commented Apr 29, 2019

I prefer to expose Process method because is not related to Http

@vncoelho
Copy link
Member Author

I think that Process method do not call the plugin's rpc call.

@erikzhang
Copy link
Member

I think you should create the "export application logs" function in the ApplicationLogs plugin.

@vncoelho
Copy link
Member Author

vncoelho commented Apr 30, 2019

I think it will be quite bad at ApplicationLogs, @erikzhang. We would need to copy and reproduce the whole ImportBlocks scheme.

Because we just want a minor modification for exporting the Transaction state as well, namely chain.acc.v2.zip.
Furthermore, we want the node to check this while importing, ensuring all states reproduced by the node (the one importing the chain) is the same as the from the chain.acc.v2.zip it is importing from.

@erikzhang
Copy link
Member

The ImportBlocks plugin should not depend on another plugin.

@vncoelho
Copy link
Member Author

vncoelho commented Apr 30, 2019

This is just for the case in which it is importing Transaction State using Application Logs.
We would specify this requirement while exporting chain with v2.

But, on the same line of reasoning, ApplicationLog should not depend on ImportBlocks, neither copy its functionalities.

Another option is to move the Application Log to the core, as already suggested in some thread. But I do not think it is necessary.
Application Log as a plugin looks ok to me.

I do not know the best solution, @erikzhang.
I think that as we are proposing looks quite simple and without impacts.

@erikzhang
Copy link
Member

I'll close this first. Let's discuss it under the PR of ImportBlocks.

@erikzhang erikzhang closed this Apr 30, 2019
@erikzhang erikzhang deleted the exposing-rpcserver-call branch April 30, 2019 16:04
Thacryba pushed a commit to simplitech/neo that referenced this pull request Feb 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants