Skip to content

Suggestions from codebase analysis#395

Open
themikemoniker wants to merge 1 commit intoolympus-btc:mainfrom
themikemoniker:suggestions
Open

Suggestions from codebase analysis#395
themikemoniker wants to merge 1 commit intoolympus-btc:mainfrom
themikemoniker:suggestions

Conversation

@themikemoniker
Copy link
Copy Markdown
Contributor

@themikemoniker themikemoniker commented Feb 15, 2026

Suggestions from Codebase Analysis

Based on a deep-dive of the v0.5.1-alpha codebase.


1. NWC (Nostr Wallet Connect) as Alternative Payment Backend

Add NIP-47 support so merchants can connect any NWC-compatible wallet alongside Phoenixd.

  • Unlocks wallet-agnostic payments (Alby, Mutiny, Phoenix mobile, Coinos, etc.)
  • No Lightning node management for merchants who don't want to run Phoenixd
  • ~200MB smaller Electron bundle when Phoenixd isn't needed
  • Complements existing Phoenixd features (channel closing, etc.) — config determines which backend is active
  • Sidesteps the autoLiquidity configuration problem ([feat]: Add phoenixd autoLiquidity configuration to setup wizard #276) entirely for NWC users

2. Security Hardening

  • GET /users and GET /users/{id} have no authentication — anyone can enumerate all users, names, roles, emails, phones
  • GET /config has no authentication — exposes business name, address, phone, email, tax ID
  • CORS set to anyHost() — enables CSRF from any origin with cookie-based auth
  • GET /wallet/seed returns raw seed phrase — even with wallet JWT, this is risky. Consider showing only during initial setup, then encrypting at rest
  • PIN hashes loggedPinHasher.kt:39 logs "Pins: " + newHash + " = " + storedHash during verification
  • --secret used for JWT signing, keystore derivation, AND PIN hashing salt — compromising one compromises all

3. Payment Reliability

  • No webhook retry — if Ambrosia is down when Phoenixd sends a payment webhook, it's lost forever. A simple retry queue or polling fallback would help
  • No invoice expiry tracking — server doesn't track when invoices expire, so the client can show stale QR codes
  • No idempotency on webhook delivery — duplicate webhooks could trigger duplicate payment confirmations
  • No polling fallback — payment detection relies entirely on WebSocket + webhook. If either drops, payment goes unnoticed until manual check

4. Existing Bugs / Rough Edges

  • InitialSetup.kt transaction rollback — if the username already exists (UNIQUE constraint), the entire transaction rolls back including the config insert. The app ends up with users but initialized = false, causing redirect loops to /onboarding
  • connection.autoCommit = false set twice in InitialSetup.kt (lines 82 and 97) — second call is redundant
  • SeedGenerator downloads wordlist from GitHub at runtime — falls back to HTTP download if local file missing, no integrity check on downloaded content
  • Products can't be added to ordersorder_dishes only references the dishes table, so store-mode products can't be added to orders via the existing order flow

5. Missing for Real Merchant Use

  • No tax support — no tax rate per product, no tax calculation, no tax-inclusive/exclusive toggle
  • No refund flow — no refund table, no credit notes
  • No customer tracking — no customer table, no purchase history
  • No pagination on most list endpoints
  • System.exit(1) on DB connection failure — crashes entire JVM instead of graceful error

🤖 Generated with Claude Code

Placeholder commit for NWC feature discussion PR.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@themikemoniker
Copy link
Copy Markdown
Contributor Author

bump up my post

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