-
Notifications
You must be signed in to change notification settings - Fork 116
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
dot/rpc: implement chain_subscribeNewHeads #836
Conversation
This is ready for another look. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the code walkthrough on this one. Nice work! Looks good to me.
method := msg["method"] | ||
// if method contains subscribe, then register subscription | ||
if strings.Contains(fmt.Sprintf("%s", method), "subscribe") { | ||
mid := msg["id"].(float64) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is the subscription ID provided by the user?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, the id provide by the user is the message id, so that can track that to the responses. The subscription id is created by the service and returned to the requester.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah okay makes sense!
dot/rpc/http.go
Outdated
// init and start block received listener routine | ||
if h.serverConfig.BlockAPI != nil { | ||
h.serverConfig.BlockAddedReceiver = make(chan *types.Block) | ||
h.serverConfig.BlockAddedReceiverDone = make(chan bool) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this can be changed to make(chan struct{})
and then in Stop()
you can do close(BlockAddedReceiverDone)
to notify the BlockState that it's done. it'll receive empty struct and unblock on <-bs.doneNotifying
it's a nitpick but I think it's usually what's used in go if you want a channel that only notifies
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, updated. I like the idea of applying patterns consistently (although I don't always do it), so thanks for pointing this out.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
overall looking really good! just a few small things
clean-up comments, move map init.
@noot I think I address all comments, so it's ready for a re-review. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good to me!
* fix formating of header digest logs * update digest type values to conform to spec * hardcode RPC results for testing * implement RPC subscribe_newHeads WebSocket call * implement subscription * header number as hex string in json responses * rpc getStorage * implement basic rpc state_getStorage call * added test for state_getStorage * cleanup comments * implement state_getPairs * implement state_getStorageHash * implement state_getStorageSize * refactor tests * cleanup comments * added tests * fix lint issues * update empty request test * lint issues * map for holding subscription connections * use channel for block listener subscription * make json results * lint issues * update tests to init coreAPI service * changed if statement to switch * Use HeaderToJSON function * add tests for WebSocket * clean up * update comments * add channel to BlockState service to notify RPC serivce * add done channel to rpc newBlock receiver * add check for open websocket connection * update chan done notifier to use struct instead of bool clean-up comments, move map init.
Changes
Basic implementation of WebSocket subscription to push new head data to client.
Tests:
Start node with RPC enabled:
Make WebSocket call to ws://localhost:8546/
OR interact via Polkadot-js
Then browse to https://polkadot.js.org/apps/#/settings and configure for custom endpoint ws://127.0.0.1:8546/
Then interact with node through polkadot-js and note errors and issues so that we can determine next areas of focus.
Checklist:
Issues: