client/asset/ltc: native litecoin spv wallet by buck54321 · Pull Request #1750 · decred/dcrdex

buck54321 · 2022-08-01T22:51:48Z

Same as #1635, but for Litecoin.

chappjc · 2022-08-01T23:23:27Z

Do we want to PR all the neutrino fixes to their fork too? The btcsuite forks and their maintenance is the main reason why I'm wary about these.

buck54321 · 2022-08-01T23:49:18Z

I haven't checked commits one-by-one, but it looks like the ltcsuite neutrino repo gets more attention than others.

If I have to maintain fix branches because we have trouble getting upstream PRs merged, so be it. I don't think it needs to stop our progress.

chappjc · 2022-08-02T00:07:33Z

I haven't checked commits one-by-one, but it looks like the ltcsuite neutrino repo gets more attention than others.

They forked from lightning labs more recently is all afaict. Neither have seen much attention since forking.

If I have to maintain fix branches because we have trouble getting upstream PRs merged, so be it. I don't think it needs to stop our progress.

That doesn't give me warm fuzzies, but I feel like if we do want neutrino wallets for BCH and LTC (and I support this with caution) we should have forks that we maintain. The upstreams are not going to see maintenance, that much is clear. We should either talk to dhilll about some separate repos under the decred github org or we make a new org for dcrdex forks of things that we admin.

On a side note, ltcd/wire still does not know how to decode their own MW blocks or txns. This means we're able to work with their neutrino by the grace of the deserialization code I added to dcrdex, which I only bring up because that suggests that no one is using their Go neutrino fork because what would you do with the blocks you pull if they fail to deserialize? None of it is reassuring to me, but as I said I cautiously support this approach since it's the only reasonable way we can add native wallets to dcrdex. EDIT: Probably the ltc-neutrino chain service is working like litecoind 0.18 on the wire, getting the older serializations with mw pieces removed by its peers, so it's simply not updated for the soft fork yet (ok for now).

chappjc · 2022-08-15T18:16:07Z

Just merged all of our neutrino-ltc PRs (and rewrote the module and import names), and PR'd the same NeutrinoClient migration that you did for BCH: dcrlabs/neutrino-ltc#5
Let's see if that works for this too.

chappjc · 2022-08-15T18:24:54Z

 		Description: "Use an external Electrum-LTC Wallet",
 		// json: DefaultConfigPath: filepath.Join(btcutil.AppDataDir("electrum-ltc", false), "config"), // e.g. ~/.electrum-ltc/config		ConfigOpts:        append(rpcOpts, commonOpts...),
-		ConfigOpts: commonOpts,
+		ConfigOpts: btc.CommonConfigOpts("LTC"),


Electrum needs the RPC options too.

chappjc · 2022-08-15T18:25:30Z

 		Description:       "Connect to litecoind",
 		DefaultConfigPath: dexbtc.SystemConfigPath("litecoin"),
-		ConfigOpts:        append(walletNameOpt, commonOpts...),
+		ConfigOpts:        append(btc.RPCConfigOpts("Litecoin", "9332"), btc.CommonConfigOpts("LTC")...),


commonOpts above is unused now.

chappjc

Looks cool.

I think splitting btc/spv.go is warranted though. See comments on that.

Also, I think every hash conversion is replaceable with an explicit type conversion, no copy.

chappjc · 2022-08-15T18:53:40Z

+// exceptions, and have some critical code that needed to be duplicated (in
+// order to avoid interface hell).
+type ltcSPVWallet struct {
+	// This section is populated in newSPVWallet.


populated in openSPVWallet

chappjc · 2022-08-15T19:11:57Z

@@ -1186,43 +1181,37 @@ func (w *spvWallet) connect(ctx context.Context, wg *sync.WaitGroup) error {

 // startWallet initializes the *btcwallet.Wallet and its supporting players and


docs still say startWallet

chappjc · 2022-08-15T19:15:07Z

+// btcsuite types to ltcsuite and vice-versa. Startup and shutdown are notable
+// exceptions, and have some critical code that needed to be duplicated (in
+// order to avoid interface hell).
+type ltcSPVWallet struct {


The btc parallel is walletExtender, created by newExtendedWallet, yes?
I would not object to s/walletExtender/btcSPVWallet to make this a more obvious design pattern... or even moving all the walletExtender methods into a separate file (or the higher level spvWallet methods into spv_wrapper.go). Their methods are all interleaved in the same file right now.

buck54321 · 2022-08-24T13:42:12Z

Rebased and uses dcrlabs now, and attempted to add an addpeer for testnet4, but having lots of troubles getting connected to a local node.

Not other review comments addressed yet.

chappjc · 2022-09-16T22:42:50Z

Needs rebase with the BCH one merged

chappjc · 2022-09-22T14:26:15Z

Considering the current conflict, I don't know that an external fee rate api makes sense for electrum since the fee rates are coming from external sources anyway (electrumx servers). It would be more an alternate source. Maybe that's useful?
EDIT: yeah might as well at least be an option with default of false for electrum.

chappjc · 2022-09-27T16:09:23Z

 	github.com/dcrlabs/neutrino-bch v0.0.0-20220809174944-921b271ba678 // indirect
+	github.com/dcrlabs/neutrino-ltc v0.0.0-20220819181220-04c154bb8ed8 // indirect


Just noticing that loadbot's go.mod still has the replace github.com/gcash/neutrino => github.com/buck54321/neutrino-bch

chappjc

Extremely clean port from BCH for the most part. (Comparing client/asset/ltc/spv.go and client/asset/bch/spv.go).
Compared master's client/asset/btc/spv.go and client/asset/btc/spv_wrapper.go and that checks out fine too, other than estimateSendTxFee migrating within the file.

chappjc · 2022-09-27T19:08:13Z

+		return w.chainParams
+	}
+	spoofParams := *w.chainParams
+	spoofParams.Net = ltcwire.TestNet3


Madness. Ok since it works, but I'll submit a PR to fix if this works:

replace github.com/ltcsuite/ltcwallet => github.com/chappjc/btcwallet v0.12.1-0.20220927170114-882f9a5992bc

Fix PR submitted after testing with the replace and this Net patching code commented out: ltcsuite/ltcwallet#8

chappjc · 2022-09-27T22:44:22Z

I may have made a poor suggestion with the spv.go -> spv_wrapper.go rename.
Git tracks the code better with some hand-holding https://github.com/decred/dcrdex/compare/master...chappjc:ltcspv2?expand=1
NBD either way now though. Already reviewed.

chappjc · 2022-09-28T16:37:11Z

Approved, but please rebase and resolve the go.mod conflict.

chappjc · 2022-09-28T16:37:24Z

May squash.

buck54321 force-pushed the ltcspv branch 2 times, most recently from 5065ee1 to 868e7b6 Compare August 5, 2022 00:34

chappjc added this to the 0.6/1.0 milestone Aug 12, 2022

chappjc reviewed Aug 15, 2022

View reviewed changes

chappjc requested changes Aug 15, 2022

View reviewed changes

buck54321 force-pushed the ltcspv branch from 868e7b6 to 79de1c6 Compare August 24, 2022 13:39

buck54321 force-pushed the ltcspv branch from 79de1c6 to 4a4afc1 Compare August 31, 2022 17:14

buck54321 force-pushed the ltcspv branch from 4a4afc1 to c63f2a3 Compare September 9, 2022 15:30

chappjc self-requested a review September 9, 2022 16:12

buck54321 force-pushed the ltcspv branch from c63f2a3 to 2dc1ed3 Compare September 17, 2022 19:26

chappjc reviewed Sep 17, 2022

View reviewed changes

Comment thread client/asset/btc/btc.go

buck54321 force-pushed the ltcspv branch from 2dc1ed3 to 32abdc0 Compare September 23, 2022 15:53

chappjc reviewed Sep 27, 2022

View reviewed changes

chappjc requested changes Sep 27, 2022

View reviewed changes

buck54321 force-pushed the ltcspv branch from 32abdc0 to 808f27d Compare September 28, 2022 15:57

chappjc approved these changes Sep 28, 2022

View reviewed changes

buck54321 added 4 commits September 28, 2022 11:46

implement native ltc spv wallet

14a6035

use dcrlabs. attempt testnet addpeer

a16b36f

reorganize btc spv. drop pointless conversion funcs

0e036d3

review followup

d0f62d9

add testnet4 seeds

8225f8f

buck54321 force-pushed the ltcspv branch from 808f27d to 8225f8f Compare September 28, 2022 17:09

chappjc merged commit 49df347 into decred:master Sep 28, 2022

		@@ -1186,43 +1181,37 @@ func (w spvWallet) connect(ctx context.Context, wg sync.WaitGroup) error {

		// startWallet initializes the *btcwallet.Wallet and its supporting players and

		github.com/dcrlabs/neutrino-bch v0.0.0-20220809174944-921b271ba678 // indirect
		github.com/dcrlabs/neutrino-ltc v0.0.0-20220819181220-04c154bb8ed8 // indirect

Conversation

buck54321 commented Aug 1, 2022

Uh oh!

chappjc commented Aug 1, 2022

Uh oh!

buck54321 commented Aug 1, 2022

Uh oh!

chappjc commented Aug 2, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chappjc commented Aug 15, 2022

Uh oh!

chappjc Aug 15, 2022

Choose a reason for hiding this comment

Uh oh!

chappjc Aug 15, 2022

Choose a reason for hiding this comment

Uh oh!

chappjc left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

chappjc Aug 15, 2022

Choose a reason for hiding this comment

Uh oh!

chappjc Aug 15, 2022

Choose a reason for hiding this comment

Uh oh!

chappjc Aug 15, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

buck54321 commented Aug 24, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chappjc commented Sep 16, 2022

Uh oh!

Uh oh!

chappjc commented Sep 22, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chappjc Sep 27, 2022

Choose a reason for hiding this comment

Uh oh!

chappjc left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

chappjc Sep 27, 2022

Choose a reason for hiding this comment

Uh oh!

chappjc Sep 28, 2022 • 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!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

chappjc commented Sep 27, 2022

Uh oh!

chappjc commented Sep 28, 2022

Uh oh!

chappjc commented Sep 28, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

chappjc commented Aug 2, 2022 •

edited

Loading

buck54321 commented Aug 24, 2022 •

edited

Loading

chappjc commented Sep 22, 2022 •

edited

Loading

chappjc Sep 28, 2022 •

edited

Loading