feat: rewrite spore-sdk in ccc #43
Conversation
🦋 Changeset detectedLatest commit: 4afc10a The changes in this PR will be included in the next version bump. This PR includes changesets to release 14 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 |
ashuralyk
changed the title
packages/spore/src/advanced.ts
Outdated
| signer: ccc.Signer, | ||
| tx: ccc.TransactionLike, | ||
| cobuildActions: UnpackResult<typeof ActionVec>, | ||
| feeRate: number = 1000, |
There was a problem hiding this comment.
The feeRate should not be set by default. Developers / Users should set this dynamically to avoid on-chain attackers.
There was a problem hiding this comment.
ok, I'll leave comment to tell users 1000 is the minimum fee rate
packages/spore/src/advanced.ts
Outdated
| import { ccc } from "@ckb-ccc/core"; | ||
| import { bytes, UnpackResult } from "@ckb-lumos/codec"; | ||
| import { Action, ActionVec, SporeAction, WitnessLayout } from "./codec"; | ||
| import { buildProtoclScript, SporeScriptInfo } from "./predefined"; |
There was a problem hiding this comment.
For commonjs compatibility, please add .js extension at the end of files path.
packages/spore/src/advanced.ts
Outdated
| cobuildActions: UnpackResult<typeof ActionVec>, | ||
| feeRate: number = 1000, | ||
| ): Promise<ccc.Transaction> { | ||
| if (feeRate < 1000) { |
There was a problem hiding this comment.
This check should be done by the node / UI.
packages/spore/src/advanced.ts
Outdated
|
|
||
| // change cell | ||
| let txSkeleton = ccc.Transaction.from(tx); | ||
| txSkeleton.addOutput({ lock }); |
There was a problem hiding this comment.
It's not necessary to add a change cell here. we can use completeFeeBy.
packages/spore/src/advanced.ts
Outdated
| sporeOutput: ccc.CellOutputLike, | ||
| sporeData: ccc.BytesLike, | ||
| ): UnpackResult<typeof Action> { | ||
| const sporeType = ccc.Script.from(sporeOutput.type!); |
There was a problem hiding this comment.
Please check if the type is null here instead of ignoring it. (And also below)
packages/spore/src/helper.ts
Outdated
| ...liveCell, | ||
| }), | ||
| ); | ||
| txSkeleton.witnesses.push("0x"); |
There was a problem hiding this comment.
Is it possible to use Signer.prepareTransaction after collecting all inputs? Extra witnesses don't look necessary.
| tx: ccc.TransactionLike, | ||
| outputIndex: number, | ||
| ): ccc.Hex { | ||
| const firstInput = tx.inputs ? tx.inputs[0] : void 0; |
There was a problem hiding this comment.
inputs is nullable in TransactionLike
packages/spore/src/predefined.ts
Outdated
| args, | ||
| ...scriptInfo[procotol], | ||
| }); | ||
| } else { |
There was a problem hiding this comment.
else here is not needed because we returned before.
packages/spore/vitest.config.mts
Outdated
| @@ -0,0 +1,17 @@ | |||
| import { defineConfig } from "vitest/config"; | |||
There was a problem hiding this comment.
The test framework for CCC is jest. CCC is also built for backend developers.
pnpm-lock.yaml
Outdated
| @@ -1,4 +1,4 @@ | |||
| lockfileVersion: '9.0' | |||
| lockfileVersion: '6.0' | |||
There was a problem hiding this comment.
Please upgrade your pnpm version.
|
@Hanssen0 I've solved all reported issues in comment, new commit has been committed, please repeat your review |
Description
spore-sdkin ccc, only contain features for spore and clustertype-burnandlock-proxyas known scripts