feat(coin): new coin package#415
Conversation
Hanssen0
commented
Jun 30, 2026
Hanssen0
commented
- I have read the Contributing Guidelines
🦋 Changeset detectedLatest commit: 627b291 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.
9481017 to
982850a
Compare
0c73462 to
497132a
Compare
b32dcb5 to
7c07413
Compare
|
/canary |
|
❌ Canary version deployment failed. View workflow run |
|
/canary |
|
❌ Canary version deployment failed. View workflow run |
|
/canary |
|
🚀 Canary version published successfully! View workflow run The following packages have been published to npm:
|
|
/canary |
|
🚀 Canary version published successfully! View workflow run The following packages have been published to npm:
|
ce82cc8 to
1b32769
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis 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: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
packages/coin/vitest.config.tsParsing error: "parserOptions.project" has been provided for 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
| ### Change to a specific address | ||
|
|
||
| ```ts | ||
| const { script: changeLock } = await signer.getRecommendedAddressObj(); | ||
| const { tx: completedTx } = await coin.completeChangeToLock(tx, changeLock); | ||
| ``` |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Make the change-address example self-contained or explicitly mark it as a continuation.
This snippet uses tx without defining it in the same example. Readers who copy only this section will encounter an undefined identifier; either include the transaction construction or label the section as continuing from the previous example.
Suggested clarification
-### Change to a specific address
+### Change to a specific address (using the `tx` from the Send example above)
```ts
+// `tx` is created in the Send example above.
const { script: changeLock } = await signer.getRecommendedAddressObj();
const { tx: completedTx } = await coin.completeChangeToLock(tx, changeLock);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ### Change to a specific address | |
| ```ts | |
| const { script: changeLock } = await signer.getRecommendedAddressObj(); | |
| const { tx: completedTx } = await coin.completeChangeToLock(tx, changeLock); | |
| ``` | |
| ### Change to a specific address (using the `tx` from the Send example above) | |
🤖 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/README.md` around lines 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.
| it("should add no inputs when amountTweak exactly cancels the output requirement", async () => { | ||
| // Output needs 100, but amountTweak is -100 (negative tweak zeroing requirement) | ||
| const tx = ccc.Transaction.from({ | ||
| inputs: [ | ||
| { | ||
| previousOutput: { | ||
| txHash: `0x${"c".repeat(63)}0`, | ||
| index: 0, | ||
| }, | ||
| }, | ||
| ], | ||
| outputs: [{ lock, type }], | ||
| outputsData: [ccc.numLeToBytes(100, 16)], | ||
| }); | ||
|
|
||
| // Already have 1 input (amount 100) matching output exactly → no more needed | ||
| const { addedCount } = await coin.completeInputsByAmount(tx); | ||
| expect(addedCount).toBe(0); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Actually exercise the negative amountTweak.
The existing input already balances the output, and the call omits -100, so this test passes without covering its stated edge case.
Proposed fix
const tx = ccc.Transaction.from({
- inputs: [
- {
- previousOutput: {
- txHash: `0x${"c".repeat(63)}0`,
- index: 0,
- },
- },
- ],
outputs: [{ lock, type }],
outputsData: [ccc.numLeToBytes(100, 16)],
});
-const { addedCount } = await coin.completeInputsByAmount(tx);
+const { addedCount } = await coin.completeInputsByAmount(tx, -100);
expect(addedCount).toBe(0);
+expect(tx.inputs).toHaveLength(0);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| it("should add no inputs when amountTweak exactly cancels the output requirement", async () => { | |
| // Output needs 100, but amountTweak is -100 (negative tweak zeroing requirement) | |
| const tx = ccc.Transaction.from({ | |
| inputs: [ | |
| { | |
| previousOutput: { | |
| txHash: `0x${"c".repeat(63)}0`, | |
| index: 0, | |
| }, | |
| }, | |
| ], | |
| outputs: [{ lock, type }], | |
| outputsData: [ccc.numLeToBytes(100, 16)], | |
| }); | |
| // Already have 1 input (amount 100) matching output exactly → no more needed | |
| const { addedCount } = await coin.completeInputsByAmount(tx); | |
| expect(addedCount).toBe(0); | |
| it("should add no inputs when amountTweak exactly cancels the output requirement", async () => { | |
| // Output needs 100, but amountTweak is -100 (negative tweak zeroing requirement) | |
| const tx = ccc.Transaction.from({ | |
| outputs: [{ lock, type }], | |
| outputsData: [ccc.numLeToBytes(100, 16)], | |
| }); | |
| // Already have 1 input (amount 100) matching output exactly → no more needed | |
| const { addedCount } = await coin.completeInputsByAmount(tx, -100); | |
| expect(addedCount).toBe(0); | |
| expect(tx.inputs).toHaveLength(0); |
🧰 Tools
🪛 ESLint
[error] 1540-1551: Unsafe assignment of an error typed value.
(@typescript-eslint/no-unsafe-assignment)
[error] 1540-1540: Unsafe call of a type that could not be resolved.
(@typescript-eslint/no-unsafe-call)
[error] 1549-1549: Unsafe assignment of an error typed value.
(@typescript-eslint/no-unsafe-assignment)
[error] 1549-1549: Unsafe assignment of an error typed value.
(@typescript-eslint/no-unsafe-assignment)
[error] 1550-1550: Unsafe call of a type that could not be resolved.
(@typescript-eslint/no-unsafe-call)
🤖 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 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.
| static from(infoLike?: CoinInfoLike) { | ||
| if (infoLike instanceof CoinInfo) { | ||
| return infoLike; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Avoid aliasing the input from CoinInfo.from.
Line 50 returns an existing CoinInfo by reference, despite the documentation promising a new instance. CoinInfo.from(existing).addAssign(...) can therefore mutate the caller’s object. Return infoLike.clone() or explicitly document and test the identity-preserving behavior.
🤖 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/coinInfo.ts` around lines 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.
b48efaf to
ebd5a39
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
| // capacity must be enough to cover the enlarged data | ||
| const minCapacity = ccc.CellOutput.from( | ||
| completedTx.outputs[1], | ||
| completedTx.outputsData[1], | ||
| ).capacity; | ||
| expect(completedTx.outputs[1].capacity).toBeGreaterThanOrEqual( | ||
| minCapacity, | ||
| ); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm whether CellOutput.from mutates the instance it is given
ast-grep run --pattern 'class CellOutput $$$' --lang typescript packages/core/src/ckb/transaction.ts 2>/dev/null | head -40
rg -nP -A 25 'static from\(\s*cellOutput' packages/core/src/ckb/transaction.ts | head -60Repository: ckb-devrel/ccc
Length of output: 152
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- transaction.ts outline ---'
ast-grep outline packages/core/src/ckb/transaction.ts --view expanded | sed -n '1,220p'
echo
echo '--- from/clone/search in transaction.ts ---'
rg -n -A 40 -B 10 'static from\(|clone\(' packages/core/src/ckb/transaction.ts
echo
echo '--- coin.test.ts around cited lines ---'
sed -n '700,740p' packages/coin/src/coin/coin.test.ts
echo
sed -n '1040,1115p' packages/coin/src/coin/coin.test.tsRepository: ckb-devrel/ccc
Length of output: 34229
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '--- file presence ---\n'
git ls-files 'packages/core/src/ckb/transaction.ts' 'packages/coin/src/coin/coin.test.ts'
printf '\n--- line counts ---\n'
wc -l packages/core/src/ckb/transaction.ts packages/coin/src/coin/coin.test.ts
printf '\n--- transaction.ts relevant declarations ---\n'
rg -n -A 30 -B 8 'from\(|clone\(|class CellOutput|interface CellOutput|type CellOutput' packages/core/src/ckb/transaction.ts
printf '\n--- coin.test.ts target ranges ---\n'
sed -n '712,732p' packages/coin/src/coin/coin.test.ts
printf '\n---\n'
sed -n '1048,1060p' packages/coin/src/coin/coin.test.ts
printf '\n---\n'
sed -n '1094,1106p' packages/coin/src/coin/coin.test.tsRepository: ckb-devrel/ccc
Length of output: 243
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- all CellOutput.from call sites ---'
rg -n 'CellOutput\.from\(' packages | sed -n '1,200p'
echo
echo '--- search for clone usage near CellOutput ---'
rg -n -A 2 -B 2 'clone\(\)' packages/core packages/coin | sed -n '1,240p'Repository: ckb-devrel/ccc
Length of output: 50373
🌐 Web query:
"ccc CellOutput.from mutates CellOutput.from(output, data) clone capacity ccc"
💡 Result:
In the CCC (CKB Common Collection) library, the CellOutput.from method is a factory function used to create a CellOutput instance [1][2]. It does not mutate the inputs provided to it; instead, it returns a new, properly initialized object based on the provided CellOutputLike configuration and optional output data [3][4]. A key feature of CellOutput.from is its ability to automatically calculate and set the cell's capacity if it is not provided or set to 0 [3][4]. When creating a cell, the capacity is calculated based on the cell's occupied size (which includes the lock script, type script, and the length of the provided output data) [3][4]. It ensures the resulting cell meets the minimum capacity requirements [3][4]. The method does not involve a manual clone operation by the user; rather, it handles the construction and necessary sizing logic internally [3][4]. If you need to produce a copy of an existing script or component (such as a type script) to pass into the factory, you should use the.clone method explicitly on that specific object before passing it into CellOutput.from if you wish to avoid shared references [5][6].
Citations:
- 1: https://github.com/ckb-devrel/ccc/blob/5f2f1d43/packages/core/src/client/jsonRpc/transformers.ts
- 2: https://deepwiki.com/ckb-devrel/ccc/2-core-layer-(`@ckb-ccccore`)
- 3: https://github.com/ckb-devrel/ccc/blob/5f2f1d43/packages/core/src/ckb/transaction.test.ts
- 4: https://github.com/ckb-devrel/ccc/blob/234a5930/packages/core/src/ckb/transaction.test.ts
- 5: https://github.com/ckb-devrel/ccc/blob/234a5930/packages/core/src/ckb/transaction.ts
- 6: https://github.com/ckb-devrel/ccc/blob/5f2f1d43/packages/core/src/ckb/transaction.ts
Use a clone before computing minCapacity. CellOutput.from(completedTx.outputs[1], completedTx.outputsData[1]) reuses the passed CellOutput instance here, so minCapacity can be read after the output has already been raised and the check becomes self-comparing. Clone the output first; the same pattern appears in the other two assertions in this file.
🤖 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 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.
| it("should add outputs correctly", async () => { | ||
| const recipientLock1 = (await signer.getRecommendedAddressObj()).script; | ||
| const recipientLock2 = (await signer.getRecommendedAddressObj()).script; | ||
|
|
||
| const { tx, outputIndexes } = await coin.transfer([ | ||
| { to: recipientLock1, amount: 100n }, | ||
| { to: recipientLock2, amount: 200n }, | ||
| ]); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Both "recipients" are the same lock, so recipient-pairing assertions can't fail.
recipientLock1 and recipientLock2 both come from signer.getRecommendedAddressObj(), making the checks at Lines 1734/1736 insensitive to a swapped or wrong to. Use a distinct second script.
💚 Proposed fix
const recipientLock1 = (await signer.getRecommendedAddressObj()).script;
- const recipientLock2 = (await signer.getRecommendedAddressObj()).script;
+ const recipientLock2 = ccc.Script.from({
+ codeHash: "0x" + "9".repeat(64),
+ hashType: "type",
+ args: "0xdeadbeef",
+ });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| it("should add outputs correctly", async () => { | |
| const recipientLock1 = (await signer.getRecommendedAddressObj()).script; | |
| const recipientLock2 = (await signer.getRecommendedAddressObj()).script; | |
| const { tx, outputIndexes } = await coin.transfer([ | |
| { to: recipientLock1, amount: 100n }, | |
| { to: recipientLock2, amount: 200n }, | |
| ]); | |
| it("should add outputs correctly", async () => { | |
| const recipientLock1 = (await signer.getRecommendedAddressObj()).script; | |
| const recipientLock2 = ccc.Script.from({ | |
| codeHash: "0x" + "9".repeat(64), | |
| hashType: "type", | |
| args: "0xdeadbeef", | |
| }); | |
| const { tx, outputIndexes } = await coin.transfer([ | |
| { to: recipientLock1, amount: 100n }, | |
| { to: recipientLock2, amount: 200n }, | |
| ]); |
🤖 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 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.
| const resolved = (async (): Promise< | ||
| [ccc.ScriptLike, ccc.CellDepLike[]] | ||
| > => { | ||
| if (script.codeHash != null && script.hashType != null) { | ||
| return [script as ccc.ScriptLike, cellDeps ?? []]; | ||
| } | ||
|
|
||
| const scriptInfo = await client.getKnownScript( | ||
| knownScript as ccc.KnownScript, | ||
| ); | ||
| return [ | ||
| { | ||
| codeHash: scriptInfo.codeHash, | ||
| hashType: scriptInfo.hashType, | ||
| args: script.args, | ||
| }, | ||
| (await client.getCellDeps(scriptInfo.cellDeps)).concat( | ||
| cellDeps?.map(ccc.CellDep.from) ?? [], | ||
| ), | ||
| ]; | ||
| })(); | ||
|
|
||
| this.script = resolved.then(([script]) => ccc.Script.from(script)); | ||
| this.cellDeps = resolved.then(([_, cellDeps]) => | ||
| cellDeps.map(ccc.CellDep.from), | ||
| ); | ||
|
|
||
| const scriptRes = this.script; | ||
| this.filter = (async () => { | ||
| return ccc.ClientIndexerSearchKeyFilter.from( | ||
| options.filter ?? { | ||
| script: await scriptRes, | ||
| outputDataLenRange: [16, "0xffffffff"], | ||
| }, | ||
| ); | ||
| })(); | ||
|
|
||
| this.coBuild = Promise.all([this.script]).then( | ||
| ([script]) => new coBuild.CoBuild(script, options.scriptInfo), | ||
| ); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Constructor promise fan-out can surface unhandled rejections.
resolved fans out into four independently-derived promises (script, cellDeps, filter, coBuild). If client.getKnownScript / getCellDeps rejects, every branch a caller does not await stays unhandled — under Node's default --unhandled-rejections=throw that terminates the process even though the caller may have handled the one promise it awaited.
🛡️ Proposed guard
this.coBuild = Promise.all([this.script]).then(
([script]) => new coBuild.CoBuild(script, options.scriptInfo),
);
+
+ // Every field is independently awaitable; mark each 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(() => {});
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const resolved = (async (): Promise< | |
| [ccc.ScriptLike, ccc.CellDepLike[]] | |
| > => { | |
| if (script.codeHash != null && script.hashType != null) { | |
| return [script as ccc.ScriptLike, cellDeps ?? []]; | |
| } | |
| const scriptInfo = await client.getKnownScript( | |
| knownScript as ccc.KnownScript, | |
| ); | |
| return [ | |
| { | |
| codeHash: scriptInfo.codeHash, | |
| hashType: scriptInfo.hashType, | |
| args: script.args, | |
| }, | |
| (await client.getCellDeps(scriptInfo.cellDeps)).concat( | |
| cellDeps?.map(ccc.CellDep.from) ?? [], | |
| ), | |
| ]; | |
| })(); | |
| this.script = resolved.then(([script]) => ccc.Script.from(script)); | |
| this.cellDeps = resolved.then(([_, cellDeps]) => | |
| cellDeps.map(ccc.CellDep.from), | |
| ); | |
| const scriptRes = this.script; | |
| this.filter = (async () => { | |
| return ccc.ClientIndexerSearchKeyFilter.from( | |
| options.filter ?? { | |
| script: await scriptRes, | |
| outputDataLenRange: [16, "0xffffffff"], | |
| }, | |
| ); | |
| })(); | |
| this.coBuild = Promise.all([this.script]).then( | |
| ([script]) => new coBuild.CoBuild(script, options.scriptInfo), | |
| ); | |
| const resolved = (async (): Promise< | |
| [ccc.ScriptLike, ccc.CellDepLike[]] | |
| > => { | |
| if (script.codeHash != null && script.hashType != null) { | |
| return [script as ccc.ScriptLike, cellDeps ?? []]; | |
| } | |
| const scriptInfo = await client.getKnownScript( | |
| knownScript as ccc.KnownScript, | |
| ); | |
| return [ | |
| { | |
| codeHash: scriptInfo.codeHash, | |
| hashType: scriptInfo.hashType, | |
| args: script.args, | |
| }, | |
| (await client.getCellDeps(scriptInfo.cellDeps)).concat( | |
| cellDeps?.map(ccc.CellDep.from) ?? [], | |
| ), | |
| ]; | |
| })(); | |
| this.script = resolved.then(([script]) => ccc.Script.from(script)); | |
| this.cellDeps = resolved.then(([_, cellDeps]) => | |
| cellDeps.map(ccc.CellDep.from), | |
| ); | |
| const scriptRes = this.script; | |
| this.filter = (async () => { | |
| return ccc.ClientIndexerSearchKeyFilter.from( | |
| options.filter ?? { | |
| script: await scriptRes, | |
| outputDataLenRange: [16, "0xffffffff"], | |
| }, | |
| ); | |
| })(); | |
| this.coBuild = Promise.all([this.script]).then( | |
| ([script]) => new coBuild.CoBuild(script, options.scriptInfo), | |
| ); | |
| // Every field is independently awaitable; mark each 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(() => {}); | |
| } |
🧰 Tools
🪛 ESLint
[error] 183-183: Unsafe construction of a type that could not be resolved.
(@typescript-eslint/no-unsafe-call)
[error] 183-183: Unsafe return of a value of type error.
(@typescript-eslint/no-unsafe-return)
🤖 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 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.
| cell.outputData = ccc.hexFrom( | ||
| ccc.bytesConcat( | ||
| ccc.numLeToBytes(normalizedAmount, 16), | ||
| ccc.bytesFrom(cell.outputData).slice(16), | ||
| ), | ||
| ); | ||
|
|
||
| return this.transformOutput(cell); |
