Wallet websocket covenant events by tynes · Pull Request #195 · handshake-org/hsd

tynes · 2019-06-11T03:06:53Z

Emit websocket events for each type of covenant when the wallet indexes the transaction. This is implemented by iterating over each output in the transaction. The topic is:

${lowercase(type)} covenant

For example bid covenant.
The payload is (walletid, namestate, details)

walletid is the id of the wallet that caused the event
namestate is the NameState json object
details is a transaction json with additional details such as the paths

Includes tests for events over websocket using bsock.

tynes · 2019-06-11T03:08:20Z

lib/wallet/http.js

@@ -1303,6 +1303,21 @@ class HTTP extends Server {
        this.to('w:*', event, wallet.id, json);
    };



We could probably use handleTX instead of creating a new function here, but the difference is in this.to

handleTX doesn't pass along enough arguments, we need to add another method or refactor handleTX. The problem is that handleTX does nothing with the tx argument, and the covenant related events pass along the argument at that position. A new function called handleCovenant was created.

codecov-io · 2019-06-11T03:09:07Z

Codecov Report

Merging #195 into master will decrease coverage by 0.1%.
The diff coverage is 76%.

@@            Coverage Diff             @@
##           master     #195      +/-   ##
==========================================
- Coverage   53.12%   53.02%   -0.11%     
==========================================
  Files         129      129              
  Lines       35748    35795      +47     
  Branches     6022     6044      +22     
==========================================
- Hits        18991    18980      -11     
- Misses      16757    16815      +58

Impacted Files	Coverage Δ
lib/wallet/txdb.js	`77.97% <68.57%> (+0.09%)`	⬆️
lib/wallet/http.js	`65.49% <93.33%> (+0.45%)`	⬆️
lib/primitives/coin.js	`67.66% <0%> (-16.55%)`	⬇️
lib/node/http.js	`47.2% <0%> (-2.1%)`	⬇️
lib/script/script.js	`58% <0%> (-1.41%)`	⬇️
lib/primitives/keyring.js	`68.96% <0%> (-0.87%)`	⬇️
lib/primitives/tx.js	`72.96% <0%> (-0.44%)`	⬇️
lib/hsd.js	`97.26% <0%> (-0.21%)`	⬇️
lib/mempool/mempool.js	`42.68% <0%> (-0.15%)`	⬇️
lib/net/framer.js	`100% <0%> (ø)`	⬆️
... and 7 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 0d5865f...b80bbc5. Read the comment docs.

lib/wallet/txdb.js

tynes · 2019-06-11T03:11:09Z

lib/wallet/txdb.js

+          this.emit(channel, {name, output, tx}, details);
+          break;
+        }
+        case types.REVEAL:


For these covenants, the name is not in the covenant data, it requires a db lookup to look up the name based on the hash. I don't think this will result in too much i/o

To alleviate the added i/o, we could refactor this.connectNames. Currently, after processing incoming covenants, the method returns a boolean that indicates whether the name state was updated. We could instead have it return a summary object which accumulates all processed covenants.

We could then pass this summary object to this.emitCovenants.

See #195 (comment)

boymanjor

As discussed out of band, I think we should emit events for tree updates as well.

test/wallet-http-test.js

lib/wallet/txdb.js

boymanjor · 2019-06-12T22:18:35Z

lib/wallet/txdb.js

+          this.emit(channel, {name, output, tx}, details);
+          break;
+        }
+        case types.REVEAL:


To alleviate the added i/o, we could refactor this.connectNames. Currently, after processing incoming covenants, the method returns a boolean that indicates whether the name state was updated. We could instead have it return a summary object which accumulates all processed covenants.

We could then pass this summary object to this.emitCovenants.

lib/wallet/txdb.js

tynes · 2019-06-22T17:07:49Z

lib/wallet/txdb.js

+      const nameHash = covenant.get(0);
+      const ns = await view.getNameState(this, nameHash);
+
+      if (EMPTY.equals(ns.name) || !ns.name) {


The view accumulates state (the name itself is what we want) during connectNames. If the name was added to the NameState object, then skip the i/o involved with looking it up. Look it up in the covenant data if its present, otherwise query the wallet database for the name based on the nameHash.

lib/wallet/txdb.js

test/wallet-http-test.js

tynes · 2019-06-22T17:12:08Z

As discussed out of band, I think we should emit events for tree updates as well.

PR for emitting tree updates #201, the node websocket topic "tree commit"

tynes · 2019-06-22T17:15:08Z

I don't think the additional i/o introduced by this PR is a problem because it first looks in the nameview that is populated in connectNames with a database lookup. It will only do another database lookup in the case that nameview does not already have the name. We could add a runtime flag to disable the event emitting codepath (including the additional i/o) if we wanted to, but I don't think its necessary.

Add tests for socket events based on auction based transactions being indexed in the wallet txdb.

Add a new method `emitCovenants` that emits an event for each auction related output in a transaction that is indexed in the walletdb.

Emit events over websocket for auction related events that are emitted by the walletdb.

lib/wallet/txdb.js

pinheadmz · 2021-01-27T17:32:48Z

concept ACK -- I'm gonna keep this on my to-review pile and maybe we can get it into v2.4.0

tynes commented Jun 11, 2019

View reviewed changes

lib/wallet/txdb.js Outdated Show resolved Hide resolved

tynes commented Jun 11, 2019

View reviewed changes

boymanjor requested changes Jun 12, 2019

View reviewed changes

tynes commented Jun 17, 2019

View reviewed changes

lib/wallet/txdb.js Show resolved Hide resolved

tynes added the wallet label Jun 21, 2019

tynes changed the title ~~Wallet websocket covenant events~~ WIP: Wallet websocket covenant events Jun 21, 2019

tynes force-pushed the wallet-covenant-events branch from f556748 to 9042ed9 Compare June 22, 2019 17:01

tynes commented Jun 22, 2019

View reviewed changes

lib/wallet/txdb.js Outdated Show resolved Hide resolved

tynes commented Jun 22, 2019

View reviewed changes

test/wallet-http-test.js Show resolved Hide resolved

tynes force-pushed the wallet-covenant-events branch from 9042ed9 to bf2842f Compare June 22, 2019 17:24

Mark Tyneway added 3 commits June 22, 2019 11:26

test: wallet auction sockets

e47bd6c

Add tests for socket events based on auction based transactions being indexed in the wallet txdb.

wallet: emit covenant related events

9722b5c

Add a new method `emitCovenants` that emits an event for each auction related output in a transaction that is indexed in the walletdb.

http: websocket events for auctions

e007f14

Emit events over websocket for auction related events that are emitted by the walletdb.

tynes force-pushed the wallet-covenant-events branch from bf2842f to e007f14 Compare June 22, 2019 17:27

tynes changed the title ~~WIP: Wallet websocket covenant events~~ Wallet websocket covenant events Jun 22, 2019

tynes added ready for review labels Jun 22, 2019

tynes commented Jun 24, 2019

View reviewed changes

lib/wallet/txdb.js Outdated Show resolved Hide resolved

txdb: remove redundant code

b80bbc5

tynes force-pushed the wallet-covenant-events branch from ae70b8d to b80bbc5 Compare August 13, 2019 08:59

nodech added tests part of the codebase and removed api labels Nov 18, 2021

nodech added wallet part of the codebase wallet-api part of the codebase wallet-db part of the codebase ready-for-review modifier needs rebase modifier waiting for the author modifier and removed ready-for-review modifier labels Nov 18, 2021

nodech added breaking-minor Backwards compatible - Release version takeover Stale PRs. and removed waiting for the author modifier labels Dec 10, 2021

		@@ -1303,6 +1303,21 @@ class HTTP extends Server {
		this.to('w:*', event, wallet.id, json);
		};

Conversation

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

Uh oh!

Uh oh!

tynes Jun 11, 2019

Choose a reason for hiding this comment

Uh oh!

tynes Jun 22, 2019

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Codecov Report

Uh oh!

Uh oh!

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

Uh oh!

Choose a reason for hiding this comment

Uh oh!

boymanjor Jun 12, 2019

Choose a reason for hiding this comment

Uh oh!

tynes Jun 22, 2019

Choose a reason for hiding this comment

Uh oh!

boymanjor left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

boymanjor Jun 12, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

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

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

tynes commented Jun 22, 2019

Uh oh!

tynes commented Jun 22, 2019

Uh oh!

Uh oh!

pinheadmz commented Jan 27, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

tynes commented Jun 11, 2019 •

edited

Loading

codecov-io commented Jun 11, 2019 •

edited

Loading

tynes Jun 11, 2019 •

edited

Loading

tynes Jun 22, 2019 •

edited

Loading