feat(coin): new coin package#415
Open
Hanssen0 wants to merge 2 commits into
Open
Conversation
Hanssen0
commented
Jun 30, 2026
Hanssen0
commented
Member
- I have read the Contributing Guidelines
🦋 Changeset detectedLatest commit: 3edf75a The changes in this PR will be included in the next version bump. This PR includes changesets to release 6 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
✅ Deploy Preview for apiccc ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for appccc ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for liveccc ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for docsccc ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
Pull request overview
This PR introduces a new @ckb-ccc/coin workspace package that provides a generic fungible-token (Coin) helper built on @ckb-ccc/core, and wires it into the monorepo’s testing/docs tooling and the @ckb-ccc/shell re-export surface.
Changes:
- Added new
packages/coinpackage (Coin implementation, tests, build/lint/test/tooling config, docs). - Updated root Vitest + TypeDoc configs to include
packages/coin. - Updated
@ckb-ccc/shellto depend on and re-export@ckb-ccc/coin; updated lockfile accordingly.
Reviewed changes
Copilot reviewed 21 out of 23 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| vitest.config.mts | Adds packages/coin to the monorepo Vitest projects list. |
| typedoc.config.mjs | Adds packages/coin to TypeDoc workspace entry points. |
| pnpm-lock.yaml | Adds the packages/coin importer and updates resolved dependency snapshots. |
| packages/shell/src/barrel.ts | Re-exports the coin namespace from @ckb-ccc/coin. |
| packages/shell/package.json | Adds @ckb-ccc/coin as a workspace dependency. |
| packages/coin/vitest.config.ts | Adds package-local Vitest configuration for tests and coverage. |
| packages/coin/typedoc.json | Adds package-local TypeDoc configuration for API generation. |
| packages/coin/tsdown.config.mts | Adds tsdown build configuration for ESM+CJS outputs and copy-basedirs. |
| packages/coin/tsconfig.json | Adds TypeScript build configuration for the coin package. |
| packages/coin/src/index.ts | Exposes barrel exports and a coin namespace export. |
| packages/coin/src/coin/index.ts | Implements the Coin class (balance/info helpers and transaction completion helpers). |
| packages/coin/src/coin/index.test.ts | Adds Vitest coverage for Coin behaviors (inputs completion, change handling, balance/info helpers). |
| packages/coin/src/coin/error.ts | Adds ErrorCoinInsufficient error type for insufficient Coin balance conditions. |
| packages/coin/src/coin/coinInfo.ts | Adds CoinInfo aggregation helper used by the Coin implementation. |
| packages/coin/src/barrel.ts | Re-exports the coin module surface from src/coin. |
| packages/coin/README.md | Adds package documentation and usage examples. |
| packages/coin/prettier.config.cjs | Adds package-local Prettier configuration. |
| packages/coin/package.json | Defines the new @ckb-ccc/coin package metadata, exports, and scripts. |
| packages/coin/misc/basedirs/dist/package.json | Ensures emitted ESM dist is treated as "type": "module". |
| packages/coin/misc/basedirs/dist.commonjs/package.json | Ensures emitted CJS dist is treated as "type": "commonjs". |
| packages/coin/eslint.config.mjs | Adds package-local ESLint + typescript-eslint configuration. |
| packages/coin/.prettierignore | Adds package-local Prettier ignore rules. |
| packages/coin/.npmignore | Adds package-local npm publish ignore rules. |
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Hanssen0
force-pushed
the
feat/coin
branch
5 times, most recently
from
June 30, 2026 08:04
9481017 to
982850a
Compare
Hanssen0
force-pushed
the
feat/coin
branch
6 times, most recently
from
July 4, 2026 07:55
0c73462 to
497132a
Compare
Hanssen0
force-pushed
the
feat/coin
branch
3 times, most recently
from
July 4, 2026 09:46
b32dcb5 to
7c07413
Compare
Hanssen0
force-pushed
the
feat/coin
branch
2 times, most recently
from
July 14, 2026 06:55
ce82cc8 to
1b32769
Compare
Member
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
Warning Review limit reached
Next review available in: 24 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (28)
📝 WalkthroughWalkthroughThis change introduces the ChangesCoin package
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant Coin
participant Client
participant Transaction
participant CoBuild
Caller->>Coin: create transfer or complete transaction
Coin->>Client: query matching cells
Client-->>Coin: return Coin cells and cell dependencies
Coin->>Transaction: add inputs and change output
Coin->>CoBuild: record transfer, mint, or burn action
CoBuild-->>Coin: return action witness index
Coin-->>Caller: return completed transaction
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/coin/README.md`:
- Around line 123-128: Update the “Change to a specific address” README example
so it is self-contained by defining tx, or explicitly label the snippet as
continuing from the preceding Send example and add that clarification in the
code block. Preserve the existing signer.getRecommendedAddressObj and
coin.completeChangeToLock calls.
In `@packages/coin/src/coin/coin.test.ts`:
- Around line 1538-1555: Update the test around completeInputsByAmount so it
passes the negative amountTweak value of -100 when invoking the method. Preserve
the existing transaction setup and addedCount expectation, ensuring the test
specifically verifies that the negative tweak cancels the output requirement.
In `@packages/coin/src/coin/coinInfo.ts`:
- Around line 49-52: Update CoinInfo.from so that when infoLike is already a
CoinInfo, it returns infoLike.clone() rather than the original reference.
Preserve the existing conversion behavior for other CoinInfoLike inputs and
maintain the documented guarantee that from always returns a new instance.
In `@packages/coin/src/xUdt/args.ts`:
- Around line 42-60: Update CoinXUdtArgsCodec and CoinXUdtArgs so xUDT extension
payloads are preserved during decode and re-encode, modeling extensionData
explicitly if supported; otherwise validate flags and encoded lengths and reject
unsupported extension data instead of silently emitting only the owner hash and
four flag bytes.
In `@packages/coin/src/xUdt/coinXUdt.ts`:
- Around line 99-115: Update the CoinXUdt constructor’s argument handling around
CoinXUdtArgs.fromBytes so extended bytes from options.script.args are preserved
when rebuilding the script, rather than always replacing them with
args.toBytes(). Keep canonical serialization for options.xUdtArgs while
retaining the original script argument bytes for the script-derived path, and
add a regression test using the extended arguments covered in xUdt.test.ts.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: b179b27b-6b93-41c2-8a16-7fa1df39ce82
⛔ Files ignored due to path filters (2)
packages/coin/misc/basedirs/dist/package.jsonis excluded by!**/dist/**pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (28)
.changeset/early-oranges-lick.mdpackages/coin/.npmignorepackages/coin/.prettierignorepackages/coin/README.mdpackages/coin/eslint.config.mjspackages/coin/misc/basedirs/dist.commonjs/package.jsonpackages/coin/package.jsonpackages/coin/prettier.config.cjspackages/coin/src/barrel.tspackages/coin/src/coBuild.tspackages/coin/src/coin/coin.test.tspackages/coin/src/coin/coin.tspackages/coin/src/coin/coinInfo.tspackages/coin/src/coin/error.tspackages/coin/src/coin/index.tspackages/coin/src/index.tspackages/coin/src/xUdt/args.tspackages/coin/src/xUdt/coinXUdt.tspackages/coin/src/xUdt/index.tspackages/coin/src/xUdt/xUdt.test.tspackages/coin/tsconfig.jsonpackages/coin/tsdown.config.mtspackages/coin/typedoc.jsonpackages/coin/vitest.config.tspackages/shell/package.jsonpackages/shell/src/barrel.tstypedoc.config.mjsvitest.config.mts
Hanssen0
force-pushed
the
feat/coin
branch
4 times, most recently
from
July 24, 2026 19:47
ebd5a39 to
627b291
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
packages/coin/src/coin/coin.ts (2)
791-893: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
transferandmintare byte-for-byte identical except the action type.Extract the shared output-building + action-append flow into a private helper parameterized by
"Transfer" | "Mint"to keep the two paths from drifting.♻️ Sketch
+ private async addCoinOutputs( + type: "Transfer" | "Mint", + items: { to: ccc.ScriptLike; amount: ccc.NumLike }[], + txLike?: ccc.TransactionLike | null, + ): Promise<{ + tx: ccc.Transaction; + outputIndexes: number[]; + witnessIndex: number; + }> { + const tx = ccc.Transaction.from(txLike ?? {}); + const outputIndexes: number[] = []; + + for (const { to, amount } of items) { + outputIndexes.push( + tx.addOutput( + await this.setAmount( + { cellOutput: { lock: to, type: await this.script } }, + amount, + ), + ) - 1, + ); + } + + return { + ...(await ( + await this.coBuild + ).appendActions( + tx, + items.map((value) => CoinAction.from({ type, value })), + )), + outputIndexes, + }; + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/coin/src/coin/coin.ts` around lines 791 - 893, Extract the duplicated transaction output-building and CoBuild action-appending logic from transfer and mint into a private helper parameterized by the action type "Transfer" | "Mint". Have transfer and mint delegate to this helper while preserving their existing inputs, outputIndexes, transaction handling, and CoinAction behavior.
377-395: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSilently swallowing every decode error hides malformed actions.
A CoBuild action that fails to decode is treated as "no mint/burn intent", so completion accounting silently diverges from what the script will enforce on-chain. Consider narrowing the swallow to decode failures and surfacing them (log/warn), so callers can diagnose a tx that later fails verification.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/coin/src/coin/coin.ts` around lines 377 - 395, Update getIntendedAmountBurned so CoinAction decoding failures are not silently ignored: catch only the expected decode error type, surface it through the established logging or warning mechanism, and let unexpected errors propagate. Preserve the existing zero contribution for successfully decoded non-mint/burn actions.packages/coin/src/coin/coin.test.ts (1)
1012-1017: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
mockImplementationOnceis fragile here — completion sources inputs in two phases.
completeChangeToOutputinvokescompleteInputsByAmounttwice and resolves each input cell viaclient.getCell, so the second and later calls silently fall back to the describe-level mocks, which don't knowhighCapCoin. Prefer a persistentmockImplementationthat resolveshighCapCoinby out point.♻️ Proposed change
- vi.spyOn(signer, "findCells").mockImplementationOnce(async function* () { - yield highCapCoin; - }); - vi.spyOn(client, "getCell").mockImplementationOnce( - async () => highCapCoin, - ); + vi.spyOn(signer, "findCells").mockImplementation(async function* () { + yield highCapCoin; + }); + vi.spyOn(client, "getCell").mockImplementation(async (outPoint) => + ccc.OutPoint.from(outPoint).eq(highCapCoin.outPoint) + ? highCapCoin + : undefined, + );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/coin/src/coin/coin.test.ts` around lines 1012 - 1017, Update the test mocks around completeChangeToOutput to use persistent mockImplementation calls instead of mockImplementationOnce for signer.findCells and client.getCell. Ensure client.getCell resolves highCapCoin by its out point on every invocation, so both completeInputsByAmount phases use the intended fixture rather than falling back to describe-level mocks.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/coin/src/coin/coin.test.ts`:
- Around line 1700-1707: Update the test setup in “should add outputs correctly”
so recipientLock1 and recipientLock2 are distinct scripts, using a separate
signer or address source for the second recipient. Keep the existing transfer
and recipient-pairing assertions unchanged so they can detect swapped or
incorrect output destinations.
- Around line 720-727: Clone completedTx.outputs[1] before passing it to
CellOutput.from when computing minCapacity, ensuring the capacity baseline is
independent of the output being validated. Apply the same
clone-before-comparison pattern to the other two related assertions in
coin.test.ts.
In `@packages/coin/src/coin/coin.ts`:
- Around line 145-184: Update the constructor promise fan-out around resolved,
script, cellDeps, filter, and coBuild so a single initialization failure is
shared through one guarded promise rather than producing independently unhandled
branch rejections. Ensure callers can still await each exposed promise while
handling getKnownScript or getCellDeps failures does not terminate the process
through unobserved sibling promises.
---
Nitpick comments:
In `@packages/coin/src/coin/coin.test.ts`:
- Around line 1012-1017: Update the test mocks around completeChangeToOutput to
use persistent mockImplementation calls instead of mockImplementationOnce for
signer.findCells and client.getCell. Ensure client.getCell resolves highCapCoin
by its out point on every invocation, so both completeInputsByAmount phases use
the intended fixture rather than falling back to describe-level mocks.
In `@packages/coin/src/coin/coin.ts`:
- Around line 791-893: Extract the duplicated transaction output-building and
CoBuild action-appending logic from transfer and mint into a private helper
parameterized by the action type "Transfer" | "Mint". Have transfer and mint
delegate to this helper while preserving their existing inputs, outputIndexes,
transaction handling, and CoinAction behavior.
- Around line 377-395: Update getIntendedAmountBurned so CoinAction decoding
failures are not silently ignored: catch only the expected decode error type,
surface it through the established logging or warning mechanism, and let
unexpected errors propagate. Preserve the existing zero contribution for
successfully decoded non-mint/burn actions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 243b1e87-efc2-487f-9007-58ca5a786248
⛔ Files ignored due to path filters (2)
packages/coin/misc/basedirs/dist/package.jsonis excluded by!**/dist/**pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (28)
.changeset/early-oranges-lick.mdpackages/coin/.npmignorepackages/coin/.prettierignorepackages/coin/README.mdpackages/coin/eslint.config.mjspackages/coin/misc/basedirs/dist.commonjs/package.jsonpackages/coin/package.jsonpackages/coin/prettier.config.cjspackages/coin/src/barrel.tspackages/coin/src/coBuild.tspackages/coin/src/coin/coin.test.tspackages/coin/src/coin/coin.tspackages/coin/src/coin/coinInfo.tspackages/coin/src/coin/error.tspackages/coin/src/coin/index.tspackages/coin/src/index.tspackages/coin/src/xUdt/args.tspackages/coin/src/xUdt/coinXUdt.tspackages/coin/src/xUdt/index.tspackages/coin/src/xUdt/xUdt.test.tspackages/coin/tsconfig.jsonpackages/coin/tsdown.config.mtspackages/coin/typedoc.jsonpackages/coin/vitest.config.tspackages/shell/package.jsonpackages/shell/src/barrel.tstypedoc.config.mjsvitest.config.mts
🚧 Files skipped from review as they are similar to previous changes (22)
- packages/coin/misc/basedirs/dist.commonjs/package.json
- packages/coin/typedoc.json
- vitest.config.mts
- packages/shell/src/barrel.ts
- packages/coin/.prettierignore
- packages/coin/src/xUdt/index.ts
- packages/coin/src/coin/error.ts
- packages/coin/src/coin/index.ts
- packages/coin/README.md
- packages/coin/prettier.config.cjs
- typedoc.config.mjs
- .changeset/early-oranges-lick.md
- packages/coin/src/barrel.ts
- packages/coin/src/index.ts
- packages/coin/tsdown.config.mts
- packages/coin/tsconfig.json
- packages/coin/vitest.config.ts
- packages/shell/package.json
- packages/coin/eslint.config.mjs
- packages/coin/src/coBuild.ts
- packages/coin/package.json
- packages/coin/src/coin/coinInfo.ts
There was a problem hiding this comment.
♻️ Duplicate comments (3)
packages/coin/src/coin/coin.test.ts (2)
1748-1749: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winBoth recipients are the same lock, so the pairing assertions can't fail.
Use a distinct script for
recipientLock2so a swappedtowould be detected at Lines 1758-1761 and 1779-1781.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/coin/src/coin/coin.test.ts` around lines 1748 - 1749, Update recipientLock2 in the test setup around recipientLock1 to use a distinct recipient signer or address script, while keeping recipientLock1 unchanged. Ensure the pairing assertions at the referenced assertion blocks compare different locks so swapped to values are detected.
753-759: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
minCapacitybaseline is derived from the same output being asserted.
ccc.CellOutput.from(completedTx.outputs[1], ...)reuses the passed instance, so the comparison can become self-referential. Clone the output before computingminCapacity; the same pattern appears at Lines 1092-1098 and 1143-1150.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/coin/src/coin/coin.test.ts` around lines 753 - 759, Update the minCapacity calculations in the test blocks around the current assertion and the matching blocks near lines 1092 and 1143 to pass a cloned completedTx.outputs[1] into ccc.CellOutput.from instead of the original output. Keep the existing capacity assertions unchanged while ensuring each baseline is computed from an independent output instance.packages/coin/src/coin/coin.ts (1)
152-169: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winPromise fan-out from
resolvedcan still surface unhandled rejections.
script,cellDeps,filter, andcoBuildare four independent chains. IfgetKnownScript/getCellDepsrejects, any branch the caller does not await stays unobserved.🛡️ Proposed guard
this.coBuild = Promise.all([this.script]).then( ([script]) => new coBuild.CoBuild(script, options.scriptInfo), ); + + // Mark every exposed branch as handled so an early resolution failure + // cannot escape as an unhandled rejection. + for (const p of [this.script, this.cellDeps, this.filter, this.coBuild]) { + void p.catch(() => {}); + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/coin/src/coin/coin.ts` around lines 152 - 169, Update the promise fan-out in the constructor around resolved, script, cellDeps, filter, and coBuild so rejection from getKnownScript/getCellDeps is observed by every branch, preventing unhandled rejections when callers await only a subset. Add the shared rejection guard or otherwise attach equivalent handling to each derived promise while preserving their existing resolved values and behavior.
🧹 Nitpick comments (2)
packages/coin/src/coin/coin.ts (2)
615-621: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRead the final amounts from
res2.txrather thantx.This only works because
ccc.Transaction.fromreturns the same instance for an existingTransaction, sotxhappens to have the inputs added bycompleteInputsByAmount. Referencingres2.txmakes the data flow explicit and immune to a future clone-on-fromchange.♻️ Proposed refactor
return ccc.Transaction.from( await change( res2.tx, - (await this.getAmountBurned(tx)) - - (await this.getIntendedAmountBurned(tx)), + (await this.getAmountBurned(res2.tx)) - + (await this.getIntendedAmountBurned(res2.tx)), ), );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/coin/src/coin/coin.ts` around lines 615 - 621, Update the amount calculations in the returned transaction flow to pass res2.tx to getAmountBurned and getIntendedAmountBurned instead of tx, while preserving the existing change and ccc.Transaction.from behavior.
322-332: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winInputs are resolved one RPC at a time.
getInputsInfoawaitsinput.getCell(client)sequentially, so latency scales linearly with input count on every call — andcompleteInputsByAmount/completecall it repeatedly. Consider resolving in parallel (e.g.Promise.allovertx.inputs) before aggregating.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/coin/src/coin/coin.ts` around lines 322 - 332, Update getInputsInfo to resolve all tx.inputs concurrently with Promise.all rather than awaiting input.getCell(client) sequentially in the async generator, then pass the resolved cells to infoFrom while preserving the existing CoinInfo result.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@packages/coin/src/coin/coin.test.ts`:
- Around line 1748-1749: Update recipientLock2 in the test setup around
recipientLock1 to use a distinct recipient signer or address script, while
keeping recipientLock1 unchanged. Ensure the pairing assertions at the
referenced assertion blocks compare different locks so swapped to values are
detected.
- Around line 753-759: Update the minCapacity calculations in the test blocks
around the current assertion and the matching blocks near lines 1092 and 1143 to
pass a cloned completedTx.outputs[1] into ccc.CellOutput.from instead of the
original output. Keep the existing capacity assertions unchanged while ensuring
each baseline is computed from an independent output instance.
In `@packages/coin/src/coin/coin.ts`:
- Around line 152-169: Update the promise fan-out in the constructor around
resolved, script, cellDeps, filter, and coBuild so rejection from
getKnownScript/getCellDeps is observed by every branch, preventing unhandled
rejections when callers await only a subset. Add the shared rejection guard or
otherwise attach equivalent handling to each derived promise while preserving
their existing resolved values and behavior.
---
Nitpick comments:
In `@packages/coin/src/coin/coin.ts`:
- Around line 615-621: Update the amount calculations in the returned
transaction flow to pass res2.tx to getAmountBurned and getIntendedAmountBurned
instead of tx, while preserving the existing change and ccc.Transaction.from
behavior.
- Around line 322-332: Update getInputsInfo to resolve all tx.inputs
concurrently with Promise.all rather than awaiting input.getCell(client)
sequentially in the async generator, then pass the resolved cells to infoFrom
while preserving the existing CoinInfo result.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 377c8388-a7cd-493a-b05f-0e8352ea940f
⛔ Files ignored due to path filters (2)
packages/coin/misc/basedirs/dist/package.jsonis excluded by!**/dist/**pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (28)
.changeset/early-oranges-lick.mdpackages/coin/.npmignorepackages/coin/.prettierignorepackages/coin/README.mdpackages/coin/eslint.config.mjspackages/coin/misc/basedirs/dist.commonjs/package.jsonpackages/coin/package.jsonpackages/coin/prettier.config.cjspackages/coin/src/barrel.tspackages/coin/src/coBuild.tspackages/coin/src/coin/coin.test.tspackages/coin/src/coin/coin.tspackages/coin/src/coin/coinInfo.tspackages/coin/src/coin/error.tspackages/coin/src/coin/index.tspackages/coin/src/index.tspackages/coin/src/xUdt/args.tspackages/coin/src/xUdt/coinXUdt.tspackages/coin/src/xUdt/index.tspackages/coin/src/xUdt/xUdt.test.tspackages/coin/tsconfig.jsonpackages/coin/tsdown.config.mtspackages/coin/typedoc.jsonpackages/coin/vitest.config.tspackages/shell/package.jsonpackages/shell/src/barrel.tstypedoc.config.mjsvitest.config.mts
🚧 Files skipped from review as they are similar to previous changes (22)
- packages/coin/typedoc.json
- packages/coin/src/barrel.ts
- typedoc.config.mjs
- packages/coin/misc/basedirs/dist.commonjs/package.json
- packages/coin/src/coin/index.ts
- packages/coin/src/xUdt/index.ts
- .changeset/early-oranges-lick.md
- vitest.config.mts
- packages/coin/vitest.config.ts
- packages/coin/prettier.config.cjs
- packages/shell/src/barrel.ts
- packages/coin/src/index.ts
- packages/coin/README.md
- packages/coin/.prettierignore
- packages/coin/tsconfig.json
- packages/coin/src/coin/error.ts
- packages/coin/tsdown.config.mts
- packages/coin/src/coin/coinInfo.ts
- packages/coin/package.json
- packages/coin/eslint.config.mjs
- packages/coin/src/coBuild.ts
- packages/coin/src/xUdt/xUdt.test.ts
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
packages/coin/src/coin/coin.test.ts (3)
930-944: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert on the returned transaction and tighten the count.
The returned
txis dropped and the assertions read the input object, so this only passes becauseTransaction.fromhands back the same instance. With 3 mock coins available,toBeGreaterThan(2)also only admits one value.♻️ Proposed refactor
- const { addedCount } = await coin.completeInputsByAmount( + const { addedCount, tx: completedTx } = await coin.completeInputsByAmount( signer, tx, ccc.Zero, // No extra Coin amount needed extraCapacityNeeded, // Extra capacity needed ); - // Should add Coins to cover the capacity requirement - expect(addedCount).toBeGreaterThan(2); - - // Should have added at least one cell with capacity - expect(await coin.getInputsAmount(tx)).toBeGreaterThan(ccc.Zero); + // All 3 available Coins are pulled in to chase the capacity requirement + expect(addedCount).toBe(3); + expect(await coin.getInputsAmount(completedTx)).toBe(ccc.numFrom(300));🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/coin/src/coin/coin.test.ts` around lines 930 - 944, Update the capacity-requirement test around coin.completeInputsByAmount to use and assert on the returned transaction rather than the original tx object. Tighten the addedCount expectation to require all three available mock coins, and read the resulting input amount from the returned transaction.
1522-1535: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMove mock cleanup into an
afterEach.
vi.restoreAllMocks()inside a single test body is order-sensitive cleanup; every other block relies on re-installing spies in its ownbeforeEach. A file-levelafterEach(orrestoreMocks: truein the vitest config) makes isolation explicit for all tests.♻️ Proposed refactor
+afterEach(() => { + vi.restoreAllMocks(); +});await expect( explicitSignerCoin.calculateInfo(signer), ).resolves.toBeDefined(); - vi.restoreAllMocks(); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/coin/src/coin/coin.test.ts` around lines 1522 - 1535, Move vi.restoreAllMocks() out of the individual “uses a signer supplied to calculateInfo” test and add file-level afterEach cleanup so mocks are restored consistently after every test. Keep the existing beforeEach spy setup and test behavior unchanged.
1047-1052: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
mockImplementationOncecouples this test to internal call counts.The enclosing
beforeEachalready installed persistent mocks overmockCoins, which does not containhighCapCoin. Any secondgetCell/findCellslookup falls through and resolvesundefined, so theinputs.length === 1assertion depends on the implementation making exactly one call each. PrefermockImplementationkeyed on the outPoint.♻️ Proposed refactor
- vi.spyOn(signer, "findCells").mockImplementationOnce(async function* () { + vi.spyOn(signer, "findCells").mockImplementation(async function* () { yield highCapCoin; }); - vi.spyOn(client, "getCell").mockImplementationOnce( - async () => highCapCoin, - ); + vi.spyOn(client, "getCell").mockImplementation(async (outPoint) => + highCapCoin.outPoint.eq(ccc.OutPoint.from(outPoint)) + ? highCapCoin + : undefined, + );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/coin/src/coin/coin.test.ts` around lines 1047 - 1052, Update the signer.findCells and client.getCell mocks in the test to use persistent mockImplementation handlers keyed by the requested outPoint, returning highCapCoin for its matching outPoint and preserving the existing fallback behavior for other lookups. Remove mockImplementationOnce so the inputs.length assertion no longer depends on exact call counts.packages/coin/src/xUdt/args.ts (1)
183-199: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAnnotate the codec generics so
DecodedTypeinfers.ESLint reports unsafe assignment on all six fields, meaning
ccc.DecodedType<typeof CoinXUdtArgsCodec>is not inferring here —ccc.Codec.fromis called without explicit type parameters at Line 89, so the decoded shape stays untyped. Supplying them (encode-in / decode-out) restores type safety for the constructor and removes the lint errors.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/coin/src/xUdt/args.ts` around lines 183 - 199, Annotate the generic parameters on the ccc.Codec.from call used to create CoinXUdtArgsCodec, supplying the encode-in and decode-out types so ccc.DecodedType<typeof CoinXUdtArgsCodec> infers the decoded shape and the six constructor assignments remain type-safe.Source: Linters/SAST tools
packages/coin/src/coin/coin.ts (1)
376-386: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSilent
catchgives no signal when a Coin action fails to decode.Tolerating undecodable actions is reasonable (the witness may carry other scripts' actions), but this value feeds the balance math in
completeInputsByAmount(Line 480) andcomplete(Lines 588, 626). A corruptedMint/Burnbelonging to this coin is silently counted as zero, skewing the change amount with no diagnostic. Consider at least documenting the intent, or emitting a debug-level log.♻️ Make the swallow intentional and observable
try { const coinAction = CoinAction.fromBytes(action.data, { isExtraFieldIgnored: true, }); acc += coinAction.match({ Mint: (mint) => -mint.amount, Burn: (burn) => burn.amount, _: () => ccc.Zero, }); - } catch (_) {} + } catch (err) { + // Actions in this witness may belong to other scripts or newer + // versions; treat anything we cannot decode as amount-neutral. + void err; + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/coin/src/coin/coin.ts` around lines 376 - 386, Update the catch block around CoinAction.fromBytes in the action balance calculation to make the intentional tolerance of undecodable actions observable, preferably by emitting a debug-level diagnostic that includes the decode error. Preserve the existing behavior of ignoring unsupported or malformed actions and continuing the accumulator calculation.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/coin/src/coin/coin.ts`:
- Around line 158-165: Update the filter initialization in the Coin constructor
so caller-provided options.filter are merged over the default filter rather than
replacing it, ensuring the coin type script constraint remains enforced while
allowing custom fields to override defaults. Preserve the existing
outputDataLenRange default unless explicitly overridden, and keep the resulting
filter compatible with completeInputs.
In `@packages/coin/src/xUdt/args.ts`:
- Around line 94-109: Update the flags calculation in the argument encoding flow
to coerce the final bitwise result to an unsigned 32-bit value before passing it
to ccc.numLeToBytes. Preserve the existing flag bits and
ownerScriptHash/extension handling, including the ownerModeInputType 0x80000000
case.
---
Nitpick comments:
In `@packages/coin/src/coin/coin.test.ts`:
- Around line 930-944: Update the capacity-requirement test around
coin.completeInputsByAmount to use and assert on the returned transaction rather
than the original tx object. Tighten the addedCount expectation to require all
three available mock coins, and read the resulting input amount from the
returned transaction.
- Around line 1522-1535: Move vi.restoreAllMocks() out of the individual “uses a
signer supplied to calculateInfo” test and add file-level afterEach cleanup so
mocks are restored consistently after every test. Keep the existing beforeEach
spy setup and test behavior unchanged.
- Around line 1047-1052: Update the signer.findCells and client.getCell mocks in
the test to use persistent mockImplementation handlers keyed by the requested
outPoint, returning highCapCoin for its matching outPoint and preserving the
existing fallback behavior for other lookups. Remove mockImplementationOnce so
the inputs.length assertion no longer depends on exact call counts.
In `@packages/coin/src/coin/coin.ts`:
- Around line 376-386: Update the catch block around CoinAction.fromBytes in the
action balance calculation to make the intentional tolerance of undecodable
actions observable, preferably by emitting a debug-level diagnostic that
includes the decode error. Preserve the existing behavior of ignoring
unsupported or malformed actions and continuing the accumulator calculation.
In `@packages/coin/src/xUdt/args.ts`:
- Around line 183-199: Annotate the generic parameters on the ccc.Codec.from
call used to create CoinXUdtArgsCodec, supplying the encode-in and decode-out
types so ccc.DecodedType<typeof CoinXUdtArgsCodec> infers the decoded shape and
the six constructor assignments remain type-safe.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: e76a77d2-c93b-426e-89d3-0c1ce3742927
⛔ Files ignored due to path filters (2)
packages/coin/misc/basedirs/dist/package.jsonis excluded by!**/dist/**pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (28)
.changeset/early-oranges-lick.mdpackages/coin/.npmignorepackages/coin/.prettierignorepackages/coin/README.mdpackages/coin/eslint.config.mjspackages/coin/misc/basedirs/dist.commonjs/package.jsonpackages/coin/package.jsonpackages/coin/prettier.config.cjspackages/coin/src/barrel.tspackages/coin/src/coBuild.tspackages/coin/src/coin/coin.test.tspackages/coin/src/coin/coin.tspackages/coin/src/coin/coinInfo.tspackages/coin/src/coin/error.tspackages/coin/src/coin/index.tspackages/coin/src/index.tspackages/coin/src/xUdt/args.tspackages/coin/src/xUdt/coinXUdt.tspackages/coin/src/xUdt/index.tspackages/coin/src/xUdt/xUdt.test.tspackages/coin/tsconfig.jsonpackages/coin/tsdown.config.mtspackages/coin/typedoc.jsonpackages/coin/vitest.config.tspackages/shell/package.jsonpackages/shell/src/barrel.tstypedoc.config.mjsvitest.config.mts
🚧 Files skipped from review as they are similar to previous changes (25)
- packages/shell/package.json
- packages/coin/typedoc.json
- .changeset/early-oranges-lick.md
- typedoc.config.mjs
- packages/coin/src/barrel.ts
- packages/coin/src/index.ts
- packages/coin/src/xUdt/index.ts
- packages/shell/src/barrel.ts
- packages/coin/prettier.config.cjs
- packages/coin/vitest.config.ts
- vitest.config.mts
- packages/coin/src/coin/error.ts
- packages/coin/eslint.config.mjs
- packages/coin/src/coin/index.ts
- packages/coin/README.md
- packages/coin/misc/basedirs/dist.commonjs/package.json
- packages/coin/package.json
- packages/coin/.npmignore
- packages/coin/.prettierignore
- packages/coin/tsconfig.json
- packages/coin/src/coin/coinInfo.ts
- packages/coin/src/xUdt/coinXUdt.ts
- packages/coin/src/coBuild.ts
- packages/coin/tsdown.config.mts
- packages/coin/src/xUdt/xUdt.test.ts
Hanssen0
force-pushed
the
feat/coin
branch
5 times, most recently
from
July 25, 2026 22:55
952c9c7 to
c4617af
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
packages/coin/src/coin/coin.ts (2)
773-838: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
transferandmintare identical except for the action type.Both bodies duplicate output creation,
setAmount, andappendActions; only the"Transfer"/"Mint"literal differs. Extracting a shared private helper keeps future changes (e.g. added action fields) from drifting between the two.♻️ Proposed extraction
+ /** Shared implementation for `transfer` and `mint`. `@internal` */ + protected async addValueOutputs( + type: "Transfer" | "Mint", + items: { to: ccc.ScriptLike; amount: ccc.NumLike }[], + txLike?: ccc.TransactionLike | null, + ): Promise<ccc.Transaction> { + let tx = ccc.Transaction.from(txLike ?? {}); + + for (const { to, amount } of items) { + const outputIndex = tx.addOutput({ lock: to, type: this.script }) - 1; + tx = await this.setAmount(tx, outputIndex, amount); + } + + const { tx: txWithActions } = await this.coBuild.appendActions( + items.map((value) => CoinAction.from({ type, value })), + tx, + ); + return txWithActions; + }
transferandmintthen become one-liners delegating with the respective action type.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/coin/src/coin/coin.ts` around lines 773 - 838, Extract the shared transaction-building logic from transfer and mint into a private helper that accepts the transfers/mints collection and the CoinAction type. Have the helper create outputs, apply setAmount, append the corresponding actions, and return the updated transaction; reduce transfer and mint to delegations using "Transfer" and "Mint" respectively.
355-361: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueInput cells are fetched strictly sequentially.
Each
input.getCell(client)awaits the previous one, so a transaction with many inputs costs N serial round-trips. If the client does not cache aggressively, consider resolving in batches (ortx.getInputsCapacity-style prefetch) while keeping the async-iterable contract forinfoFrom.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/coin/src/coin/coin.ts` around lines 355 - 361, Update the async generator passed to infoFrom in the transaction input flow so input.getCell(client) requests are resolved concurrently or in batches instead of awaiting each input strictly sequentially. Preserve the async-iterable contract and yield each resolved cell with the existing input order and behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/coin/src/coin/coin.ts`:
- Around line 183-189: Update the normalized filter construction around
ClientIndexerSearchKeyFilter.from so caller-provided options are merged with the
default script, script length range, and output data length range rather than
replacing them; ensure the identifying normalizedScript remains pinned while
allowing supported caller fields to override defaults as intended.
- Around line 394-410: Update getIntendedAmountBurned so CoinAction.fromBytes or
action matching failures are surfaced instead of silently swallowed, allowing
malformed relevant actions to fail change computation; remove the empty catch or
rethrow with useful context. Add TSDoc describing the method’s intended amount
calculation and its failure behavior, consistent with the surrounding public
API.
In `@packages/coin/src/xUdt/coinXUdt.ts`:
- Around line 126-141: Update the coin option handling around the script branch
so a partially specified script containing only codeHash or only hashType is
rejected rather than routed to the known xUDT fallback. Preserve the existing
fully specified custom script path and the knownScript default only when no
script identifiers are supplied, allowing Coin.resolveOptions to surface the
invalid partial configuration.
---
Nitpick comments:
In `@packages/coin/src/coin/coin.ts`:
- Around line 773-838: Extract the shared transaction-building logic from
transfer and mint into a private helper that accepts the transfers/mints
collection and the CoinAction type. Have the helper create outputs, apply
setAmount, append the corresponding actions, and return the updated transaction;
reduce transfer and mint to delegations using "Transfer" and "Mint"
respectively.
- Around line 355-361: Update the async generator passed to infoFrom in the
transaction input flow so input.getCell(client) requests are resolved
concurrently or in batches instead of awaiting each input strictly sequentially.
Preserve the async-iterable contract and yield each resolved cell with the
existing input order and behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: c7478433-39db-4838-af39-e55c8e02bf92
⛔ Files ignored due to path filters (2)
packages/coin/misc/basedirs/dist/package.jsonis excluded by!**/dist/**pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (28)
.changeset/early-oranges-lick.mdpackages/coin/.npmignorepackages/coin/.prettierignorepackages/coin/README.mdpackages/coin/eslint.config.mjspackages/coin/misc/basedirs/dist.commonjs/package.jsonpackages/coin/package.jsonpackages/coin/prettier.config.cjspackages/coin/src/barrel.tspackages/coin/src/coBuild.tspackages/coin/src/coin/coin.test.tspackages/coin/src/coin/coin.tspackages/coin/src/coin/coinInfo.tspackages/coin/src/coin/error.tspackages/coin/src/coin/index.tspackages/coin/src/index.tspackages/coin/src/xUdt/args.tspackages/coin/src/xUdt/coinXUdt.tspackages/coin/src/xUdt/index.tspackages/coin/src/xUdt/xUdt.test.tspackages/coin/tsconfig.jsonpackages/coin/tsdown.config.mtspackages/coin/typedoc.jsonpackages/coin/vitest.config.tspackages/shell/package.jsonpackages/shell/src/barrel.tstypedoc.config.mjsvitest.config.mts
🚧 Files skipped from review as they are similar to previous changes (25)
- .changeset/early-oranges-lick.md
- packages/shell/package.json
- packages/shell/src/barrel.ts
- packages/coin/.prettierignore
- packages/coin/src/xUdt/index.ts
- packages/coin/src/barrel.ts
- typedoc.config.mjs
- packages/coin/typedoc.json
- packages/coin/src/coin/index.ts
- vitest.config.mts
- packages/coin/README.md
- packages/coin/prettier.config.cjs
- packages/coin/vitest.config.ts
- packages/coin/misc/basedirs/dist.commonjs/package.json
- packages/coin/src/index.ts
- packages/coin/src/coin/error.ts
- packages/coin/tsconfig.json
- packages/coin/package.json
- packages/coin/tsdown.config.mts
- packages/coin/src/coin/coinInfo.ts
- packages/coin/src/coBuild.ts
- packages/coin/eslint.config.mjs
- packages/coin/src/xUdt/args.ts
- packages/coin/src/xUdt/xUdt.test.ts
- packages/coin/src/coin/coin.test.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.