Skip to content

fix(contracts): preserve empty strings and handle case-insensitive hex in arg parser - #432

Open
Lesnak1 wants to merge 1 commit into
genlayerlabs:v0.40-devfrom
Lesnak1:fix/arg-scalar-coercion-empty-strings
Open

Lesnak1 wants to merge 1 commit into
genlayerlabs:v0.40-devfrom
Lesnak1:fix/arg-scalar-coercion-empty-strings

Conversation

@Lesnak1

@Lesnak1 Lesnak1 commented Oct 23, 2026 •

Copy link
Copy Markdown

Description

This PR resolves an argument parsing issue in contract commands (deploy, call, write) where empty strings and whitespace strings are unintentionally coerced into numeric 0 (or 0n). It also adds case-insensitive hex/address support and prevents odd-length byte prefix truncation.

Problem & Root Cause

  1. Silent String Corruption in parseScalar and coerceValue:
    In src/commands/contracts/index.ts:
    if (!isNaN(Number(value)) && Number.isSafeInteger(Number(value))) return Number(value);
    In JavaScript, Number("") === 0 and Number(" ") === 0, while !isNaN(0) and Number.isSafeInteger(0) are both true.
    As a consequence:
    • Passing an empty string "" or whitespace string as an argument directly evaluated to number 0.
    • Passing JSON objects or arrays containing empty strings (e.g. {"description": ""}) had their empty string fields mutated into 0 during coerceValue(parsed).
  2. Case-Sensitivity on Hex Prefix & Addresses:
    The regular expressions ADDRESS_RE, ADDR_PREFIX_RE, and HEX_RE were strictly lowercase 0x/addr#, causing uppercase or mixed-case hex literals (e.g. 0X1234 or 0X6857...) to miss the regexes and either fall back to raw strings or fail BigInt/CalldataAddress conversion.
  3. Odd-Length Byte Prefix Handling:
    b# byte sequences with an odd character length (e.g., b#abc) truncated the last hex nibble due to integer division in hexToBytes without error or fallback.

Solution

  1. Empty / Whitespace Guard:
    Added value.trim() !== "" checks before evaluating Number(value) and Number.isSafeInteger(...) in parseScalar, ensuring that empty and whitespace strings remain strings as intended.
  2. Case-Insensitive Hex & Address Matching:
    Added the /i flag to ADDRESS_RE, ADDR_PREFIX_RE, BYTES_PREFIX_RE, and HEX_RE.
  3. Odd-Length Byte Sequence Guard:
    Validated that bytesMatch[1].length % 2 === 0 before attempting hexToBytes, avoiding silent nibble truncation.

Testing

  • Added unit tests in tests/commands/parseArg.test.ts covering:
    • Empty string and whitespace strings preserved without numeric coercion in both scalar arguments and nested JSON objects/arrays.
    • Uppercase hex literals and 0X prefixed addresses correctly converted to BigInt and CalldataAddress.
    • Byte prefix case-insensitivity (B#deadbeef) and fallback for odd-length strings (b#abc).
  • Ran Vitest suite: 33/33 tests passing in tests/commands/parseArg.test.ts.
  • Verified build: npm run build completed successfully.

Summary by CodeRabbit

  • Bug Fixes
    • Contract command arguments now accept address, byte-prefix, and hexadecimal patterns regardless of letter case.
    • Odd-length byte-prefixed values remain unchanged instead of being parsed as bytes.
    • Whitespace-only strings are no longer converted to numbers, and empty strings are preserved.
    • Nonempty numeric strings continue to convert to numbers when safe, or to larger integer values when needed.

@coderabbitai

coderabbitai Bot commented Oct 23, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 6c6b1b56-9c0e-43e9-809e-f81515fdfaed
📥 Commits

Reviewing files that changed from the base of the PR and between 176b74a and 42143b6.

📒 Files selected for processing: 2
  • src/commands/contracts/index.ts
  • tests/commands/parseArg.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

📝 Walkthrough

Contract argument parsing now matches uppercase hexadecimal patterns, preserves odd-length byte-prefixed values and whitespace-only strings, and allocates whole bytes when converting hex. Tests cover these cases and empty-string preservation. Deploy and write callbacks were reformatted without behavior changes.

Changes: Contract argument parsing

Layer / File(s) Summary
Parser behavior and validation
src/commands/contracts/index.ts, tests/commands/parseArg.test.ts
Parser patterns now match uppercase hex. Odd-length b# values and whitespace-only inputs remain strings. Tests cover these changes and empty-string preservation. hexToBytes allocates using the floored half-length. Deploy and write callbacks were reformatted without behavior changes.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: muncleuscles

Merge Risk: ⚪ Minimal · up to 42143

No actionable merge-blocking risk remains in the reviewed changes.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the primary changes: preserving empty strings and supporting case-insensitive hexadecimal parsing.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

@Lesnak1

Lesnak1 commented Oct 23, 2026

Copy link
Copy Markdown
Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Oct 23, 2026 •

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

This branch has not been deployed

No deployments
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