PendingSignerRequest.signerPubkey is unreliable
Labels: bug
Problem
signerPubkey is the only thing attributing a pending request to an account. It is
not trustworthy today, which blocks any multi-account aggregation.
Empty during login (NIP-46 and NIP-07)
packages/ndk/lib/data_layer/repositories/signers/nip46_event_signer.dart:132
signerPubkey: cachedPublicKey ?? '',
cachedPublicKey is only assigned once getPublicKeyAsync() resolves (line 268),
and that call goes through remoteRequest itself. So the first requests, connect
and get_public_key, carry an empty string. That is exactly when the UI most needs
to say who it is waiting on.
packages/ndk_flutter/lib/signers/src/nip07_event_signer_web.dart:59 has the
identical pattern, with cachedPublicKey assigned at line 213 inside a
_trackRequest call.
Related: #237, which covers the sync/async getPublicKey design that forces this
caching in the first place.
npub vs hex (NIP-55)
packages/ndk_flutter/lib/data_layer/repositories/signers/nip55_event_signer.dart:84
uses the raw constructor-injected publicKey, which may be an npub. Line 49 decodes
it to hex for _currentUser, but neither signerPubkey nor getPublicKey() (line
147) do.
Accounts.addAccount keys accounts on signer.getPublicKey(), so a NIP-55 account
can sit under an npub key while a NIP-46 account sits under hex. An aggregated map
would then hold two entries for what may be the same identity.
Ambiguous semantics
The doc comment says "The public key of the signer (the account making the request)".
For a bunker those are two different keys: the user pubkey and
connection.remotePubkey. The field currently holds the former. This needs to be
decided explicitly, and possibly split into two fields, if the UI is ever to show
which signer a request is waiting on.
Suggested fix
- NIP-46 and NIP-07: populate the pubkey as early as possible instead of
?? ''.
- NIP-55: normalize to hex.
- Entity: pin down the semantics in the doc comment.
Breaking change to decide
Normalizing NIP-55 to hex changes what getPublicKey() returns, therefore the key
the account is registered under in Accounts.accounts. Existing callers doing
hasAccount(npub) or switchAccount(pubkey: npub) would break.
The alternative is to normalize in Accounts.addAccount, which fixes every signer at
once but has a wider blast radius.
Preference: the narrow fix in NIP-55 plus a changelog entry, since NIP-55 specifies
hex ("All pubkeys in this NIP are in hex format") and the npub form is already an
anomaly there.
PendingSignerRequest.signerPubkeyis unreliableLabels:
bugProblem
signerPubkeyis the only thing attributing a pending request to an account. It isnot trustworthy today, which blocks any multi-account aggregation.
Empty during login (NIP-46 and NIP-07)
packages/ndk/lib/data_layer/repositories/signers/nip46_event_signer.dart:132cachedPublicKeyis only assigned oncegetPublicKeyAsync()resolves (line 268),and that call goes through
remoteRequestitself. So the first requests,connectand
get_public_key, carry an empty string. That is exactly when the UI most needsto say who it is waiting on.
packages/ndk_flutter/lib/signers/src/nip07_event_signer_web.dart:59has theidentical pattern, with
cachedPublicKeyassigned at line 213 inside a_trackRequestcall.Related: #237, which covers the sync/async
getPublicKeydesign that forces thiscaching in the first place.
npub vs hex (NIP-55)
packages/ndk_flutter/lib/data_layer/repositories/signers/nip55_event_signer.dart:84uses the raw constructor-injected
publicKey, which may be an npub. Line 49 decodesit to hex for
_currentUser, but neithersignerPubkeynorgetPublicKey()(line147) do.
Accounts.addAccountkeysaccountsonsigner.getPublicKey(), so a NIP-55 accountcan sit under an npub key while a NIP-46 account sits under hex. An aggregated map
would then hold two entries for what may be the same identity.
Ambiguous semantics
The doc comment says "The public key of the signer (the account making the request)".
For a bunker those are two different keys: the user pubkey and
connection.remotePubkey. The field currently holds the former. This needs to bedecided explicitly, and possibly split into two fields, if the UI is ever to show
which signer a request is waiting on.
Suggested fix
?? ''.Breaking change to decide
Normalizing NIP-55 to hex changes what
getPublicKey()returns, therefore the keythe account is registered under in
Accounts.accounts. Existing callers doinghasAccount(npub)orswitchAccount(pubkey: npub)would break.The alternative is to normalize in
Accounts.addAccount, which fixes every signer atonce but has a wider blast radius.
Preference: the narrow fix in NIP-55 plus a changelog entry, since NIP-55 specifies
hex ("All pubkeys in this NIP are in hex format") and the npub form is already an
anomaly there.