Skip to content

fix(d402): @example amount uses 10^18 (ETH) but Demos OS_PER_DEM is 10^9#92

Open
cX3po wants to merge 2 commits into
kynesyslabs:mainfrom
cX3po:fix-d402-middleware-dem-denomination
Open

fix(d402): @example amount uses 10^18 (ETH) but Demos OS_PER_DEM is 10^9#92
cX3po wants to merge 2 commits into
kynesyslabs:mainfrom
cX3po:fix-d402-middleware-dem-denomination

Conversation

@cX3po

@cX3po cX3po commented Jun 1, 2026

Copy link
Copy Markdown

Problem. src/d402/server/middleware.ts @example sets amount: 5000000000000000000 // 5 DEM in smallest unit — that's 5×10^18, an 18-decimal (ETH-style) assumption. Per the 3.0.0-rc.1 osDenomination migration Demos uses 9 decimals: OS_PER_DEM = 10^9 (denomination.demToOs(1) === 1_000_000_000n). So 5 DEM in the smallest unit (OS) is 5_000_000_000, not 5×10^18. A builder copying this d402Required example would over-charge by 10^9×.

Section & path. src/d402/server/middleware.ts (the d402Required JSDoc @example).

Fix. amount: 5000000000, // 5 DEM in smallest unit (OS); OS_PER_DEM = 10^9.

One implementer's read from the PATH-OS Labs DACS reference-impl pass — happy to adjust the wording.

@qodo-code-review

Copy link
Copy Markdown
Contributor

Review Summary by Qodo

Fix d402 middleware @example amount denomination from 10^18 to 10^9

🐞 Bug fix

Grey Divider

Walkthroughs

Description
• Corrects @example amount from 10^18 to 10^9 decimals
• Aligns with Demos OS_PER_DEM = 10^9 denomination standard
• Adds clarification comment about OS unit denomination
• Prevents builder over-charging by 10^9× factor
Diagram
flowchart LR
  A["@example amount<br/>5×10^18 ETH-style"] -->|"Fix to Demos<br/>9-decimal standard"| B["amount: 5×10^9<br/>OS_PER_DEM = 10^9"]

Loading

Grey Divider

File Changes

1. src/d402/server/middleware.ts 🐞 Bug fix +1/-1

Fix @example amount denomination in d402Required JSDoc

• Updated @example amount value from 5000000000000000000 to 5000000000
• Corrected denomination assumption from 18 decimals to 9 decimals
• Enhanced comment to clarify OS unit and OS_PER_DEM constant reference
• Prevents incorrect over-charging in builder implementations

src/d402/server/middleware.ts


Grey Divider

Qodo Logo

@coderabbitai

coderabbitai Bot commented Jun 1, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

@cX3po, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 42 minutes and 41 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 3b4655e5-71f9-460e-a6d7-6ac58b2c1ed3

📥 Commits

Reviewing files that changed from the base of the PR and between d6adf4f and 2725bbe.

📒 Files selected for processing (1)
  • src/d402/server/middleware.ts

Warning

Walkthrough skipped

File diffs could not be summarized.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@qodo-code-review

qodo-code-review Bot commented Jun 1, 2026

Copy link
Copy Markdown
Contributor

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider


Action required

1. OS amount typed as number ✓ Resolved 🐞 Bug ≡ Correctness
Description
The d402Required JSDoc example sets an OS-denominated smallest-unit value as a number, but
number amounts are interpreted as DEM and normalized via demToOs, making the required OS amount
5e18 instead of 5e9. Copying the example will cause correct 5e9-OS payments to be rejected as
underpayment (or lead to attempts to pay 10^9× more).
Code

src/d402/server/middleware.ts[52]

Evidence
The middleware/options/types document that number means DEM while string means OS, and the
server’s normalization converts numbers via demToOs but parses strings as OS; therefore a numeric
5000000000 will be treated as 5e9 DEM and converted to 5e18 OS. The denomination constants also
state the wire format is an OS amount serialized as a decimal string.

src/d402/server/middleware.ts[12-18]
src/d402/server/middleware.ts[45-56]
src/d402/server/D402Server.ts[16-67]
src/d402/server/D402Server.ts[212-232]
src/denomination/constants.ts[8-12]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The `d402Required` JSDoc example documents `amount` as OS (smallest unit) but provides it as a numeric literal. In this codebase, `number` carriers are treated as DEM and converted to OS via `demToOs`, so the example still implies a 10^9× incorrect requirement when used in validation.
### Issue Context
- `D402MiddlewareOptions.amount` is dual-shape: `number` = DEM (legacy), `string` = OS decimal-integer string (post-fork).
- The server normalizes `number` by converting DEM->OS (`demToOs`), and normalizes `string` by parsing as OS (`parseOsString`).
### Fix Focus Areas
- src/d402/server/middleware.ts[45-56]
### Proposed change
Update the example to pass the OS amount as a string:

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

Comment thread src/d402/server/middleware.ts Outdated
@greptile-apps

greptile-apps Bot commented Jun 1, 2026

Copy link
Copy Markdown

Greptile Summary

This PR corrects a denomination error in the d402Required JSDoc @example. The old value (5000000000000000000 as a JS number) was an 18-decimal (ETH-style) figure; Demos uses 9 decimals (OS_PER_DEM = 10^9). The replacement ('5000000000' as a string) correctly routes through _normalizeD402AmountToOsBigintparseOsString, yielding 5_000_000_000n OS = exactly 5 DEM, which was the intended amount.

  • amount: '5000000000' (string) takes the parseOsString branch in _normalizeD402AmountToOsBigint, giving the correct 5 DEM in OS units. A bare number would instead call demToOs and multiply by 10^9, causing a 10^9× overcharge — so the type (string vs number) is the critical distinction, and the updated comment now explains this distinction inline.
  • No runtime behavior is changed; the fix is documentation-only.

Confidence Score: 5/5

Safe to merge — the change is a one-line JSDoc correction with no runtime logic touched.

The only modification is the @example amount value and its inline comment. The string '5000000000' correctly routes through parseOsString in _normalizeD402AmountToOsBigint, producing exactly 5_000_000_000n OS (5 DEM). No application logic, types, or behavior are altered.

No files require special attention.

Important Files Changed

Filename Overview
src/d402/server/middleware.ts JSDoc @example updated: amount changed from the number 5000000000000000000 (18-decimal ETH-style) to the string '5000000000', correctly routing through parseOsString (OS path) for 5 DEM at 9-decimal precision. No runtime logic changed.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["d402Required({ amount })"] --> B{typeof amount}
    B -->|"number (e.g. 5)"| C["demToOs(value)\n× 10^9\n→ 5_000_000_000n OS"]
    B -->|"string (e.g. '5000000000')"| D["parseOsString(value)\n→ 5_000_000_000n OS"]
    B -->|bigint| E["passthrough\n(must be ≥ 0)"]
    C --> F["_normalizeD402AmountToOsBigint result (OS bigint)"]
    D --> F
    E --> F

    style D fill:#90ee90
    style A fill:#f0f0f0
Loading

Reviews (2): Last reviewed commit: "fix: @example amount must be a string (O..." | Re-trigger Greptile

Comment thread src/d402/server/middleware.ts
Addresses Greptile/Qodo review: a bare number is normalized via demToOs (DEM), so 5000000000 as a number = 5e18 OS — the same overcharge. String routes through parseOsString (OS).
@cX3po

cX3po commented Jun 1, 2026

Copy link
Copy Markdown
Author

Good catch by the review bots — fixed. The @example now uses the string form '5000000000' (OS path via parseOsString); a bare number would be treated as DEM and demToOs-scaled, reproducing the same 10^9× overcharge this PR fixes. Updated the comment to spell out the string=OS / number=DEM distinction.

@sonarqubecloud

sonarqubecloud Bot commented Jun 1, 2026

Copy link
Copy Markdown

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.

1 participant