bonds: UI updates and import/export #2200

chappjc · 2023-03-03T14:11:56Z

This PR contains:

some final UI updates for the fee-->bonds change
includes bonds when doing account import/export, fix need to restart after import
a few minor client/core fixes
dep updates flagged by dependabot

martonp

During registration, on the wallet funding screen, it allows you to only deposit the amount required for 1 bond, but then when it's time to post the bond, it will reject if there is not enough for the second bond. Either we should allow the user to not have to reserve enough for the second bond, or not go to the next page.

Another issue I found was when exporting the account. I exported, created a fresh dexc with the same seed, then imported. The imported server did not show up, but after I restarted the dexc, I could see it. Might be a pre-existing issue.

martonp · 2023-03-14T20:47:15Z

client/webserver/locales/en-us.go

-	"Confirm Registration":           "Confirm Registration",
-	"app_pw_reg":                     "Enter your app password to confirm DEX registration.",
-	"reg_confirm_submit":             `When you submit this form, funds will be spent from your wallet to pay the registration fee.`,
+	"Skip Registration":              "No account (view-only mode)",


Should these be re-translated as well?

Would be nice if we had a "stale" flag like with the notifications because some of these aren't bad still.

Removed some and flagged some others with comments if they were still accurate.

martonp · 2023-03-14T20:51:49Z

client/core/bond.go

@@ -396,7 +396,9 @@ func (c *Core) rotateBonds(ctx context.Context) {

 		bondAsset := bondAssets[bondAssetID]
 		if bondAsset == nil {
-			c.log.Warnf("Bond asset %d not supported by DEX %v", bondAssetID, dc.acct.host)
+			if targetTier > 0 {
+				c.log.Warnf("Bond asset %d not supported by DEX %v", bondAssetID, dc.acct.host)


Would this only happen if the dex stopped supporting a bond asset they previously used to support?

chappjc · 2023-03-14T21:24:01Z

The imported server did not show up, but after I restarted the dexc, I could see it. Might be a pre-existing issue.

Yeah, this is the annoying initializeDEXconnections call. I'll look at avoiding the restart but I think it might need to stay this way for now.

chappjc · 2023-03-15T16:08:24Z

During registration, on the wallet funding screen, it allows you to only deposit the amount required for 1 bond, but then when it's time to post the bond, it will reject if there is not enough for the second bond. Either we should allow the user to not have to reserve enough for the second bond, or not go to the next page.

This was one of the things I was still considering. You're absolutely right of course, with the current UI. What I was pondering is a new user that might want to post a single bond without maintenance. Seeing as we'll need to add a check box for that and probably handle other edges, I think we can put that idea on the back burner until 1.0 or maybe 0.6.1. The normal user will have a maintained account.

Will change the wallet wait form so it waits for the amount that it suggests, which accounts for the overlap with maintenance running.

chappjc · 2023-03-15T20:12:05Z

client/webserver/locales/pl-pl.go

-	"1 Sync the Blockchain":       "1: Zsynchronizuj blockchain",
-	"Progress":                    "Postęp",
-	"remaining":                   "pozostało",
-	"2 Fund the Registration Fee": "2: Wnieś opłatę rejestracyjną",


I couldn't see a way to tame gofmt here, not without leaving these incorrect translations.

chappjc · 2023-03-15T20:59:05Z

(I missed the conf countdown).

chappjc · 2023-03-16T15:25:17Z

The need to restart after import should also be resolved @martonp but I've not tested that. If the required changes somehow turn out to be much bigger we'll have to punt on this.

martonp · 2023-03-16T19:52:58Z

The import basically works now, but if there was an active bond then it is not able to do a refund

2023-03-16 15:47:05.836 [WRN] CORE: Bond output not found, possibly already spent or never mined! Marking refunded. Backup refund transaction: 
2023-03-16 15:47:05.836 [ERR] CORE: Failed to mark bond as refunded: bond bucket does not exist: 376aae28be249c5c0494b21433d5df1144085b50cf957a3a2cba850e72cba8ac

I think this will require a more extensive change.

martonp · 2023-03-16T20:02:28Z

It's not a huge deal, but I think the text You may deposit as much as you like might make people think you can deposit just the 10 DCR. Maybe below the Bond Amount: 10 DCR there could also be Minimum Deposit: 20.01 in the same size text.

chappjc · 2023-03-16T20:19:51Z

The import basically works now, but if there was an active bond then it is not able to do a refund
2023-03-16 15:47:05.836 [WRN] CORE: Bond output not found, possibly already spent or never mined! Marking refunded. Backup refund transaction: 
2023-03-16 15:47:05.836 [ERR] CORE: Failed to mark bond as refunded: bond bucket does not exist: 376aae28be249c5c0494b21433d5df1144085b50cf957a3a2cba850e72cba8ac
I think this will require a more extensive change.

Was there really no refund tx in the log or did you remove it for brevity? I don't see why it wouldn't be exported and imported given

RefundTx   []byte `json:"refundTx"`

but it really needs to be.

chappjc · 2023-03-16T20:28:19Z

Oh wait, I think the server may have reported an unknown bond there. I guess I'll toy with this some more.

chappjc · 2023-03-16T21:23:16Z

Ah I need to update the async exportAccount () { in dexsettings.ts. The JSON that the browser downloads lacks bonds.

chappjc · 2023-03-17T16:15:31Z

Yeesh, finally got bond import working. General cases:

Restore from same seed, discover account done, then import account used to import previous bonds. The bond key index will derive the correct private key. We need to update any placeholder live and unspent bonds when importing, otherwise append new unspent bonds that are likely expired and need refund.
New seed used, registered fresh, then import account used to import previous bonds. The bonds do not apply to the account, so we must block this. Further, the bond key index will NOT derive the correct private key. The user must manually broadcast the refund tx from the JSON file.
New seed used, import the account with its bonds. The bonds do apply to the account, but as with the previous case the bond key index will NOT derive the correct private key, so automatic refund must use the backup refund tx (in rotateBonds). However, this pays to the old wallet. This is a messy case and should only be considered as a recovery aid.

In the last two points above, this is an unfortunate consequence of how we switched from storing the bond private key in the DB to storing the key index since that is only helpful if the app seed is unchanged. Since the backup refund tx pays to the old wallet, this is suboptimal, but at least they can restore the wallet if they had the original seed.

martonp

LGTM, all the import cases work well. Could optionally export the bond private keys so they can be used when imported with a dexc that was initialized with another seed. It's probably a niche use case though so probably not worth the time.

buck54321

I sent the exact recommended amount, but the registration sequence did not progress. I had to send more.

client/core/account.go

buck54321 · 2023-03-17T22:31:42Z

client/core/account.go

+			// TODO: less heavy handed approach to append or update
+			// dc.acct.{bonds,pendingBonds,expiredBonds}, using server config...


Is the more sophisticated technique to roughly 1) add the bond to appropriate map, 2) update tier, and 3) call monitorBondConfs?

add or update. Probably other error prone stuff.

client/webserver/site/src/js/forms.ts

chappjc · 2023-03-20T15:07:12Z

I sent the exact recommended amount, but the registration sequence did not progress. I had to send more.

Dang, that was a whoopsie from my previous fixup for @martonp's review. Updated and fixed in f55b3f2. Verification:

walletwaitfix.mp4

One more commit coming shortly for the other nits.

Also, connect acct on import. On import detect and log foreign account keys. Import now tries to merge bonds if the account already exists. If the account ID and dex pubkey do not match, import is blocked.

client/{asset,core} This adds the BondsFeeBuffer asset.Bonder method to return a generous estimate (e.g. 150%) of how much extra the user should fund their wallet to successfully post their initial bond with ReserveBondFunds. This also adds a Core method to obtain this amount for a bond wallet. client/webserver Add the "/bondsfeebuffer" endpoint, which is intended to inform the UI's WalletWaitForm.

This silences a spammy log message that is show when a DEX server does not support the user's selected bond asset AND bond maintenance is enabled for that server. This would only happen if they were using bonds with that server and it had then stopped supporting that asset.

This rewords the UI's registration forms for the fee->bond change. This also changes the wallet funding step to use the new /bondsfeebuffer http API endpoint. The overlap factor (2) is *advised* at the level of the frontend, assuming the user wants uninterrupted bond maintainance. However, the wallet fail form will allow proceeding with the just bond amount plus a fee buffer. RESERVES DEFICIT may show in balance if the user posts a single bond and then decided to enable bond maintanence (target tier). remove stale web translations

chappjc added this to the 0.6 milestone Mar 10, 2023

chappjc force-pushed the bond-ui branch from 4125c08 to 7d39ed6 Compare March 13, 2023 21:30

chappjc mentioned this pull request Mar 14, 2023

client/core: Unlock wallet during PostBond #2222

Closed

martonp reviewed Mar 14, 2023

View reviewed changes

chappjc force-pushed the bond-ui branch from 7d39ed6 to 26b1129 Compare March 15, 2023 20:10

chappjc commented Mar 15, 2023

View reviewed changes

chappjc force-pushed the bond-ui branch 2 times, most recently from 2d1e13f to e6547ec Compare March 15, 2023 20:56

chappjc force-pushed the bond-ui branch from e6547ec to ba03c47 Compare March 15, 2023 20:59

chappjc marked this pull request as ready for review March 15, 2023 20:59

chappjc force-pushed the bond-ui branch from ba03c47 to 8cf67fa Compare March 15, 2023 21:15

chappjc force-pushed the bond-ui branch 2 times, most recently from a133419 to bcb8286 Compare March 17, 2023 15:36

chappjc force-pushed the bond-ui branch from bcb8286 to 5d29193 Compare March 17, 2023 16:16

martonp approved these changes Mar 17, 2023

View reviewed changes

buck54321 requested changes Mar 20, 2023

View reviewed changes

chappjc force-pushed the bond-ui branch from 54fe051 to f55b3f2 Compare March 20, 2023 15:06

chappjc added 2 commits March 20, 2023 10:20

client/{db,webserver}: bonds import/export with account

6c8ee4b

Also, connect acct on import. On import detect and log foreign account keys. Import now tries to merge bonds if the account already exists. If the account ID and dex pubkey do not match, import is blocked.

chappjc added 2 commits March 20, 2023 10:21

client/core: postbond automatically unlocks the wallet

1cc2347

buck54321 approved these changes Mar 20, 2023

View reviewed changes

chappjc added 4 commits March 20, 2023 10:24

ui: hide bond strength form until it has utility

0df3ef3

npm audit fix: CVE-2023-28154 (webpack 5.76)

431c711

deps: golang.org/x repos for net CVE

08185ca

chappjc force-pushed the bond-ui branch from d1dd0f1 to 08185ca Compare March 20, 2023 15:26

chappjc merged commit ac18594 into decred:master Mar 20, 2023

chappjc deleted the bond-ui branch March 20, 2023 15:36

chappjc added the bonds fidelity bonds label Mar 31, 2023

chappjc mentioned this pull request Jun 16, 2023

[0.6.2] btc: more sensible bond fee buffer #2392

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

bonds: UI updates and import/export #2200

bonds: UI updates and import/export #2200

chappjc commented Mar 3, 2023 •

edited

Loading

martonp left a comment

martonp Mar 14, 2023

chappjc Mar 15, 2023 •

edited

Loading

chappjc Mar 15, 2023

martonp Mar 14, 2023

chappjc commented Mar 14, 2023

chappjc commented Mar 15, 2023

chappjc Mar 15, 2023

chappjc commented Mar 15, 2023

chappjc commented Mar 16, 2023

martonp commented Mar 16, 2023

martonp commented Mar 16, 2023

chappjc commented Mar 16, 2023

chappjc commented Mar 16, 2023

chappjc commented Mar 16, 2023

chappjc commented Mar 17, 2023 •

edited

Loading

martonp left a comment

buck54321 left a comment •

edited

Loading

buck54321 Mar 17, 2023

chappjc Mar 20, 2023

chappjc commented Mar 20, 2023 •

edited

Loading

		// TODO: less heavy handed approach to append or update
		// dc.acct.{bonds,pendingBonds,expiredBonds}, using server config...

bonds: UI updates and import/export #2200

bonds: UI updates and import/export #2200

Conversation

chappjc commented Mar 3, 2023 • edited Loading

martonp left a comment

Choose a reason for hiding this comment

martonp Mar 14, 2023

Choose a reason for hiding this comment

chappjc Mar 15, 2023 • edited Loading

Choose a reason for hiding this comment

chappjc Mar 15, 2023

Choose a reason for hiding this comment

martonp Mar 14, 2023

Choose a reason for hiding this comment

chappjc commented Mar 14, 2023

chappjc commented Mar 15, 2023

chappjc Mar 15, 2023

Choose a reason for hiding this comment

chappjc commented Mar 15, 2023

chappjc commented Mar 16, 2023

martonp commented Mar 16, 2023

martonp commented Mar 16, 2023

chappjc commented Mar 16, 2023

chappjc commented Mar 16, 2023

chappjc commented Mar 16, 2023

chappjc commented Mar 17, 2023 • edited Loading

martonp left a comment

Choose a reason for hiding this comment

buck54321 left a comment • edited Loading

Choose a reason for hiding this comment

buck54321 Mar 17, 2023

Choose a reason for hiding this comment

chappjc Mar 20, 2023

Choose a reason for hiding this comment

chappjc commented Mar 20, 2023 • edited Loading

chappjc commented Mar 3, 2023 •

edited

Loading

chappjc Mar 15, 2023 •

edited

Loading

chappjc commented Mar 17, 2023 •

edited

Loading

buck54321 left a comment •

edited

Loading

chappjc commented Mar 20, 2023 •

edited

Loading