(#1919) Migrate to AddWatchedAddresses #1920

hoffmabc · 2019-12-12T19:23:00Z

Helps fix #1919

This will hopefully fix:

cpacia · 2019-12-13T18:29:00Z

It doesn't look like the wallet interface and wallet changes were merged into this branch right?

Also, since this is a varadic function you should just be able to do:

AddWatchedAddresses(addr)

instead of:

AddWatchedAddresses([]btcutil.Address{addr}...)

hoffmabc · 2019-12-13T18:35:05Z

My GoLand IDE was bitching about it when I did that. Are you sure?

placer14

We need to include the vendor changes on this PR before this can be merged.

coveralls · 2019-12-17T02:50:51Z

Coverage decreased (-0.04%) to 38.429% when pulling aa1ca08 on brian.addWatchedAddresses into 85332fc on master.

placer14 · 2019-12-17T16:20:19Z

@hoffmabc mentioned there was still details which need to be included in this PR. I also noticed that the go-eth wallet changes are not consistent with the PR opened in its repo. If we're going to change the wallet-interface, it would be my preference that all dependencies which are expected to be cast into this interface to be working. I've recommended a unit test be added to these dependency's tests suite to ensure this interface is satisfied.

Something like:

func TestWalletSatisfiesInterface(t *testing.T) {
  if _, ok := NewWalletConstruction.(WalletInterface); !ok {
    t.Fatal("wallet does not satisfy wallet-interface")
  }
}

hoffmabc · 2019-12-18T16:56:27Z

@placer14 here are the associated vendor PRs:

hoffmabc · 2019-12-18T17:17:00Z

@hoffmabc mentioned there was still details which need to be included in this PR. I also noticed that the go-eth wallet changes are not consistent with the PR opened in its repo. If we're going to change the wallet-interface, it would be my preference that all dependencies which are expected to be cast into this interface to be working. I've recommended a unit test be added to these dependency's tests suite to ensure this interface is satisfied.

Something like:
func TestWalletSatisfiesInterface(t *testing.T) {
  if _, ok := NewWalletConstruction.(WalletInterface); !ok {
    t.Fatal("wallet does not satisfy wallet-interface")
  }
}

I was going to try and implement this but I'm really not convinced we should be unit testing this. I mean as soon as you try to instantiate an object using this interface you're going to get a compile time error. This doesn't really by us anything here.

Any thoughts @cpacia @tyler-smith ?

hoffmabc · 2019-12-18T17:25:14Z

This is to you @placer14. The PR is completed in terms of feedback along with linked dependencies.

hoffmabc · 2019-12-18T18:21:33Z

Ok this is ready @placer14

placer14

This looks ready to merge now. Would appreciate another look by @cpacia at the recent race fix and some additional tidying up.

* origin/master: (23 commits) (#1919) Migrate to AddWatchedAddresses (#1920) Do not mark wallet address as watched + fix earlier txn errors Refactor zcash buildtx Import latest zcash changes Update make openbazaard command to not include tag Point Dockfiles at public Docker Hub repo; Add make help Bump version to 0.13.7 Update zcash branchID Remove unused imports Fix linter issues Update transports Update Bitcoin Cash rates dialer Refactor Dockerfile.dev to derive from latest Dockerfile.qa Refactor Dockerfile.qa Add openbazaard and qa targets to Makefile; Fix docker build targets Add QA docker commands to Makefile QA dockerfile Add qa.sh script Fix transports for message retriever and images Fix formatted logging messages ... Conflicts: Makefile net/service/handlers.go qa/runtests.sh vendor/github.com/OpenBazaar/multiwallet/bitcoin/wallet.go vendor/github.com/OpenBazaar/multiwallet/bitcoincash/exchange_rates.go vendor/github.com/OpenBazaar/multiwallet/bitcoincash/wallet.go vendor/github.com/OpenBazaar/multiwallet/litecoin/wallet.go vendor/github.com/OpenBazaar/multiwallet/zcash/exchange_rates.go vendor/github.com/OpenBazaar/multiwallet/zcash/wallet.go

* Migrate to AddWatchedAddresses * Use variadic parameters * Vendored fixes * Re-watch addresses on socket restart * Convert Mutex to RWMutex * Protect with listenLock * Update to use bulk call * Eliminate redundant conversion * Move statements into the socketclient section * Remove duplicate lock * Refactored to be closer to it’s usage * DB Transaction for Putting PubKeys * Fix linter errors * Migrate to PutAll for watched scripts * Fix missing initialization of err * Interface check for implemented wallets * Handle errors from transaction; Report Rollback failures * Append to BlockBookClient.listenAddrs instead of replace * Fix race bug and remove redundant replay handling in c/b/client.go Co-authored-by: Mike Greenberg <[email protected]>

Unresolved merges from go-ethwallet, spvwallet, and multiwallet * ethereum-master: (178 commits) Bump version to v0.13.8 [#1923] Set default profile.Version only if empty [#1923] Add version to profile protobuf Vendor latest changes for multiwallet Error on no inputs available (#1919) Migrate to AddWatchedAddresses (#1920) Remove unused function Do not mark wallet address as watched + fix earlier txn errors Refactor zcash buildtx Import latest zcash changes Update make openbazaard command to not include tag Point Dockfiles at public Docker Hub repo; Add make help Bump version to 0.13.7 Update zcash branchID Remove unused imports Fix linter issues Update transports Update Bitcoin Cash rates dialer Refactor Dockerfile.dev to derive from latest Dockerfile.qa Refactor Dockerfile.qa ... Conflicts: api/jsonapi_test.go core/completion.go core/order.go core/profile.go core/refunds.go mobile/node.go repo/db/purchases.go repo/db/watched_scripts.go repo/migrations/Migration014.go repo/profile.go repo/profile_test.go test/factory/profile.go

* Migrate to AddWatchedAddresses * Use variadic parameters * Vendored fixes * Re-watch addresses on socket restart * Convert Mutex to RWMutex * Protect with listenLock * Update to use bulk call * Eliminate redundant conversion * Move statements into the socketclient section * Remove duplicate lock * Refactored to be closer to it’s usage * DB Transaction for Putting PubKeys * Fix linter errors * Migrate to PutAll for watched scripts * Fix missing initialization of err * Interface check for implemented wallets * Handle errors from transaction; Report Rollback failures * Append to BlockBookClient.listenAddrs instead of replace * Fix race bug and remove redundant replay handling in c/b/client.go Co-authored-by: Mike Greenberg <[email protected]>

* ethereum-master: (41 commits) Update latest multiwallet dependency Commit latest changes from spvwallet Remove single-currency BitcoinCash/Zcash startup options Fix remaining build errors Update/add github.com/OpenBazaar/go-ethwallet/.. deps [#1930] Remove bitcoind-wallet and zcashd-wallet dependencies [#1965] Deprecate Migration014 Pick patches from ae0282f index on reapply-lost-patch: 361c5e4 Restore patch from fdf79f5..c110900 Restore patch from fdf79f5..c110900 Bump version to v0.13.8 [#1923] Set default profile.Version only if empty [#1923] Add version to profile protobuf Vendor latest changes for multiwallet Error on no inputs available (#1919) Migrate to AddWatchedAddresses (#1920) Remove unused function Do not mark wallet address as watched + fix earlier txn errors Refactor zcash buildtx Import latest zcash changes ...

* commit '1ab22c8e2fb4e59122d38bb2d7cb0dbee890efee': (30 commits) Vendor latest changes for multiwallet Error on no inputs available (OpenBazaar#1919) Migrate to AddWatchedAddresses (OpenBazaar#1920) Do not mark wallet address as watched + fix earlier txn errors Update make openbazaard command to not include tag Point Dockfiles at public Docker Hub repo; Add make help Remove unused imports Fix linter issues Update transports Update Bitcoin Cash rates dialer Refactor Dockerfile.dev to derive from latest Dockerfile.qa Refactor Dockerfile.qa Add openbazaard and qa targets to Makefile; Fix docker build targets Add QA docker commands to Makefile QA dockerfile Add qa.sh script Fix transports for message retriever and images Fix formatted logging messages Add dates to timestamps for desktop server Add date to the timestamps in Haven logging ...

Migrate to AddWatchedAddresses

bb121cc

hoffmabc requested review from placer14 and cpacia December 12, 2019 19:23

hoffmabc self-assigned this Dec 12, 2019

drwasho added the ordering ordering issues label Dec 12, 2019

drwasho added the 🔍 readyForReview Issue or PR ready for code review prior to closing. label Dec 12, 2019

hoffmabc mentioned this pull request Dec 13, 2019

[WIP] Adding AddWatchedAddresses/ListenAddresses #1914

Closed

hoffmabc added ⏰ changes requested 🚧 in progress Issue or PR is currently being worked on and not in final form. and removed 🔍 readyForReview Issue or PR ready for code review prior to closing. labels Dec 13, 2019

Use variadic parameters

4a30937

drwasho changed the title ~~Migrate to AddWatchedAddresses~~ (#1919) Migrate to AddWatchedAddresses Dec 16, 2019

placer14 suggested changes Dec 16, 2019

View reviewed changes

drwasho added this to the 0.13.8 milestone Dec 16, 2019

Vendored fixes

0bce59e

hoffmabc added needs confirmation 🔍 readyForReview Issue or PR ready for code review prior to closing. and removed ⏰ changes requested 🚧 in progress Issue or PR is currently being worked on and not in final form. labels Dec 17, 2019

hoffmabc requested a review from placer14 December 17, 2019 02:43

Re-watch addresses on socket restart

239be7e

placer14 added 🚧 in progress Issue or PR is currently being worked on and not in final form. and removed 🔍 readyForReview Issue or PR ready for code review prior to closing. labels Dec 17, 2019

Merge branch 'master' into brian.addWatchedAddresses

cf38f35

Fix missing initialization of err

e932184

hoffmabc added 🔍 readyForReview Issue or PR ready for code review prior to closing. and removed 🚧 in progress Issue or PR is currently being worked on and not in final form. labels Dec 18, 2019

Interface check for implemented wallets

4d4fbb9

placer14 added 2 commits December 18, 2019 17:19

Handle errors from transaction; Report Rollback failures

fc99f8a

Append to BlockBookClient.listenAddrs instead of replace

97d6fc8

placer14 force-pushed the brian.addWatchedAddresses branch from 450a2ef to 97d6fc8 Compare December 18, 2019 22:19

cpacia approved these changes Dec 19, 2019

View reviewed changes

placer14 force-pushed the brian.addWatchedAddresses branch from a5db5a3 to b6aaa9e Compare December 19, 2019 20:16

placer14 approved these changes Dec 19, 2019

View reviewed changes

placer14 requested a review from cpacia December 19, 2019 20:54

Fix race bug and remove redundant replay handling in c/b/client.go

aa1ca08

placer14 force-pushed the brian.addWatchedAddresses branch from b6aaa9e to aa1ca08 Compare December 19, 2019 20:56

cpacia approved these changes Dec 19, 2019

View reviewed changes

placer14 merged commit edb5cc6 into master Dec 19, 2019

placer14 deleted the brian.addWatchedAddresses branch December 19, 2019 22:33

hoffmabc mentioned this pull request Jan 7, 2020

Multiwallet websocket connection to blockbook intermittently fails OpenBazaar/multiwallet#105

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

(#1919) Migrate to AddWatchedAddresses #1920

(#1919) Migrate to AddWatchedAddresses #1920

hoffmabc commented Dec 12, 2019 •

edited by drwasho

Loading

cpacia commented Dec 13, 2019

hoffmabc commented Dec 13, 2019

placer14 left a comment

coveralls commented Dec 17, 2019 •

edited

Loading

placer14 commented Dec 17, 2019

hoffmabc commented Dec 18, 2019 •

edited

Loading

hoffmabc commented Dec 18, 2019

hoffmabc commented Dec 18, 2019

hoffmabc commented Dec 18, 2019

placer14 left a comment •

edited

Loading

(#1919) Migrate to AddWatchedAddresses #1920

(#1919) Migrate to AddWatchedAddresses #1920

Conversation

hoffmabc commented Dec 12, 2019 • edited by drwasho Loading

cpacia commented Dec 13, 2019

hoffmabc commented Dec 13, 2019

placer14 left a comment

Choose a reason for hiding this comment

coveralls commented Dec 17, 2019 • edited Loading

placer14 commented Dec 17, 2019

hoffmabc commented Dec 18, 2019 • edited Loading

hoffmabc commented Dec 18, 2019

hoffmabc commented Dec 18, 2019

hoffmabc commented Dec 18, 2019

placer14 left a comment • edited Loading

Choose a reason for hiding this comment

hoffmabc commented Dec 12, 2019 •

edited by drwasho

Loading

coveralls commented Dec 17, 2019 •

edited

Loading

hoffmabc commented Dec 18, 2019 •

edited

Loading

placer14 left a comment •

edited

Loading