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

Add proper eth subscribe support #9985

Closed
jennijuju opened this issue Jan 11, 2023 · 1 comment · Fixed by #10027
Closed

Add proper eth subscribe support #9985

jennijuju opened this issue Jan 11, 2023 · 1 comment · Fixed by #10027
Assignees
Labels
P2 P2: Should be resolved

Comments

@jennijuju
Copy link
Member

jennijuju commented Jan 11, 2023

from: #9863 (comment)

  • subscribe is not supported
    • The only real way to do this correctly requires a bunch of changes is go-jsonrpc.
      • I'm surprised that what we have today for eth_subscribe works at all
        • It doesn't return the subscription id correctly
        • It doesn't invoke the correct notification on the client
        • The params in the notifications are in the wrong format (array with [id, value] vs object with those)
        • eth_unsubscribe id has nothing to do with the id used by go-jsonrpc when handling the channel
      • To fix this we'll need to add proper support for reverse calls in go-jsonrpc, and add support for object-based parameters
        • Or allow hooking into the raw rpc stream, but that has performance implications
        • Or add support for custom channel logic, but that's about the same amout of work needed for reverse calling, and less flexible
        • Implementing reverse calls is probably 1-2 days, but is that worth it now for proper eth_subscribe support?
@jennijuju
Copy link
Member Author

blocked on filecoin-project/go-jsonrpc#88

@jennijuju jennijuju added the P2 P2: Should be resolved label Jan 11, 2023
@jennijuju jennijuju linked a pull request Jan 16, 2023 that will close this issue
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P2 P2: Should be resolved
Projects
No open projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants