fix(keys): accept uppercase 0X prefix in isHexString - #198
kutluhaneth46 wants to merge 1 commit into
Conversation
Align the hex type guard with hexStringToBytes, which already strips a case-insensitive 0x prefix. Values like 0XABCDEF decoded successfully but failed isHexString.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 2 included reviews per hour; 0 remain after this review. Walkthrough
ChangesHex prefix validation
Estimated code review effort: 1 (Trivial) | ~5 minutes Merge Risk: ⚪ Minimal · up to This change makes uppercase 0X-prefixed hexadecimal strings valid in isHexString, matching existing decoder behavior. The new behavior is directly covered by a regression test, with no remaining current-head merge risk identified. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files. (1 skipped: 1 unsupported.) ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
72a7ce5 to
b744238
Compare
|
Thank you for the contribution. What isHexString should accept, including prefix variants, is already an open question on #161, and we would like to settle it once rather than widen the function case by case. Closing this one; feel free to add your case to that discussion. |
isHexString now recognizes an uppercase "0X" prefix in addition to
"0x", matching hexStringToBytes which already lower-cases the value
before checking for the prefix. Both isHexString("0XABCDEF") and the
bare isHexString("0X") now return true, so the two hex helpers accept
the same prefix variants.
This answers the "which prefix variants should be accepted" question
raised in agentcommercekit#161 that venables asked to resolve when closing agentcommercekit#198.
Summary
isHexStringonly stripped a lowercase0xprefix, whilehexStringToBytesalready accepts0X….0XABCDEF.This is distinct from #161 (bare
0x/ empty body); that PR does not make the prefix check case-insensitive.Test plan
pnpm --filter @agentcommercekit/keys exec vitest run src/encoding/hex.test.tsisHexString('0XABCDEF') === trueMade with Cursor
Summary by CodeRabbit
Bug Fixes
0xand uppercase0Xprefixes, matching hex string conversion behavior.Tests
0Xprefix.