Skip to content
This repository has been archived by the owner on Sep 26, 2024. It is now read-only.

feat(frontend): Implementation of new design on Nodes page #613

Merged
merged 12 commits into from
May 18, 2021

Conversation

shelegdmitriy
Copy link
Contributor

@shelegdmitriy shelegdmitriy commented Apr 22, 2021

Resolves #370

Test plan

  • Existing tests pass;
  • ValidatorsList unit test covering a list of validators and proposals with all data;
  • ValidatorsRow unit test covering a single validator row with all data;
  • ValidatorsRow unit test covering a single validator row with partial data;
  • NodeEpoch unit test covering a section with information about epoch status;

@shelegdmitriy shelegdmitriy requested a review from frol April 22, 2021 13:52
@shelegdmitriy shelegdmitriy self-assigned this Apr 22, 2021
@render
Copy link

render bot commented Apr 22, 2021

@shelegdmitriy shelegdmitriy force-pushed the feat/design_dashboard_for_validators branch from 7e1afc0 to 256eb68 Compare April 22, 2021 13:53
@shelegdmitriy
Copy link
Contributor Author

@frol, currently I don't know how to get some data. But I have some thoughts of how to do it

backend/src/wamp.js Outdated Show resolved Hide resolved
@frol frol mentioned this pull request May 4, 2021
2 tasks
@shelegdmitriy shelegdmitriy force-pushed the feat/design_dashboard_for_validators branch from 4a3e2a4 to 5fd1044 Compare May 5, 2021 10:06
backend/src/index.js Outdated Show resolved Hide resolved
backend/src/near.js Outdated Show resolved Hide resolved
backend/src/near.js Outdated Show resolved Hide resolved
backend/src/near.js Outdated Show resolved Hide resolved
frontend/src/components/nodes/CumuativeStakeChart.tsx Outdated Show resolved Hide resolved
@frol frol changed the title feat(fronted): Implementation of new design on Nodes page feat(frontend): Implementation of new design on Nodes page May 5, 2021
@shelegdmitriy shelegdmitriy force-pushed the feat/design_dashboard_for_validators branch 6 times, most recently from 42c0105 to 0237158 Compare May 6, 2021 14:47
@shelegdmitriy shelegdmitriy marked this pull request as ready for review May 7, 2021 07:48
Comment on lines 12 to 17
<div className="total-value" style={{ width: `${value.total}%` }} />
<div className="current-value" />
<div className="cumulative-stake-label">{value.current}%</div>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is just a coincidence that the width of the dark blue chunk has a constant size

image

On the second line the light-blue block on the left represents 27% of the width, and the width of the dark blue line is the % of the current stake out of the total stake (8% in that case). In other words, 27%+8% = 35%

Comment on lines 30 to 33
Number(
utils.format.formatNearAmount(amount.toString(), 0).replace(/,/g, "")
) /
10 ** 6
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You do not need to format the amount and then convert it back to Number. If amount is BN, just divide it by 10 ** (24 + 9)

Comment on lines 45 to 64
const currentTimestamp = moment() as moment.Moment;
const endEpochTimestamp = moment(
this.props.epochStartBlock.timestamp
).add(12, "h");
const duration = moment
.duration(
endEpochTimestamp.diff(moment(this.props.epochStartBlock.timestamp))
)
.asSeconds();
const durationProseed = moment
.duration(
currentTimestamp.diff(moment(this.props.epochStartBlock.timestamp))
)
.asSeconds();
const durationValue = (durationProseed / duration) * 100;
this.setState({
timeRemaining: moment
.duration(endEpochTimestamp.diff(currentTimestamp))
.asMilliseconds(),
epochProseed: Number(durationValue.toFixed(2)),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

epochProgress = (latestBlock.height - epochStartBlock.height) / epochLength * 100
timeRemaining = (latestBlock.timestamp - epochStartBlock.timestamp) / epochProgress * (100 - epochProgress)

Comment on lines 101 to 105
(
{timeRemaining
? moment(timeRemaining)?.format("HH:mm:ss")
: "00:00:00"}{" "}
remaining)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel this is easier to read:

Suggested change
(
{timeRemaining
? moment(timeRemaining)?.format("HH:mm:ss")
: "00:00:00"}{" "}
remaining)
{`({timeRemaining
? moment(timeRemaining)?.format("HH:mm:ss")
: "00:00:00"} remaining)`}

render() {
const { node } = this.props;
const { node, index, cellCount, validatorType } = this.props;
let persntStake = 0;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The naming looks odd to me O_o

class ValidatorsList extends React.PureComponent<Props> {
calculateStake = (nodeIndex: number, totalStake: BN) => {
let total = new BN(0);
let networkHolderIndex = [];
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't need an array here, you only care about the very first value. Just initialize it as null and only set it if the accumulated stake is over 33% and also this variable is still null.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could not figure out of how to do it differently here

for (let i = 0; i <= nodeIndex; i++) {
total = total.add(new BN(this.props.validators[i].stake));

if (total.gt(totalStake.div(new BN(3)))) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Precompute the totalStake.divn(3) in a const before the loop, so you don't need to re-compute it over and over again in the loop.

@@ -5,19 +5,23 @@ import { OverlayTrigger, Tooltip } from "react-bootstrap";

const FRAC_DIGITS = 5;

const Balance = ({ amount }) => {
const Balance = ({ amount, label = null, className = "" }) => {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It think it is better to leave className undefined if it was not defined

Suggested change
const Balance = ({ amount, label = null, className = "" }) => {
const Balance = ({ amount, label = null, className }) => {

return () => {
explorerApi.unsubscribe("node-stats");
};
}, []);
}, [epochStartHeight]);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Create a separate useEffect since otherwise, this code will re-subscribe to node-stats for no good reason.

@@ -285,6 +319,24 @@ async function main() {
};
setTimeout(regularCheckNodeStatus, 0);

// Periodic check of validator's fee and delegators
const regularCheckValidatorsExtraInfo = async (validators) => {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use a dedicated setTimeout for this regularCheckValidatorsExtraInfo, and don't call it from regularCheckNodeStatus. Fetch the validators (current validators and current proposals) inside this regular job, so you do not need to call it from regularCheckNodeStatus.

@frol
Copy link
Collaborator

frol commented May 7, 2021

Bugreport:

image

B, M are off.

Seat price precision should does not need the float precision.

Comment on lines 313 to 356
const regularCheckValidatorsExtraInfo = async () => {
if (validators) {
validatorsExtraInfo = validatorsExtraInfo || validators;

for (let i = 0; i < validatorsExtraInfo.length; i++) {
const { account_id } = validatorsExtraInfo[i];
validatorsExtraInfo[i].fee = await nearRpc.callViewMethod(
account_id,
"get_reward_fee_fraction",
{}
);
validatorsExtraInfo[i].delegators = await nearRpc.callViewMethod(
account_id,
"get_number_of_accounts",
{}
);
}
}
return validators;

if (proposals) {
proposalsExtraInfo = proposalsExtraInfo || proposals;

for (let i = 0; i < proposalsExtraInfo.length; i++) {
const { account_id } = proposalsExtraInfo[i];
proposalsExtraInfo[i].fee = await nearRpc.callViewMethod(
account_id,
"get_reward_fee_fraction",
{}
);
proposalsExtraInfo[i].delegators = await nearRpc.callViewMethod(
account_id,
"get_number_of_accounts",
{}
);
}
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@frol , I don't know is it a good idea or not... Could you please look at it and tell me your mind?

Comment on lines 34 to 38
value = (Number(amount) / 10 ** (24 + 6)).toFixed(1);
} else if (type === "totalStakeAmount") {
value =
(
Number(
utils.format.formatNearAmount(amount.toString(), 0).replace(/,/g, "")
) /
10 ** 3
).toFixed(1) + "M";
value = (Number(amount) / 10 ** (24 + 3)).toFixed(1);
} else if (type === "seatPriceAmount") {
value = (Number(amount) / 10 ** 24).toFixed(1);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@frol , also this looks strange but with BN it fails

@frol
Copy link
Collaborator

frol commented May 12, 2021

@shelegdmitriy I had to move a lot of things around to hopefully name things a little bit better. There are lots of things that remain to be named in a way that is hard to reason about, but I did not want to block this PR even more.

Please, go over the Figma design and especially font sizes as it seems that they might be off.

Bug report:

image

  1. missing "M" (million) suffix
  2. let's make total stake the same magnitude as total supply ("M")
  3. Let's add comma-formatting

@shelegdmitriy shelegdmitriy force-pushed the feat/design_dashboard_for_validators branch from ecb0a96 to fb7ba55 Compare May 13, 2021 06:05
@shelegdmitriy shelegdmitriy force-pushed the feat/design_dashboard_for_validators branch 2 times, most recently from 12360ad to 2075cf5 Compare May 13, 2021 07:02
@shelegdmitriy shelegdmitriy force-pushed the feat/design_dashboard_for_validators branch from 2075cf5 to c3c092d Compare May 13, 2021 07:18
@frol
Copy link
Collaborator

frol commented May 14, 2021

Bugs on mobile:

image

image

@frol frol merged commit 31209e4 into master May 18, 2021
@frol frol deleted the feat/design_dashboard_for_validators branch May 18, 2021 14:59
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Design Dashboard for Validators
2 participants