Move app runtime flows onto SDK boundaries#33
Conversation
There was a problem hiding this comment.
Code Review
This pull request refactors the workspace to introduce a new @ickb/node-utils package and centralizes protocol planning logic within the SDK. The browser interface is updated to support small-balance "dust" conversions with dedicated UI notices, and query stability is improved through a new object identity tracking mechanism. Additionally, the sampler and tester apps were refactored for better testability and standardized logging. Feedback identifies potential issues with script deduplication in both the interface and node utilities, which could lead to redundant RPC calls, and notes that pretty-printing in the logging utility would break the required NDJSON format.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request refactors the monorepo by introducing the @ickb/node-utils package and updating the interface, sampler, and tester applications to utilize improved SDK integrations and shared utilities. Key enhancements include the implementation of dust conversion notices in the UI, more granular query key management for state invalidation, and standardized logging and error handling for Node-based apps. Feedback was provided to ensure that ccc.Script deduplication logic uses value-based comparisons (e.g., hex representations) rather than object identity, as scripts are frequently recreated as new instances.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request refactors the workspace by introducing a new @ickb/node-utils package for shared Node application logic and delegating protocol planning from the browser interface to the SDK. Key updates include improved state tracking and query key generation in the interface, support for small-balance iCKB conversions with inline notices, and enhanced sampling and testing utilities. Feedback was provided regarding the use of Math.pow for block numbers in the sampler app, which could lead to precision loss; using bigint arithmetic was suggested as a safer alternative to handle blockchain-scale values.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a new @ickb/node-utils package to centralize shared logic for Node-based applications and refactors the interface, sampler, and tester apps to utilize it. Key changes include improved transaction preview logic in the interface, better handling of small iCKB balances, and enhanced state tracking via unique query keys and state IDs. The documentation has been updated with a workspace map and specific app details. I have no feedback to provide.
|
LGTM Phroi %223 |
Why
Changes
@ickb/node-utilsand make sampler import-safe and testable with large-height-safe sampling.