search

nspcc-dev / neo-go

Go Node and SDK for the NEO blockchain
MIT License
118 stars 77 forks source link

Unifying notification standards #1081

Open corollari opened 4 years ago

corollari commented 4 years ago

Hi! I just found out about the inclusion of a notifications system into neo-go, and, given that I'm the creator of neo-PubSub, I thought I'd chime in and get the discussion started on creating a standard for notifications, as that would enable interoperability in libraries such as neon-js, which currently only supports neo-PubSub but for which I could see a neo3 release (the maintainer announced that he doesn't plan on doing any additional releases for neo2) that lets users decide on whether to use public infrastructure (neo-PubSub), which requires trust on the person hosting that, or a node hosted by themselves (neo-go).

Furthermore, it might also be interesting to include the maintainers of neo-python into this, as they have built a notifications system that, at the moment, requires direct integration in python (similar to neo-node C# plugins), but could be easily changed to serve notifications over the network.

Anyway, I've taken a look at your specification and noticed that you've adopted a format that tries to be consistent with the RPC methods, but, apart from that, the information provided for smart contract notifications is pretty similar to the one provided by neo-PubSUb, with the exception of the transaction id of the call that triggered the notification, therefore it should be pretty easy to merge both systems.

What are your thoughts on this?

roman-khimov commented 4 years ago

I'm all for standards and wider usage of these standards, that's one of the reasons for writing our specification in the first place. For some reason I've only discovered neo-PubSub just two days ago when I'd been preparing the Medium post about our system.

So now we have two somewhat similar and different at the same time approaches. My view of these differences is the following:

I think that the first two are unavoidable given that our solution is directly tied to the node's JSON-RPC interface while yours is like connect-and-get. Which means that we would have two basic modes of operation client-side, but it shouldn't be a problem.

And as for the third, that's the place where we can and should provide a perfect match between different approaches.

Furthermore, it might also be interesting to include the maintainers of neo-python into this

Sure, @ixje might be interested in this.

I've taken a look at your specification

Just as a side note --- we have two versions of it for Neo 2 and Neo 3 and the version for Neo 3 should be considered to be a draft-level document as it is going to be updated following the changes being made here (different blocks, different transactions, different getapplicationlog results and there also is a big question of block-level (native contract's OnPersist) code executions).

you've adopted a format that tries to be consistent with the RPC methods

Yep, that was a natural choice for us and we've tried not to reinvent solutions for already solved problems. IIUC it should make us compatible for block and transaction notifications as these are well-defined by the Neo JSON-RPC API.

the exception of the transaction id of the call that triggered the notification

At the moment our format for notifications generated during execution is just a part of the regular getapplicationlog response, but this extension seems to be useful and I think we can easily add it.

ixje commented 4 years ago

Furthermore, it might also be interesting to include the maintainers of neo-python into this, as they have built a notifications system that, at the moment, requires direct integration in python (similar to neo-node C# plugins), but could be easily changed to serve notifications over the network.

👋 Similar to neon-js I don't expect todo any new changes/releases for neo-python (2.x) other than security related or really necessary to stay in working state. This is mostly driven by lack of resources.

However, for 3.x I'm also in favour of standards. As you noticed the notification system in neo-python is meant to be consumed from inside Python. A very similar system will exist for 3.x given that it is not limited to just smart contract notifications and I want to keep it uniform. However, we should be able to include a wrapper that exposes the smart contract notifications on a websocket in a common format we agree on here. I believe that's what your asking for, right?