Skip to content

Add Satoshi Genesis sample wallet to self-hosted onboarding#234

Merged
schjonhaug merged 4 commits intomasterfrom
feature/satoshi-genesis-sample-wallet
Mar 4, 2026
Merged

Add Satoshi Genesis sample wallet to self-hosted onboarding#234
schjonhaug merged 4 commits intomasterfrom
feature/satoshi-genesis-sample-wallet

Conversation

@schjonhaug
Copy link
Owner

Summary

  • Adds a second sample wallet ("Satoshi Genesis") to the self-hosted onboarding flow alongside Bacon, using the real genesis block coinbase public key on mainnet and a deterministic funded address on regtest
  • Refactors the sample wallet system from single-wallet to a multi-wallet array with per-network descriptor overrides and inline form filling (no page navigation)
  • Sample wallet prompt now stays visible until user adds a non-sample wallet, filtering out already-added sample wallets from the buttons

Test plan

  • cd frontend && pnpm build — no type/build errors
  • cd frontend && pnpm test — all 146 tests pass
  • cd backend && cargo build && cargo test -- --test-threads=1 — backend compiles and passes
  • Run dev.sh reset && dev.sh init — verify "satoshi-genesis" wallet is created and funded with 0.5 BTC
  • Visit /wallets/add with no wallets → both "Use Bacon Wallet" and "Use Satoshi Genesis Address" buttons visible
  • Click "Use Bacon Wallet" → form fills inline with Bacon name/descriptor (no navigation)
  • Click "Use Satoshi Genesis Address" → form fills inline with Satoshi (Genesis) name and regtest address
  • Submit Bacon → wallet created, return to add page → only Satoshi Genesis button remains
  • Submit Satoshi Genesis → wallet created, return to add page → prompt auto-hides
  • Add a non-sample wallet → prompt no longer appears on subsequent visits

Add a second sample wallet option alongside Bacon in the self-hosted
onboarding flow. On mainnet/testnet, it uses Satoshi's real genesis
block coinbase public key. On regtest, it uses a deterministic funded
address for a useful onboarding experience.

Refactor the sample wallet system from single-wallet to multi-wallet:
- Replace SAMPLE_WALLET_SLUG + network-keyed record with SampleWallet[]
  array supporting per-network descriptor overrides
- Sample wallet buttons now fill the form inline instead of navigating
  to separate routes
- Prompt stays visible until user adds a non-sample wallet, with
  already-added sample wallets filtered from the buttons
- Generalize bacon-specific naming throughout (isBaconWallet →
  isSampleWallet, baconPrefilled → samplePrefilled with {name} param)
- Update all 9 locale files with new translation keys
- Add deterministic satoshi-genesis wallet to dev.sh (funded 0.5 BTC)
@github-actions
Copy link

github-actions bot commented Mar 4, 2026

🤖 Codex Code Review

  1. Medium: sample-wallet detection is name-based, which can produce false positives/negatives
  • Changed logic treats a wallet as “sample” purely by wallet.name:
  • This can misclassify wallets:
    • A user-created non-sample wallet named "Bacon" hides the Bacon sample option.
    • A sample wallet renamed at creation may still appear as “not yet added,” allowing duplicate sample onboarding.
  • Suggestion: track sample wallets by a stable identifier (e.g., known descriptor/checksum/metadata) instead of display name.
  1. Low: test coverage doesn’t cover the above edge cases
  • New tests validate happy-path visibility and selection, but all sample/non-sample distinction assertions use only name fixtures:
  • Add cases for:
    • Non-sample wallet with sample-like name.
    • Sample wallet added with edited name.
    • This would lock in intended behavior and prevent regressions.

No additional performance or security issues stood out in the modified diff.

@github-actions
Copy link

github-actions bot commented Mar 4, 2026

🤖 Claude Code Review

Pull Request Review: Satoshi Genesis Sample Wallet & Onboarding Refactor

Commit Summary

Commit Description
8922681 Core feature: add Satoshi Genesis sample wallet, refactor single-to-multi sample wallet system
f968e21 Dead code removal: handleSelectSampleWallet and unused SAMPLE_WALLETS import
ae7b41b Dead code removal: handleSkipToForm, new test for all-samples-added edge case
9f167c0 Fix: translate guidesPrompt key across all 9 locale files

Overall Assessment

This is a well-structured set of commits. The refactor cleanly generalises the single-wallet Bacon pattern into an extensible multi-wallet system, dead code is removed promptly as a follow-up, and the guidesPrompt translation oversight is addressed in a separate commit. The commit history is readable and messages explain the "why" clearly.

No critical issues were found — no security vulnerabilities, no breaking API contract changes, no data integrity risks.


Issues & Improvements

Medium — Bitcoin Correctness: Bare pubkey used as descriptor on mainnet

File: frontend/src/components/add-wallet-form.tsx, lines 31 and 46

const GENESIS_PUBKEY = "04678afdb0fe5548271967f1a67130b7105cd6a828e03909a67962e0ea1f61deb..."

{
  descriptor: GENESIS_PUBKEY,  // bare hex pubkey, not a BDK descriptor
  networkOverrides: {
    regtest: { descriptor: "bcrt1q20lu6ldqtssq7y7ewarlamlzldnmyk5w4n3e97" },
  },
}

The bare uncompressed pubkey hex is not a valid BDK output descriptor. On mainnet, the backend will receive this raw hex as the descriptor field. This is worth verifying — if the frontend's DESCRIPTOR_REGEX validation runs on this, it may reject the form before submission. The comment in source acknowledges testnet "will be empty," but doesn't clarify whether mainnet actually works end-to-end.

Recommendation: Verify with a live mainnet test, and add a code comment explaining what the backend does with this raw pubkey and whether it produces a usable result.


Low — Logic: hasOnlySampleWallets relies on vacuous truth

File: frontend/src/app/wallets/add/[[...slug]]/page.tsx, line 98

const hasOnlySampleWallets = wallets.every(w => sampleWalletNames.includes(w.name))

Array.every() returns true on an empty array. This works today since an empty wallet list should show the sample prompt, but the variable name implies "has at least one sample wallet." This is a latent readability/maintenance hazard.

Recommendation:

const hasOnlySampleWallets = wallets.length === 0 || wallets.every(w => sampleWalletNames.includes(w.name))

Low — Robustness: Name-based deduplication is fragile

File: frontend/src/components/wallet-wizard/wallet-type-selector.tsx, lines 43–44

const addedWalletNames = new Set(wallets.map(w => w.name))
const availableSampleWallets = SAMPLE_WALLETS.filter(sw => !addedWalletNames.has(sw.name))

A user who creates a real wallet named "Bacon" or "Satoshi (Genesis)" will suppress the corresponding sample wallet button, even though the descriptor is different. Wallet identity everywhere else uses checksum. This is low-risk for an onboarding aid, but diverges from the rest of the system's identity model.


Low — State Management: selectedSampleSlug not reset when wallet list changes

File: frontend/src/components/wallet-wizard/wallet-type-selector.tsx, lines 40–44

If a user submits a sample wallet successfully and returns to the page before redirect completes, selectedSampleSlug would still be set to a slug that's been filtered out of availableSampleWallets. The form would remain prefilled while no button appears active. A useEffect that resets the slug when it's no longer in availableSampleWallets would make state self-consistent.


Very Low — Maintainability: Slug-as-JSON-key constraint is undocumented

"sampleWallets": {
  "satoshi-genesis": "Use Satoshi Genesis Address"
}

Accessed via t(`add.wizard.sampleWallets.${wallet.slug}`). This pattern requires slugs to be valid next-intl key path characters. A slug containing a . would silently fail lookup. A brief comment in the SampleWallet interface would prevent future issues.


Test Coverage

The test suite is thorough. The following scenarios are covered:

Scenario Status
Both sample buttons shown for zero wallets
Neither shown when a non-sample wallet exists
One button shown when the other sample is already added
Prompt hidden when all samples added
Neither shown in cloud mode
Bacon fills form inline
Satoshi fills form inline

One gap: No test verifies that the amber-banner text changes from trySampleWallet to samplePrefilled after a sample wallet button is clicked. Minor, since both are derived from the same selectedSample object.


i18n Completeness

All 9 locale files (da, de-DE, en-US, es-419, fr-FR, ja, nb, pt-BR, sv) are fully updated with the new keys. The guidesPrompt key was initially missing in non-English locales and correctly fixed in 9f167c0.


Dead Code Removal Quality

Clean. All three removed items — handleSelectSampleWallet, handleSkipToForm, and the SAMPLE_WALLETS import in useWalletWizard — are fully absent from the codebase with no remaining references.


Summary

Category Finding Severity
Bitcoin correctness Bare pubkey hex used as descriptor on mainnet; may fail frontend validation Medium
Logic hasOnlySampleWallets relies on vacuous truth; variable name misleading Low
Robustness Name-based sample wallet deduplication can be fooled by user-named wallets Low
State management selectedSampleSlug not reset if wallet list changes while mounted Low
Test coverage No test for amber-banner text change after sample button click Very Low
i18n All 9 locales fully covered ✅ Clean
Dead code All removed items fully absent ✅ Clean
Security No issues ✅ Clean

…import

Sample wallet selection is now inline (no navigation), so the
handleSelectSampleWallet handler and its wiring are no longer needed.
Address review feedback: remove handleSkipToForm dead code from the
wizard hook, and add a test verifying the sample prompt hides when
both sample wallets have already been added.
@schjonhaug schjonhaug merged commit 3e0fc6d into master Mar 4, 2026
4 of 5 checks passed
@schjonhaug schjonhaug deleted the feature/satoshi-genesis-sample-wallet branch March 4, 2026 12:30
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