feat: Logs polish

[ovitrif]

Description

• Bump version to 0.0.17 (code 17)
• Standardize operation names in LightningRepo logs using camelCase function names (e.g., sync, connectToTrustedPeers, payInvoice)
• Use fixed-width path format in log output for better alignment (%-36s for location)
• Consolidate performance logging to single measured() function, removing withPerformanceLogging
• Fix JSON log type field to use simple class name instead of full serialized type
• Rename Logger.performance() to Logger.perf() for brevity
• Integrate peer connection fix from fix: add peer detail full address handling #541 - handles both host:port and nodeId@host:port formats
• Add fallback to Env peers when no trusted peers connect successfully

Preview

Peer connected on a fresh wallet

QA Notes

• Verify logs are properly formatted with aligned columns
• Verify peer connections work correctly with LSP nodes
• Run ./gradlew testDevDebugUnitTest --tests PeerDetailsTest to verify tests pass
• Check WakeNodeWorker logs show properly formatted performance timing

[ovitrif]

• needs merging with changes in fix: add peer detail full address handling #541
• first going to merge that one

[claude]

Code Review: Logs Polish

Overview

This PR improves logging consistency, readability, and performance measurement across the codebase. The changes are well-structured and follow the project's conventions outlined in CLAUDE.md.

---

✅ Strengths

1. Logging Standardization

• Excellent: Operation names now use camelCase function names (sync, connectToTrustedPeers, payInvoice) instead of mixed formats, improving consistency and making logs easier to grep.
• Good: Error messages now use single quotes around operation names for better readability

2. Log Formatting Improvements

• Smart: Fixed-width path format (%-36s) for location field ensures aligned columns in logs, making them much easier to read.
• Cleaner: Simplified performance logging from verbose withPerformanceLogging to concise measured() function.
• Better: Logger.perf() is more concise than Logger.performance() while remaining clear.

3. Duration Formatting

The new Duration.formatted() extension is well-designed:

• Handles multiple time units (hours, minutes, seconds, milliseconds)
• Only shows non-zero values to avoid clutter
• Fallback to ms if everything is zero ensures something is always displayed

4. Peer Connection String Parsing Fix ⭐

• Critical Bug Fix: The toPeerDetails() function now correctly handles connection strings in both formats:
  • "host:port" → host:port
  • "pubkey@host:port" → host:port (extracts address after @)
• Excellent Test Coverage: 15 new unit tests for PeerDetailsExt cover edge cases including IPv6, empty lists, and multiple connection strings.

5. Fallback Peer Connection Logic

• Resilient: New verifyTrustedPeersOrFallback() method adds fallback to Env.trustedLnPeers when no trusted peers connect successfully.
• Good UX: Logs connection status for debugging.

6. JSON Log Type Field Fix

• Clean: Using simple class name (value::class.simpleName) instead of full serialized type makes logs more readable.

---

🔍 Issues & Recommendations

1. Thread Safety in verifyTrustedPeersOrFallback() ⚠️

Issue: The new method calls node.listPeers() and node.connect() without error handling if the node state changes between calls.

Recommendation: Wrap in runCatching or add checks for node state before listing peers to prevent potential crashes.

2. Performance Logging Context

Observation: measured() function at Perf.kt:24 calls Logger.perf() but doesn't pass a context parameter, so it will use an empty context.

Recommendation: Update signature to accept optional context parameter for better log traceability.

3. IPv6 Address Parsing Edge Case

Minor concern: No test for malformed IPv6 (e.g., "pubkey@2001:db8::1:9735" without brackets). Consider adding a test to document expected behavior.

4. Unit Test Improvements

Good: Test changes follow CLAUDE.md guidelines (using runBlocking in setUp(), avoiding wheneverBlocking in expression bodies).

Observation: LightningRepoTest.kt lines 602-627 now test with full connection strings, which validates the bug fix. However, consider adding a test that verifies the old format still works to ensure backwards compatibility.

---

🔐 Security Considerations

✅ No security concerns identified. The changes are purely logging improvements with no impact on authentication, authorization, or data validation.

---

🚀 Performance Considerations

✅ Positive Impact:

• Consolidating to a single measured() function reduces code duplication.
• Using kotlin.time.measureTime is more efficient than manual System.currentTimeMillis() calculations.
• Duration formatting is only done when needed (in the log message).

⚠️ Minor Concern: The new formatLog() function creates a location string on every log call. Since logs are called frequently, consider if this adds noticeable overhead. (Likely negligible, but worth profiling if logs become a bottleneck.)

---

🧪 Test Coverage

✅ Excellent: 15 new unit tests for PeerDetailsExt covering:

• Basic parsing and formatting
• Connection string formats (with/without pubkey)
• Edge cases (IPv6, empty lists, multiple strings)
• Error handling (invalid URIs)

✅ Good: Updated LightningRepoTest to use full connection strings, validating the fix.

Recommendation: Run ./gradlew testDevDebugUnitTest to ensure all tests pass (per CLAUDE.md).

---

📋 Code Quality & Best Practices

Follows CLAUDE.md Guidelines ✅

• Uses Result pattern in repositories
• Logs use context = TAG
• Returns early with guard clauses
• String resources preserved (no hardcoded strings added)
• Minimal changes to fix issues (follows YAGNI principle)

Detekt Baseline Cleanup ✅

• Removed 10 suppressed issues from detekt-baseline.xml, indicating code quality improvements.

Recommendation: Run ./gradlew detekt to ensure no new lint issues (per CLAUDE.md).

---

🎯 Suggested Action Items

Before Merge:

1. ✅ Run ./gradlew compileDevDebugKotlin to verify compilation
2. ✅ Run ./gradlew testDevDebugUnitTest to verify tests pass
3. ✅ Run ./gradlew detekt to check for lint issues
4. ⚠️ Add error handling to verifyTrustedPeersOrFallback()
5. 💡 Consider adding context parameter to measured() function

Nice to Have:

• Add test for backwards compatibility of "host:port" format in toPeerDetails()
• Add test for malformed IPv6 addresses

---

✅ Overall Assessment

Verdict: ✅ Approve with minor recommendations

This is a high-quality PR that meaningfully improves the codebase:

• Fixes a critical bug in peer connection string parsing
• Improves log readability and consistency
• Adds excellent test coverage
• Follows project conventions and best practices

The suggested improvements are minor and could be addressed in a follow-up PR if needed. Great work! 🎉

---

📝 Commit Message Suggestions

Per CLAUDE.md guidelines, here are 3 suggested commit messages:

1. feat: polish logging with aligned format and perf metrics
2. fix: peer connection string parsing with @ separator
3. refactor: standardize operation logging and duration formatting

[ben-kaufman]

utAck