-
Notifications
You must be signed in to change notification settings - Fork 18
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
EVM Staking - Validator Tables & Selection updates #1929
Conversation
✅ Deploy Preview for tangle-dapp ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for hubble-stats ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for testnet-leaderboard ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for zkexplorer ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Can you provide a video of test/QA? |
CleanShot.2024-01-03.at.09.54.16.mp4 |
@drewstone Currently on the testnet, there are zero validators in the waiting pool. This is why we can't see any waiting validators in the above screen recording. I was able to setup a validator locally and test it. The video below demonstrates the working changes. Thanks to @1xstj for helping me run the validator locally ❤️ First-time nomination flowCleanShot.2024-01-03.at.13.04.39.mp4Second time nomination flowCleanShot.2024-01-03.at.13.06.46.mp4 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left some comments.
|
||
useEffect(() => { | ||
getMaxNominationQuota().then((maxNominationQuota) => { | ||
setMaxNominationQuota(maxNominationQuota ? maxNominationQuota : 16); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The variable maxNominationQuota
can potentially be undefined
, 0
, or NaN
. I recommend implementing a more stringent check for this scenario.
const allValidators = useMemo(() => { | ||
if (!activeValidatorsData || !waitingValidatorsData) return []; | ||
|
||
return [...activeValidatorsData, ...waitingValidatorsData]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there any case where we have duplicate validators? Just out of curiosity.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No there won't be.
const [maxNominationQuota, setMaxNominationQuota] = useState<number>(0); | ||
|
||
useEffect(() => { | ||
getMaxNominationQuota().then((maxNominationQuota) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here.
const { data: waitingValidatorsData } = useSWR( | ||
[getWaitingValidators.name], | ||
([, ...args]) => getWaitingValidators(...args) | ||
); | ||
|
||
const [maxNominationQuota, setMaxNominationQuota] = useState<number>(0); | ||
|
||
useEffect(() => { | ||
getMaxNominationQuota().then((maxNominationQuota) => { | ||
setMaxNominationQuota(maxNominationQuota ? maxNominationQuota : 16); | ||
}); | ||
}, []); | ||
|
||
const allValidators = useMemo(() => { | ||
if (!activeValidatorsData || !waitingValidatorsData) return []; | ||
|
||
return [...activeValidatorsData, ...waitingValidatorsData]; | ||
}, [activeValidatorsData, waitingValidatorsData]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please deduplicate this code? You can achieve this by creating a custom hook to encapsulate the logic. Afterwards, ensure to reuse the hook in both this file and apps/tangle-dapp/containers/DelegateTxContainer/DelegateTxContainer.tsx
.
Summary of changes
Proposed area of change
apps/bridge-dapp
apps/hubble-stats
apps/stats-dapp
apps/tangle-dapp
apps/testnet-leaderboard
apps/faucet
libs/webb-ui-components
Reference issue to close (if applicable)
Screenshots