Laan Tungir
9afbb8fcbd
The CLI was hand-building cJSON params and calling the low-level nsigner_client_call for all 16 verbs. Now it uses the high-level nostr_signer_t typed wrappers from nostr_core_lib for 14 of 16 verbs: get_info, get_public_key (alg + nostr), sign_event, mine_event, nip04/44 encrypt+decrypt, sign, verify, derive, encapsulate, decapsulate, derive_shared_secret, otp encrypt/decrypt. The 'call' verb (raw passthrough) and 'derive --algorithm' still use the low-level nsigner_client_call on the shared connection (created via nostr_signer_nsigner_from_client). The 'list' verb uses nsigner_transport_list_unix directly. The client shrank from 945 to ~840 lines, with the per-verb cJSON building logic now in the library. Error messages use nostr_signer_last_error() to surface the raw n_signer RPC error text (path_not_allowed, unknown_role, etc.). Also adds two plan docs: - plans/client_breaking_change_audit.md: audit of n_signer breaking changes vs all ~/lt/ client repos - plans/nostr_core_lib_full_verb_coverage.md: analysis of the library verb coverage gap that motivated this refactor Tests: 45/45 pass, 0 fail, 2 skip (test_n_signer_client.sh).
180 lines
8.8 KiB
Markdown
180 lines
8.8 KiB
Markdown
# Audit: n_signer Breaking Changes vs Client Repos
|
||
|
||
## 1. The breaking changes made to n_signer
|
||
|
||
Three changes on the n_signer wire protocol are breaking for every existing
|
||
client. All three are landed in `src/` and documented in `README.md` §4.
|
||
|
||
### 1.1 Verb renames (legacy names removed)
|
||
|
||
Source: [`plans/legacy_verb_aliases.md`](legacy_verb_aliases.md) — COMPLETED.
|
||
|
||
| Old wire verb | New wire verb |
|
||
|--------------------|----------------------------|
|
||
| `sign_event` | `nostr_sign_event` |
|
||
| `mine_event` | `nostr_mine_event` |
|
||
| `nip04_encrypt` | `nostr_nip04_encrypt` |
|
||
| `nip04_decrypt` | `nostr_nip04_decrypt` |
|
||
| `nip44_encrypt` | `nostr_nip44_encrypt` |
|
||
| `nip44_decrypt` | `nostr_nip44_decrypt` |
|
||
| `get_public_key` (role branch) | `nostr_get_public_key` |
|
||
|
||
The role-based `get_public_key` was split: algorithm-based stays
|
||
`get_public_key`; Nostr-protocol key selection is now `nostr_get_public_key`.
|
||
The old alias names are **gone** — no shim, no fallthrough.
|
||
|
||
### 1.2 Selector model rewrite (nostr_index / index removed for nostr verbs)
|
||
|
||
Source: [`plans/role_path_authorization.md`](role_path_authorization.md).
|
||
|
||
- `nostr_index` selector → **removed**, rejected with error `2006
|
||
nostr_index_deprecated` (see [`src/dispatcher.c`](../src/dispatcher.c:1815)).
|
||
- `index` on `nostr_*` verbs → **removed**, rejected with `2007
|
||
index_deprecated`.
|
||
- The **only** accepted selector for `nostr_*` verbs is now `{"role":"<name>",
|
||
"role_path":"<full-path>"}` sent **together**. Either field alone is
|
||
rejected: `2008 role_required` / `2009 path_required`
|
||
([`README.md`](../README.md) §4.6).
|
||
- No backward compatibility. `--nostr-index` / `--index` on the client CLI are
|
||
removed; replaced by `--role` + `--path`.
|
||
|
||
### 1.3 OTP encoding values changed
|
||
|
||
`encrypt` / `decrypt` (algorithm `otp`) now take `encoding` =
|
||
`"ascii"` (ASCII-armored, default) or `"binary"` (base64 raw `.otp` blob)
|
||
([`src/dispatcher.c`](../src/dispatcher.c:1452), [`src/otp_pad.c`](../src/otp_pad.c:347)).
|
||
|
||
Note: [`client/n_signer_client.c`](../client/n_signer_client.c:51) help text
|
||
still advertises `--encoding <base64|hex>` — that is a **stale doc string**
|
||
inside n_signer's own client and should be fixed to `ascii|binary`.
|
||
|
||
---
|
||
|
||
## 2. Are these reflected in the nostr_core_lib repo? — NO
|
||
|
||
`nostr_core_lib` is the shared client library that every C-based n_signer
|
||
client links against. It is **out of date** and will fail against current
|
||
n_signer. Specific gaps:
|
||
|
||
### 2.1 Still emits the removed `nostr_index` selector
|
||
|
||
[`nostr_core_lib/nostr_core/nostr_signer.c`](../../nostr_core_lib/nostr_core/nostr_signer.c:309)
|
||
`signer_remote_params_with_selector()` emits `{"nostr_index":N}` when set
|
||
(lines 320–327). n_signer now rejects this with `2006 nostr_index_deprecated`.
|
||
|
||
The public API
|
||
[`nostr_signer_nsigner_set_nostr_index()`](../../nostr_core_lib/nostr_core/nostr_signer.c:858)
|
||
still exists and is the documented way to select a key — it is now a dead end.
|
||
|
||
### 2.2 Sends `role` without `role_path`
|
||
|
||
When `nostr_index` is not set, the same helper emits only `{"role":"..."}`
|
||
(line 341) with no `role_path`. n_signer now requires both and rejects
|
||
role-only with `2009 path_required`.
|
||
|
||
The `nostr_signer_nsigner_*` factory constructors
|
||
([`nostr_signer.h`](../../nostr_core_lib/nostr_core/nostr_signer.h:51)) take a
|
||
single `const char* role` parameter — there is no way to pass a `role_path`
|
||
through the high-level API at all.
|
||
|
||
### 2.3 `derive` (HMAC) path is half-broken
|
||
|
||
[`nostr_signer.c`](../../nostr_core_lib/nostr_core/nostr_signer.c:560) builds
|
||
`{"algorithm":"secp256k1","index":N}` for the `derive` verb. The
|
||
algorithm-based `derive` verb still accepts `index`, so the `nostr_index` branch
|
||
works. But the `role`-only branch (line 563) sends `{"role":"..."}` with no
|
||
`index` — `derive` requires `index` and will reject it.
|
||
|
||
### 2.4 Documentation is stale
|
||
|
||
[`NSIGNER_INTEGRATION.md`](../../nostr_core_lib/nostr_core/NSIGNER_INTEGRATION.md:123)
|
||
still tells integrators to use `nostr_index` and `role`-only selectors, and
|
||
[`plans/nostr_core_lib_client_updates.md`](../../nostr_core_lib/plans/nostr_core_lib_client_updates.md)
|
||
proposes `nostr_index` support as the chosen design — both predate the
|
||
selector rewrite.
|
||
|
||
### 2.5 What needs to change in nostr_core_lib
|
||
|
||
1. Replace the `role`-only + `nostr_index` selector model with a combined
|
||
`role` + `role_path` selector. Concretely: change the `nostr_signer_nsigner_*`
|
||
constructors (or add new ones / a selector struct) to accept both a role
|
||
name and a full path.
|
||
2. Remove `nostr_signer_nsigner_set_nostr_index` (or repurpose it to set
|
||
`role` + `role_path` from an index by expanding the NIP-06 template
|
||
`m/44'/1237'/N'/0/0` client-side).
|
||
3. Update `signer_remote_params_with_selector` to always emit both `role` and
|
||
`role_path`.
|
||
4. Fix the `derive` remote path to always include `index`.
|
||
5. Update `NSIGNER_INTEGRATION.md`, `nostr_core_lib_client_updates.md`, and
|
||
`tests/nsigner_client_test.c` (which sends `nostr_get_public_key` with a
|
||
`nostr_index` selector at line 297).
|
||
|
||
---
|
||
|
||
## 3. Repos in ~/lt/ that need client edits
|
||
|
||
### Tier 1 — Direct n_signer wire clients (BROKEN now)
|
||
|
||
These talk the n_signer JSON-RPC protocol directly and will fail against
|
||
current n_signer:
|
||
|
||
| Repo | Files | Problem |
|
||
|------|-------|---------|
|
||
| **nostr_core_lib** | `nostr_core/nostr_signer.c`, `nostr_signer.h`, `nsigner_client.c`, `NSIGNER_INTEGRATION.md`, `tests/nsigner_client_test.c`, `examples/note_poster.c` | Emits removed `nostr_index`; sends `role` without `role_path`. Shared lib — fixing this fixes all C clients that link it. |
|
||
| **nostr_terminal** | `src/nsigner_client.c`, `include/nsigner_client.h`, `src/signer.c`, `src/menu_login.c`, `src/menu_profile.c`, `plans/n_signer_integration.md` | Has its own hand-rolled `nsigner_client` that sends `{"nostr_index":N}` ([`nsigner_client.c`](../../nostr_terminal/src/nsigner_client.c:617)). Selector struct is `has_nostr_index`/`nostr_index`/`role` with no `role_path`. Login menu prompts for "index" only. |
|
||
| **sovereign_browser** | `src/login_dialog.c`, `src/agent_login.c`, `src/key_store.c`, `src/key_store.h` | Uses `nostr_signer_nsigner_*` from nostr_core_lib + `nostr_signer_nsigner_set_nostr_index`. UI has a nostr_index spin button. Breaks via the lib, and the UI needs a role+path input. |
|
||
| **laantungir_website** | `scripts/publish_nostr.js`, `scripts/get_nsigner_pubkey.js` | Raw JSON-RPC over qrexec sending `{"nostr_index": N}` ([`publish_nostr.js`](../../laantungir_website/scripts/publish_nostr.js:103)). Will get `2006`. |
|
||
|
||
### Tier 2 — Indirect (breaks once Tier 1 lib is fixed, or uses nostr_core_lib local signing only)
|
||
|
||
| Repo | Status | Action |
|
||
|------|--------|--------|
|
||
| **n_signer** (this repo) | `client/n_signer_client.c` help text says `--encoding <base64\|hex>` but server wants `ascii\|binary`; the client itself already uses `--role`+`--path` correctly per [`role_path_authorization.md`](role_path_authorization.md). | Fix the stale `--encoding` help string. |
|
||
|
||
### Not affected (use local nostr_core_lib signing, not n_signer remote)
|
||
|
||
These call `nostr_create_and_sign_event` / `nostr_signer_local` with a local
|
||
private key — they do not speak the n_signer wire protocol and are unaffected:
|
||
|
||
- `open_wire` (local `sign_event` helper, not n_signer RPC)
|
||
- `raspberry_pi_zero_nostr` (local `nostr_create_and_sign_event`)
|
||
- `esp32_playground` (local `nostr_create_and_sign_event`)
|
||
|
||
### Not affected (NIP-46 to arbitrary remote signers, not n_signer)
|
||
|
||
These use NIP-46 method names (`sign_event`, `nip04_encrypt`, …) per the NIP-46
|
||
spec, targeting generic remote signers / browser extensions — not n_signer's
|
||
renamed verbs. No change needed unless they specifically add an n_signer
|
||
backend:
|
||
|
||
- `primal-web-app` (`src/lib/nip46/nip46.ts`)
|
||
- `super_ball` (`web/nostr.bundle.js`)
|
||
- `nips` (spec docs)
|
||
|
||
---
|
||
|
||
## 4. Recommended remediation order
|
||
|
||
1. **nostr_core_lib** first — it is the shared dependency. Introduce a
|
||
`role` + `role_path` selector (struct or new constructors), remove
|
||
`nostr_index` emission, fix `derive`, update tests + integration doc.
|
||
2. **sovereign_browser** — update login UI to collect role + path instead of
|
||
index; switch to the new nostr_core_lib API.
|
||
3. **nostr_terminal** — rewrite its hand-rolled `nsigner_client` selector to
|
||
`role` + `role_path`; update login/profile menus and the integration plan.
|
||
4. **laantungir_website** — switch the two JS scripts from `nostr_index` to
|
||
`role` + `role_path`.
|
||
5. **n_signer** — fix the stale `--encoding` help string in
|
||
`client/n_signer_client.c`.
|
||
|
||
A Mermaid overview of the dependency order:
|
||
|
||
```mermaid
|
||
flowchart LR
|
||
NS[n_signer wire changes] --> NCL[nostr_core_lib]
|
||
NCL --> SB[sovereign_browser]
|
||
NCL --> NT[nostr_terminal]
|
||
NS --> LW[laantungir_website]
|
||
NS --> NSC[n_signer client help text]
|
||
```
|