Skip to content

Fix critical bugs and harden error handling for production readiness#51

Open
vincenzopalazzo wants to merge 1 commit into
mainfrom
claude/nervous-noyce
Open

Fix critical bugs and harden error handling for production readiness#51
vincenzopalazzo wants to merge 1 commit into
mainfrom
claude/nervous-noyce

Conversation

@vincenzopalazzo

Copy link
Copy Markdown
Collaborator

Summary

  • P2P graph node2 bug: add_channel used node1 ID for both endpoints, so node2 was never added to the graph — completely breaking P2P topology for probabilistic routing
  • LDK route amount/delay calculation: was accumulating fees instead of computing correct forward amounts per hop, producing routes with wrong amounts at every hop
  • 5 panic sites removed: replaced unwrap()/expect() with proper error propagation in probabilistic strategy (construct_route_params, convert_route_to_output), plugin on_init, pay handler (cln_rpc_path), and gossip channel conversion
  • Error messages fixed: amount validation now distinguishes "both specified" vs "both missing" instead of showing the same confusing message for both
  • Random seed: replaced hardcoded [0; 32] with time+payment-based entropy for route randomization
  • Clippy cleanup: clone_on_copy, derivable_impls, unused import

Production readiness plan

These fixes address the critical correctness and crash bugs. Remaining work for feature-complete production readiness:

P1 — Must have for real-money testing:

  1. Basic retry logic (at least 1 retry on transient WIRE_TEMPORARY_* failures)
  2. CLTV safety validation (minimum 18 blocks, overflow check, max bound)
  3. Fee bounds (maxfee parameter with hard ceiling enforcement)
  4. waitsendpay timeout (currently blocks indefinitely)

P2 — Required for production:
5. Multi-part payments (MPP) — without this, payments > single channel capacity always fail
6. Gossip map caching in plugin state (currently reloads gossip_store on every call)
7. Migrate from deprecated decodepay to decode RPC
8. Implement barqrouteinfo for dry-run route inspection

P3 — Competitive with renepay:
9. Liquidity learning from payment attempt results (Bayesian updates)
10. Payment splitting via MCF optimization
11. Shadow routing for privacy
12. Configurable scoring parameters

Test plan

  • cargo build passes
  • cargo clippy --all --tests passes (remaining warnings are pre-existing: unused root_path field, unimplemented route_info dead code)
  • cargo test --all — 2/3 pass, 1 pre-existing failure (test_rapid_gossip_sync_network_not_empty fails on main too due to empty Testnet gossip data)
  • Integration tests require CLN + bitcoind setup (not run in this PR)

🤖 Generated with Claude Code

…eadiness

- Fix P2P graph builder using node1 ID for both endpoints (node2 was
  never added to the graph, breaking topology)
- Fix LDK route conversion computing wrong amounts and delays per hop
  (was accumulating fees instead of computing correct forward amounts)
- Replace all panic-prone unwrap/expect calls with proper error
  propagation in probabilistic strategy, plugin init, and pay handler
- Fix misleading error messages in amount_msat validation (both
  conflict and missing cases showed same message)
- Generate time-based random seed bytes instead of hardcoded zeros
  for probabilistic route scoring
- Handle GossipChannel conversion gracefully when satoshi value or
  wire serialization is unavailable
- Fix clippy warnings (clone_on_copy, derivable_impls, unused imports)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant