Show staked tokens on the staking page #591

jessepinho · 2024-02-21T23:39:58Z

🚨 300+ line changes are test files

In this PR

Build a hook that provides the data structure needed for rendering validators on a per-account basis.
Reorganize some of the components related to rendering validators.
Add tests for assetPatterns, and fix their regex for security implications.

Punting for now

The delegate / undelegate buttons don't do anything yet (but will after Issue delegation actions #542).
The delegation/unbonding tokens currently display quite messily. I'll need to revisit this, but this PR is already a bit large so wanted to get it merged first.

Closes #587
Closes #588

jessepinho · 2024-02-23T04:52:41Z

apps/webapp/src/components/staking/accounts/staking-actions.tsx

+}) => {
+  const hasTokens = !!delegationTokens || !!unbondingTokens;
+
+  const handleClickAction = () => alert('Not yet implemented; coming soon!');


jessepinho · 2024-02-23T05:13:14Z

apps/webapp/src/components/staking/validator-info-row.tsx

-    {/** @todo: Render an icon for each state */}
-    <TableCell>{getStateLabel(validatorInfo)}</TableCell>
-
-    {/** @todo: Render an icon for each state */}
-    <TableCell>{getBondingStateLabel(validatorInfo)}</TableCell>


Per @hdevalence these aren't really needed

jessepinho · 2024-02-23T05:16:07Z

apps/webapp/src/components/staking/validators-table.tsx


+export const ValidatorsTable = ({


Two main changes in this file:

Remove the <Card> wrapper, since that's now provided by either <AllValidators /> or <Account />.

Accept props for the validator infos/etc., rather than grabbing them from the context. This is needed because the parent components may be filtering the validator infos (such as the <Account /> component, which only renders the validator infos for a given account).

jessepinho · 2024-02-23T05:22:17Z

apps/webapp/src/components/staking/accounts/account.tsx

@@ -1,42 +0,0 @@
-import { getDisplayDenomFromView } from '@penumbra-zone/types';


Moved to account/index.tsx

grod220

I agree, the design feels a bit clunky. But honestly, no obvious notes from me how to improve it. We don't really have reference designs for the direction we are going.

In general, is the idea that the top boxes will show you who you've delegated to and those buttons allow you to delegate more to them specifically? When you click that button, will that give you a dialog box (or send you to a page) that prefills that validator in? Is a table the best design if the width doesn't allow all of the data to fit?

Overall, I think I'm far more accustomed to staking pages that say, look like Dimension's:

grod220 · 2024-02-23T11:27:24Z

apps/webapp/src/components/staking/accounts/account/helpers.ts

What do you think about located this with the rest of our Getters? Or does this feel logically different?

Yeah I'd thought about that too, but this feels different because it's actually converting the value from one type to another, kind of like the translators directory. I don't know ... It certainly would be quite ergonomic to be able to chain this to a series of getters. Maybe there could be a class of getters that starts with as, to indicate that they transform a value rather than just getting it? e.g., asBech32String?

grod220 · 2024-02-23T11:43:12Z

apps/webapp/src/components/staking/accounts/account/helpers.ts

+  if (assetPatterns.delegationToken.test(displayDenom))
+    return displayDenom.replace(assetPatterns.delegationToken, '$<bech32IdentityKey>');


How about we just grab the capture group. That way it's a little more typesafe if it changes:

const delegationMatch = assetPatterns.delegationToken.exec(displayDenom); if (delegationMatch) { const { bech32IdentityKey } = delegationMatch.groups as DelegationCaptureGroups; return bech32IdentityKey; }

whoops, good call. Fixed.

grod220 · 2024-02-23T11:54:07Z

apps/webapp/src/components/staking/validator-info-provider.tsx

Instead of react-context, could we compose our state within zustand? I hesitate having multiple kinds of state managers in the app.

Whoops 🤦🏻‍♂️ Will do

@grod220 ~~OK, I think this was the last step. Commit is here~~.

@grod220 ah crap, hang on, gotta fix a few issues I missed

@grod220 OK ready now, see here

grod220 · 2024-02-23T12:01:34Z

packages/constants/assets/index.ts

  unbondingToken: new RegExp(
-    /.*unbonding_epoch_(?<epoch>[0-9]+)_penumbravalid1(?<id>[a-zA-HJ-NP-Z0-9]+)$/,
+    /^uunbonding_epoch_(?<epoch>[0-9]+)_(?<bech32IdentityKey>penumbravalid1(?<id>[a-zA-HJ-NP-Z0-9]+))$/,


Can you add bech32IdentityKey to interface UnbondingCaptureGroups?

hdevalence · 2024-02-23T17:09:54Z

Yes, that UI is great. We should structure the four groups of rows I described to look like that.

Importantly, we should only show one account at a time, rather than multiplexing different accounts in one view.

hdevalence · 2024-02-23T18:25:08Z

apps/webapp/src/components/staking/validators-table.tsx

+            <ValidatorInfoRow
+              key={getValidator(validatorInfo).name}
+              loading={loading}
+              validatorInfo={validatorInfo}
+              votingPowerByValidatorInfo={votingPowerByValidatorInfo}
+              staking={renderStakingActions(validatorInfo)}
+            />


We should work towards the design in #569 (comment) , this should be a component for rendering a delegation token, and it should be the same component as is used for the user's delegations.

hdevalence · 2024-02-23T18:51:35Z

Does this PR try to work towards #569 (comment) ?

It looks like we have separate logic for the different sections. If we have a single component rendering delegation tokens, I think this will be much simpler. We can orient that component's visual design around the screenshot that @grod220 posted.

jessepinho · 2024-02-23T19:09:11Z

@hdevalence, sort of :) I started with the plan of creating a component that accepted both a value view and validator info, per your comment. However, I realized that it would mean that validators would be displayed in duplicate if a user has both unbonding tokens and delegation tokens for a given validator.

So instead, I grouped the delegation and unbonding tokens for a given validator together with the relevant validator info, so that each validator is only rendered once per account that has delegation OR unbonding tokens for it.

That said, is your concern more with the visual presentation of the tokens + validators? For the moment, I was just using the existing validator info rows from my previous PR, and then adding the token info in the rightmost column. I could try to redesign it a bit to highlight the token data more than the validator info, but that might be tricky since the only information to display about delegation/unbonding tokens is their balances.

jessepinho · 2024-02-23T19:10:01Z

(Also, note that I haven't yet filtered the "Active validators" table at the bottom to only include validators that the user has no delegation/unbonding tokens in, FYI. That'll come later.)

jessepinho · 2024-02-23T19:14:14Z

Sorry, one more note @hdevalence — I would like to redesign the ValueViewComponent for delegation/unbonding tokens specifically, so it's a bit more readable and portable. This design isn't very usable:

But to me, that's a separate concern from how we group ValueViewComponent with validator info in this PR.

jessepinho · 2024-02-23T19:31:12Z

apps/webapp/src/state/staking.test.ts

@@ -0,0 +1,96 @@
+import { beforeEach, describe, expect, it, vi } from 'vitest';


This file is pretty much moved from use-validator-infos.test.ts, but with some changes to account for using Zustand instead of context

jessepinho force-pushed the jessepinho/web-587-show-staked-tokens branch from 7f799f8 to 6352274 Compare February 22, 2024 03:20

Base automatically changed from jessepinho/web-586-show-unstaked-tokens to main February 22, 2024 17:06

jessepinho force-pushed the jessepinho/web-587-show-staked-tokens branch 8 times, most recently from 0e91381 to fa0942b Compare February 23, 2024 04:35

jessepinho commented Feb 23, 2024

View reviewed changes

jessepinho changed the title ~~WIP: Show staked tokens on the staking page~~ Show staked tokens on the staking page Feb 23, 2024

jessepinho marked this pull request as ready for review February 23, 2024 05:22

jessepinho requested review from grod220 and turbocrime February 23, 2024 05:22

grod220 reviewed Feb 23, 2024

View reviewed changes

jessepinho force-pushed the jessepinho/web-587-show-staked-tokens branch from b8a6a41 to 89a4885 Compare February 23, 2024 17:08

hdevalence reviewed Feb 23, 2024

View reviewed changes

jessepinho force-pushed the jessepinho/web-587-show-staked-tokens branch from da28e32 to 931d0fc Compare February 23, 2024 19:27

jessepinho commented Feb 23, 2024

View reviewed changes

jessepinho force-pushed the jessepinho/web-587-show-staked-tokens branch from 931d0fc to 50bf143 Compare February 23, 2024 19:34

jessepinho requested a review from grod220 February 23, 2024 19:37

jessepinho added 24 commits February 26, 2024 10:02

Display validator info alongside the delegation token balance

697fe03

Check for unbonding tokens when determining whether to render

36d2db0

Refactor ValidatorsTable to accept args rather than using context

5154746

Remove bonding state from the UI

ab51bf0

Tweak the unbonding token pattern

9be3817

Fix token regexes

1bd0672

Make a match group for the identity key in unbonding tokens

f90718b

Create helper to get identity key from value view

7e3f850

Export a staking token const

1b6dc16

Build out staking column

9b1242e

Reorganize

dd561ca

Extract lots of logic to a hook

fa05271

Fix bug

63a45f9

Write tests

dc6c28f

Add docs

d0bceb4

Add another test

ce05b39

Tweak var name; add docs

2d9e3f5

Tweak prop name

dc60a31

Fix test names

54b7bf7

Tweak comment

cb172e4

Add component docs

98b9e18

Add comment

62e15cd

Use typed match groups

ea1f4cd

Use a staking slice instead of a context for staking state

8e93235

jessepinho force-pushed the jessepinho/web-587-show-staked-tokens branch from 50bf143 to 8e93235 Compare February 26, 2024 18:04

jessepinho marked this pull request as ready for review February 26, 2024 18:04

Fix import

91f4700

grod220 approved these changes Feb 26, 2024

View reviewed changes

jessepinho merged commit 02bf373 into main Feb 26, 2024
7 checks passed

jessepinho deleted the jessepinho/web-587-show-staked-tokens branch February 26, 2024 18:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Show staked tokens on the staking page #591

Show staked tokens on the staking page #591

jessepinho commented Feb 21, 2024 •

edited

Loading

jessepinho Feb 23, 2024

jessepinho Feb 23, 2024

jessepinho Feb 23, 2024

jessepinho Feb 23, 2024

grod220 left a comment

grod220 Feb 23, 2024

jessepinho Feb 23, 2024

grod220 Feb 23, 2024

jessepinho Feb 23, 2024

grod220 Feb 23, 2024

jessepinho Feb 23, 2024

jessepinho Feb 23, 2024 •

edited

Loading

jessepinho Feb 23, 2024

jessepinho Feb 23, 2024

grod220 Feb 23, 2024

jessepinho Feb 23, 2024

jessepinho Feb 23, 2024

hdevalence commented Feb 23, 2024

hdevalence Feb 23, 2024

hdevalence commented Feb 23, 2024

jessepinho commented Feb 23, 2024

jessepinho commented Feb 23, 2024

jessepinho commented Feb 23, 2024

jessepinho Feb 23, 2024

		@@ -1,42 +0,0 @@
		import { getDisplayDenomFromView } from '@penumbra-zone/types';

		if (assetPatterns.delegationToken.test(displayDenom))
		return displayDenom.replace(assetPatterns.delegationToken, '$<bech32IdentityKey>');

		@@ -0,0 +1,96 @@
		import { beforeEach, describe, expect, it, vi } from 'vitest';

Show staked tokens on the staking page #591

Show staked tokens on the staking page #591

Conversation

jessepinho commented Feb 21, 2024 • edited Loading

In this PR

Punting for now

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

grod220 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

jessepinho Feb 23, 2024 • edited Loading

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

hdevalence commented Feb 23, 2024

Choose a reason for hiding this comment

hdevalence commented Feb 23, 2024

jessepinho commented Feb 23, 2024

jessepinho commented Feb 23, 2024

jessepinho commented Feb 23, 2024

Choose a reason for hiding this comment

jessepinho commented Feb 21, 2024 •

edited

Loading

jessepinho Feb 23, 2024 •

edited

Loading