Skip to content

Fix CPU RandomX hashrate units#3262

Open
maiqiu-cat wants to merge 2 commits into
tari-project:mainfrom
maiqiu-cat:codex/fix-cpu-hashrate-units
Open

Fix CPU RandomX hashrate units#3262
maiqiu-cat wants to merge 2 commits into
tari-project:mainfrom
maiqiu-cat:codex/fix-cpu-hashrate-units

Conversation

@maiqiu-cat
Copy link
Copy Markdown

Description

Fixes CPU hashrate display units so RandomX CPU mining is shown as H/s, kH/s, MH/s, etc. GPU/C29 display remains on the existing G/s-based unit scale.

Motivation and Context

Closes #3210. CPU RandomX hashrate values were formatted with the GPU/C29 base unit, which caused CPU mining to appear as G/s on macOS.

How Has This Been Tested?

  • npx vitest run src/utils/formatters.test.ts
  • npx tsc --noEmit
  • npm run lint

What process can a PR reviewer use to test or verify this change?

Start CPU mining and confirm the CPU tile/pill reports RandomX hashrate with H/s-derived units instead of G/s-derived units. GPU mining should continue to use G/s-derived units.

Breaking Changes

  • None
  • Requires data directory on base node to be deleted
  • Requires hard fork
  • Other - Please specify

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request refactors hashrate formatting by replacing algorithm-specific logic with a generic HashrateUnit type across the CPU, GPU, and Miner components. The changes include removing the getSelectedMiner dependency and adding unit tests for the new 'H' unit. Feedback highlights that the formatHashrate implementation contains inconsistent spacing for units and fails to respect the joinUnit parameter for values below 1000.

Comment thread src/utils/formatters.ts
const unit = 'G';
export type HashrateUnit = 'H' | 'G';

export function formatHashrate(hashrate: number, joinUnit = true, unit: HashrateUnit = 'G'): Hashrate {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The implementation of formatHashrate (lines 132-167) contains a few issues that affect the new 'H' unit:

  1. Inconsistent Spacing: Base units (e.g., H/s) are formatted without a leading space (line 135), while prefixed units (e.g., kH/s) include one (lines 141, 147, etc.). This leads to inconsistent UI presentation (e.g., "500H/s" vs "1.5 kH/s").
  2. joinUnit Bug: The joinUnit parameter is ignored for hashrates under 1000 (lines 132-137), always returning the full unit string even when joinUnit is false.

Since this PR modifies the function signature and introduces the HashrateUnit type, it's a good opportunity to fix these implementation details for better consistency.

@maiqiu-cat
Copy link
Copy Markdown
Author

Addressed the Gemini Code Assist feedback in 200ee62: formatHashrate now returns unit strings without embedded leading spaces, and joinUnit=false is respected for values under 1000 as well. I also added coverage for the under-1000 joinUnit case.

Verified locally:

  • npx vitest run src/utils/formatters.test.ts
  • npx tsc --noEmit
  • npm run lint

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Tari Universe 1.6.11 reports rate in G/s on Mac for CPU

1 participant