Persist primary/fallback wallet addresses in the settings domain#1102
Persist primary/fallback wallet addresses in the settings domain#1102bitcoin-coder-bob wants to merge 7 commits into
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (1)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![arkanaai[bot]](https://avatars.githubusercontent.com/u/170820085?s=60&v=4){kind=small}
There was a problem hiding this comment.
Arkana Code Review — arkd#1102
Scope: Settings domain, DB persistence (postgres + sqlite + badger + redis), admin API, bootstrap config.
Risk level: Medium-high — wallet connection addresses control which gRPC endpoint handles signing, address derivation, and sweeps. A malicious wallet_addr redirects all fund operations.
🔴 Bug: Cannot clear fallback addresses via admin API
internal/interface/grpc/handlers/adminservice.go (new code in parseSettings):
var walletFallbackAddrs *[]string
if len(settings.WalletFallbackAddrs) > 0 {
t := settings.GetWalletFallbackAddrs()
walletFallbackAddrs = &t
}Proto3 repeated fields default to an empty slice. Sending wallet_fallback_addrs: [] is indistinguishable from not sending the field at all. Once fallback addresses are set, an operator cannot clear them back to zero through the admin API — the if len(...) > 0 guard silently drops the update.
Fix: Use a different sentinel, or accept that an empty repeated field means "clear all fallbacks". At minimum, document the limitation. Compare with how other optional fields use != nil checks — repeated fields need different handling.
🟡 Fragile serialization: comma-separated addresses
internal/infrastructure/db/postgres/settings_repo.go:17-22 and identical code in sqlite/settings_repo.go:17-22:
func splitFallbackAddrs(s string) []string {
if s == "" { return nil }
return strings.Split(s, ",")
}And the write path uses strings.Join(s.WalletFallbackAddrs, ",").
- No validation prevents commas in addresses. A wallet address like
host:6060,metadata=foowould be silently split into two broken addresses on the next read. Add a validation check inUpdate()or at the persistence layer: reject addresses containing commas. - Code duplication:
splitFallbackAddrsis copy-pasted identically in bothpostgres/settings_repo.goandsqlite/settings_repo.go. Extract to a shared utility (e.g., a package-level helper in the parentdbpackage or indomain).
🟡 No validation of wallet addresses at persist time
internal/core/domain/settings.go — Update() and Validate():
The new WalletAddr and WalletFallbackAddrs fields are intentionally excluded from Validate() (per the PR description). But Update() also performs no validation. An admin could set wallet_addr to an empty string, garbage, or a syntactically invalid gRPC target. The server won't discover the problem until the next restart, when walletService() fails and the node refuses to boot.
Suggestion: At minimum, validate non-empty for wallet_addr when it's being explicitly updated (i.e., when u.WalletAddr != nil). A basic format check (contains :?) would catch fat-finger errors early rather than at next restart.
🟡 Security surface expansion — document the threat model
internal/config/config.go:780-791 (new code in walletService()):
if c.repo != nil {
settings, err := c.repo.Settings().Get(context.Background())
if err == nil && settings != nil {
if settings.WalletAddr != "" {
arkWallet = settings.WalletAddr
}
fallbackAddrs = settings.WalletFallbackAddrs
}
}This is the core change: the wallet gRPC target is now sourced from the DB rather than env-only. An attacker who compromises the admin macaroon can persist a malicious wallet_addr pointing to an attacker-controlled gRPC server. On next restart, the node would dial the attacker's server for all wallet operations — address derivation, signing, sweeps. This is equivalent to full fund theft.
The existing trust model (admin macaroon = full operator trust) arguably already covers this — an operator with the macaroon could also SSH in and change the env var. But the attack surface is now remote and persistent (a single API call persists the change, vs. requiring shell access).
Recommendations:
- Add a comment in
walletService()documenting this trust boundary - Consider logging the effective
wallet_addrat INFO level on startup so operators can audit it - In a follow-up, consider requiring a restart confirmation or a separate "dangerous settings" permission for wallet address changes
🟢 Minor nits
-
internal/core/domain/settings.go:362-365: The fallback addrs update doesn't check for nil slice vs. empty slice semantics. SettingWalletFallbackAddrsto&[]string{}stores an empty slice, but the comma-join produces"", whichsplitFallbackAddrsdecodes back tonil. Round-trip is lossless (both mean "no fallbacks") but the types differ — could surprise tests that usereflect.DeepEqual. -
internal/infrastructure/live-store/redis/settings.go: The DTO correctly storesWalletFallbackAddrs []stringas a native slice (JSON array). Good — no comma-separation issue here. But verify the redis DTO handlesnilvs[]string{}consistently on round-trip (JSONnullvs[]).
✅ What looks good
- Proto field numbers (23, 24) are sequential and don't collide
- Badger backend is struct-based — new fields serialize automatically, no migration needed
- Migration scripts for postgres and sqlite are correct (ADD COLUMN with NOT NULL DEFAULT '')
- The "env seeds first boot, settings win after" model is sound and well-documented
Update()correctly skipsValidate()bypass — it validates after applying all changes- Restart-only application is the right call for now; live re-dial is correctly deferred
- Test coverage for domain
Update()of the new fields is present - DB round-trip tests updated in
service_test.go - No cross-repo breakage: no downstream SDKs currently consume the
Settingsproto message - Not protocol-critical: doesn't touch VTXO handling, forfeit paths, round lifecycle, or exit paths directly
Verdict
Request changes on the parseSettings bug (cannot clear fallbacks) — this is a functional defect that will bite operators. The comma-injection and missing-validation issues are lower priority but should be addressed before this leaves draft.
Good overall structure and the stacked-PR approach is clean. The "settings as source of truth" model is the right architecture.
There was a problem hiding this comment.
Arkana Follow-up Review — arkd#1102 (commit 8a8c6d3)
Reviewing only the delta since my previous review.
What's fixed ✅
-
config.go:787-800— Error handling upgraded from silentif err == nilto aswitchwithlog.Warnf. Good. Thelen > 0guard on fallback addrs correctly preserves env-configured addresses through upgrades. -
splitFallbackAddrs(postgres + sqlite) — Now trims whitespace and drops empty entries. Handles edge cases like trailing commas or"a, b"from manual DB edits. Solid improvement. -
adminservice.go:900-903— The "can't clear fallbacks" limitation is now documented with a workaround (reconfigure env + re-seed). Acceptable for a draft/v1 — proto3 repeated semantics make this inherent.
Remaining nits (non-blocking)
- Code duplication:
splitFallbackAddrsis still identical inpostgres/settings_repo.go:17-28andsqlite/settings_repo.go:17-28. Extract to a shared package before this ships to reduce maintenance burden. - Write-path validation: Addresses containing commas will still corrupt the comma-separated column. A one-line check in
Update()orUpsert()would prevent this cheaply. - Trust boundary comment:
walletService()now sources gRPC targets from the DB — a comment noting this is admin-macaroon-trust-equivalent would help future auditors.
None of these are blocking. The core design is sound, the upgrade path is safe, and the new commit shows good attention to the review feedback.
Approving — not protocol-critical (no VTXO/signing/forfeit/round/exit path changes). The nits can land in a follow-up.
8a8c6d3 to
b53a45e
Compare
There was a problem hiding this comment.
Arkana Follow-up Review — arkd#1102 (commit b53a45e)
Delta since my last review (approval at 14:43Z). This commit addresses all three non-blocking nits:
- Deduplication ✅ —
splitFallbackAddrsremoved from both SQL repos, replaced by shareddomain.EncodeFallbackAddrs/domain.DecodeFallbackAddrs. All call sites updated. - Comma validation ✅ —
Validate()now rejects commas inWalletAddrandWalletFallbackAddrs. Closes the write-path corruption vector. Tests added. - Trust boundary comment + audit log ✅ —
walletService()now documents the trust boundary and logs the effective wallet target at startup.
No new issues. Approval stands.
b9db0bc to
49779ee
Compare
b53a45e to
8d85e08
Compare
There was a problem hiding this comment.
Arkana Follow-up Review — arkd#1102 (rebase + commit 8d85e08)
Branch was rebased; only new content is 8d85e085 docs: document wallet address settings in settings.md.
Docs change ✅ — Adds ARKD_WALLET_ADDR / ARKD_WALLET_FALLBACK_ADDRS to the seed env var table in docs/settings.md and clarifies wallet addresses follow the first-boot-only rule. Accurate, matches code behavior.
All three fixes from prior reviews remain present post-rebase:
- Shared
domain.EncodeFallbackAddrs/domain.DecodeFallbackAddrs(no more copy-paste) - Comma validation in
Validate() - Trust boundary comment + audit log in
walletService()
No new issues. Approval stands.
49779ee to
8affa20
Compare
8d85e08 to
f716630
Compare
8affa20 to
44379b3
Compare
f716630 to
5816204
Compare
44379b3 to
ef4ac4d
Compare
0a44e86 to
619219c
Compare
There was a problem hiding this comment.
Arkana Follow-up Review — arkd#1102 (rebase to 619219c)
Rebase-only push; same 4 commits with new SHAs. No content changes since my last review.
All prior fixes remain intact:
- Shared
domain.EncodeFallbackAddrs/domain.DecodeFallbackAddrs✅ - Comma validation in
Validate()✅ - Trust boundary comment + audit log in
walletService()✅ - Docs in
settings.md✅
Approval stands.
89271cc to
4f89460
Compare
619219c to
41f0a4a
Compare
4f89460 to
ccb8a46
Compare
41f0a4a to
c4850ec
Compare
ccb8a46 to
a248505
Compare
c4850ec to
214bc06
Compare
Move the primary and fallback arkd-wallet connection addresses into the DB-backed settings: seeded from env on first boot, sourced from the settings to dial the wallets, and settable via the admin Settings API. Wallet address changes are persisted immediately but only take effect on the next restart (re-dialing live is a follow-up).
- walletService only overrides env wallet addresses when the settings carry a value, so an upgrade (whose migrated row defaults them to empty) doesn't drop env-configured fallback wallets; log instead of swallowing a settings read error. - splitFallbackAddrs trims and drops empty entries, matching the env parser. - Document that the fallback list can be replaced but not cleared via the API.
- Validate() rejects a comma in the primary or any fallback wallet address, since they're persisted comma-separated (a comma would corrupt the column). - Move the comma-separated encode/decode into domain (EncodeFallbackAddrs / DecodeFallbackAddrs) so postgres and sqlite share one implementation. - Document the wallet-target trust boundary in walletService and log the effective primary wallet address at startup so operators can audit it.
a248505 to
b6bb1a1
Compare
214bc06 to
c4103c5
Compare
There was a problem hiding this comment.
Arkana Follow-up Review — arkd#1102 (rebase to c4103c5)
Rebase-only push; same 4 commits, identical diff (verified via checksum). No content changes since my last review.
All prior fixes remain intact. Approval stands — no new findings.
Resolve config.go dialFallbackWallets: combine #1102's settings-sourced fallbackAddrs parameter with sweep-fallback's []FallbackWallet return and dedup/self-reference validation. Pass the resolved primary (arkWallet) so the self-reference check is correct when the primary comes from settings, not just the WALLET_ADDR env var. Update the test call sites to the 2-arg signature.
The previous note read as a contradiction ('an exception: like the other
settings'). Spell out that wallet addresses are seeded+persisted like settings
but, unlike other settings, the env var stays a fallback on every boot when the
stored value is empty, which is what keeps upgrades (empty migrated columns)
working.
There was a problem hiding this comment.
Arkana Follow-up Review — arkd#1102 (commits e609b13, 4e9d540)
Scope: merge of bob/arkd-sweep-fallback base + docs clarification.
What's new
-
FallbackWalletstruct (config.go:674-677) — pairsAddr+Serviceso errors surface the wallet address instead of a positional index. Clean improvement for operator debugging. -
Dedup + self-reference validation (
config.go:900-910) —dialFallbackWalletsnow takesprimaryAddrand rejects a fallback that equals the primary or another fallback before dialing. Already-dialed wallets are properly closed on failure. Good. -
primaryThenFallbackshelper removed — inlined into bothsweeper.signingWallets()andadminService.signingWallets(). Both create a fresh slice (append([]ports.WalletService{...}, ...)) so no aliasing bug. Fine. -
Error messages in
ensureWalletReady(service.go) — switched from%d(index) to%q(address). Matches the newFallbackWallet.Addrfield. Good. -
Test coverage (
config_test.go:369-413) — new"fallback equal to primary hard-fails"and"duplicate fallback hard-fails"test cases verify the dedup logic, including that the dial count is correct (rejected entries are never dialed) and already-dialed wallets are closed. Solid. -
Docs (
docs/settings.md) — clarifies the wallet-address hybrid behavior (env stays a fallback when stored value is empty). Removes the contradictory wording from the previous version.
Observations (non-blocking)
-
String-equality dedup:
addr == primaryAddratconfig.go:902is string comparison, solocalhost:6060and127.0.0.1:6060would pass the check but point to the same wallet. This is an inherent limitation of address-string dedup and not a real-world problem (operators control both values), but worth noting. -
File rename:
sweep_fallback_internal_test.go→sweeper_test.gois a clean rename, no content changes. Good housekeeping.
Verdict
All new code since c4103c50 is correct, well-tested, and improves operability. No protocol-critical concerns in this delta. Previous review findings still apply to the earlier commits.
LGTM on the incremental changes. The full PR still touches wallet-address persistence (protocol-adjacent) — human review required before merge per standing policy.
Trim verbose comments across the wallet-address settings code (proto, config walletService, domain Settings, admin UpdateSettings, adminservice parse) and remove the UpdateSettings log that warned wallet-address changes apply on the next restart.
There was a problem hiding this comment.
Arkana Follow-up Review — arkd#1102 (commit 368bd80)
Scope: Comment trimming + removal of the restart warning log.
Changes reviewed
Commit 368bd808 is a cosmetic refactor:
- Trims verbose comments in
admin.proto,config.go,domain/settings.go,adminservice.go - Moves
walletAddr/walletFallbackAddrsvariable declarations to the top of thevarblock inparseSettings— cleaner - Removes the restart warning log from
admin.go:UpdateSettingsthat fired whenwallet_addrorwallet_fallback_addrswere changed
One minor note
admin.go — removed restart warning: The deleted code logged "settings field %q updated; wallet address changes take effect on restart" when wallet addresses were changed via the admin API. This was useful operator feedback — without it, an operator who changes the wallet address via the API gets no indication that a restart is required. The behavior is documented in settings.md, but runtime logs are the first thing operators see.
Not blocking — it's a UX nicety, not a correctness issue. If you want to keep the code lean, the doc is sufficient. If you want belt-and-suspenders, consider keeping a single log.Infof (not Warnf) like "wallet address updated in settings; takes effect on next restart".
Verdict
No correctness, security, or protocol issues introduced. All prior review feedback has been addressed. LGTM on this commit. ✅
There was a problem hiding this comment.
Review: Persist primary/fallback wallet addresses in the settings domain
Verdict: Approve. Clean, well-structured settings plumbing. Not protocol-critical (no VTXO/signing/forfeit/exit path changes). A few items worth addressing before merge.
Issues to address
1. Cannot clear fallback addresses via admin API — adminservice.go:928-930
if len(settings.WalletFallbackAddrs) > 0 {
t := settings.GetWalletFallbackAddrs()
walletFallbackAddrs = &t
}Proto3 repeated fields can't distinguish "not provided" from "empty list", so an empty list is silently treated as no-op. Once fallbacks are set, they can never be removed via the API. The PR mentions this in the body but there's no guard, doc comment, or API error telling the operator. At minimum, add a comment on the WalletFallbackAddrs field in SettingsUpdate explaining this limitation. Ideally, add a sentinel (e.g. a separate clear_wallet_fallback_addrs bool field) in a follow-up.
2. Upsert ON CONFLICT always overwrites wallet addresses — postgres/sqlc/query.sql:503-504, sqlite/sqlc/query.sql:512-513
wallet_addr = EXCLUDED.wallet_addr,
wallet_fallback_addrs = EXCLUDED.wallet_fallback_addrs,The upsert unconditionally overwrites. If Upsert is ever called after the initial seed (e.g. another settings field is seeded/updated via this code path), it would clobber admin-configured wallet addresses with whatever is in the Settings struct at that point. I believe the seed is gated on settings table being empty so this is safe today, but it's brittle. Confirm the seed is truly one-shot and add a comment on the upsert query noting this assumption.
3. No address format validation — settings.go:255-262
Only commas are rejected. An admin can set wallet_addr to "lol" or "" and it'll be persisted. The empty-primary case is caught at dial time (startup failure), but garbage addresses cause a needlessly opaque gRPC dial error on next boot instead of a clean validation error at update time. Consider a lightweight host:port parse check in Validate(). Low severity — admin-only, fail-fast on boot.
Observations (non-blocking)
4. Redis cache backward compat during rolling upgrade
If a new binary reads a Redis-cached settingsDTO written by the old binary, WalletAddr will be "" and WalletFallbackAddrs will be nil. The walletService() env fallback handles the primary correctly. Fallbacks will briefly not dial until the cache is refreshed. Acceptable for a config-level field, but worth noting in the operator docs.
5. DecodeFallbackAddrs on empty string — settings.go:268-278
strings.Split("", ",") returns [""], which the empty-entry filter correctly drops → returns nil. Good. Edge case handled.
6. Migration default values — postgres/migration, sqlite/migration
Both add NOT NULL DEFAULT ''. Existing rows get empty strings, which trigger the env fallback in walletService(). Upgrade path is sound.
7. Test coverage
- Domain
Validatecomma-rejection: ✅ - Domain
Updatechangelog: ✅ NewSettingswith new params: ✅- DB round-trip (
service_test.go): ✅ dialFallbackWalletssignature change: ✅- Missing: no test for the
walletService()settings-override-env logic itself (thec.repo != nilbranch). Consider a unit test that mocks the repo to return a settings with a different wallet addr and verifies it's used over the env value.
Not protocol-critical
This PR is pure settings/config plumbing. It does not touch VTXOs, transaction signing, forfeit paths, round lifecycle, exit paths, or connector trees. The wallet address is infrastructure config — the actual signing happens inside the wallet service, not here.
No cross-repo breakage: proto fields 26-27 are additive optional fields. No downstream consumer currently uses the admin Settings API.
LGTM. Merge when the comment on item #1 is added and item #2 is confirmed.
There was a problem hiding this comment.
Review: Persist primary/fallback wallet addresses in the settings domain
Verdict: Approve. Clean, well-structured settings plumbing. Not protocol-critical (no VTXO/signing/forfeit/exit path changes). A few items worth addressing before merge.
Issues to address
1. Cannot clear fallback addresses via admin API — adminservice.go:928-930
if len(settings.WalletFallbackAddrs) > 0 {
t := settings.GetWalletFallbackAddrs()
walletFallbackAddrs = &t
}Proto3 repeated fields can't distinguish "not provided" from "empty list", so an empty list is silently treated as no-op. Once fallbacks are set, they can never be removed via the API. The PR mentions this in the body but there's no guard, doc comment, or API error telling the operator. At minimum, add a comment on the WalletFallbackAddrs field in SettingsUpdate explaining this limitation. Ideally, add a sentinel (e.g. a separate clear_wallet_fallback_addrs bool field) in a follow-up.
2. Upsert ON CONFLICT always overwrites wallet addresses — postgres/sqlc/query.sql:503-504, sqlite/sqlc/query.sql:512-513
wallet_addr = EXCLUDED.wallet_addr,
wallet_fallback_addrs = EXCLUDED.wallet_fallback_addrs,The upsert unconditionally overwrites. If Upsert is ever called after the initial seed (e.g. another settings field is seeded/updated via this code path), it would clobber admin-configured wallet addresses with whatever is in the Settings struct at that point. I believe the seed is gated on settings table being empty so this is safe today, but it's brittle. Confirm the seed is truly one-shot and add a comment on the upsert query noting this assumption.
3. No address format validation — settings.go:255-262
Only commas are rejected. An admin can set wallet_addr to "lol" or "" and it'll be persisted. The empty-primary case is caught at dial time (startup failure), but garbage addresses cause a needlessly opaque gRPC dial error on next boot instead of a clean validation error at update time. Consider a lightweight host:port parse check in Validate(). Low severity — admin-only, fail-fast on boot.
Observations (non-blocking)
4. Redis cache backward compat during rolling upgrade
If a new binary reads a Redis-cached settingsDTO written by the old binary, WalletAddr will be "" and WalletFallbackAddrs will be nil. The walletService() env fallback handles the primary correctly. Fallbacks will briefly not dial until the cache is refreshed. Acceptable for a config-level field, but worth noting in the operator docs.
5. DecodeFallbackAddrs on empty string — settings.go:268-278
strings.Split("", ",") returns [""], which the empty-entry filter correctly drops → returns nil. Good. Edge case handled.
6. Migration default values — postgres/migration, sqlite/migration
Both add NOT NULL DEFAULT ''. Existing rows get empty strings, which trigger the env fallback in walletService(). Upgrade path is sound.
7. Test coverage
- Domain
Validatecomma-rejection: ✅ - Domain
Updatechangelog: ✅ NewSettingswith new params: ✅- DB round-trip (
service_test.go): ✅ dialFallbackWalletssignature change: ✅- Missing: no test for the
walletService()settings-override-env logic itself (thec.repo != nilbranch). Consider a unit test that mocks the repo to return a settings with a different wallet addr and verifies it's used over the env value.
Not protocol-critical
This PR is pure settings/config plumbing. It does not touch VTXOs, transaction signing, forfeit paths, round lifecycle, exit paths, or connector trees. The wallet address is infrastructure config — the actual signing happens inside the wallet service, not here.
No cross-repo breakage: proto fields 26-27 are additive optional fields. No downstream consumer currently uses the admin Settings API.
LGTM. Merge when the comment on item #1 is added and item #2 is confirmed.
|
Closing this. We've decided LP wallet addresses should be infrastructure config (env vars), not runtime-mutable settings, so the settings-domain persistence + admin CRUD this PR adds is not something we want to carry. The env-var path is already complete in the stack:
Both env vars are already documented in the README on the branches that introduced them ( |
1 participant
Why
The final step of the multi-LP work: make the settings domain the source of truth for the primary/fallback
arkd-walletdesignation, instead of reading the addresses straight from env on every boot. Now that #939 (settings domain) has merged, the wallet addresses can be persisted, seeded from env on first boot, dialed from the settings, and changed via the admin API.Stacked PRs (merge top-down):
add Settings domain with DB persistence and admin CRUD API #939— ✅ merged intomastermasterbob/arkd-sweep-fallback(Sweep fallback across primary and fallback arkd-wallets #1101); merges lastWhat changes
Domain (
core/domain/settings.go)SettingsgainsWalletAddr stringandWalletFallbackAddrs []string;NewSettingstakes them;SettingsUpdate+Update()make them partially-updatable (changelog:wallet_addr,wallet_fallback_addrs).Validate()only rejects a comma in any address (the SQL list separator); an empty primary is caught at dial time, not inValidate(). They're left out of the infoDigest(operator connection config, not a client-facing protocol param).Persistence (all three backends)
wallet_addr/wallet_fallback_addrscolumns (postgres + sqlite); sqlc regenerated. Fallbacks are stored comma-separated. Badger is struct-based (automatic). Redis cache DTO carries both. The first-boot seed writes them through.Bootstrap (
config.go)getSettings()seeds the addresses from env (ARKD_WALLET_ADDR/ARKD_WALLET_FALLBACK_ADDRS).walletService()now dials the primary + fallbacks from the persisted settings (the settings table is seeded before the wallet is dialed, so the ordering is sound), falling back to env if the repo isn't wired yet. This is what makes an admin address change actually take effect (on restart).Admin API (
admin.proto+ handler)Settingsproto gainswallet_addr(optional) andwallet_fallback_addrs(repeated);GetSettingsreturns them andUpdateSettingsaccepts them. Awallet_addr/wallet_fallback_addrschange is persisted but only takes effect on the next restart.Tests
Updateof wallet addresses (changelog + values).go build ./...,make lint(0 issues), and the touched non-infra packages pass.Open: live re-dial (why this is a draft)
This PR applies wallet-address changes on restart, not live. Re-dialing at runtime is the remaining piece and is deliberately deferred because:
The "applied on restart" behaviour is footgun-free: addresses are changeable via the admin API (and env still seeds first boot), unlike a seed-only-immutable approach.
Operator note
ARKD_WALLET_ADDR/ARKD_WALLET_FALLBACK_ADDRSnow seed the settings on first boot only; after that the persisted settings win and the addresses are changed via the admin Settings API (effective on restart). See [docs/settings.md] for the broader env-seeds-first-boot model.