feat: add Fiber (FNN) support to the local devnet - #483
Open
humble-little-bear wants to merge 2 commits into
Open
feat: add Fiber (FNN) support to the local devnet#483humble-little-bear wants to merge 2 commits into
humble-little-bear wants to merge 2 commits into
Conversation
Add a local Fiber development environment to offckb: - Genesis: the devnet now carries the Fiber contracts auth, funding_lock and commitment_lock, copied from the new ckb/fiber submodule pinned to the FNN v0.9.0-rc7 commit (bc361aa). They are appended after the existing system cells so existing script type ids (accounts, sudt, xudt, ...) stay unchanged; the genesis tx hash changes, so cell dep out points are always read from a fresh `ckb list-hashes` at start. - FNN install: download/cache of the tested FNN release (0.9.0-rc7, portable tarballs), keeping the bundled config/testnet/config.yml as the devnet config template; --binary-path/--fnn-binary-path run a locally built FNN, using its sibling testnet config or the shipped fallback. - offckb node --fiber: start CKB, miner, RPC proxy and FNN nodes with one command (daemon mode included). Plain local devnet only: mainnet/testnet and forked devnets (any fork.json) are rejected before any daemon respawn. - offckb fiber start/stop/status/logs/clean: manage FNNs on an already-running devnet. Node N uses built-in account N+2, RPC port 21713+N and P2P port 8343+N (1-16 nodes). Each node writes only to its own fnn.log; per-node FNN config overrides live in fiber/nodes.yml; config.yml is regenerated every start from the FNN testnet config with unknown fields preserved. - Startup checks: genesis hash agreement between list-hashes, CKB RPC and every FNN node_info, node identity key vs fiber/sk, funding account vs the expected built-in account, and available balance; then node 1 connects to the other nodes (verified via list_peers). - Process management: a shared .offckb-devnet.lock, runtime.json for manager/node records, daemon PID files with identity checks, stop only ever signals recorded managers (never per-FNN kills), store LOCK verification before cleans; offckb clean removes fiber stores with --data and refuses while a daemon or live store lock is confirmed. - fiber status reports CKB and per-node state (starting/running/stopped/unknown/conflict) plus an OFFCKB-managed column, as a table or --json. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 12 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 selected for processing (16)
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdded Fiber support for local devnets. The changes add Fiber contracts and configuration, FNN installation and lifecycle management, CLI commands, node integration, cleanup safeguards, validation, tests, and documentation. ChangesFiber devnet integration
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant fiberStart
participant resolveFnnBinary
participant startFiberEnvironment
participant FNN
participant CKB
fiberStart->>resolveFnnBinary: resolve binary and testnet configuration
fiberStart->>startFiberEnvironment: start configured Fiber nodes
startFiberEnvironment->>FNN: spawn node processes
FNN->>CKB: query chain and account state
startFiberEnvironment->>FNN: connect peers and verify readiness
startFiberEnvironment-->>fiberStart: return running Fiber environment
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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: 12
🧹 Nitpick comments (7)
src/fiber/scripts.ts (1)
91-93: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
SystemScriptNameinstead of the double cast and string literals.Line 91 erases the typed record with
as unknown as Record<string, SystemScript | undefined>, and line 93 repeats the script names as bare strings. A rename of an enum member then compiles but fails at runtime. Index the typed record withSystemScriptNamemembers.♻️ Suggested refactor
- const scripts = resolved.scripts as unknown as Record<string, SystemScript | undefined>; - - const missing = ['auth', 'funding_lock', 'commitment_lock'].filter((name) => scripts[name] == null); + const scripts = resolved.scripts; + + const required = [SystemScriptName.auth, SystemScriptName.funding_lock, SystemScriptName.commitment_lock]; + const missing = required.filter((name) => scripts[name] == null);Import
SystemScriptNamealongsideSystemScripton line 2.🤖 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 `@src/fiber/scripts.ts` around lines 91 - 93, Update the script lookup around resolved.scripts to import and use SystemScriptName alongside SystemScript, removing the double cast and replacing the string literals in the missing filter with the corresponding SystemScriptName members. Index the typed scripts record with those enum values so renames remain compile-safe.Makefile (1)
56-62: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winFail with a clear message when the
ckb/fibersubmodule is absent.The target copies files from
ckb/fiber, which is a submodule. If a user clones without--recurse-submodules,cpfails with a bare "No such file or directory". Add an explicit check or initialize the submodule first.♻️ Suggested guard
fiber: `@echo` "Copying Fiber contracts via submodule" + `@test` -d ckb/fiber/tests/deploy/contracts || \ + (echo "ckb/fiber submodule is missing. Run: git submodule update --init ckb/fiber" && exit 1) mkdir -p ckb/devnet/specs/fiber🤖 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 `@Makefile` around lines 56 - 62, Update the fiber target to detect whether the ckb/fiber submodule is available before creating the destination or copying files. If it is absent, fail immediately with a clear message explaining that the submodule must be initialized, while preserving the existing copy behavior when present.ckb/devnet/specs/fiber/auth (1)
1-1: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConfirm and document the committed Fiber contract binaries.
make fibercopiesauth,funding_lock, andcommitment_lockfromckb/fiber, but these files are tracked whileckb/devnet/specsis not ignored. Add a small comment near theMakefile.fibertarget with the pinned Fiber commit/Fnn revision if the committed copies are intended for distribution or offline use.🤖 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 `@ckb/devnet/specs/fiber/auth` at line 1, Document the committed Fiber contract binaries near the Makefile.fiber target by adding a concise comment containing the pinned Fiber commit or Fnn revision. State that auth, funding_lock, and commitment_lock are intentionally tracked for distribution or offline use, and preserve the existing build behavior.src/fiber/install.ts (1)
84-117: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the downloaded tarball after extraction.
tempFilePathstays inos.tmpdir()after the install completes. Each install or reinstall leaves another archive behind. Delete it in afinallyblock so a failed extraction also cleans up.♻️ Proposed cleanup
logger.info(`downloading ${downloadURL} ..`); const response = await Request.send(downloadURL); const arrayBuffer = await response.arrayBuffer(); fs.writeFileSync(tempFilePath, Buffer.from(arrayBuffer)); - const extractDir = path.join(settings.bins.downloadPath, `fnn_v${version}`); - fs.rmSync(extractDir, { recursive: true, force: true }); - await unZipFile(tempFilePath, extractDir, true); + const extractDir = path.join(settings.bins.downloadPath, `fnn_v${version}`); + try { + fs.rmSync(extractDir, { recursive: true, force: true }); + await unZipFile(tempFilePath, extractDir, true); + } finally { + fs.rmSync(tempFilePath, { force: true }); + }🤖 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 `@src/fiber/install.ts` around lines 84 - 117, Update downloadFnnAndUnzip to remove tempFilePath in a finally block surrounding the download, extraction, and installation workflow, ensuring the temporary tarball is deleted on both success and failure while preserving existing error propagation.src/fiber/status.ts (1)
66-76: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider sharing the funding-lock comparison helper.
lockMatchesduplicates the inline lock comparison insrc/fiber/manager.ts(lines 213-217). Both comparecode_hash,hash_type, andargscase-insensitively againstaccount.lockScript. Export one helper and use it in both places, so a future change to the comparison rules cannot diverge.🤖 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 `@src/fiber/status.ts` around lines 66 - 76, Export the lockMatches helper from status.ts and replace the duplicate inline comparison in the manager.ts account.lockScript validation with calls to this shared helper. Preserve the existing case-insensitive comparisons of code_hash, hash_type, and args, including handling an undefined actual lock.src/cmd/clean.ts (2)
16-24: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winThe node daemon PID path is rebuilt from literals in three files. Each site joins
settings.devnet.dataPath,'logs', and'daemon.pid'independently, whilesrc/cmd/node.tsalready ownsresolveDaemonPathswith theDAEMON_LOG_DIRandDAEMON_PID_FILEconstants. A change to that layout breaks each stop and clean safety check silently. Export one path helper (for examplenodeDaemonPaths(settings)insrc/util/daemon.ts) and use it at every site.
src/cmd/clean.ts#L16-L24: replace the inlinepath.joininassertCkbDaemonStoppedwith the shared helper.src/fiber/status.ts#L59-L62: replace the inlinepath.join(settings.devnet.dataPath, 'logs', 'daemon.pid')inresolveOffckbManagedwith the shared helper.src/fiber/daemon.ts#L308-L311: deleteresolveNodeDaemonPathsand call the shared helper.🤖 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 `@src/cmd/clean.ts` around lines 16 - 24, The daemon PID path is duplicated across three sites instead of using shared path definitions. Export a shared helper such as nodeDaemonPaths(settings) from src/util/daemon.ts, then update assertCkbDaemonStopped in src/cmd/clean.ts and resolveOffckbManaged in src/fiber/status.ts to use it; remove resolveNodeDaemonPaths from src/fiber/daemon.ts and call the shared helper there, preserving the existing DAEMON_LOG_DIR and DAEMON_PID_FILE layout.
26-34: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReuse one Fiber node enumeration helper.
fiberStoreDirsrepeats the pattern insrc/fiber/clean.ts: read<fiber>/nodes, keep entries that match/^\d+$/, then map to a per-node path.existingStoreLockFiles(Lines 18-26) and the store listing infiberClean(Lines 79-85) do the same. Export onefiberNodeIds(settings)helper fromsrc/fiber/paths.tsand map the wanted path in each caller.♻️ Proposed helper
// src/fiber/paths.ts export function fiberNodeIds(settings: Settings = readSettings()): number[] { const nodesDir = path.join(fiberRootPath(settings), 'nodes'); if (!isFolderExists(nodesDir)) return []; return fs .readdirSync(nodesDir) .filter((entry) => /^\d+$/.test(entry)) .map((entry) => Number(entry)); }function fiberStoreDirs(settings: ReturnType<typeof readSettings>): string[] { - const nodesDir = path.join(fiberRootPath(settings), 'nodes'); - if (!isFolderExists(nodesDir)) return []; - return fs - .readdirSync(nodesDir) - .filter((entry) => /^\d+$/.test(entry)) - .map((entry) => fiberNodePaths(Number(entry), settings).fiberStoreDir) + return fiberNodeIds(settings) + .map((id) => fiberNodePaths(id, settings).fiberStoreDir) .filter((storeDir) => isFolderExists(storeDir)); }🤖 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 `@src/cmd/clean.ts` around lines 26 - 34, Centralize Fiber node enumeration by adding and exporting fiberNodeIds in src/fiber/paths.ts, preserving the existing missing-directory, numeric-entry filtering, and number-conversion behavior. Update fiberStoreDirs, existingStoreLockFiles, and the store listing in fiberClean to call fiberNodeIds(settings) and map each ID to the required per-node path instead of reading and filtering the nodes directory themselves.
🤖 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 `@ckb/devnet/specs/fiber/commitment_lock`:
- Line 1: Rebuild the contract binaries without embedded developer filesystem
paths by applying Rust path remapping or replacing them with the upstream
release artifacts. Update ckb/devnet/specs/fiber/commitment_lock at lines 1-1 to
remove the specified /Users/quake paths, and ckb/devnet/specs/fiber/funding_lock
at lines 1-1 to remove the specified /home/quake paths.
In `@ckb/fiber`:
- Line 1: Update the ckb/fiber submodule reference from unreachable commit
bc361aaaa40d1394b83e6a1808869b0b06c48c13 to an accessible Fiber commit, or
publish that pinned commit while preserving the required tests/deploy/contracts
and config/testnet assets used by the Fiber target.
In `@src/cmd/fiber.ts`:
- Around line 36-43: Update printFiberSummary to use the shared
fiberAccountIndex(node.id) helper when formatting each node’s account number,
replacing the inline node.id + 2 calculation and keeping the existing summary
output unchanged otherwise.
In `@src/cmd/node.ts`:
- Around line 712-726: Update waitForFiberRuntimeRunning to use a 10-minute
timeout, matching FIBER_DAEMON_READY_TIMEOUT_MS in the Fiber daemon, so
first-run FNN downloads can complete without terminating the child process.
Preserve the existing polling and timeout error behavior.
In `@src/fiber/config-gen.ts`:
- Around line 87-98: Add ckb.udt_whitelist to MANAGED_CONFIG_PATHS in
nodes-yml.ts so mergeNodeConfig preserves the resolved
options.chainScripts.udtWhitelist value and prevents per-node configuration from
overriding it.
In `@src/fiber/daemon.ts`:
- Around line 190-195: Move the storeLockFilesForRuntime call before
terminateProcess in the shutdown flow, capturing the lock-file list while
runtime.json still exists; then reuse that captured lockFiles value for
waitForStoreLocksReleased and the existing warning without changing the
termination or wait behavior.
In `@src/fiber/manager.ts`:
- Around line 376-385: Update waitForChildExit and the liveness checks near the
existing exitCode conditions to also treat a non-null signalCode as exited.
Preserve the current immediate-return behavior for normally exited children and
ensure signal-terminated children do not wait for the timeout.
- Around line 313-325: Update the FNN spawning flow around spawnFnn to collect
handles incrementally instead of using an all-or-nothing nodes.map call. If a
later spawn throws, stop every previously collected handle before propagating
the original failure; only construct and write the FiberRuntime after all nodes
spawn successfully.
In `@src/fiber/nodes-yml.ts`:
- Around line 138-145: Update the warning inside the removed-node loop to
explicitly state that each node’s hand-written config overrides are also
discarded when its nodes.yml entry is removed, while preserving the existing
directory-retention and cleanup guidance.
In `@src/fiber/scripts.ts`:
- Around line 98-102: Update the script validation around requireScript in the
script-loading flow to cover sudt and xudt as well as auth, funding_lock, and
commitment_lock, or otherwise ensure their failures use the same full contextual
error message instead of raw missing:<name> output. Keep the existing
required-script behavior unchanged.
In `@src/fiber/store-lock.ts`:
- Around line 37-45: Update the error handling in the lock inspection function
around the `execFileSync` catch block to return `null` when the `lsof` process
is terminated by the timeout signal or exits with any status other than 1. Only
interpret stdout for the no-holder status 1 case; preserve the existing `ENOENT`
handling and ensure inspection failures cannot return `false` to the cleanup
flow.
In `@src/util/daemon.ts`:
- Around line 158-180: Update getProcessCommandLine’s Windows command selection
to use PowerShell with Get-CimInstance Win32_Process, or fall back to it when
the WMIC invocation is unavailable, while preserving the existing PID filtering
and command-line parsing. Ensure Windows systems without WMIC still resolve the
managed process command line so verifyDaemonIdentity can proceed.
---
Nitpick comments:
In `@ckb/devnet/specs/fiber/auth`:
- Line 1: Document the committed Fiber contract binaries near the Makefile.fiber
target by adding a concise comment containing the pinned Fiber commit or Fnn
revision. State that auth, funding_lock, and commitment_lock are intentionally
tracked for distribution or offline use, and preserve the existing build
behavior.
In `@Makefile`:
- Around line 56-62: Update the fiber target to detect whether the ckb/fiber
submodule is available before creating the destination or copying files. If it
is absent, fail immediately with a clear message explaining that the submodule
must be initialized, while preserving the existing copy behavior when present.
In `@src/cmd/clean.ts`:
- Around line 16-24: The daemon PID path is duplicated across three sites
instead of using shared path definitions. Export a shared helper such as
nodeDaemonPaths(settings) from src/util/daemon.ts, then update
assertCkbDaemonStopped in src/cmd/clean.ts and resolveOffckbManaged in
src/fiber/status.ts to use it; remove resolveNodeDaemonPaths from
src/fiber/daemon.ts and call the shared helper there, preserving the existing
DAEMON_LOG_DIR and DAEMON_PID_FILE layout.
- Around line 26-34: Centralize Fiber node enumeration by adding and exporting
fiberNodeIds in src/fiber/paths.ts, preserving the existing missing-directory,
numeric-entry filtering, and number-conversion behavior. Update fiberStoreDirs,
existingStoreLockFiles, and the store listing in fiberClean to call
fiberNodeIds(settings) and map each ID to the required per-node path instead of
reading and filtering the nodes directory themselves.
In `@src/fiber/install.ts`:
- Around line 84-117: Update downloadFnnAndUnzip to remove tempFilePath in a
finally block surrounding the download, extraction, and installation workflow,
ensuring the temporary tarball is deleted on both success and failure while
preserving existing error propagation.
In `@src/fiber/scripts.ts`:
- Around line 91-93: Update the script lookup around resolved.scripts to import
and use SystemScriptName alongside SystemScript, removing the double cast and
replacing the string literals in the missing filter with the corresponding
SystemScriptName members. Index the typed scripts record with those enum values
so renames remain compile-safe.
In `@src/fiber/status.ts`:
- Around line 66-76: Export the lockMatches helper from status.ts and replace
the duplicate inline comparison in the manager.ts account.lockScript validation
with calls to this shared helper. Preserve the existing case-insensitive
comparisons of code_hash, hash_type, and args, including handling an undefined
actual lock.
🪄 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: 7e83b12b-a6a7-4f88-bb8a-d562bb5ae816
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (41)
.changeset/fiber-devnet.md.gitmodulesMakefileREADME.mdckb/devnet/specs/dev.tomlckb/devnet/specs/fiber/authckb/devnet/specs/fiber/commitment_lockckb/devnet/specs/fiber/funding_lockckb/devnet/specs/fiber/testnet-config.ymlckb/fiberpackage.jsonsrc/cfg/setting.tssrc/cli.tssrc/cmd/clean.tssrc/cmd/config.tssrc/cmd/fiber.tssrc/cmd/node.tssrc/fiber/accounts.tssrc/fiber/ckb-env.tssrc/fiber/clean.tssrc/fiber/config-gen.tssrc/fiber/daemon.tssrc/fiber/env-lock.tssrc/fiber/install.tssrc/fiber/manager.tssrc/fiber/nodes-yml.tssrc/fiber/paths.tssrc/fiber/rpc.tssrc/fiber/runtime.tssrc/fiber/scripts.tssrc/fiber/status.tssrc/fiber/store-lock.tssrc/scripts/public.tssrc/scripts/type.tssrc/util/daemon.tstests/fiber-accounts.test.tstests/fiber-config-gen.test.tstests/fiber-env-lock.test.tstests/fiber-nodes-yml.test.tstests/fiber-scripts.test.tstests/node-command.test.ts
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 12
🧹 Nitpick comments (7)
src/fiber/scripts.ts (1)
91-93: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
SystemScriptNameinstead of the double cast and string literals.Line 91 erases the typed record with
as unknown as Record<string, SystemScript | undefined>, and line 93 repeats the script names as bare strings. A rename of an enum member then compiles but fails at runtime. Index the typed record withSystemScriptNamemembers.♻️ Suggested refactor
- const scripts = resolved.scripts as unknown as Record<string, SystemScript | undefined>; - - const missing = ['auth', 'funding_lock', 'commitment_lock'].filter((name) => scripts[name] == null); + const scripts = resolved.scripts; + + const required = [SystemScriptName.auth, SystemScriptName.funding_lock, SystemScriptName.commitment_lock]; + const missing = required.filter((name) => scripts[name] == null);Import
SystemScriptNamealongsideSystemScripton line 2.🤖 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 `@src/fiber/scripts.ts` around lines 91 - 93, Update the script lookup around resolved.scripts to import and use SystemScriptName alongside SystemScript, removing the double cast and replacing the string literals in the missing filter with the corresponding SystemScriptName members. Index the typed scripts record with those enum values so renames remain compile-safe.Makefile (1)
56-62: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winFail with a clear message when the
ckb/fibersubmodule is absent.The target copies files from
ckb/fiber, which is a submodule. If a user clones without--recurse-submodules,cpfails with a bare "No such file or directory". Add an explicit check or initialize the submodule first.♻️ Suggested guard
fiber: `@echo` "Copying Fiber contracts via submodule" + `@test` -d ckb/fiber/tests/deploy/contracts || \ + (echo "ckb/fiber submodule is missing. Run: git submodule update --init ckb/fiber" && exit 1) mkdir -p ckb/devnet/specs/fiber🤖 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 `@Makefile` around lines 56 - 62, Update the fiber target to detect whether the ckb/fiber submodule is available before creating the destination or copying files. If it is absent, fail immediately with a clear message explaining that the submodule must be initialized, while preserving the existing copy behavior when present.ckb/devnet/specs/fiber/auth (1)
1-1: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConfirm and document the committed Fiber contract binaries.
make fibercopiesauth,funding_lock, andcommitment_lockfromckb/fiber, but these files are tracked whileckb/devnet/specsis not ignored. Add a small comment near theMakefile.fibertarget with the pinned Fiber commit/Fnn revision if the committed copies are intended for distribution or offline use.🤖 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 `@ckb/devnet/specs/fiber/auth` at line 1, Document the committed Fiber contract binaries near the Makefile.fiber target by adding a concise comment containing the pinned Fiber commit or Fnn revision. State that auth, funding_lock, and commitment_lock are intentionally tracked for distribution or offline use, and preserve the existing build behavior.src/fiber/install.ts (1)
84-117: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the downloaded tarball after extraction.
tempFilePathstays inos.tmpdir()after the install completes. Each install or reinstall leaves another archive behind. Delete it in afinallyblock so a failed extraction also cleans up.♻️ Proposed cleanup
logger.info(`downloading ${downloadURL} ..`); const response = await Request.send(downloadURL); const arrayBuffer = await response.arrayBuffer(); fs.writeFileSync(tempFilePath, Buffer.from(arrayBuffer)); - const extractDir = path.join(settings.bins.downloadPath, `fnn_v${version}`); - fs.rmSync(extractDir, { recursive: true, force: true }); - await unZipFile(tempFilePath, extractDir, true); + const extractDir = path.join(settings.bins.downloadPath, `fnn_v${version}`); + try { + fs.rmSync(extractDir, { recursive: true, force: true }); + await unZipFile(tempFilePath, extractDir, true); + } finally { + fs.rmSync(tempFilePath, { force: true }); + }🤖 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 `@src/fiber/install.ts` around lines 84 - 117, Update downloadFnnAndUnzip to remove tempFilePath in a finally block surrounding the download, extraction, and installation workflow, ensuring the temporary tarball is deleted on both success and failure while preserving existing error propagation.src/fiber/status.ts (1)
66-76: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider sharing the funding-lock comparison helper.
lockMatchesduplicates the inline lock comparison insrc/fiber/manager.ts(lines 213-217). Both comparecode_hash,hash_type, andargscase-insensitively againstaccount.lockScript. Export one helper and use it in both places, so a future change to the comparison rules cannot diverge.🤖 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 `@src/fiber/status.ts` around lines 66 - 76, Export the lockMatches helper from status.ts and replace the duplicate inline comparison in the manager.ts account.lockScript validation with calls to this shared helper. Preserve the existing case-insensitive comparisons of code_hash, hash_type, and args, including handling an undefined actual lock.src/cmd/clean.ts (2)
16-24: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winThe node daemon PID path is rebuilt from literals in three files. Each site joins
settings.devnet.dataPath,'logs', and'daemon.pid'independently, whilesrc/cmd/node.tsalready ownsresolveDaemonPathswith theDAEMON_LOG_DIRandDAEMON_PID_FILEconstants. A change to that layout breaks each stop and clean safety check silently. Export one path helper (for examplenodeDaemonPaths(settings)insrc/util/daemon.ts) and use it at every site.
src/cmd/clean.ts#L16-L24: replace the inlinepath.joininassertCkbDaemonStoppedwith the shared helper.src/fiber/status.ts#L59-L62: replace the inlinepath.join(settings.devnet.dataPath, 'logs', 'daemon.pid')inresolveOffckbManagedwith the shared helper.src/fiber/daemon.ts#L308-L311: deleteresolveNodeDaemonPathsand call the shared helper.🤖 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 `@src/cmd/clean.ts` around lines 16 - 24, The daemon PID path is duplicated across three sites instead of using shared path definitions. Export a shared helper such as nodeDaemonPaths(settings) from src/util/daemon.ts, then update assertCkbDaemonStopped in src/cmd/clean.ts and resolveOffckbManaged in src/fiber/status.ts to use it; remove resolveNodeDaemonPaths from src/fiber/daemon.ts and call the shared helper there, preserving the existing DAEMON_LOG_DIR and DAEMON_PID_FILE layout.
26-34: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReuse one Fiber node enumeration helper.
fiberStoreDirsrepeats the pattern insrc/fiber/clean.ts: read<fiber>/nodes, keep entries that match/^\d+$/, then map to a per-node path.existingStoreLockFiles(Lines 18-26) and the store listing infiberClean(Lines 79-85) do the same. Export onefiberNodeIds(settings)helper fromsrc/fiber/paths.tsand map the wanted path in each caller.♻️ Proposed helper
// src/fiber/paths.ts export function fiberNodeIds(settings: Settings = readSettings()): number[] { const nodesDir = path.join(fiberRootPath(settings), 'nodes'); if (!isFolderExists(nodesDir)) return []; return fs .readdirSync(nodesDir) .filter((entry) => /^\d+$/.test(entry)) .map((entry) => Number(entry)); }function fiberStoreDirs(settings: ReturnType<typeof readSettings>): string[] { - const nodesDir = path.join(fiberRootPath(settings), 'nodes'); - if (!isFolderExists(nodesDir)) return []; - return fs - .readdirSync(nodesDir) - .filter((entry) => /^\d+$/.test(entry)) - .map((entry) => fiberNodePaths(Number(entry), settings).fiberStoreDir) + return fiberNodeIds(settings) + .map((id) => fiberNodePaths(id, settings).fiberStoreDir) .filter((storeDir) => isFolderExists(storeDir)); }🤖 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 `@src/cmd/clean.ts` around lines 26 - 34, Centralize Fiber node enumeration by adding and exporting fiberNodeIds in src/fiber/paths.ts, preserving the existing missing-directory, numeric-entry filtering, and number-conversion behavior. Update fiberStoreDirs, existingStoreLockFiles, and the store listing in fiberClean to call fiberNodeIds(settings) and map each ID to the required per-node path instead of reading and filtering the nodes directory themselves.
🤖 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 `@ckb/devnet/specs/fiber/commitment_lock`:
- Line 1: Rebuild the contract binaries without embedded developer filesystem
paths by applying Rust path remapping or replacing them with the upstream
release artifacts. Update ckb/devnet/specs/fiber/commitment_lock at lines 1-1 to
remove the specified /Users/quake paths, and ckb/devnet/specs/fiber/funding_lock
at lines 1-1 to remove the specified /home/quake paths.
In `@ckb/fiber`:
- Line 1: Update the ckb/fiber submodule reference from unreachable commit
bc361aaaa40d1394b83e6a1808869b0b06c48c13 to an accessible Fiber commit, or
publish that pinned commit while preserving the required tests/deploy/contracts
and config/testnet assets used by the Fiber target.
In `@src/cmd/fiber.ts`:
- Around line 36-43: Update printFiberSummary to use the shared
fiberAccountIndex(node.id) helper when formatting each node’s account number,
replacing the inline node.id + 2 calculation and keeping the existing summary
output unchanged otherwise.
In `@src/cmd/node.ts`:
- Around line 712-726: Update waitForFiberRuntimeRunning to use a 10-minute
timeout, matching FIBER_DAEMON_READY_TIMEOUT_MS in the Fiber daemon, so
first-run FNN downloads can complete without terminating the child process.
Preserve the existing polling and timeout error behavior.
In `@src/fiber/config-gen.ts`:
- Around line 87-98: Add ckb.udt_whitelist to MANAGED_CONFIG_PATHS in
nodes-yml.ts so mergeNodeConfig preserves the resolved
options.chainScripts.udtWhitelist value and prevents per-node configuration from
overriding it.
In `@src/fiber/daemon.ts`:
- Around line 190-195: Move the storeLockFilesForRuntime call before
terminateProcess in the shutdown flow, capturing the lock-file list while
runtime.json still exists; then reuse that captured lockFiles value for
waitForStoreLocksReleased and the existing warning without changing the
termination or wait behavior.
In `@src/fiber/manager.ts`:
- Around line 376-385: Update waitForChildExit and the liveness checks near the
existing exitCode conditions to also treat a non-null signalCode as exited.
Preserve the current immediate-return behavior for normally exited children and
ensure signal-terminated children do not wait for the timeout.
- Around line 313-325: Update the FNN spawning flow around spawnFnn to collect
handles incrementally instead of using an all-or-nothing nodes.map call. If a
later spawn throws, stop every previously collected handle before propagating
the original failure; only construct and write the FiberRuntime after all nodes
spawn successfully.
In `@src/fiber/nodes-yml.ts`:
- Around line 138-145: Update the warning inside the removed-node loop to
explicitly state that each node’s hand-written config overrides are also
discarded when its nodes.yml entry is removed, while preserving the existing
directory-retention and cleanup guidance.
In `@src/fiber/scripts.ts`:
- Around line 98-102: Update the script validation around requireScript in the
script-loading flow to cover sudt and xudt as well as auth, funding_lock, and
commitment_lock, or otherwise ensure their failures use the same full contextual
error message instead of raw missing:<name> output. Keep the existing
required-script behavior unchanged.
In `@src/fiber/store-lock.ts`:
- Around line 37-45: Update the error handling in the lock inspection function
around the `execFileSync` catch block to return `null` when the `lsof` process
is terminated by the timeout signal or exits with any status other than 1. Only
interpret stdout for the no-holder status 1 case; preserve the existing `ENOENT`
handling and ensure inspection failures cannot return `false` to the cleanup
flow.
In `@src/util/daemon.ts`:
- Around line 158-180: Update getProcessCommandLine’s Windows command selection
to use PowerShell with Get-CimInstance Win32_Process, or fall back to it when
the WMIC invocation is unavailable, while preserving the existing PID filtering
and command-line parsing. Ensure Windows systems without WMIC still resolve the
managed process command line so verifyDaemonIdentity can proceed.
---
Nitpick comments:
In `@ckb/devnet/specs/fiber/auth`:
- Line 1: Document the committed Fiber contract binaries near the Makefile.fiber
target by adding a concise comment containing the pinned Fiber commit or Fnn
revision. State that auth, funding_lock, and commitment_lock are intentionally
tracked for distribution or offline use, and preserve the existing build
behavior.
In `@Makefile`:
- Around line 56-62: Update the fiber target to detect whether the ckb/fiber
submodule is available before creating the destination or copying files. If it
is absent, fail immediately with a clear message explaining that the submodule
must be initialized, while preserving the existing copy behavior when present.
In `@src/cmd/clean.ts`:
- Around line 16-24: The daemon PID path is duplicated across three sites
instead of using shared path definitions. Export a shared helper such as
nodeDaemonPaths(settings) from src/util/daemon.ts, then update
assertCkbDaemonStopped in src/cmd/clean.ts and resolveOffckbManaged in
src/fiber/status.ts to use it; remove resolveNodeDaemonPaths from
src/fiber/daemon.ts and call the shared helper there, preserving the existing
DAEMON_LOG_DIR and DAEMON_PID_FILE layout.
- Around line 26-34: Centralize Fiber node enumeration by adding and exporting
fiberNodeIds in src/fiber/paths.ts, preserving the existing missing-directory,
numeric-entry filtering, and number-conversion behavior. Update fiberStoreDirs,
existingStoreLockFiles, and the store listing in fiberClean to call
fiberNodeIds(settings) and map each ID to the required per-node path instead of
reading and filtering the nodes directory themselves.
In `@src/fiber/install.ts`:
- Around line 84-117: Update downloadFnnAndUnzip to remove tempFilePath in a
finally block surrounding the download, extraction, and installation workflow,
ensuring the temporary tarball is deleted on both success and failure while
preserving existing error propagation.
In `@src/fiber/scripts.ts`:
- Around line 91-93: Update the script lookup around resolved.scripts to import
and use SystemScriptName alongside SystemScript, removing the double cast and
replacing the string literals in the missing filter with the corresponding
SystemScriptName members. Index the typed scripts record with those enum values
so renames remain compile-safe.
In `@src/fiber/status.ts`:
- Around line 66-76: Export the lockMatches helper from status.ts and replace
the duplicate inline comparison in the manager.ts account.lockScript validation
with calls to this shared helper. Preserve the existing case-insensitive
comparisons of code_hash, hash_type, and args, including handling an undefined
actual lock.
🪄 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: 7e83b12b-a6a7-4f88-bb8a-d562bb5ae816
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (41)
.changeset/fiber-devnet.md.gitmodulesMakefileREADME.mdckb/devnet/specs/dev.tomlckb/devnet/specs/fiber/authckb/devnet/specs/fiber/commitment_lockckb/devnet/specs/fiber/funding_lockckb/devnet/specs/fiber/testnet-config.ymlckb/fiberpackage.jsonsrc/cfg/setting.tssrc/cli.tssrc/cmd/clean.tssrc/cmd/config.tssrc/cmd/fiber.tssrc/cmd/node.tssrc/fiber/accounts.tssrc/fiber/ckb-env.tssrc/fiber/clean.tssrc/fiber/config-gen.tssrc/fiber/daemon.tssrc/fiber/env-lock.tssrc/fiber/install.tssrc/fiber/manager.tssrc/fiber/nodes-yml.tssrc/fiber/paths.tssrc/fiber/rpc.tssrc/fiber/runtime.tssrc/fiber/scripts.tssrc/fiber/status.tssrc/fiber/store-lock.tssrc/scripts/public.tssrc/scripts/type.tssrc/util/daemon.tstests/fiber-accounts.test.tstests/fiber-config-gen.test.tstests/fiber-env-lock.test.tstests/fiber-nodes-yml.test.tstests/fiber-scripts.test.tstests/node-command.test.ts
🛑 Comments failed to post (1)
ckb/devnet/specs/fiber/commitment_lock (1)
1-1: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Information Disclosure (CWE-200): Exposure of Sensitive Information to an Unauthorized Actor
Reachability: Unreachable
Committed contract binaries embed developer build paths. Both contracts were compiled locally without path remapping, so Rust panic metadata retains the builder's home directory in the published artifact.
ckb/devnet/specs/fiber/commitment_lock#L1-L1: rebuild without the/Users/quake/.rustup/...and/Users/quake/.cargo/...strings, using--remap-path-prefixor the upstream release artifact.ckb/devnet/specs/fiber/funding_lock#L1-L1: rebuild without the/home/quake/.rustup/...and/home/quake/.cargo/...strings the same way.📍 Affects 2 files
ckb/devnet/specs/fiber/commitment_lock#L1-L1(this comment)ckb/devnet/specs/fiber/funding_lock#L1-L1🤖 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 `@ckb/devnet/specs/fiber/commitment_lock` at line 1, Rebuild the contract binaries without embedded developer filesystem paths by applying Rust path remapping or replacing them with the upstream release artifacts. Update ckb/devnet/specs/fiber/commitment_lock at lines 1-1 to remove the specified /Users/quake paths, and ckb/devnet/specs/fiber/funding_lock at lines 1-1 to remove the specified /home/quake paths.
- Sanitize builder home paths (/Users/quake, /home/quake) embedded in the committed funding_lock/commitment_lock binaries with equal-length replacements; make fiber reproduces the sanitized copies and fails with a clear message when the ckb/fiber submodule is missing - Keep already-spawned FNNs from being orphaned when a later spawn fails - Treat signal-terminated children as exited (exitCode is null there) - Capture store lock files before signaling the manager; it removes runtime.json during its own shutdown - isStoreLockHeld: only lsof exit 1 means 'no holder'; any other exit status or a timeout kill now reports 'unknown' instead of 'free' - Align the node --fiber --daemon readiness wait with the fiber daemon's 10-minute budget (first run may download FNN) - Mark ckb.udt_whitelist as a managed nodes.yml field so per-node overrides cannot silently break UDT payments - Replace deprecated wmic with PowerShell Get-CimInstance for Windows process command-line lookup - Give requireScript a full contextual error instead of raw missing:<name> - Warn that shrinking nodes.yml discards the removed nodes' config overrides - Share helpers: fiberAccountIndex in the start summary, lockMatches between status and manager, nodeDaemonPaths across cmd/fiber modules, fiberNodeIds for node enumeration, SystemScriptName for script lookup - Delete the downloaded FNN tarball after install (also on failure) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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
1 participant
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.
Summary
Implements the Fiber devnet plan: offckb can now start and manage a local Fiber development environment — 1 CKB node + miner + RPC proxy + N FNN nodes (default 2), with the Fiber contracts in the local chain's genesis block.
What changed
auth,funding_lock,commitment_lockcopied from the newckb/fibersubmodule pinned to FNN v0.9.0-rc7 (bc361aa), appended after the existing system cells. Type IDs are derived from the cellbase input + output index, so existing scripts (accounts, sUDT, xUDT, omnilock, …) keep their code hashes; only the genesis tx hash changes, and cell dep out points are always computed from a freshckb list-hashesat start. The three contracts appear inSystemScriptName/offckb system-scriptsasauth,funding_lock,commitment_lock.0.9.0-rc7, portable tarballs), keeps the full extracted layout so the bundledconfig/testnet/config.ymlcan seed the devnet config;--binary-path/--fnn-binary-pathrun a local FNN with its sibling testnet config (unparseable → error) or the shipped fallback.config.ymlfrom the FNN testnet config parsed as a generic mapping (unknown fields from future FNN versions survive), replacing chain/listeners/bootnodes/scripts/RPC/UDT/services with the devnet values, then merging per-node overrides fromfiber/nodes.yml(managed fields rejected).FundingLock/CommitmentLockget their own cell + the sharedauthcell dep; the sUDT/xUDT whitelist anchors to the account-19 issuer lock hash (^0x…$).devnet/fiber/{nodes.yml,runtime.json,logs/,nodes/<id>/{config.yml,ckb/key,fiber/sk,fiber/store,fnn.log,password}}; node N uses built-in account N+2, RPC21713+N, P2P8343+N, max 16 nodes. Every FNN logs only to its ownfnn.log.node_info.chain_hash; node identity vsfiber/sk; funding account vs the expected built-in account; available balance; then node 1 connects to the other nodes (verified once vialist_peers). Any failure stops everything started in that run..offckb-devnet.lock(sibling ofdevnet/),runtime.jsonmanager records, daemon PID files with identity verification (no overwrite), stop only ever signals recorded managers — never per-FNN kills by port/path/version — with one SIGTERM, a store-LOCK wait, and at most one SIGKILL.offckb fiber stopalso stops the combinednode --fiber --daemonmanager (with a clear message that CKB stops too);offckb node stoprefuses while a separate fiber daemon manages FNNs.offckb cleantakes the env lock, refuses on live daemons/store locks, and removes fiber stores with--data.--network mainnet|testnetwith--fibererrors, and anyfork.json(valid or not) rejects Fiber before any daemon respawn. Port conflicts are reported, never killed. Devnets initialized by older offckb versions lack the contracts and get a guided "stop everything,offckb clean, restart" message.Verification
pnpm test),tsc --noEmitand eslint clean; new unit tests cover nodes.yml rules, config generation/merge, list-hashes → FNN script building, env lock, key material.node --fiberstarts the full environment;fiber status/--jsonreportrunning; opened a 200 CKB channel node1→node2 (funding tx confirmed →ChannelReady) and completed a 10 CKB invoice payment; foreground SIGINT and both daemons shut down cleanly (runtime.json/PID files removed, store locks released); supervisor stops the whole group when one FNN dies;fiber clean --datapreserves node identities,fiber clean/offckb cleanwork; pre-fiber devnets and fork.json are rejected with the designed messages; existingaccounts/balance/udt issueflows verified unchanged on the new genesis.Notes
taskkill /T), matching the existing daemon helper — the graceful console/job-object helper from the plan is future work.offckb config set fnn-version <v>sets the default FNN version.🤖 Generated with Claude Code