Adopt core protocol libraries#60
Conversation
|
Warning Gemini encountered an error creating the review. You can try again by commenting |
|
/gemini review |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a major refactoring across the @ickb monorepo packages, including core, dao, order, and sdk. Key updates include transitioning relative imports to .ts extensions, adding api-extractor configurations, updating package scripts, and introducing comprehensive JSDoc documentation. Additionally, the IckbUdt class has been refactored to extend ccc.Udt with custom completion logic, and Nervos DAO output limit checks have been streamlined, removing redundant mocks and assertions from the test suites. There are no review comments to address, and the changes appear to be a clean and thorough modernization of the codebase.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
CCC cleanup note: this branch now removes the custom local CCC fork workflow ( |
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request refactors the workspace to simplify dependencies by resolving CCC packages directly through the lockfile, removing the local CCC fork build step, and updating import paths to use .ts extensions. It restructures and modularizes the core, dao, and order packages, introducing API Extractor configurations, robust transaction-shaping helpers, and comprehensive test suites. The review feedback highlights several opportunities to improve debuggability by replacing vague error messages with descriptive, context-rich errors that include transaction hashes, block numbers, or outpoints. Additionally, a critical issue was identified in packages/core/src/udt.ts where a missing input cell could lead to a runtime crash during transaction completion.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
/gemini review |
|
Warning Gemini encountered an error creating the review. You can try again by commenting |
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request simplifies the workspace by removing the local CCC fork build workflow and resolving CCC packages as standard package dependencies. It also refactors and reorganizes the core, dao, and order packages, integrating api-extractor and updating build scripts. Additionally, the matching logic in the order package has been modularized, and IckbUdt in the core package has been refactored to handle transaction completion directly. Feedback on the changes includes a recommendation to use a TypeScript type guard in order_match_sequence.ts to properly narrow the type of filtered order matchers.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request simplifies the workspace setup by resolving CCC packages as normal package dependencies, removing the need for a local CCC fork build step. It also refactors the core, dao, and order packages by migrating relative imports to .ts extensions, adding comprehensive JSDoc documentation, extracting internal helper functions, and reorganizing test suites into dedicated test directories. Feedback on the changes suggests avoiding optional chaining when retrieving elements from internal collections using a previously validated index in packages/core/src/owned_owner.ts, recommending throwing an explicit error instead to make invariant breaks visible.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
/gemini review |
|
Warning Gemini encountered an error creating the review. You can try again by commenting |
|
/gemini review |
|
Warning Gemini encountered an error creating the review. You can try again by commenting |
phroi
force-pushed
the
review/core-protocol-libraries
branch
from
811bbd1 to
23ab1d4
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request removes the local CCC fork build workflow, transitioning the workspace to resolve CCC packages as standard package dependencies via pnpm catalogs and the lockfile. It also migrates packages/core, packages/dao, and packages/order to use tsgo and api-extractor for building and API linting, updates relative imports to use .ts extensions, and restructures tests into dedicated directories. Additionally, the codebase is refactored to introduce stricter validation for Nervos DAO cells, receipt payloads, and limit-order matching. As there were no review comments provided, I have no further feedback to offer.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
LGTM Phroi %355 |
1 participant
Why
Continue the migration after the shared utility packages by moving the core protocol surfaces into their package-ready shape before broader SDK, app, and validation follow-ups.
This also removes the custom local CCC fork workflow so the stack builds and tests against mainline published
@ckb-ccc/*packages from the workspace catalog and lockfile.Changes
@ickb/core,@ickb/dao, and@ickb/orderaround explicit package entry points, build configs, API Extractor configs, and package-local tests.forks:ccc, localforks/ccc/repo, fork smoke/override checks, and deploy/supervisor CCC build gates).@ckb-ccc/*catalog versions for the migrated protocol packages.