Extended websocket interface #1335

Flowneee · 2019-06-03T14:12:14Z

Overview

This PR introduce additional functionality to websocket interface. Old version allowed to subscribe only for new blocks events. New version add support of subscribing to new transactions (all or only from specific services with specific ID) and sending transactions via websocket connection.

Details

Added new explorer endpoints:

v1/transactions/subscribe: this endpoint automatically subscribe to new transaction events. This endpoint accept optional parameters: service_id and message_id (message_id as in derive macro TransactionSet);
v1/ws: this endpoint open websocket connection and allow to set multiple subscription (for new blocks and transaction, filtered by service and transaction id) and send transactions (in hex, like explorer) to blockchain.

Messages, sent to websocket, must have format: {"type": "...", "payload": ...} (currently possible types: set-subsctiptions, transaction), where payload differs for different types.

Response for each sent message can be:

success: {"result": "success"} or {"result": "success", "response": ...}, if action assume some response;
error: {"result": "error", "description": "..."}.

Note: you can also set multiple subscription and set transactions through v1/blocks/subscribe and v1/transactions/subscribe, so they are like v1/ws, but with default filter.

Examples

`v1/blocks/subscribe`

for subscribing to new blocks: ws://localhost:8080/api/explorer/v1/blocks/subscribe.

`v1/transactions/subscribe`

for subscribing to all new transactions: ws://localhost:8080/api/explorer/v1/transactions/subscribe;
for subscribing to new transactions of service with id 1: ws://localhost:8080/api/explorer/v1/transactions/subscribe?service_id=1;
for subscribing to new transactions of service with id 1 and message id 2: ws://localhost:8080/api/explorer/v1/transactions/subscribe?service_id=1&message_id=2.

`v1/ws` (multiple filters)

Initiate connection: ws://localhost:8080/api/explorer/v1/ws;
Send text message (via websocket) with list of filter (for example, blocks and transactions of service1 with message id 2): {"type":"set-subscriptions","payload":[{"type":"blocks"},{"type":"transactions","filter":{"service_id":1,"message_id":2}}]};
Update filter (leave only subscription for blocks), send text message: {"type":"set-subscriptions","payload":[{"type":"blocks"}]}.

`v1/ws` (send transaction)

Initiate connection: ws://localhost:8080/api/explorer/v1/ws;
Send text message (via websocket) with transaction hex (like in explorer): {'type":"transaction","payload":{"tx_body":"53ef10e1b06e60d69ad72b21c628d11ef5beb1a1cfe828d6dcf1e4055093e1470000000000000a220a2053ef10e1b06e60d69ad72b21c628d11ef5beb1a1cfe828d6dcf1e4055093e1471205416c696365c50061fe2442b846179420c51cd19e6efafb4c20dc5272247dbb1d2549bcead9aee0805161fac631ed8483a41fdff3957a44cbacdbd8b13fcb827a0f913a120e"}}.

Added: - new endpoint: "v1/transactions/subscribe". This endpoint automatically subscribe to new transaction events. This endpoint accept optional parameters: `service_id` and `transaction_id`; - new enspoint: "v1/ws". This endpoint open websocket connection and allow to set multiple subscription (for blocks and transaction, filtered by service and transaction id) and send transactions (in hex, like explorer) to blockchain.

codecov · 2019-06-03T14:42:55Z

Codecov Report

Merging #1335 into master will increase coverage by 0.3%.
The diff coverage is 86.8%.

@@            Coverage Diff            @@
##           master    #1335     +/-   ##
=========================================
+ Coverage   93.23%   93.53%   +0.3%     
=========================================
  Files         162      164      +2     
  Lines       25032    25354    +322     
=========================================
+ Hits        23339    23716    +377     
+ Misses       1693     1638     -55

Impacted Files	Coverage Δ
exonum/src/api/mod.rs	`98.11% <ø> (+0.94%)`	⬆️
exonum/src/messages/mod.rs	`95.34% <100%> (+0.22%)`	⬆️
exonum/src/explorer/mod.rs	`93.55% <100%> (ø)`	⬆️
exonum/tests/websocket/blockchain/mod.rs	`77.77% <77.77%> (ø)`
exonum/src/api/websocket.rs	`88.05% <86.07%> (+88.05%)`	⬆️
exonum/src/api/node/public/explorer.rs	`83.82% <89.65%> (+7.93%)`	⬆️
exonum/tests/websocket/main.rs	`90% <90%> (ø)`
exonum/src/node/mod.rs	`92.13% <0%> (+0.68%)`	⬆️
... and 6 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 61992e9...6904bc5. Read the comment docs.

… tests.

alekseysidorov

At the first sight, this PR looking good for me except of a several moments. Also may you write entry to the CHANGELOG.md?

exonum/src/api/websocket.rs

Changed: - `usize` type replaced by `u64` in websocket.rs; - `sub_types` field in `Subscribe` type renamed to `subscribers`; - `subscribers` field of `Server` now use `BTreeMap`.

aleksuss · 2019-06-04T10:33:16Z

@Flowneee good job. Thanks a lot. But what do you think about to rename a query parameter tranisaction_id to message_id ? We use a message_id definition to differ messages (transactions) in our codebase. Take a look here for example.

…y with rest of code base).

Flowneee · 2019-06-04T11:42:00Z

@aleksuss Yes, this make sense. Done. Also added in changelog how to determine message_id (just mentioned TransactionSet macro).

Flowneee · 2019-06-04T12:42:56Z

@aleksuss Error occurred during tests, on command cd $TRAVIS_BUILD_DIR/testkit/server && npm run proto && npm run test:unix. I don't think that it is because of my change. Can you look into it?

aleksuss · 2019-06-04T12:45:28Z

@Flowneee I've restarted the job. I suppose it will fix that issue. It happens some times.

Flowneee · 2019-06-05T11:31:00Z

At the first sight, this PR looking good for me except of a several moments. Also may you write entry to the CHANGELOG.md?

@alekseysidorov I added entry to CHANGELOG.md and fixed all your comments. Can you check it?

aleksuss · 2019-06-06T09:00:19Z

exonum/src/api/websocket.rs

+    where
+        T: AsRef<dyn Snapshot> + IndexAccess,
+    {
+        let tx = schema.transactions().get(tx_hash).unwrap();


@Flowneee What if we don't have a transaction with hash tx_hash in the blockchain ? What do you think about to return a Result or Option here ?

@aleksuss Currently there is always will be a transaction, because the only place where TransactionInfo called is here (tx_hash in that case is always already committed transaction). Generally yes, probably good idea. And I even think that method should be not new, but something like new_committed to indicate that we want info about committed transaction, and this method should return None instead of each .unwrap(). Another option is to rename type to something like CommittedTransactionInfo. What do you say?

@Flowneee I've got your point and it's true. But unwrap() hurts my eyes in this place. I'd prefer a case with None instead of each .unwrap() but also you can use .expect() with a message like ("BUG. We don't have committed transaction in transactions index with hash: {:?}, tx_hash). Because the transaction is really MUST BE in the index. So. It's up to you.

Okay, done. TransactionInfo renamed to CommittedTransactionSummary, added logging of situation, when summary cannot be built.

@alekseysidorov @aleksuss @popzxc Can you check it again?

…oesn't exist in blockchain

aleksuss

LGTM

Flowneee added 2 commits June 3, 2019 15:35

Warning fixes.

1bc2064

Flowneee marked this pull request as ready for review June 3, 2019 14:18

Flowneee requested review from alekseysidorov, aleksuss and popzxc as code owners June 3, 2019 14:18

Flowneee added 2 commits June 3, 2019 18:26

Fixed warnings in tests.

5a4f86d

Fixed typo in comment, fmt, and failed test. Increased timeout for ws…

04ba7d7

… tests.

alekseysidorov suggested changes Jun 4, 2019

View reviewed changes

exonum/src/api/websocket.rs Outdated Show resolved Hide resolved

exonum/src/api/websocket.rs Outdated Show resolved Hide resolved

exonum/src/api/websocket.rs Outdated Show resolved Hide resolved

Flowneee added 3 commits June 4, 2019 12:00

Review fixes.

5985119

Changed: - `usize` type replaced by `u64` in websocket.rs; - `sub_types` field in `Subscribe` type renamed to `subscribers`; - `subscribers` field of `Server` now use `BTreeMap`.

Fmt fixes

9b0b65d

Updated CHANGELOG.md

49866be

aleksuss requested a review from ivan-ochc June 4, 2019 10:24

TransactionFilter::transaction_id renamed to message_id (to compl…

5b833cf

…y with rest of code base).

alekseysidorov previously approved these changes Jun 5, 2019

View reviewed changes

popzxc previously approved these changes Jun 6, 2019

View reviewed changes

aleksuss previously approved these changes Jun 6, 2019

View reviewed changes

aleksuss reviewed Jun 6, 2019

View reviewed changes

Flowneee and others added 2 commits June 6, 2019 12:38

Merge branch 'master' into extended-websocket-interface

f6c2c77

Proper handle of situation, when data about "committed" transaction d…

6904bc5

…oesn't exist in blockchain

Flowneee dismissed stale reviews from aleksuss, popzxc, and alekseysidorov via 6904bc5 June 6, 2019 10:12

aleksuss approved these changes Jun 6, 2019

View reviewed changes

alekseysidorov approved these changes Jun 6, 2019

View reviewed changes

aleksuss merged commit 8e5dafd into exonum:master Jun 6, 2019

Extended websocket interface #1335

Extended websocket interface #1335

Uh oh!

Conversation

Flowneee commented Jun 3, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Overview

Details

Examples

v1/blocks/subscribe

v1/transactions/subscribe

v1/ws (multiple filters)

v1/ws (send transaction)

Uh oh!

codecov bot commented Jun 3, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

alekseysidorov left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

aleksuss commented Jun 4, 2019

Uh oh!

Flowneee commented Jun 4, 2019

Uh oh!

Flowneee commented Jun 4, 2019

Uh oh!

aleksuss commented Jun 4, 2019

Uh oh!

Flowneee commented Jun 5, 2019

Uh oh!

aleksuss Jun 6, 2019

Choose a reason for hiding this comment

Uh oh!

Flowneee Jun 6, 2019

Choose a reason for hiding this comment

Uh oh!

aleksuss Jun 6, 2019

Choose a reason for hiding this comment

Uh oh!

Flowneee Jun 6, 2019

Choose a reason for hiding this comment

Uh oh!

Flowneee Jun 6, 2019

Choose a reason for hiding this comment

Uh oh!

aleksuss left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

4 participants

Flowneee commented Jun 3, 2019 •

edited

Loading

`v1/blocks/subscribe`

`v1/transactions/subscribe`

`v1/ws` (multiple filters)

`v1/ws` (send transaction)

codecov bot commented Jun 3, 2019 •

edited

Loading