dot/rpc implement state_subscribeStorage RPC WebSocket method #983
Conversation
|
I see that code climate is complaining, let me know if you want me to refactor. |
dot/rpc/modules/state.go
Outdated
| // TODO implement change storage trie so that block hash parameter works (See issue #834) | ||
| // SubscribeStorage Storage subscription. If storage keys are specified, it creates a message for each block which | ||
| // changes the specified storage keys. If none are specified, then it creates a message for every block. | ||
| // This endpoint communicates over the Websockt protocol, but this func should remain here so it's added to rpc_methods list |
| // SubscribeStorage Storage subscription. If storage keys are specified, it creates a message for each block which | ||
| // changes the specified storage keys. If none are specified, then it creates a message for every block. | ||
| // This endpoint communicates over the Websockt protocol, but this func should remain here so it's added to rpc_methods list | ||
| func (sm *StateModule) SubscribeStorage(r *http.Request, req *StateStorageQueryRangeRequest, res *StorageChangeSetResponse) error { |
There was a problem hiding this comment.
I'm also wondering, is there an unsubscribe method corresponding to the subscribe methods? if so, it would be nice to return the subscription ID
dot/rpc/websocket.go
Outdated
| wss := &WebSocketSubscription{ | ||
| WSConnection: conn, | ||
| SubscriptionType: subscriptionType, | ||
| Filter: filter, | ||
| } | ||
| wssub[sub] = wss | ||
| h.serverConfig.WSSubscriptions = wssub |
There was a problem hiding this comment.
the WSSubscriptions should be stored in the HTTPServer struct (or in a separate wsServer struct) not in the config
dot/rpc/websocket.go
Outdated
| if change != nil { | ||
| for i, sub := range h.serverConfig.WSSubscriptions { | ||
| if sub.SubscriptionType == SUB_STORAGE { | ||
| // check it change key is in subscription filter |
dot/state/storage.go
Outdated
| Key: key, | ||
| Value: value, | ||
| } | ||
| //fmt.Printf("SetStorage Key: %x Value: %x\n", key, value) |
dot/state/storage.go
Outdated
| Value: value, | ||
| } | ||
| //fmt.Printf("SetStorage Key: %x Value: %x\n", key, value) | ||
| // TODO ed add channel notify here |
There was a problem hiding this comment.
should also be added to ClearStorage
dot/rpc/websocket.go
Outdated
| for change := range h.storageChan { | ||
|
|
||
| if change != nil { | ||
| for i, sub := range h.serverConfig.WSSubscriptions { |
There was a problem hiding this comment.
I'm wondering if it would be simpler to start a goroutine for each subscription, what do you think?
There was a problem hiding this comment.
Yes, I've refactor websockets to better handle this.
|
I've updated this, it's ready for another look. Let me know your thoughts. |
dot/rpc/http.go
Outdated
| type WSConn struct { | ||
| wsconn *websocket.Conn | ||
| mu sync.Mutex | ||
| serverConfig *HTTPServerConfig |
There was a problem hiding this comment.
does this need the whole server config? or just the state interfaces?
There was a problem hiding this comment.
No just BlockAPI and StorageAPI, I've updated.
dot/rpc/http.go
Outdated
| wsconn *websocket.Conn | ||
| mu sync.Mutex | ||
| serverConfig *HTTPServerConfig | ||
| logger log.Logger |
There was a problem hiding this comment.
might be nice to make the logger global to the rpc package
dot/rpc/websocket.go
Outdated
| // Listen implementation of Listen interface to listen for channel changes | ||
| func (l *StorageChangeListener) Listen() { | ||
| for change := range l.channel { | ||
| if change != nil { |
There was a problem hiding this comment.
nitpick but it might be nice to do
if change == nil {
continue
}
// check if change key ....
same for below
Changes
Tests
OR connect a websocket client to node and send request:
Checklist
Issues