Skip to content

Conversation

JohnnyWyles
Copy link
Contributor

@JohnnyWyles JohnnyWyles commented Oct 3, 2025

What is the purpose of the change:

Allows leading decimals to input fields - the cause of the add initial pool liquidity at pool creation crashing the frontend and interrupting the flow.

Allows trailing zeros - an issue with creating precise liquidity positions where 0.00502 could not be entered as a position for example.

Main places to check here are therefore the position setting fields when creating a new CL position, and the initial liquidity addition after creating a new pool. I've created a new pool and tried to break this myself through keyboard entry with no errors showing, for info any further QA incurs the pool creation fee (20 USDC).
Also check other numerical entry fields in case any are broken by this, however this only adds additional checks.

Linear Task

https://linear.app/osmosis/issue/FE-1325/invalid-decimal-error-on-initial-liquidity-add-to-a-pool

Brief Changelog

Changes Number field types to Text
Adds Decimal input type
Adjusts and adds validation of these fields to allow:
- Support for leading decimals (e.g., .5, .05)
- Support for trailing zeros and trailing decimal points (e.g., 0.00502, 10.)
- Length limit check (max 50 characters)
- Allows intermediate input states like "0.", "." for smooth typing

Copy link

vercel bot commented Oct 3, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
osmosis-frontend Ready Ready Preview Comment Oct 8, 2025 3:28pm
4 Skipped Deployments
Project Deployment Preview Comments Updated (UTC)
osmosis-frontend-datadog Ignored Ignored Oct 8, 2025 3:28pm
osmosis-frontend-dev Ignored Ignored Oct 8, 2025 3:28pm
osmosis-frontend-edgenet Ignored Ignored Oct 8, 2025 3:28pm
osmosis-testnet Ignored Ignored Oct 8, 2025 3:28pm

cursor[bot]

This comment was marked as outdated.

@JohnnyWyles
Copy link
Contributor Author

Main points this fixes are:

  • Add liquidity to a newly created supercharged pool - which is only accessible after spending 20 USDC to create a pool
  • Position creation in CL pools

Impacts more number entry locations but I tested all I could find on previous (before prettier) builds.

@JohnnyWyles JohnnyWyles marked this pull request as ready for review October 7, 2025 15:43
cursor[bot]

This comment was marked as outdated.

Copy link
Contributor

coderabbitai bot commented Oct 7, 2025

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

Walkthrough

Centralized numeric raw-input validation into utils; added safeInputToDec normalization across price and input flows; preserved raw input strings for display; switched many numeric inputs from type="number" to type="text" with inputMode="decimal"; exposed inputMode on InputBox; tightened initial-liquidity input sanitization and gating.

Changes

Cohort / File(s) Summary of changes
Validation utilities
packages/utils/src/assertion.ts, packages/web/hooks/input/use-amount-input.ts, packages/web/hooks/input/__tests__/use-amount-input.spec.ts, packages/web/modals/review-order.tsx, packages/web/hooks/limit-orders/use-place-limit.ts
Introduced/updated isValidNumericalRawInput(input: string): boolean (trim, length cap, single-decimal regex, numeric range guards). Removed local validator re-export in use-amount-input.ts; updated imports/tests to use the shared util.
Safe input normalization & price handling
packages/stores/src/ui-config/price.ts, packages/web/components/control/crypto-fiat-input.tsx, packages/web/components/place-limit-tool/index.tsx
Added internal safeInputToDec helper (treats "" or "." as "0") and wrapped raw inputs with it before constructing Dec/PricePretty; PriceConfig.input now uses isValidNumericalRawInput to ignore invalid raw inputs; toString preserves raw input.
InputBox API enhancement
packages/web/components/input/input-box.tsx
Added optional inputMode prop to InputBox props and passed it to the underlying <input /> to control mobile keyboard (includes decimal).
Switch numeric inputs to text + decimal mode
packages/web/components/cl-deposit-input-group/index.tsx, packages/web/components/complex/add-conc-liquidity.tsx, packages/web/components/complex/add-liquidity.tsx, packages/web/components/complex/pool/create/step1-set-ratios.tsx, packages/web/components/complex/pool/create/step2-add-liquidity.tsx, packages/web/components/complex/pool/create/cl/add-initial-liquidity.tsx
Replaced type="number" with type="text" and added inputMode="decimal" for many inputs to accept decimals and avoid native numeric spinners.
Initial liquidity sanitization & gating
packages/web/components/complex/pool/create/cl/add-initial-liquidity.tsx
Added isPositiveDecAmount and safeParseDecOrZero; input sanitization (max length 32, prepend 0 for leading ., allow digits with single decimal point, ignore invalid/empty); tightened render gating to require positive base/quote amounts and safe parsing for balance checks.
Other input normalization updates
packages/web/components/place-limit-tool/index.tsx, packages/stores/src/ui-config/price.ts, packages/web/components/control/crypto-fiat-input.tsx
Replaced direct Dec constructions with safeInputToDec-based conversions; adjusted conversions to handle trailing dots and preserve raw display strings.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor User
  participant Input as InputBox
  participant Validator as isValidNumericalRawInput
  participant Normalizer as safeInputToDec
  participant Dec as Dec/PricePretty
  participant UI

  User->>Input: type raw value ("0.50", ".", "", "abc")
  Input->>Validator: isValidNumericalRawInput(raw)
  alt valid
    Validator-->>Input: true
    Input->>Normalizer: safeInputToDec(raw)
    Normalizer-->>Dec: construct Dec(normalized)
    Dec-->>UI: update computed values
    UI-->>User: render using preserved raw string
  else invalid
    Validator-->>Input: false
    Input-->>UI: ignore update / keep prior state
    UI-->>User: no change
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title Check ✅ Passed The title concisely describes the main change by highlighting support for leading decimal points and trailing zeros in input fields, directly reflecting the PR’s purpose without unnecessary detail. It is clear, specific, and matches the summary of changes across various input components. This helps teammates scanning the history quickly grasp the primary update.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
Description Check ✅ Passed The pull request description clearly explains the purpose of the change, includes the link to the Linear Task, and provides a detailed brief changelog that matches the required template’s sections, though it does not include a “Testing and Verifying” section.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch jw/allow-0-at-end-of-inputs

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.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

♻️ Duplicate comments (1)
packages/stores/src/ui-config/price.ts (1)

7-14: Consolidate duplicated safeInputToDec helper.

This is a duplicate of the same helper in packages/web/components/control/crypto-fiat-input.tsx. As mentioned in my earlier comment, consider extracting this to @osmosis-labs/utils for reuse.

🧹 Nitpick comments (3)
packages/web/components/control/crypto-fiat-input.tsx (1)

25-32: Consolidate duplicated safeInputToDec helper.

The safeInputToDec helper is duplicated in multiple files (this file and packages/stores/src/ui-config/price.ts). This creates maintenance overhead and risks inconsistencies if one implementation diverges from another.

Consider extracting this to a shared utility module (e.g., @osmosis-labs/utils) alongside isValidNumericalRawInput:

+// In packages/utils/src/input.ts
+export function safeInputToDec(input: string): string {
+  const trimmed = input.trim();
+  if (trimmed === "" || trimmed === ".") return "0";
+  return trimmed;
+}

Then import from both files:

import { isValidNumericalRawInput, safeInputToDec } from "@osmosis-labs/utils";
packages/web/components/place-limit-tool/index.tsx (1)

67-74: Consider using existing validation utilities for consistency.

The safeInputToDec helper duplicates input normalization logic found elsewhere in the codebase. The relevant code snippet from packages/stores/src/ui-config/price.ts (lines 59-75) shows a more robust approach that uses isValidNumericalRawInput to validate the input pattern before normalization.

The current implementation only guards against empty strings and lone decimals but doesn't validate that the trimmed input is a valid number. This could allow malformed inputs to reach the Dec constructor.

Consider extracting and reusing the validation logic from price.ts or consolidating both implementations:

+import { isValidNumericalRawInput } from "@osmosis-labs/utils";
+
-function safeInputToDec(input: string): string {
-  const trimmed = input.trim();
-  if (trimmed === "" || trimmed === ".") return "0";
-  return trimmed;
-}
+function safeInputToDec(input: string): string {
+  const trimmed = input.trim();
+  if (trimmed === "" || trimmed === ".") return "0";
+  // Validate to prevent invalid inputs from reaching Dec constructor
+  if (!isValidNumericalRawInput(trimmed)) return "0";
+  return trimmed;
+}
packages/web/components/complex/pool/create/cl/add-initial-liquidity.tsx (1)

232-255: Inconsistent validation approach across the codebase.

The inline regex validation (/^\d*\.?\d*$/) duplicates validation logic that exists in isValidNumericalRawInput from @osmosis-labs/utils (imported in other files like place-limit-tool/index.tsx). This creates maintenance burden and potential inconsistencies.

Consider using the shared validation utility for consistency:

+import { isValidNumericalRawInput } from "@osmosis-labs/utils";
+
 onChange={(e) => {
   let inputValue = e.target.value;

   // we might have to adjust this treshold
   if (inputValue.length > 32) return;
   if (inputValue === "") return setter();

   // Handle leading decimal point
   if (inputValue.startsWith(".")) {
     inputValue = "0" + inputValue;
   }

-  // Validate input: only allow valid numerical input with optional decimal point
-  // Allows: "1", "1.", "1.0", "1.00", ".5", "0.5", etc.
-  // Rejects: "1.2.3", "abc", "1a", etc.
-  const validPattern = /^\d*\.?\d*$/;
-  if (!validPattern.test(inputValue)) return;
+  // Validate using shared utility
+  if (!isValidNumericalRawInput(inputValue)) return;

   setter(inputValue);
 }}
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e32cf28 and d8cb4ac.

📒 Files selected for processing (12)
  • packages/stores/src/ui-config/price.ts (2 hunks)
  • packages/utils/src/assertion.ts (1 hunks)
  • packages/web/components/cl-deposit-input-group/index.tsx (1 hunks)
  • packages/web/components/complex/add-conc-liquidity.tsx (1 hunks)
  • packages/web/components/complex/add-liquidity.tsx (1 hunks)
  • packages/web/components/complex/pool/create/cl/add-initial-liquidity.tsx (2 hunks)
  • packages/web/components/complex/pool/create/step1-set-ratios.tsx (2 hunks)
  • packages/web/components/complex/pool/create/step2-add-liquidity.tsx (1 hunks)
  • packages/web/components/control/crypto-fiat-input.tsx (3 hunks)
  • packages/web/components/input/input-box.tsx (3 hunks)
  • packages/web/components/place-limit-tool/index.tsx (4 hunks)
  • packages/web/hooks/input/use-amount-input.ts (1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-11-23T16:36:41.685Z
Learnt from: greg-nagy
PR: osmosis-labs/osmosis-frontend#3954
File: packages/web/components/alert/prettify.ts:43-44
Timestamp: 2024-11-23T16:36:41.685Z
Learning: In `packages/web/components/alert/prettify.ts`, when handling overspend error messages related to `uusdc`, assuming 6 decimal places is acceptable because `uusdc` uses 6 decimals.

Applied to files:

  • packages/web/components/place-limit-tool/index.tsx
🧬 Code graph analysis (4)
packages/web/components/place-limit-tool/index.tsx (1)
packages/stores/src/ui-config/price.ts (1)
  • input (60-76)
packages/utils/src/assertion.ts (1)
packages/stores/src/ui-config/price.ts (1)
  • input (60-76)
packages/web/hooks/input/use-amount-input.ts (1)
packages/stores/src/ui-config/price.ts (1)
  • input (60-76)
packages/web/components/control/crypto-fiat-input.tsx (1)
packages/stores/src/ui-config/price.ts (1)
  • input (60-76)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Summary
🔇 Additional comments (14)
packages/web/components/control/crypto-fiat-input.tsx (1)

136-139: LGTM! Safe decimal construction.

The use of safeInputToDec before constructing Dec values correctly handles edge cases (empty string, lone decimal) and prevents runtime errors from invalid decimal values.

packages/utils/src/assertion.ts (1)

9-40: LGTM! Robust validation with a false positive security alert.

The validation logic is comprehensive and handles all necessary edge cases:

  • Allows intermediate states (empty, lone ".")
  • Enforces reasonable length limit
  • Uses a linear-time regex pattern
  • Guards against NaN, negative values, and unsafe integers

Regarding the GitHub Advanced Security alert about "polynomial regular expression": The regex /^\d*\.?\d*$/ is not vulnerable to catastrophic backtracking. It runs in linear time O(n) because it contains no nested quantifiers or alternation. The alert is a false positive.

For reference, a vulnerable pattern would look like /^(\d+)+$/ (nested quantifiers) or /^(a+)+$/ (exponential backtracking).

packages/stores/src/ui-config/price.ts (2)

68-75: LGTM! Proper input validation with early return.

The validation now correctly uses isValidNumericalRawInput and preserves the previous valid state when invalid input is detected. This prevents invalid values from corrupting the internal state.


94-99: LGTM! Preserves user input format.

Returning _decRaw directly preserves trailing zeros (e.g., "0.0502"), which improves the user experience by maintaining the format they typed. This aligns with the PR objectives.

packages/web/components/complex/pool/create/step2-add-liquidity.tsx (1)

75-76: LGTM! Enables decimal input.

Changing to type="text" with inputMode="decimal" correctly enables entry of leading decimals and trailing zeros while preserving mobile keyboard optimization. The validation is handled upstream by amountConfig.setAmount, which uses the centralized validation logic.

packages/web/components/complex/add-liquidity.tsx (1)

226-227: LGTM! Consistent decimal input implementation.

The change aligns with other input components in this PR, enabling decimal entry while maintaining validation through the onInputAmount handler.

packages/web/components/cl-deposit-input-group/index.tsx (1)

137-138: LGTM! Past concern about invalid characters is addressed.

The change to type="text" with inputMode="decimal" enables decimal input as intended. While the past review comment raised a valid concern about text inputs accepting non-numeric characters, this is properly handled by validation in the onUpdate callback chain, which ultimately uses isValidNumericalRawInput to reject invalid input.

packages/web/components/complex/add-conc-liquidity.tsx (1)

861-862: LGTM! Enables precise price range input.

The change to type="text" with inputMode="decimal" for the price input boxes enables users to enter precise decimal values for concentrated liquidity ranges, which is essential for the use case described in the PR objectives.

packages/web/components/place-limit-tool/index.tsx (3)

372-376: LGTM! Safe Dec construction with input normalization.

The use of safeInputToDec prevents Dec construction errors when tokenAmount is empty or a lone decimal point.


400-404: LGTM! Consistent input sanitization.

Properly normalizes the fiat input before division by price.


650-650: LGTM! Input validation with safe Dec construction.

The sanitization prevents runtime errors in the validation logic.

packages/web/components/complex/pool/create/cl/add-initial-liquidity.tsx (1)

244-247: Good input normalization for leading decimals.

Prepending "0" to inputs starting with "." enables the user experience improvements described in the PR objectives.

packages/web/components/input/input-box.tsx (2)

42-51: Well-designed API enhancement.

The new inputMode prop is properly typed with standard HTML5 values and includes clear documentation. This enables better mobile keyboard experiences for numeric inputs throughout the application.


154-154: LGTM! Proper prop propagation.

The inputMode is correctly passed to the native input element, enabling the mobile keyboard mode.

cursor[bot]

This comment was marked as outdated.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Nitpick comments (1)
packages/web/hooks/input/__tests__/use-amount-input.spec.ts (1)

349-381: Add test coverage for leading decimals and trailing zeros.

The test suite for isValidNumericalRawInput doesn't cover the new features mentioned in the PR objectives:

  • Leading decimals (e.g., ".5")
  • Trailing zeros (e.g., "0.00502")

Additionally, since isValidNumericalRawInput is now imported from the external library @osmosis-labs/utils, consider whether these tests should:

  1. Be removed if the external library has its own comprehensive test coverage
  2. Be enhanced to include the new use cases if you want to verify the library's behavior for this project's specific needs

If you choose to keep and enhance these tests, apply this diff to add coverage for the new features:

   it("should return true for MAX_SAFE_INTEGER", () => {
     expect(isValidNumericalRawInput("9007199254740991")).toBe(true); // MAX_SAFE_INTEGER
   });
+
+  it("should return true for leading decimals", () => {
+    expect(isValidNumericalRawInput(".5")).toBe(true);
+    expect(isValidNumericalRawInput(".123")).toBe(true);
+    expect(isValidNumericalRawInput(".0")).toBe(true);
+  });
+
+  it("should return true for trailing zeros", () => {
+    expect(isValidNumericalRawInput("0.00502")).toBe(true);
+    expect(isValidNumericalRawInput("1.100")).toBe(true);
+    expect(isValidNumericalRawInput("5.000")).toBe(true);
+  });
 });
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between fa7e88c and 6ddbea7.

📒 Files selected for processing (4)
  • packages/stores/src/ui-config/price.ts (2 hunks)
  • packages/web/components/complex/pool/create/cl/add-initial-liquidity.tsx (3 hunks)
  • packages/web/hooks/input/__tests__/use-amount-input.spec.ts (1 hunks)
  • packages/web/modals/review-order.tsx (1 hunks)
✅ Files skipped from review due to trivial changes (1)
  • packages/web/modals/review-order.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/web/components/complex/pool/create/cl/add-initial-liquidity.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
packages/stores/src/ui-config/price.ts (1)
packages/utils/src/assertion.ts (1)
  • isValidNumericalRawInput (9-40)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: Cursor Bugbot
  • GitHub Check: Analyze (javascript)
  • GitHub Check: Summary
🔇 Additional comments (3)
packages/stores/src/ui-config/price.ts (3)

3-3: LGTM: Import and defensive normalization helper.

The import of isValidNumericalRawInput and the safeInputToDec helper work together to ensure safe Dec construction. The helper provides a safety net by normalizing edge cases (empty string, lone decimal) to "0" before Dec instantiation, preventing runtime errors.

Also applies to: 7-14


85-89: LGTM: Defensive normalization in conversion methods.

Using safeInputToDec in both toDec and toDecWithCurrencyDecimals ensures safe Dec construction by normalizing edge cases before conversion. The special handling of trailing "." in toDec (line 84-88) correctly allows users to type values like "0." while ensuring computation uses "0".

Also applies to: 94-94


97-101: LGTM: Documentation accurately describes toString behavior.

The updated documentation clearly explains that toString() preserves the raw user input, including trailing zeros and terminal ".", which aligns with the PR objective to support inputs like "0.00502" for precise liquidity position entry.

cursor[bot]

This comment was marked as outdated.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
packages/stores/src/ui-config/price.ts (1)

7-14: LGTM! Defensive normalization helper.

The safeInputToDec helper provides a good safety net when constructing Dec objects from raw input strings. This defensive approach prevents runtime errors from edge cases.

Minor: The comment mentions strings that "can't be parsed," but technically "." passes validation—it's just normalized to "0" for safety. Consider clarifying to "Returns '0' for empty strings or lone decimals for safe Dec construction."

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6ddbea7 and 5e240dc.

📒 Files selected for processing (3)
  • packages/stores/src/ui-config/price.ts (2 hunks)
  • packages/web/components/complex/pool/create/cl/add-initial-liquidity.tsx (3 hunks)
  • packages/web/hooks/input/use-amount-input.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/web/hooks/input/use-amount-input.ts
🧰 Additional context used
🧬 Code graph analysis (1)
packages/stores/src/ui-config/price.ts (1)
packages/utils/src/assertion.ts (1)
  • isValidNumericalRawInput (9-40)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (18)
  • GitHub Check: production
  • GitHub Check: staging
  • GitHub Check: Cursor Bugbot
  • GitHub Check: Analyze (actions)
  • GitHub Check: Analyze (javascript)
  • GitHub Check: test (22.x, unit)
  • GitHub Check: wait-for-deployment
  • GitHub Check: lint-workspace
  • GitHub Check: test (22.x, tx)
  • GitHub Check: test (22.x, pools)
  • GitHub Check: test (22.x, utils)
  • GitHub Check: test (22.x, web)
  • GitHub Check: test (22.x, stores)
  • GitHub Check: test (22.x, server)
  • GitHub Check: test (22.x, math)
  • GitHub Check: test (22.x, bridge)
  • GitHub Check: test (22.x, proto-codecs)
  • GitHub Check: Summary
🔇 Additional comments (8)
packages/stores/src/ui-config/price.ts (5)

3-3: LGTM! Good use of centralized validation.

The import and use of isValidNumericalRawInput from @osmosis-labs/utils centralizes numeric input validation across the codebase, improving consistency and maintainability.


60-85: Excellent validation and normalization logic.

The refactored input handling correctly addresses the previous concerns about validation bypass and empty input handling:

  • Validation with isValidNumericalRawInput now occurs before the leading decimal normalization (line 74 before line 80), preventing invalid inputs like ".abc" from being stored.
  • Empty and whitespace-only inputs are consistently normalized to "0" at lines 68-71.
  • Leading decimals (e.g., ".5") are properly validated then normalized to "0.5".
  • Invalid inputs are rejected while preserving the previous valid value (line 76).

This implementation enables the PR objectives: allowing leading decimals, preserving trailing zeros, and preventing crashes on invalid input.

Note: Past review comments about validation bypass and empty input handling are now resolved.


89-96: LGTM! Proper handling of trailing decimals.

The toDec method correctly handles the intermediate state when users type a trailing decimal point:

  • Trailing "." is sliced off before conversion (lines 91-93)
  • safeInputToDec normalizes edge cases before passing to removeCurrencyDecimals
  • This ensures calculations use valid numeric values while preserving the user's typing state in the display

99-104: LGTM! Consistent conversion logic.

The toDecWithCurrencyDecimals method mirrors the toDec logic for handling trailing decimals, ensuring consistency across conversion methods. The use of safeInputToDec before Dec construction prevents errors from edge cases.


106-111: Perfect! Preserves raw input as intended.

The updated toString method directly returns _decRaw, preserving the user's exact input including trailing zeros and decimal points. This is essential for the PR objectives:

  • Allows typing precise values like "0.00502" without auto-formatting removing the trailing zeros
  • Enables proper handling of intermediate states like "0." during typing
  • Fixes the add-initial-pool-liquidity crash by not mangling decimal inputs

The comprehensive JSDoc comment clearly documents this behavior.

packages/web/components/complex/pool/create/cl/add-initial-liquidity.tsx (3)

26-33: Excellent addition of safe decimal validation.

The isPositiveDecAmount helper correctly addresses previous concerns about Dec construction errors by wrapping the validation in a try-catch block. This prevents crashes from invalid inputs while ensuring only positive decimal amounts are accepted.


114-116: Improved validation consistency.

The updated condition correctly uses isPositiveDecAmount for both amounts, ensuring consistent and safe validation. This addresses previous concerns about inconsistent validation between base and quote amounts.


240-241: Correct input type for decimal entry.

Switching to type="text" with inputMode="decimal" is the appropriate solution for allowing leading decimals (e.g., ".5") and trailing zeros (e.g., "0.0050"). This resolves the limitations of HTML type="number" inputs while maintaining good mobile keyboard UX.

cursor[bot]

This comment was marked as outdated.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
packages/web/components/complex/pool/create/cl/add-initial-liquidity.tsx (1)

140-151: Add validation to prevent submitting zero or invalid amounts.

The button's disabled condition uses safeParseDecOrZero which prevents crashes but doesn't validate that amounts are positive. This allows the button to be enabled with zero or invalid input (e.g., "0", "0.", "0.00"). The onClick guard on line 151 only performs a truthy check, which passes for "0".

This partially addresses a past review comment that suggested adding isPositiveDecAmount checks, but the validation was not fully implemented.

Apply this diff to add positive amount validation:

          <button
            disabled={
              isTxLoading ||
+             !isPositiveDecAmount(baseAmount) ||
+             !isPositiveDecAmount(quoteAmount) ||
              safeParseDecOrZero(baseAmount).gt(
                baseAssetBalanceData?.amount?.toDec() ?? new Dec(0)
              ) ||
              safeParseDecOrZero(quoteAmount).gt(
                quoteAssetBalanceData?.amount?.toDec() ?? new Dec(0)
              )
            }
🧹 Nitpick comments (1)
packages/web/components/complex/pool/create/cl/add-initial-liquidity.tsx (1)

273-285: Consider gating USD display on positive amount.

While safeParseDecOrZero prevents crashes, the USD value could display "$0.00" for zero or intermediate input states (e.g., "0", "0."). Consider adding isPositiveDecAmount(value) to the condition for clearer UX.

Apply this diff:

          <span className="caption h-3.5 text-osmoverse-400">
            {isQuote &&
              value &&
+             isPositiveDecAmount(value) &&
              "~" +
                formatPretty(

This was suggested in a previous review and would prevent displaying "$0.00" for zero amounts.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5e240dc and 540dabf.

📒 Files selected for processing (2)
  • packages/web/components/complex/pool/create/cl/add-initial-liquidity.tsx (5 hunks)
  • packages/web/hooks/limit-orders/use-place-limit.ts (1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-11-23T16:36:41.685Z
Learnt from: greg-nagy
PR: osmosis-labs/osmosis-frontend#3954
File: packages/web/components/alert/prettify.ts:43-44
Timestamp: 2024-11-23T16:36:41.685Z
Learning: In `packages/web/components/alert/prettify.ts`, when handling overspend error messages related to `uusdc`, assuming 6 decimal places is acceptable because `uusdc` uses 6 decimals.

Applied to files:

  • packages/web/components/complex/pool/create/cl/add-initial-liquidity.tsx
🧬 Code graph analysis (1)
packages/web/components/complex/pool/create/cl/add-initial-liquidity.tsx (1)
packages/keplr-hooks/src/tx/amount.ts (1)
  • amount (116-136)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: Cursor Bugbot
  • GitHub Check: preview-swap-usdc-tests
  • GitHub Check: Summary
🔇 Additional comments (5)
packages/web/hooks/limit-orders/use-place-limit.ts (2)

915-923: LGTM! Leading decimal handling is correct.

The code properly normalizes inputs starting with "." by prepending "0" before validation, which aligns with the PR objective to support leading decimals. The validation flow is correct: normalize → validate → process.


8-8: Confirm isValidNumericalRawInput handles leading decimals and trailing zeros

The import change centralizes numeric validation to @osmosis-labs/utils. Please verify the external function supports:

  • Leading decimals (e.g., “.5”)
  • Trailing zeros (e.g., “0.00502”)

Implementation isn’t available locally—confirm via the utils package source or its tests.

packages/web/components/complex/pool/create/cl/add-initial-liquidity.tsx (3)

24-40: LGTM! Excellent defensive programming.

The new helper functions isPositiveDecAmount and safeParseDecOrZero effectively prevent runtime errors from invalid Dec constructions. Both use try-catch blocks to handle edge cases like empty strings, invalid input, and zero values safely.


121-138: Good fix for implied value validation.

The condition now consistently validates both baseAmount and quoteAmount using isPositiveDecAmount, addressing previous concerns about inconsistent validation. This ensures the Dec constructions on lines 131-133 only execute when both amounts are valid positive decimals.


246-271: Excellent input sanitization implementation.

The change from type="number" to type="text" with inputMode="decimal" combined with the comprehensive sanitization logic properly addresses the PR objectives:

  • ✅ Allows leading decimals (e.g., ".5" → "0.5")
  • ✅ Preserves trailing zeros (e.g., "0.00502")
  • ✅ Provides smooth typing UX for intermediate states (e.g., "0.", "0.0")
  • ✅ Validates input with regex to prevent malformed numbers
  • ✅ Limits length to prevent excessive input

The inputMode="decimal" provides optimal mobile keyboard while maintaining full control over validation.

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.

2 participants