fix(rpc module): unsubscribe according ethereum pubsub spec #693
Conversation
| let err = to_json_raw_value(&format!( | ||
| "Invalid subscription ID={}", |
There was a problem hiding this comment.
The only downside to this PR is that we loose this information (I think?).
With this PR, when an unsubscribe call fails, what do users get back?
There was a problem hiding this comment.
yeah, unfortunately they will just receive a bool set to false and then the client(s) have to figure it on their own :(
There was a problem hiding this comment.
another route is to actually require users to customize their unsubscribe impl as jsonrpc does but probably not worth the effort we still only support String and u64 as subscription ID.
There was a problem hiding this comment.
Let's document this properly in the macro docs (and on this method): "When attempting to unsubscribe with an unknown subscription ID this call will return false" or something along those lines?
There was a problem hiding this comment.
I guess it's better to document that we implement the ethereum pubsub specification when we refer to subscriptions and it's possible to provide custom subscription IDs but that's it.
Sure, we could add some documentation to both RpcModule::register_subscription and the macros.
As we already have adopted the ethereum pubsub spec for subscriptions let's stick to it completely.