Skip to content

fix: #1928 rename parseNativeValueWei to parseNativeValueEther - #2276

Open
tenk-earn wants to merge 1 commit into
KeeperHub:stagingfrom
tenk-earn:fix/issue-1928-rename-parse-native-value-ether
Open

fix: #1928 rename parseNativeValueWei to parseNativeValueEther#2276
tenk-earn wants to merge 1 commit into
KeeperHub:stagingfrom
tenk-earn:fix/issue-1928-rename-parse-native-value-ether

Conversation

@tenk-earn

Copy link
Copy Markdown

Closes #1928.

Rename-only, matching the accepted plan on the issue (build against the maintainer comment, not the filed wei-parse proposal).

parseNativeValueWei called ethers.parseEther. Ether is the published API contract (docs/api/direct-execution.md). The Wei name was the defect; changing the parse would 1e18x every existing caller who read the docs.

  • Rename parseNativeValueWeiparseNativeValueEther
  • Update re-exports, execute routes, and the existing unit describe
  • Behavior unchanged: same parseEther, same valueWei return
  • No local app/test run (identifier rename)

The function parses ether via ethers.parseEther; the Wei name installed the wrong unit. Behavior unchanged. Closes KeeperHub#1928.
@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

About the build check on this pull request

This pull request comes from a fork, so GitHub does not pass it the credentials build normally uses for our image registry cache and staging build configuration. The build still runs and still compiles the image, so a red build here is real; it just takes longer than on team branches.

Every workflow run on a pull request from a fork also waits for a maintainer to approve it, so checks can sit at "awaiting approval" for a while after each push. Nothing is needed from you for either of these.

@suisuss suisuss left a comment

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.

Welcome, and thanks for this - the contributing guide is in CONTRIBUTING.md, and ISSUES.md covers when a change needs an issue first.

What this changes

parseNativeValueWei is renamed to parseNativeValueEther across seven files, plus two doc-comment lines explaining the name. The body is untouched: still ethers.parseEther(rawAmount), same empty-to-"0", same negative rejection, same valueWei output.

I checked the two things a rename like this can get wrong. The parsing semantics did not move - input is still a human-decimal ether string, output is still wei, and the test expectations are byte-identical ("1.5" to "1500000000000000000"). And no caller is left behind: all 24 occurrences on staging are updated across lib/execute/native-value.ts, lib/execute/reserved-value.ts, app/api/execute/_lib/reserved-value.ts, the three execute routes and the unit test. No sub-package references the symbol.

Does it match the description

Matches.

Blocking

  • The lint job fails as pushed. .github/workflows/pr-checks.yml runs pnpm check unconditionally on every pull request, and four newly introduced violations trip it:

    • tests/unit/reserved-value.test.ts:19 - the three extra characters push the line to 82 columns, over the formatter's width. -> "Formatter would have printed the following content", job red. -> Let it break across lines.
    • app/api/execute/_lib/reserved-value.ts:6-7 - the export block lists parseNativeValueLamports before parseNativeValueEther, which no longer sorts. Swap them.
    • app/api/execute/transfer/route.ts:36-37 - same inversion in the import block. Swap them.
    • lib/execute/reserved-value.ts:4-5 and :12-13 - same inversion in both the import and the re-export block.

    pnpm exec biome check --write on the changed files fixes all four in one pass.

Mechanical - actionable as-is

  • ISSUES.md:80 still reads "parseNativeValueWei parses with ethers.parseEther ... the misleading thing is the internal function name". It is prose about this exact rename, and it now names a symbol that does not exist. Update it to the new name, or reword it as a past-tense account.

Verdict

Changes requested - the rename is complete and semantically clean, but four formatter and import-sort violations fail lint as pushed.

Worth knowing for the follow-up: parseNativeValueLamports directly below is named for its output unit, as is parseNodeNativeValueWei, so after this the module names two functions by output and one by input. That is what the new comment is compensating for. Not something to change in this PR - and ReservedValue.valueWei carrying lamports on the Solana path (reserved-value.ts:78,108) is the larger unit confusion in the module, untouched here and correctly left alone.

@suisuss suisuss added the changes-requested Triage: reviewed, changes needed from the contributor label Sep 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changes-requested Triage: reviewed, changes needed from the contributor

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(execute): value is parsed as ether by a function named parseNativeValueWei

2 participants