fix(governance): handle null validator scores properly (#5459)

apps/governance/src/routes/staking/home/validator-tables/consensus-validators-table.spec.tsx

@@ -16,7 +16,7 @@ import type { ValidatorsView } from './validator-tables';
 const nodeFactory = (
   overrides?: PartialDeep<NodesFragmentFragment>
 ): NodesFragmentFragment => {
-  const defaultNode = {
+  const defaultNode: NodesFragmentFragment = {
     id: 'ccc022b7e63a4d0a6d3a193c3940c88574060e58a184964c994998d86835a1b4',
     name: 'high',
     avatarUrl: 'https://upload.wikimedia.org/wikipedia/en/2/25/Marvin-TV-3.jpg',
@@ -288,7 +288,7 @@ describe('Consensus validators table', () => {
 
     expect(
       grid.querySelector('[role="gridcell"][col-id="totalPenalties"]')
-    ).toHaveTextContent('10.07%');
+    ).toHaveTextContent('13.16%');
 
     expect(
       grid.querySelector('[role="gridcell"][col-id="normalisedVotingPower"]')

apps/governance/src/routes/staking/home/validator-tables/consensus-validators-table.tsx

@@ -185,6 +185,15 @@ export const ConsensusValidatorsTable = ({
           const { rawValidatorScore: previousEpochValidatorScore } =
             getLastEpochScoreAndPerformance(previousEpochData, id);
 
+          const overstakingPenalty = calculateOverallPenalty(
+            id,
+            allNodesInPreviousEpoch
+          );
+          const totalPenalty = calculateOverstakedPenalty(
+            id,
+            allNodesInPreviousEpoch
+          );
+
           return {
             id,
             [ValidatorFields.RANKING_INDEX]: stakedTotalRanking,
@@ -213,11 +222,11 @@ export const ConsensusValidatorsTable = ({
               2
             ),
             [ValidatorFields.OVERSTAKING_PENALTY]: formatNumberPercentage(
-              calculateOverstakedPenalty(id, allNodesInPreviousEpoch),
+              overstakingPenalty,
               2
             ),
             [ValidatorFields.TOTAL_PENALTIES]: formatNumberPercentage(
-              calculateOverallPenalty(id, allNodesInPreviousEpoch),
+              totalPenalty,
               2
             ),
             [ValidatorFields.PENDING_STAKE]: pendingStake,

apps/governance/src/routes/staking/home/validator-tables/standby-pending-validators-table.tsx

@@ -124,6 +124,15 @@ export const StandbyPendingValidatorsTable = ({
             }
           }
 
+          const overstakingPenalty = calculateOverallPenalty(
+            id,
+            allNodesInPreviousEpoch
+          );
+          const totalPenalty = calculateOverstakedPenalty(
+            id,
+            allNodesInPreviousEpoch
+          );
+
           return {
             id,
             [ValidatorFields.RANKING_INDEX]: stakedTotalRanking,
@@ -154,11 +163,11 @@ export const StandbyPendingValidatorsTable = ({
               2
             ),
             [ValidatorFields.OVERSTAKING_PENALTY]: formatNumberPercentage(
-              calculateOverstakedPenalty(id, allNodesInPreviousEpoch),
+              overstakingPenalty,
               2
             ),
             [ValidatorFields.TOTAL_PENALTIES]: formatNumberPercentage(
-              calculateOverallPenalty(id, allNodesInPreviousEpoch),
+              totalPenalty,
               2
             ),
             [ValidatorFields.PENDING_STAKE]: pendingStake,

apps/governance/src/routes/staking/node/validator-table.tsx

@@ -266,7 +266,9 @@ export const ValidatorTable = ({
 
                 <Tooltip description={t('OverstakedPenaltyDescription')}>
                   <span data-testid="overstaking-penalty">
-                    {formatNumberPercentage(penalties.overstaked, 2)}
+                    {penalties.overstaked
+                      ? formatNumberPercentage(penalties.overstaked, 2)
+                      : '-'}
                   </span>
                 </Tooltip>
               </KeyValueTableRow>
@@ -285,7 +287,9 @@ export const ValidatorTable = ({
                 </span>
                 <span data-testid="total-penalties">
                   <strong>
-                    {formatNumberPercentage(penalties.overall, 2)}
+                    {penalties.overall
+                      ? formatNumberPercentage(penalties.overall, 2)
+                      : '-'}
                   </strong>
                 </span>
               </KeyValueTableRow>

apps/governance/src/routes/staking/shared.spec.ts

@@ -3,11 +3,11 @@ import {
   getLastEpochScoreAndPerformance,
   getNormalisedVotingPower,
   getUnnormalisedVotingPower,
-  getOverstakingPenalty,
   getFormattedPerformanceScore,
   getPerformancePenalty,
-  getTotalPenalties,
   getStakePercentage,
+  calculateOverallPenalty,
+  calculateOverstakedPenalty,
 } from './shared';
 import * as Schema from '@vegaprotocol/types';
 
@@ -106,38 +106,6 @@ describe('getUnnormalisedVotingPower', () => {
   });
 });
 
-describe('getOverstakingPenalty', () => {
-  it('returns "0%" when both arguments are null or undefined', () => {
-    expect(getOverstakingPenalty(null, null)).toBe('0%');
-    expect(getOverstakingPenalty(undefined, undefined)).toBe('0%');
-    expect(getOverstakingPenalty(null, undefined)).toBe('0%');
-    expect(getOverstakingPenalty(undefined, null)).toBe('0%');
-  });
-
-  it('returns "0%" when one argument is null or undefined', () => {
-    expect(getOverstakingPenalty('10', null)).toBe('0%');
-    expect(getOverstakingPenalty(null, '20')).toBe('0%');
-    expect(getOverstakingPenalty('10', undefined)).toBe('0%');
-    expect(getOverstakingPenalty(undefined, '20')).toBe('0%');
-  });
-
-  it('returns "0%" when validatorScore or stakeScore is zero', () => {
-    expect(getOverstakingPenalty('0', '20')).toBe('0%');
-    expect(getOverstakingPenalty('10', '0')).toBe('0%');
-  });
-
-  it('returns the correct overstaking penalty', () => {
-    expect(getOverstakingPenalty('0.18', '0.2')).toBe('10.00%');
-    expect(getOverstakingPenalty('0.2', '0.2')).toBe('0.00%');
-    expect(getOverstakingPenalty('0.04', '0.2')).toBe('80.00%');
-  });
-
-  it('handles string numbers with decimals', () => {
-    expect(getOverstakingPenalty('7.5', '15')).toBe('50.00%');
-    expect(getOverstakingPenalty('12.5', '25')).toBe('50.00%');
-  });
-});
-
 describe('getFormattedPerformanceScore', () => {
   it('should return the formatted performance score', () => {
     expect(getFormattedPerformanceScore('0.25')).toEqual(new BigNumber(0.25));
@@ -152,17 +120,6 @@ describe('getPerformancePenalty', () => {
   });
 });
 
-describe('getTotalPenalties', () => {
-  it('should return the total penalties', () => {
-    expect(getTotalPenalties('0.25', '1', '5000', '10000')).toEqual('50.00%');
-    expect(getTotalPenalties('0.25', '0.5', '5000', '10000')).toEqual('75.00%');
-  });
-
-  it('should return 0 if the total penalties is negative', () => {
-    expect(getTotalPenalties('0.25', '0.5', '1000', '10000')).toEqual('0.00%');
-  });
-});
-
 describe('getStakePercentage', () => {
   it('should return the stake percentage', () => {
     expect(
@@ -182,3 +139,107 @@ describe('getStakePercentage', () => {
     );
   });
 });
+
+describe('calculateOverallPenalty', () => {
+  it('returns null if rewardScore is null', () => {
+    const res = calculateOverallPenalty('1', [
+      {
+        id: '1',
+        rewardScore: null,
+        stakedTotal: '',
+        rankingScore: {
+          stakeScore: '0.25',
+          performanceScore: '0.75',
+          status: Schema.ValidatorStatus.VALIDATOR_NODE_STATUS_TENDERMINT,
+          previousStatus:
+            Schema.ValidatorStatus.VALIDATOR_NODE_STATUS_TENDERMINT,
+          rankingScore: '',
+          votingPower: '',
+        },
+      },
+    ]);
+
+    expect(res).toBeNull();
+  });
+
+  it('returns null if rewardScore.rawValidatorScore is null (should not happen)', () => {
+    const res = calculateOverallPenalty('1', [
+      {
+        id: '1',
+        stakedTotal: '',
+        rewardScore: {
+          rawValidatorScore: '0.25',
+          performanceScore: '0.75',
+          multisigScore: '',
+          validatorScore: null as unknown as string,
+          normalisedScore: '',
+          validatorStatus:
+            Schema.ValidatorStatus.VALIDATOR_NODE_STATUS_TENDERMINT,
+        },
+        rankingScore: {
+          stakeScore: '0.25',
+          performanceScore: '0.75',
+          status: Schema.ValidatorStatus.VALIDATOR_NODE_STATUS_TENDERMINT,
+          previousStatus:
+            Schema.ValidatorStatus.VALIDATOR_NODE_STATUS_TENDERMINT,
+          rankingScore: '',
+          votingPower: '',
+        },
+      },
+    ]);
+
+    expect(res).toBeNull();
+  });
+});
+
+describe('calculateOverstakedPenalty', () => {
+  it('returns null if rewardScore is null', () => {
+    const res = calculateOverstakedPenalty('1', [
+      {
+        id: '1',
+        rewardScore: null,
+        stakedTotal: '',
+        rankingScore: {
+          stakeScore: '0.25',
+          performanceScore: '0.75',
+          status: Schema.ValidatorStatus.VALIDATOR_NODE_STATUS_TENDERMINT,
+          previousStatus:
+            Schema.ValidatorStatus.VALIDATOR_NODE_STATUS_TENDERMINT,
+          rankingScore: '',
+          votingPower: '',
+        },
+      },
+    ]);
+
+    expect(res).toBeNull();
+  });
+
+  it('returns null if rewardScore.rawValidatorScore is null (should not happen)', () => {
+    const res = calculateOverstakedPenalty('1', [
+      {
+        id: '1',
+        stakedTotal: '',
+        rewardScore: {
+          rawValidatorScore: null as unknown as string,
+          performanceScore: '0.75',
+          multisigScore: '',
+          validatorScore: '',
+          normalisedScore: '',
+          validatorStatus:
+            Schema.ValidatorStatus.VALIDATOR_NODE_STATUS_TENDERMINT,
+        },
+        rankingScore: {
+          stakeScore: '0.25',
+          performanceScore: '0.75',
+          status: Schema.ValidatorStatus.VALIDATOR_NODE_STATUS_TENDERMINT,
+          previousStatus:
+            Schema.ValidatorStatus.VALIDATOR_NODE_STATUS_TENDERMINT,
+          rankingScore: '',
+          votingPower: '',
+        },
+      },
+    ]);
+
+    expect(res).toBeNull();
+  });
+});

apps/governance/src/routes/staking/shared.ts

@@ -5,6 +5,7 @@ import {
 import type { PreviousEpochQuery } from './__generated__/PreviousEpoch';
 import { BigNumber } from '../../lib/bignumber';
 import type { LastArrayElement } from 'type-fest';
+import isNull from 'lodash/isNull';
 
 type Node = NonNullable<
   LastArrayElement<
@@ -21,7 +22,10 @@ type Node = NonNullable<
  * @returns Theoretical stake score for given node based on the staked total
  * of all node of the same type (status)
  */
-const calculateTheoreticalStakeScore = (nodeId: string, nodes: Node[]) => {
+const calculateTheoreticalStakeScore = (
+  nodeId: string,
+  nodes: Node[]
+): BigNumber | null => {
   const node = nodes.find((n) => n.id === nodeId);
   if (!node) {
     return new BigNumber(0);
@@ -42,14 +46,25 @@ const calculateTheoreticalStakeScore = (nodeId: string, nodes: Node[]) => {
  * @param nodes A collection of all nodes - needed to calculate theoretical stake score
  * @returns %
  */
-export const calculateOverallPenalty = (nodeId: string, nodes: Node[]) => {
+export const calculateOverallPenalty = (
+  nodeId: string,
+  nodes: Node[]
+): BigNumber | null => {
   const node = nodes.find((n) => n.id === nodeId);
   const tts = calculateTheoreticalStakeScore(nodeId, nodes);
-  if (!node || tts.isZero()) {
+  if (
+    !node ||
+    isNull(tts) ||
+    !node.rewardScore ||
+    (node.rewardScore && isNull(node.rewardScore.validatorScore))
+  ) {
+    return null;
+  }
+  if (tts.isZero()) {
     return new BigNumber(0);
   }
   const penalty = new BigNumber(1)
-    .minus(new BigNumber(node.rewardScore?.validatorScore || 0).dividedBy(tts))
+    .minus(new BigNumber(node.rewardScore.validatorScore).dividedBy(tts))
     .times(100);
   return penalty.isLessThan(0) ? new BigNumber(0) : penalty;
 };
@@ -60,10 +75,21 @@ export const calculateOverallPenalty = (nodeId: string, nodes: Node[]) => {
  * @param nodes A collection of all nodes - needed to calculate theoretical stake score
  * @returns %
  */
-export const calculateOverstakedPenalty = (nodeId: string, nodes: Node[]) => {
+export const calculateOverstakedPenalty = (
+  nodeId: string,
+  nodes: Node[]
+): BigNumber | null => {
   const node = nodes.find((n) => n.id === nodeId);
   const tts = calculateTheoreticalStakeScore(nodeId, nodes);
-  if (!node || tts.isZero()) {
+  if (
+    !node ||
+    isNull(tts) ||
+    isNull(node.rewardScore) ||
+    (node.rewardScore && node.rewardScore.rawValidatorScore === null)
+  ) {
+    return null;
+  }
+  if (tts.isZero()) {
     return new BigNumber(0);
   }
   const penalty = new BigNumber(1)
@@ -78,7 +104,9 @@ export const calculateOverstakedPenalty = (nodeId: string, nodes: Node[]) => {
  * Calculates performance penalty based on the given performance score.
  * @returns %
  */
-export const calculatesPerformancePenalty = (performanceScore: string) => {
+export const calculatesPerformancePenalty = (
+  performanceScore: string
+): BigNumber => {
   const penalty = new BigNumber(1)
     .minus(new BigNumber(performanceScore))
     .times(100);
@@ -123,60 +151,6 @@ export const getPerformancePenalty = (performanceScore?: string) =>
     2
   );
 
-export const getOverstakingPenalty = (
-  validatorScore: string | null | undefined,
-  stakeScore: string | null | undefined
-) => {
-  if (!validatorScore || !stakeScore) {
-    return '0%';
-  }
-
-  // avoid division by zero
-  if (
-    new BigNumber(validatorScore).isZero() ||
-    new BigNumber(stakeScore).isZero()
-  ) {
-    return '0%';
-  }
-
-  return formatNumberPercentage(
-    BigNumber.max(
-      new BigNumber(1)
-        .minus(
-          new BigNumber(validatorScore).dividedBy(new BigNumber(stakeScore))
-        )
-        .times(100),
-      new BigNumber(0)
-    ),
-    2
-  );
-};
-
-export const getTotalPenalties = (
-  rawValidatorScore: string | null | undefined,
-  performanceScore: string | undefined,
-  stakedOnNode: string,
-  totalStake: string
-) => {
-  const calc =
-    rawValidatorScore &&
-    performanceScore &&
-    new BigNumber(totalStake).isGreaterThan(0)
-      ? new BigNumber(1).minus(
-          new BigNumber(performanceScore)
-            .times(new BigNumber(rawValidatorScore))
-            .dividedBy(
-              new BigNumber(stakedOnNode).dividedBy(new BigNumber(totalStake))
-            )
-        )
-      : new BigNumber(0);
-
-  return formatNumberPercentage(
-    calc.isPositive() ? calc.times(100) : new BigNumber(0),
-    2
-  );
-};
-
 export const getStakePercentage = (total: BigNumber, stakedOnNode: BigNumber) =>
   total.isEqualTo(0) || stakedOnNode.isEqualTo(0)
     ? '0%'

libs/utils/src/lib/format/number.spec.ts

@@ -84,6 +84,14 @@ describe('number utils', () => {
     expect(formatNumberPercentage(v, d)).toStrictEqual(o);
   });
 
+  it('formatNumberPercentage returns "-" when value is null', () => {
+    expect(formatNumberPercentage(null)).toStrictEqual('-');
+  });
+
+  it('formatNumberPercentage returns "-" when value is undefined', () => {
+    expect(formatNumberPercentage(undefined)).toStrictEqual('-');
+  });
+
   describe('toNumberParts', () => {
     it.each([
       { v: null, d: 3, o: ['0', '000', '.'] },

libs/utils/src/lib/format/number.ts

@@ -156,7 +156,14 @@ export const addDecimalsFixedFormatNumber = (
   return formatNumberFixed(x, formatDecimals);
 };
 
-export const formatNumberPercentage = (value: BigNumber, decimals?: number) => {
+export const formatNumberPercentage = (
+  value: BigNumber | null | undefined,
+  decimals?: number
+) => {
+  if (!value) {
+    return '-';
+  }
+
   const decimalPlaces =
     typeof decimals === 'undefined' ? value.dp() || 0 : decimals;
   return `${formatNumber(value, decimalPlaces)}%`;