-
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
feat: dot/rpc: Add support for submit and watch extrinisic #1415
Conversation
This pull request introduces 1 alert when merging a066327 into f9a40ff - view on LGTM.com new alerts:
|
This pull request introduces 1 alert when merging 276e61b into 7f506cf - view on LGTM.com new alerts:
|
This pull request introduces 1 alert when merging b03cde1 into 451474b - view on LGTM.com new alerts:
|
dot/rpc/websocket.go
Outdated
return c.wsconn.WriteJSON(msg) | ||
err := c.wsconn.WriteJSON(msg) | ||
if err != nil { | ||
logger.Warn("error sending websocket message", "error", err) |
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.
I wonder if this should be warn or debug, since warn will pollute the logs if there's some issue... what do you think?
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.
Good idea.
dot/rpc/websocket.go
Outdated
return c.wsconn.WriteJSON(res) | ||
err := c.wsconn.WriteJSON(res) | ||
if err != nil { | ||
logger.Warn("error sending websocket message", "error", err) |
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.
same here
} | ||
|
||
// ExtrinsicSubmitListener to handle listening for extrinsic events | ||
type ExtrinsicSubmitListener struct { |
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.
maybe this and the related logic could go into another file?
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.
Yes, I was thinking I should move the listener logic into another file, I'll clean-up in another PR.
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! in terms of tests, is it possible to add a polkadot.js test that explicitly tests submitAndWatchExtrinsic
?
Edward Mack: feat: dot/rpc: Add support for submit and watch extrinisic (#1415) * add check for call to websocket * stub rpc author_submitAndWatchExtrinsic * add block import listener * implement state_subscribeRuntimeVersion * add test for apply extrinsic * implement inBlock status for author_submitAndWatchExtrinsic * handle submit extrinsic in HandleSubmittedExtrinsic * remove print lines * add unit tests * add tests for apply extrinsic * add test to build and apply extrinsic * add tests * add subkey to tests.yml * fix SubscriptionParams parameter * skip test failing in CI due to subkey setup * remove unused reference * address comments * remove unused comments * change logging level for websocket issues
Changes
Tests
I need to build tests, any suggestions on how I can init a test so that submit extrinsic I can follow it through all the steps.
Checklist
Issues