refactor of kos-web demo to match newer code#244
Conversation
📝 WalkthroughWalkthroughWeb demo UI, assets, and Vite/TypeScript configuration for kos-web were removed; a Node.js ESM demo script (index.js) plus src/package.json were added. The kos-go demo go.mod dependency was bumped from v0.2.37 to v0.2.40. Changes
Sequence DiagramsequenceDiagram
participant Script as index.js
participant KOS as "@klever/kos-web"
participant Wallet as Wallet
participant TxOpts as TransactionChainOptions
Script->>KOS: import Wallet, TransactionChainOptions, helpers
Script->>Wallet: derive account (fromMnemonicIndex / PathOptions)
Wallet-->>Script: address & publicKey
Script->>Script: raw tx hex → bytes
Script->>TxOpts: build Bitcoin sign options (amounts, prevScripts)
Script->>Wallet: sign(rawTxBytes, TxOpts)
Wallet->>KOS: invoke signing routines
KOS-->>Wallet: signed tx bytes
Wallet-->>Script: signed tx bytes
Script->>Script: signed bytes → hex, verify expected result
Script-->>Console: log success / exit on mismatch
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/kos-web/demo/src/package.json (1)
2-15: Consider marking this demo package as private.Since this manifest lives under
packages/kos-web/demo/src, adding"private": truewould help prevent accidentally publishing the demo as a real package.Suggested change
"name": "kos-web-demo", "version": "1.0.0", + "private": true, "description": "",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/kos-web/demo/src/package.json` around lines 2 - 15, The package manifest lacks the private flag which risks accidental publishing; open the package.json manifest and add the property "private": true at the top level of the JSON object (alongside "name", "version", "type", etc.) to mark this demo package as non-publishable; ensure the JSON remains valid and that existing fields like "name", "version", "type", and "dependencies" are preserved.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/kos-web/demo/src/index.js`:
- Around line 76-89: When signing fails, the script currently only logs and
exits with status 0; update the mismatch branch (the if checking signedRaw !==
expectedRaw in demo's main flow) to call process.exit(1) after logging the
expected/got messages so the process fails; also in the catch block that
currently logs error.message and error.stack, call process.exit(1) (or rethrow)
after logging so exceptions produce a non‑zero exit code. Ensure you modify the
signedRaw !== expectedRaw branch and the catch handler in the main function in
packages/kos-web/demo/src/index.js.
In `@packages/kos-web/demo/src/package.json`:
- Around line 6-8: The current "test" script in package.json is a failing
placeholder that causes workspace test sweeps to report false failures; update
the "scripts"."test" entry so it exits successfully (for example replace the
failing command with a no-op/pass-through test command or an echo that returns
exit code 0) so running npm test for the demo no longer returns code 1; edit the
package.json scripts object (the "test" key) accordingly and leave "dev"
unchanged.
---
Nitpick comments:
In `@packages/kos-web/demo/src/package.json`:
- Around line 2-15: The package manifest lacks the private flag which risks
accidental publishing; open the package.json manifest and add the property
"private": true at the top level of the JSON object (alongside "name",
"version", "type", etc.) to mark this demo package as non-publishable; ensure
the JSON remains valid and that existing fields like "name", "version", "type",
and "dependencies" are preserved.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3cfcc93c-3129-4f4e-8d4c-3a526a6d022c
⛔ Files ignored due to path filters (1)
packages/kos-go/demo/go.sumis excluded by!**/*.sum
📒 Files selected for processing (14)
packages/kos-go/demo/go.modpackages/kos-web/demo/README.mdpackages/kos-web/demo/index.htmlpackages/kos-web/demo/package.jsonpackages/kos-web/demo/src/components/cryptography-demo.tspackages/kos-web/demo/src/components/transaction-signer.tspackages/kos-web/demo/src/components/wallet-generator.tspackages/kos-web/demo/src/index.jspackages/kos-web/demo/src/main.tspackages/kos-web/demo/src/package.jsonpackages/kos-web/demo/src/style.csspackages/kos-web/demo/src/vite-env.d.tspackages/kos-web/demo/tsconfig.jsonpackages/kos-web/demo/vite.config.ts
💤 Files with no reviewable changes (11)
- packages/kos-web/demo/src/main.ts
- packages/kos-web/demo/README.md
- packages/kos-web/demo/package.json
- packages/kos-web/demo/tsconfig.json
- packages/kos-web/demo/index.html
- packages/kos-web/demo/src/components/cryptography-demo.ts
- packages/kos-web/demo/src/vite-env.d.ts
- packages/kos-web/demo/vite.config.ts
- packages/kos-web/demo/src/components/transaction-signer.ts
- packages/kos-web/demo/src/style.css
- packages/kos-web/demo/src/components/wallet-generator.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/kos-web/demo/src/index.js (1)
31-59: Share one validated hex decoder.These two helpers duplicate the same parsing loop, and malformed hex currently degrades into a later signing mismatch instead of failing at decode time. Decoding once with length/byte validation will make this smoke test much easier to trust.
♻️ Proposed refactor
- // Convert hex to base64 for JavaScript binding - const hexToBase64 = (hex) => { - const bytes = new Uint8Array(hex.length / 2); - for (let i = 0; i < hex.length; i += 2) { - bytes[i / 2] = parseInt(hex.substr(i, 2), 16); - } - return btoa(String.fromCharCode(...bytes)); - }; + const decodeHex = (hex) => { + if (hex.length % 2 !== 0) { + throw new Error("Invalid hex string length"); + } + + const bytes = new Uint8Array(hex.length / 2); + for (let i = 0; i < hex.length; i += 2) { + const byte = Number.parseInt(hex.slice(i, i + 2), 16); + if (Number.isNaN(byte)) { + throw new Error(`Invalid hex byte at offset ${i}`); + } + bytes[i / 2] = byte; + } + return bytes; + }; + + const hexToBase64 = (hex) => + btoa(String.fromCharCode(...decodeHex(hex))); @@ - // Convert raw transaction hex string to bytes using custom hex decoder - const hexToUint8Array = (hex) => { - const bytes = new Uint8Array(hex.length / 2); - for (let i = 0; i < hex.length; i += 2) { - bytes[i / 2] = parseInt(hex.substr(i, 2), 16); - } - return bytes; - }; + // Convert raw transaction hex string to bytes using the validated decoder + const hexToUint8Array = decodeHex;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/kos-web/demo/src/index.js` around lines 31 - 59, Both helpers duplicate hex parsing and lack validation; replace them with a single validated decoder function (e.g., hexToBytes) that validates even length and hex characters and throws on malformed input, then reuse it: call hexToBytes for TransactionChainOptions.newBitcoinSignOptions (convert result to base64 for prevScript1/prevScript2) and pass the Uint8Array or converted Big-endian byte arrays for hexToUint8Array usage; update references to hexToBase64 and hexToUint8Array to use hexToBytes + a deterministic base64 conversion so decoding fails early and consistently for inputAmounts/prevScripts/TransactionChainOptions.newBitcoinSignOptions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/kos-web/demo/src/index.js`:
- Around line 1-7: The import currently brings in a symbol named toString which
shadows the restricted global and triggers Biome; update the import statement to
either remove toString if unused or rename it via an alias (e.g., import
toString as bytesToString) so the local symbol no longer conflicts; look for the
import line importing Wallet, PathOptions, TransactionChainOptions, toBytes,
toString from '@klever/kos-web' and modify it accordingly.
---
Nitpick comments:
In `@packages/kos-web/demo/src/index.js`:
- Around line 31-59: Both helpers duplicate hex parsing and lack validation;
replace them with a single validated decoder function (e.g., hexToBytes) that
validates even length and hex characters and throws on malformed input, then
reuse it: call hexToBytes for TransactionChainOptions.newBitcoinSignOptions
(convert result to base64 for prevScript1/prevScript2) and pass the Uint8Array
or converted Big-endian byte arrays for hexToUint8Array usage; update references
to hexToBase64 and hexToUint8Array to use hexToBytes + a deterministic base64
conversion so decoding fails early and consistently for
inputAmounts/prevScripts/TransactionChainOptions.newBitcoinSignOptions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 37bd9315-d4f4-49b1-95a0-0af13ffd676e
📒 Files selected for processing (2)
packages/kos-web/demo/src/index.jspackages/kos-web/demo/src/package.json
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/kos-web/demo/src/package.json
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/kos-web/demo/src/index.js (1)
30-57: Consider consolidating duplicate hex-to-bytes logic and replacing deprecatedsubstr.
hexToBase64(lines 30-36) andhexToUint8Array(lines 51-57) share identical hex-to-bytes conversion logic. Also,substr()is deprecated in favor ofsubstring()orslice().♻️ Proposed refactor to consolidate and modernize
- // Convert hex to base64 for JavaScript binding - const hexToBase64 = (hex) => { - const bytes = new Uint8Array(hex.length / 2); - for (let i = 0; i < hex.length; i += 2) { - bytes[i / 2] = parseInt(hex.substr(i, 2), 16); - } - return btoa(String.fromCharCode(...bytes)); - }; + // Convert hex string to Uint8Array + const hexToUint8Array = (hex) => { + const bytes = new Uint8Array(hex.length / 2); + for (let i = 0; i < hex.length; i += 2) { + bytes[i / 2] = parseInt(hex.slice(i, i + 2), 16); + } + return bytes; + }; + + // Convert hex to base64 for JavaScript binding + const hexToBase64 = (hex) => { + return btoa(String.fromCharCode(...hexToUint8Array(hex))); + };Then remove the duplicate
hexToUint8Arraydefinition at lines 51-57.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/kos-web/demo/src/index.js` around lines 30 - 57, Both hexToBase64 and hexToUint8Array duplicate the same hex-to-bytes logic and use deprecated substr(); refactor by creating a single helper (e.g., hexToBytes) that converts a hex string to a Uint8Array using slice() or substring() instead of substr(), then have hexToBase64 call that helper and remove the duplicate hexToUint8Array definition so all hex-to-bytes logic is centralized and updated; reference the existing function names hexToBase64 and hexToUint8Array when making the replacement.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/kos-web/demo/src/index.js`:
- Around line 30-57: Both hexToBase64 and hexToUint8Array duplicate the same
hex-to-bytes logic and use deprecated substr(); refactor by creating a single
helper (e.g., hexToBytes) that converts a hex string to a Uint8Array using
slice() or substring() instead of substr(), then have hexToBase64 call that
helper and remove the duplicate hexToUint8Array definition so all hex-to-bytes
logic is centralized and updated; reference the existing function names
hexToBase64 and hexToUint8Array when making the replacement.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a233c7fa-13a2-4d0e-ba4d-191e14f56f6c
📒 Files selected for processing (1)
packages/kos-web/demo/src/index.js
Summary by CodeRabbit
New Features
Chores