platform-wallet: out-of-wallet identities lose their public keys, contradicting the load-side doc

[Claudius-Maginificent]

TL;DR

platform-wallet's "unowned identity" storage silently drops an identity's public keys when it's added, but the docs on the read side promise the opposite — that a loaded identity is "usable without a second call." A consumer that trusts that doc for an out-of-wallet identity gets a silent, empty key map instead of an error.

User story

As a downstream consumer of platform-wallet/platform-wallet-storage (e.g. dash-evo-tool, storing masternode/evonode identities it doesn't own a wallet for), I want the out-of-wallet identity API to either persist the keys I pass it or clearly document that it won't, so that I don't build a feature on a doc promise the storage layer doesn't keep.

Scenario

Actual behavior: IdentityManager::add_out_of_wallet_identity takes a full dpp::identity::Identity (public keys included) but converts it via ManagedIdentity::new_out_of_wallet(identity), which — per its own doc comment — produces "ManagedIdentity rows with wallet_id == None and an empty public-key map." Unlike the wallet-owned path (add_identity), which explicitly calls managed_identity.keys_snapshot_changeset() and persists an identity_keys changeset alongside the identity, add_out_of_wallet_identity persists only an IdentityChangeSet — no keys changeset at all, ever.

Meanwhile, SqlitePersister::load_unowned_identities's doc says: "Keys are folded in exactly as load() does for wallet-owned identities, so a returned ManagedIdentity is usable without a second call." That's accurate about the fold mechanism, but it creates a false impression for any row that entered through add_out_of_wallet_identity — there was never anything to fold in, so the "usable" ManagedIdentity comes back with a permanently empty key map, silently.

The gap is invisible from the writer's side too: nothing in add_out_of_wallet_identity or its callers is told the keys it was handed were discarded.

Expected behavior: One of:

1. add_out_of_wallet_identity persists the identity's public keys the same way add_identity does (call keys_snapshot_changeset() and include it in the stored changeset) — the natural fix, since the caller already has the keys in hand; or
2. If keeping the out-of-wallet bucket key-less is intentional (e.g. because upstream write paths for this bucket are meant to be read-only/observed identities without signing capability), say so explicitly on add_out_of_wallet_identity itself, and update load_unowned_identities's doc to not claim the loaded identity is "usable without a second call" when it never held keys in the first place.

Detailed discussion

Found while reviewing dash-evo-tool PR #955, which uses add_out_of_wallet_identity to mirror masternode/evonode identities (identities DET has no owning wallet for, but for which it does hold real keys — e.g. the masternode voting key — in its own vault) into the upstream store so they survive wallet-removal cascades and boot reconciliation can find them. DET consumes only the identity IDs from the mirror today (a diff against its own local state), so this gap is currently inert for that specific caller — but it's a trap for the next consumer who reads a mirrored identity's public_keys and trusts the load-path doc.

Relevant code (as of 4784de03):

• rs-platform-wallet/src/wallet/identity/state/manager/lifecycle.rs — add_identity (persists keys via keys_snapshot_changeset()) vs. add_out_of_wallet_identity (does not).
• rs-platform-wallet/src/wallet/identity/state/managed_identity/identity_ops.rs:52 — keys_snapshot_changeset is pub(crate), so an external consumer can't call it to compensate on its own side even if it wanted to.
• rs-platform-wallet-storage/src/sqlite/persister.rs:575 — the load_unowned_identities doc comment making the "usable without a second call" claim.

Option 1 above (persist the keys) seems like the more consistent fix given add_identity's own precedent, but I don't have visibility into whether the out-of-wallet bucket was deliberately designed to stay signing-incapable (its own doc separately describes it as "observed read-only," which DET's actual usage — a device that can sign with some of these identities — doesn't quite match either; that's a separate discussion for dash-evo-tool, not filed here). Flagging both options in case the read-only framing is intentional and only the doc needs fixing.

_{🤖 Co-authored by Claudius the Magnificent AI Agent}

[Claudius-Maginificent]

Two related observations from the same PR #955 review, both about the out-of-wallet/"unowned" bucket this issue already covers — adding here rather than opening separate issues since they're the same mechanism.

The anti-promotion defense's own justifying comment is now false

rs-platform-wallet-storage/src/sqlite/schema/identities.rs:135-138:

/// Scope-keying also covers the promotion case (an existing NULL-parented
/// row being pulled into a wallet that already fills the slot) at no
/// extra cost. That case is defensive only: no production write creates a
/// `wallet_id IS NULL` `identities` row, since the flush scope is always
/// the persister's bound wallet id.

add_out_of_wallet_identity (this issue's subject) is exactly such a production write — DET calls it from WalletBackend::register_unowned_identity on every masternode/evonode it discovers. The defense the comment calls "defensive only" is load-bearing today: it's what keeps a wallet-less identity's row from being silently claimed the next time some wallet's manager flushes. Worth updating the comment so the next reader doesn't conclude the code path is dead and remove it.

The bucket's own "observed read-only" framing doesn't match at least one real consumer

Three places describe the out-of-wallet bucket as strictly observational:

• manager/mod.rs:20: out_of_wallet_identities[identity_id] — observed read-only.
• managed_identity/identity_ops.rs:95-96: "signing / derivation paths must guard on these and reject the out-of-wallet case explicitly."
• manager/lifecycle.rs:95: "Add an identity to the out-of-wallet (observed read-only) bucket."

DET's masternode/evonode identities are not observational in that sense — DET holds real signing keys for at least some of them (e.g. the masternode voting key) in its own vault, and does sign with them; it just has no wallet backing them, which is the only thing this bucket actually models. The data model conflates two different things: "has no owning wallet" and "is read-only/unsigned." They're coincident for DET's usage today, but not the same property, and a future upstream feature that treats "out-of-wallet" as license to prune, GC, or otherwise deprioritize these rows (reasonable, given the doc) would take DET's real node records with it without warning.

Not asking for a specific fix here, since I don't know upstream's intended direction for this bucket — flagging so it's visible if/when add_out_of_wallet_identity (or the bucket's semantics generally) gets revisited alongside the keys gap this issue is about.

_{🤖 Co-authored by Claudius the Magnificent AI Agent}