client/dcr: Add staking methods. #2290

JoeGruffins · 2023-04-07T08:55:55Z

part of #2264

This pr only adds capabilities to the asset and simnet testing framework.

Many of the changes are files copied from vspd and wallet. Hopefully the fee paying logic can be exported from wallet so we won't need to add here.

This will be updated.

martonp · 2023-04-11T15:55:19Z

Would it make sense to have a separate VSP library in a new repo containing all the files you copied? And then vspd, dex, and other projects could just import it.

buck54321 · 2023-04-11T16:20:34Z

Would it make sense to have a separate VSP library in a new repo containing all the files you copied? And then vspd, dex, and other projects could just import it.

@jholdstock wanted to put it in the vspd repo anyway. Maybe @JoeGruffins could open up a pull request over there too. But we don't want to wait for them either. We'll delete this stuff once it's available.

chappjc · 2023-04-11T16:38:10Z

If decred/dcrwallet#2224 (or something like it) happens, are we still left with a bunch of copied files? I didn't see the discussion with @jholdstock about putting this stuff in the vspd repo.

buck54321 · 2023-04-11T17:59:42Z

If decred/dcrwallet#2224 (or something like it) happens, are we still left with a bunch of copied files? I didn't see the discussion with @jholdstock about putting this stuff in the vspd repo.

Matrix link

I didn't know about #2224. @JoeGruffins shouldn't we be targeting a move to vspd?

JoeGruffins · 2023-04-12T04:40:25Z

Asking on matrix if there were plans to move the rest of the dcrwallet/internal/vsp package to the decred/vspd repo.

Not sure about the files in dex/testing/dcr/vspdtemplates but the simnet vspd panics without them so I guess we need to copy to use vspd from the test folder location. I believe dexc also used to require files in a certain location like this but we build them into the executable at some point.

chappjc · 2023-04-12T19:12:04Z

Echoing the diffs proposed on matrix:

buck54321 · 2023-04-24T18:41:06Z

Could do this while waiting for decred/vspd#382.

buck54321 · 2023-04-24T23:08:38Z

I was playing with this a little. Please consider these changes.

Consolidates pure getters into a single call
Adds extra ticket details
Prepares for pagination

*untested

JoeGruffins · 2023-04-25T01:30:48Z

replace github.com/decred/vspd/client/v2 => github.com/jholdstock/vspd/client/v2 v2.0.0-20230424165325-5d3f56b5c4e4 Wow didn't realize that was a thing we could do. Will adjust.

JoeGruffins · 2023-04-25T04:16:02Z

Made the buck change suggestions. For some reason go mod tidy does not error when I run it but does with the testing script. invalid version: unknown revision 000000000000

Oh yeah, loadbot's go.mod. Ok now.

buck54321

Do we need to do something with (*AutoClient).ProcessUnprocessedTickets?

client/asset/dcr/dcr.go

buck54321 · 2023-05-24T01:22:30Z

client/asset/dcr/dcr.go

+}
+
+// PurchaseTickets purchases n amout of tickets. Ability to purchase should be
+// checked with CanPurchaseTickets. Part of the asset.TicketBuyer interface.


CanPurchaseTickets?

Yeah. The original plan was to check the wallet config before purchasing over rpc, and we would deny if not using a vsp. But, giving up on that since it seems we cannot get the config. User will just have to know what they are doing. Anyway, removing the reference to the removed method.

client/asset/interface.go

buck54321 · 2023-05-24T01:28:31Z

client/asset/dcr/dcr.go

+	if !dcr.connected.Load() {
+		return nil, errors.New("not connected, login first")
+	}


I'm wondering what these dcr.connected checks do for us. We could theoretically add this check to almost any ExchangeWallet method, but we don't because we expect the caller to know better, and continuing without being connected would generate an error anyway. What would happen if we didn't check this?

My memory is fuzzy, but these will panic if the dcr.ctx is nil, which is so before the first connect.

Yeah, I added this to any methods that use dcr.ctx to avoid using a nil context and panic inside the rpcclient. Should I handle this differently?

buck54321 · 2023-05-24T01:48:19Z

client/asset/dcr/rpcwallet.go

+
+func (w *rpcWallet) Tickets(ctx context.Context) ([]*asset.Ticket, error) {
+	const includeImmature = true
+	// GetTickets only works for clients with a dcrd backend.


Wow. I didn't know that. That's unfortunate. It looks like it's just for the extraTickets checks in (*Wallet).LiveTicketHashes. Otherwise all the data is just from the db.

client/asset/dcr/spv.go

buck54321 · 2023-05-24T14:53:15Z

client/asset/dcr/spv.go

+			w.log.Warnf("unable to get VSP associated with ticket %s: %v", hash, err)
+			return nil
+		}
+		info, err := vspInfo(vspHost)


Probably shouldn't call vspInfo in a loop since it contains an http request. Maybe keep a cache map[string]*vspclient.AutoClient that you can check first.

Good idea. A map local to the function good enough?

client/asset/dcr/spv_test.go

buck54321 · 2023-06-05T20:14:17Z

Can get rid of the replace and rebase now, I think. Probably good to pull out of draft too.

JoeGruffins · 2023-06-06T08:11:37Z

vsp still has several replaces, https://github.com/decred/vspd/blob/master/go.mod

If we import vspd I guess we also need to use those replaces? And does the entire repo need to move to dcrwallet/v3 then?

JoeGruffins · 2023-06-08T06:39:07Z

buck changes https://github.com/decred/dcrdex/compare/0eaf8a3d498cc05e4d7646cba9e89e1851ea757d..3f91df5d9253ec15136656a877e44e29ded1b22d

JoeGruffins · 2023-06-08T07:48:56Z

Do we need to do something with (*AutoClient).ProcessUnprocessedTickets?

Maybe. Client could do this if wallet was reset from seed I think. Is it ok to make a todo for now? Because of automatic revocations now, its not a HUGE deal even if the vsp is not set up. Also, if the fee was paid, and the vspd is ok, the wallet doesnt need to worry about it, really. The vsp will vote and the wallet will get the funds back at some point.

JoeGruffins · 2023-06-08T07:52:38Z

Made a TODO for managing unmanaged tickets https://github.com/decred/dcrdex/compare/be02793d87ec38b68e79012b4a1b0c047643f761..82014c37e3f680b799345fd1f08bdb429089594f

JoeGruffins · 2023-06-08T07:54:14Z

So, copying the vspd client logic for now but using dcrwallet/v2. Hopefully can be removed sooner or later, but this seems like the only way to continue immediately.

buck54321 · 2023-06-16T21:01:51Z

vspd cleaned up their go.mod and chappjc is working through the dcrwallet v3 upgrade now.

chappjc · 2023-07-13T05:59:16Z

Nice work. 💪

client/webserver/site/package-lock.json

JoeGruffins · 2023-07-14T01:23:01Z

sed amout -> amount https://github.com/decred/dcrdex/compare/ce32862ef975be44d1370b19382bc543dbc814bb..674e1795ce0c32b30b324993854a137a514b77a8

chappjc · 2023-07-17T16:18:11Z

client/cmd/dexc-desktop/go.mod

@@ -5,7 +5,7 @@ go 1.18
 replace decred.org/dcrdex => ../../..

 require (
-	decred.org/dcrdex v0.6.1
+	decred.org/dcrdex v0.6.0


looks like an unintentional rollback

Fixed! I guess this value doesn't really matter with the replace?

Sorry to dwell on this, but all the indirect depends resulting from this are still changed. Best just to restore these files from master e.g. git restore --source=master go.mod

Ok! Will add that command to my knowledge.

dex/testing/dcr/vspdtemplates/header.html

ukane-philemon

utAck. Good work @JoeGruffins.

client/asset/dcr/dcr.go

JoeGruffins · 2023-07-18T08:48:42Z

Just clone vspd for web files and fix some names https://github.com/decred/dcrdex/compare/83e734a95df1af93f6a5c3b1c4eb345bd816b10f..2f64728b70a0292a544718cd0b83d29c08eaa914

Fix dexc-desktop version in go.mod https://github.com/decred/dcrdex/compare/2f64728b70a0292a544718cd0b83d29c08eaa914..e35ffb5416101fe991099a47bfe0aabfbefd1039

JoeGruffins · 2023-07-19T01:48:04Z

git restore --source=master go.mod https://github.com/decred/dcrdex/compare/e35ffb5416101fe991099a47bfe0aabfbefd1039..577da2e864436624888cd50a34fbf7b0d7c76ed5

JoeGruffins · 2023-07-19T01:53:45Z

go mod tidy... https://github.com/decred/dcrdex/compare/577da2e864436624888cd50a34fbf7b0d7c76ed5..29d50b0a7e546a0a63499f5b889df74e8448c05d

client/asset/dcr/dcr.go

ukane-philemon · 2023-07-28T00:08:41Z

client/asset/dcr/dcr.go

+
+// SetVSP sets the VSP provider. Ability to set should be checked with CanSetVSP
+// first. Part of the asset.TicketBuyer interface.
+func (dcr *ExchangeWallet) SetVSP(url string) error {


It seems SetVSP and other TicketBuyer methods are not consumed anywhere, except in simnet_test.go, this means consumption of these new methods/features would be in a follow-up PR, no?

Yes. With the rpcserver in #2316 and webserver not done yet.

client/asset/dcr/simnet_test.go

client/asset/dcr/dcr.go

ukane-philemon · 2023-07-28T00:22:50Z

client/asset/dcr/wallet.go

+	// PurchaseTickets purchases n tickets. vsp arguments only needed for
+	// internal wallets.
+	PurchaseTickets(ctx context.Context, n int, vspHost, vspPubKey string) ([]string, error)


Please can we specify the exact arguments that are not needed for RPC wallets? I see only vspHost and vspPubKey are ignored and all other arguments are provided. vsp arguments might not ring a bell the first time.

vspHost and vspPubKey only needed for internal wallets. okay?

buck54321

Mostly just some nits for now. Testing well.

dex/testing/dcr/create-vspd.sh

client/asset/dcr/dcr.go

client/asset/dcr/simnet_test.go

buck54321 · 2023-07-30T06:39:47Z

dex/testing/dcr/harness.sh

+MANUAL_TICKETS="1"
+"${HARNESS_DIR}/create-wallet.sh" "$SESSION:7" "vspdwallet" ${VSPD_WALLET_SEED} \
+${VSPD_WALLET_RPC_PORT} ${USE_SPV} ${ENABLE_VOTING} "_" ${MANUAL_TICKETS}


I may not understand the config option, but why manualtickets=1 for this wallet?

Oh I don't really know, just following the guide.

dcrwallet is also required to have the manual tickets option (--manualtickets) enabled which disables dcrwallet adding tickets arriving over the network.

https://github.com/decred/vspd/blob/master/docs/deployment.md#voting-servers

client/asset/dcr/dcr.go

client/asset/dcr/simnet_test.go

client/asset/dcr/dcr.go

client/asset/interface.go

JoeGruffins · 2023-08-05T10:46:00Z

Addressing reviews https://github.com/decred/dcrdex/compare/467337e1993823a1ffcac08d0e966282cc789548..c3b9e76eae170af3e91a6573d20c870ad2b36857

Notable changes are removing the vsp file and just getting the fee and pubkey on wallet start and adding some test parameters.

JoeGruffins · 2023-08-07T04:24:55Z

ty @buck54321 https://github.com/decred/dcrdex/compare/877601896fc96501c2ba2ac01b837d234d21187b..bd79ca33db163d89bd2ae4e1080afb5109c2e1b8

Also using the separate file to save again.

buck54321

Good stuff. I'll follow up with a couple of tweaks.

itswisdomagain · 2023-08-15T13:37:20Z

client/asset/dcr/dcr.go

@@ -5134,6 +5174,125 @@ func (dcr *ExchangeWallet) EstimateSendTxFee(address string, sendAmount, feeRate
 	return finalFee, isValidAddress, nil
 }

+func (dcr *ExchangeWallet) isSPV() bool {
+	return dcr.walletType == walletTypeSPV


This reminds me. There's a dcr.wallet.SpvMode() which may be true even for a wallet connected via rpc. Put another way, isSPV could be true even if walletType isn't walletTypeSPV.

JoeGruffins force-pushed the dcrstaking branch from 46cc4e7 to 13f8233 Compare April 7, 2023 09:46

JoeGruffins force-pushed the dcrstaking branch 2 times, most recently from be0e3d7 to 80ef47a Compare April 13, 2023 06:57

JoeGruffins force-pushed the dcrstaking branch from 80ef47a to cb2868e Compare April 20, 2023 08:19

JoeGruffins mentioned this pull request Apr 20, 2023

client/dcr: Add rpc staking. #2316

Merged

JoeGruffins force-pushed the dcrstaking branch from cb2868e to 485c9c1 Compare April 25, 2023 04:14

JoeGruffins force-pushed the dcrstaking branch from 485c9c1 to 427095d Compare April 25, 2023 04:24

buck54321 requested changes May 24, 2023

View reviewed changes

JoeGruffins force-pushed the dcrstaking branch 2 times, most recently from 0eaf8a3 to 3f91df5 Compare June 8, 2023 06:38

JoeGruffins force-pushed the dcrstaking branch from 3f91df5 to be02793 Compare June 8, 2023 07:20

JoeGruffins force-pushed the dcrstaking branch from be02793 to 82014c3 Compare June 8, 2023 07:51

JoeGruffins marked this pull request as ready for review June 8, 2023 07:52

JoeGruffins commented Jul 13, 2023

View reviewed changes

client/webserver/site/package-lock.json Outdated Show resolved Hide resolved

JoeGruffins force-pushed the dcrstaking branch 2 times, most recently from ce32862 to 674e179 Compare July 14, 2023 01:21

chappjc reviewed Jul 17, 2023

View reviewed changes

dex/testing/dcr/vspdtemplates/header.html Outdated Show resolved Hide resolved

ukane-philemon suggested changes Jul 17, 2023

View reviewed changes

JoeGruffins force-pushed the dcrstaking branch 3 times, most recently from 2f64728 to e35ffb5 Compare July 18, 2023 08:46

JoeGruffins force-pushed the dcrstaking branch from e35ffb5 to 577da2e Compare July 19, 2023 01:47

JoeGruffins force-pushed the dcrstaking branch from 577da2e to 29d50b0 Compare July 19, 2023 01:53

ukane-philemon suggested changes Jul 28, 2023

View reviewed changes

buck54321 requested changes Jul 30, 2023

View reviewed changes

itswisdomagain reviewed Jul 31, 2023

View reviewed changes

client/asset/dcr/dcr.go Show resolved Hide resolved

client/asset/interface.go Outdated Show resolved Hide resolved

JoeGruffins force-pushed the dcrstaking branch 2 times, most recently from 467337e to c3b9e76 Compare August 5, 2023 10:44

JoeGruffins force-pushed the dcrstaking branch from c3b9e76 to 8776018 Compare August 7, 2023 04:04

client/dcr: Add staking methods.

bd79ca3

JoeGruffins force-pushed the dcrstaking branch from 8776018 to bd79ca3 Compare August 7, 2023 04:23

buck54321 approved these changes Aug 7, 2023

View reviewed changes

buck54321 merged commit ae95dca into decred:master Aug 7, 2023

itswisdomagain reviewed Aug 15, 2023

View reviewed changes

buck54321 mentioned this pull request Sep 17, 2023

btc: redo update btcwallet for panic fix #2528

Merged

client/dcr: Add staking methods. #2290

client/dcr: Add staking methods. #2290

Conversation

JoeGruffins commented Apr 7, 2023

martonp commented Apr 11, 2023

buck54321 commented Apr 11, 2023

chappjc commented Apr 11, 2023 • edited Loading

buck54321 commented Apr 11, 2023

JoeGruffins commented Apr 12, 2023

chappjc commented Apr 12, 2023

buck54321 commented Apr 24, 2023

buck54321 commented Apr 24, 2023 • edited Loading

JoeGruffins commented Apr 25, 2023

JoeGruffins commented Apr 25, 2023 • edited Loading

buck54321 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

buck54321 commented Jun 5, 2023

JoeGruffins commented Jun 6, 2023

JoeGruffins commented Jun 8, 2023

JoeGruffins commented Jun 8, 2023

JoeGruffins commented Jun 8, 2023

JoeGruffins commented Jun 8, 2023

buck54321 commented Jun 16, 2023

chappjc commented Jul 13, 2023 • edited Loading

JoeGruffins commented Jul 14, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ukane-philemon left a comment

Choose a reason for hiding this comment

JoeGruffins commented Jul 18, 2023

JoeGruffins commented Jul 19, 2023

JoeGruffins commented Jul 19, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

buck54321 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JoeGruffins commented Aug 5, 2023

JoeGruffins commented Aug 7, 2023

buck54321 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chappjc commented Apr 11, 2023 •

edited

Loading

buck54321 commented Apr 24, 2023 •

edited

Loading

JoeGruffins commented Apr 25, 2023 •

edited

Loading

chappjc commented Jul 13, 2023 •

edited

Loading