Cli/add chain inbound limits

[evgeniko]

Clone of #794 by @martin0995

This pull request introduces a new system for configuring and validating inbound transfer limits between chains in the CLI deployment configuration. It adds interactive prompts to ensure that all required inbound limits are set (or warns if they are missing when prompts are skipped), improving deployment safety and user experience. The main changes are the addition of a new limits.ts module, integration of inbound limit configuration into CLI commands, and enhancements to the prompt utilities.

Inbound limits configuration and validation:

• Added a new module cli/src/limits.ts that provides utilities to detect missing or zero inbound transfer limits between chains, prompt the user to set them (with validation), and update the deployment configuration accordingly. This includes logic to group missing limits, provide sensible defaults, and handle both new chain additions and configuration pulls.
• Integrated configureInboundLimitsForNewChain and configureInboundLimitsForPull into the CLI command flow in cli/src/index.ts, so that users are prompted to fill in missing inbound limits when adding a new chain or pulling configuration. The system can also warn when limits are missing and prompts are skipped (e.g., with --yes). [1] [2] [3]

Prompt utility improvements:

• Enhanced the prompt utilities in cli/src/prompts.ts to support aborting on Ctrl+C (SIGINT), which now immediately exits with a specific code if enabled. This is used in the new inbound limits prompts for a better user experience. [1] [2]
• Updated promptYesNo to accept the new abort option and to delegate it to promptLine, ensuring consistent behavior across prompts.

Summary by CodeRabbit

• New Features

  • Interactive inbound-limit configuration flows for adding chains and when pulling updates.

• Improvements

  • Better numeric limit formatting validation with clearer error and warning messages (including zero-limit detection).
  • Prompts now support graceful interrupt handling (SIGINT) to abort flows cleanly.

• Tests

  • Expanded test coverage for config validation, limit formatting, and inbound-limit aggregation.

• Chores

  • Made test/script invocations non-interactive by default.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

Adds numeric formatting/validation utilities, deployment config error checking, and inbound-limit collection/configuration flows; integrates them into the CLI and enhances prompts with SIGINT abort handling. Extensive tests for formatting, config errors, and inbound-limit logic are added.

Changes

Cohort / File(s)	Summary
Limit Formatting Utilities cli/src/limitFormatting.ts	New pure utilities: formatNumber, checkNumberFormatting, getDecimalsFromLimit, isZeroLimit, isValidLimit.
Config Validation cli/src/configErrors.ts	New checkConfigErrors that validates per-deployment outbound/inbound formatting, logs errors/warnings, and returns error count.
Inbound Limits Configuration cli/src/limits.ts	New functions to collect and apply missing inbound limits: collectMissingInboundGroups, collectMissingInboundGroupsForAll, configureInboundLimitsForNewChain, configureInboundLimitsForPull, plus helpers for grouping, warnings, and applying limits.
Prompt Enhancement cli/src/prompts.ts	promptLine and promptYesNo extended with abortOnSigint option (new PromptOptions type) to exit with code 130 on SIGINT when enabled.
Main CLI Integration cli/src/index.ts	Replaced in-file formatting/checks with imports from new modules and wired inbound-limit flows into add-chain and pull workflows.
Tests: Limit Formatting cli/__tests__/limitFormatting.test.ts	New tests covering formatting, validation, and getDecimalsFromLimit edge cases.
Tests: Config Errors cli/__tests__/configErrors.test.ts	New tests for checkConfigErrors, asserting error/warn console calls and error counts.
Tests: Inbound Limits cli/__tests__/limits.test.ts	Comprehensive tests for missing inbound collection and grouping behavior, zero/malformed handling, and aggregation across chains.
Test Script Updates cli/test/sepolia-bsc.sh	Added --yes flag to two ntt add-chain commands to bypass interactive prompts in CI.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant CLI
    participant Collector as InboundCollector
    participant Validator as LimitFormatting
    participant Prompt as Prompts
    participant Config as Deployments

    User->>CLI: add-chain / pull
    CLI->>Collector: collectMissingInboundGroups(newChain?)
    Collector->>Validator: checkNumberFormatting(outbound/inbound)
    Validator-->>Collector: valid / invalid
    Collector-->>CLI: MissingInboundGroup[]
    alt groups found
        CLI->>Prompt: if not skipPrompts -> ask for values
        Prompt->>User: prompt for inbound limit
        User-->>Prompt: enter value
        Prompt->>Validator: isValidLimit(value, decimals)
        Validator-->>Prompt: valid/invalid
        Prompt-->>CLI: validated limits
        CLI->>Config: setInboundLimit(...) per source/destination
    else skipPrompts
        CLI->>CLI: warn about missing inbound limits
    end
    CLI-->>User: complete (updated? hadMissing?)

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Suggested reviewers

• dvgui
• nik-suri

Poem

🐰 Hoppity-hop through decimals and signs,
I hop the dots and count the lines.
Errors logged and warnings told,
Missing inbounds placed in fold,
A rabbit cheers: configurations aligned!

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 58.82% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The PR title 'Cli/add chain inbound limits' is directly related to the main change: adding a system for configuring inbound transfer limits between chains in the CLI. It clearly summarizes the primary feature introduced.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch cli/add-chain-inbound-limits

Tip

Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord.

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@cli/src/limitFormatting.ts`:
- Around line 1-9: formatNumber currently treats the minus sign as part of the
digit string, producing malformed output for negative bigints; update
formatNumber to detect negativity (e.g., check num < 0n), store the sign
separately, convert and operate on the absolute value (use -num for the abs),
then run the existing padding/splitting logic on the absolute value (variables:
num, str, padded, splitIndex) and finally prepend the stored sign to the
returned string (also handle the decimals === 0 case by returning
`${sign}${str}.`).

🧹 Nitpick comments (6)

cli/src/prompts.ts (1)

29-36: Minor code path clarity issue in SIGINT handler.

When abortOnSigint is true, process.exit(130) terminates immediately, making lines 34-35 unreachable in that branch. While functionally harmless, the code flow would be clearer with an early return or else block.

♻️ Optional: Make control flow explicit

     rl.once("SIGINT", () => {
       process.stdout.write("\n");
       if (abortOnSigint) {
         process.exit(130);
+        return; // Unreachable but clarifies intent
       }
       settle("");
       rl.close();
     });

cli/__tests__/limitFormatting.test.ts (2)

8-14: Consider adding edge case tests for negative bigints.

The formatNumber function accepts bigint which can be negative. The current tests only cover non-negative values. If negative values are not expected, consider documenting this constraint; otherwise, add test cases to verify behavior.

// Example test cases to consider:
expect(formatNumber(-1234n, 2)).toBe("-12.34"); // or should this throw?

---

53-58: Consider testing edge case: empty string.

The isZeroLimit function returns true for empty strings (after removing dots, normalized.length === 0). If this is intentional behavior, consider adding an explicit test case to document it.

// Add to clarify expected behavior:
expect(isZeroLimit("")).toBe(true);  // Is this intended?
expect(isZeroLimit(".")).toBe(true); // Only a dot

cli/src/limitFormatting.ts (1)

19-22: Redundant undefined check.

After verifying parts.length !== 2, accessing parts[1] is guaranteed to exist. The undefined check on line 20 is redundant. This is a minor style nit - the code is functionally correct.

♻️ Optional: Simplify by removing redundant check

 export function checkNumberFormatting(
   formatted: string,
   decimals: number
 ): boolean {
   const parts = formatted.split(".");
   if (parts.length !== 2) {
     return false;
   }
-  const fraction = parts[1];
-  if (fraction === undefined) {
-    return false;
-  }
-  if (fraction.length !== decimals) {
-    return false;
-  }
-  return true;
+  return parts[1]!.length === decimals;
 }

cli/src/limits.ts (2)

87-144: Consider extracting shared logic between collection functions.

collectMissingInboundGroups and collectMissingInboundGroupsForAll share significant code (chain validation, outbound limit checking, grouping logic). The main difference is the filtering predicate. Consider extracting a common helper to reduce duplication.

♻️ Optional: Extract shared logic

// Example shared helper approach:
function collectMissingInboundGroupsCore(
  chainsConfig: Config["chains"],
  shouldIncludePair: (source: Chain, destination: Chain) => boolean
): MissingInboundGroup[] {
  // ... shared validation and grouping logic
  // Use shouldIncludePair to filter pairs
}

export function collectMissingInboundGroups(chainsConfig, newChain) {
  return collectMissingInboundGroupsCore(chainsConfig, (src, dst) =>
    src === newChain || dst === newChain
  );
}

export function collectMissingInboundGroupsForAll(chainsConfig) {
  return collectMissingInboundGroupsCore(chainsConfig, () => true);
}

---

157-159: Placeholder outbound limit format may cause inconsistency.

The placeholder "0.0" (1 decimal) may not match the decimal precision expected by other chains. If setInboundLimit is ever called without an existing outbound limit configured, this could create format inconsistencies. The comment acknowledges this is a fallback, but consider documenting the expected caller invariant more explicitly or using a format that matches the source chain's decimals.

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@cli/src/configErrors.ts`:
- Line 1: The file fails CI due to Prettier formatting; run the formatter and
commit the updated file so imports and spacing match project style.
Specifically, run the project's formatter command (e.g., `bun run prettier`) and
stage the resulting changes for this file (which contains the import of
assertChain and Chain from "@wormhole-foundation/sdk") so the import line and
overall file formatting conform to Prettier rules.

🧹 Nitpick comments (2)

cli/src/limits.ts (2)

97-99: Placeholder outbound "0.0" has a hardcoded 1-decimal format.

If this fallback is ever reached, the outbound limit format won't match the destination's actual decimals, which would then fail checkNumberFormatting in configErrors.ts. The comment acknowledges this is unlikely, but a more defensive approach would derive the format from the destination's known decimals.

♻️ Suggested improvement

-    destinationConfig.limits = { outbound: "0.0", inbound: {} };
+    // Use a zero outbound matching the destination's expected decimal count.
+    // This path is defensive; normal flows only call setInboundLimit when outbound exists.
+    destinationConfig.limits = { outbound: "0.", inbound: {} };

Though even "0." (0 decimals) is arbitrary. Consider passing decimals into setInboundLimit and using formatNumber(0n, decimals) for a correct placeholder.

---

175-201: Per-source prompt loop is correct but consider a max-retry guard.

The while (true) on line 176 loops until valid input or Ctrl+C (via abortOnSigint). This is standard for interactive CLI prompts, but in automated/scripted contexts where stdin is piped, a non-matching input could loop indefinitely. Since skipPrompts guards the outer path, this is low risk.

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@cli/src/configErrors.ts`:
- Line 4: The import statement for the Deployment type in configErrors.ts uses a
bare "./validation" path which will fail under ESM resolution; update the import
to reference the emitted module file by changing the specifier to
"./validation.js" (i.e., modify the import that brings in Deployment) and search
for other occurrences of "from './validation'" in the codebase to make the same
change so runtime ESM resolution succeeds.