Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[NWC] Add an encryption tag to negotiate upgrading to NIP44. #1780

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
141 changes: 120 additions & 21 deletions 47.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,16 @@ Fundamentally NWC is communication between a **client** and **wallet service** b

4. Once the payment is complete the **wallet service** will send an encrypted `response` (kind 23195) to the **user** over the relay(s) in the URI.

5. The **wallet service** may send encrypted notifications (kind 23196) of wallet events (such as a received payment) to the **client**.
5. The **wallet service** may send encrypted notifications (kind 23197 or 23196) of wallet events (such as a received payment) to the **client**.

## Events

There are four event kinds:

- `NIP-47 info event`: 13194
- `NIP-47 request`: 23194
- `NIP-47 response`: 23195
- `NIP-47 notification event`: 23196
- `NIP-47 notification event`: 23197 (23196 for backwards compatibility with NIP-04)

### Info Event

Expand All @@ -46,34 +47,72 @@ The content should be a plaintext string with the supported capabilities space-s

If the **wallet service** supports notifications, the info event SHOULD contain a `notifications` tag with the supported notification types space-separated, eg. `payment_received payment_sent`.

It should also contain supported encryption modes as described in the [Encryption](#encryption) section. For example:

```jsonc
{
"kind": 13194,
"tags": [
["encryption", "nip44_v2 nip04"], // List of supported encryption schemes as described in the Encryption section.
["notifications", "payment_received payment_sent"]
// ...
],
"content": "pay_invoice get_balance make_invoice lookup_invoice list_transactions get_info notifications",
// ...
}
```

### Request and Response Events

Both the request and response events SHOULD contain one `p` tag, containing the public key of the **wallet service** if this is a request, and the public key of the **client** if this is a response. The response event SHOULD contain an `e` tag with the id of the request event it is responding to.
Optionally, a request can have an `expiration` tag that has a unix timestamp in seconds. If the request is received after this timestamp, it should be ignored.

The content of requests and responses is encrypted with [NIP04](04.md), and is a JSON-RPCish object with a semi-fixed structure:
The content of requests and responses is encrypted with [NIP44](44.md), and is a JSON-RPCish object with a semi-fixed structure.

Request:
```jsonc
**Important note for backwards-compatibility:** The initial version of the protocol used [NIP04](04.md). If a **wallet service** or client app does not include the `encryption` tag in the
`info` or request events, it should be assumed that the connection is using NIP04 for encryption. See the [Encryption](#encryption) section for more information.

Example request:

```js
{
"method": "pay_invoice", // method, string
"params": { // params, object
"invoice": "lnbc50n1..." // command-related data
}
"kind" 23194,
"tags": [
["encryption", "nip44_v2"],
["p", "03..." ] // public key of the wallet service.
// ...
],
"content": nip44_encrypt({ // Encryption type corresponds to the `encryption` tag.
"method": "pay_invoice", // method, string
"params": { // params, object
"invoice": "lnbc50n1..." // command-related data
}
}),
}
```

Response:
```jsonc
Example response:

```js
{
"result_type": "pay_invoice", //indicates the structure of the result field
"error": { //object, non-null in case of error
"code": "UNAUTHORIZED", //string error code, see below
"message": "human readable error message"
},
"result": { // result, object. null in case of error.
"preimage": "0123456789abcdef..." // command-related data
}
"kind" 23195,
"tags": [
["encryption", "nip44_v2"],
["p", "03..." ] // public key of the requesting client app
["e", "1234"] // id of the request event this is responding to
// ...
],
"content": nip44_encrypt({
"result_type": "pay_invoice", //indicates the structure of the result field
"error": { //object, non-null in case of error
"code": "UNAUTHORIZED", //string error code, see below
"message": "human readable error message"
},
"result": { // result, object. null in case of error.
"preimage": "0123456789abcdef..." // command-related data
}
})
// ...
}
```

Expand All @@ -83,9 +122,9 @@ If the command was successful, the `error` field must be null.

### Notification Events

The notification event SHOULD contain one `p` tag, the public key of the **client**.
The notification event is a kind 23197 event (23196 if using nip04) SHOULD contain one `p` tag, the public key of the **user**.

The content of notifications is encrypted with [NIP04](04.md), and is a JSON-RPCish object with a semi-fixed structure:
The content of notifications is encrypted with [NIP44](44.md) (or NIP-04 for legacy client apps), and is a JSON-RPCish object with a semi-fixed structure:

```jsonc
{
Expand All @@ -96,6 +135,7 @@ The content of notifications is encrypted with [NIP04](04.md), and is a JSON-RPC
}
```

_Note on backwards-compatibility:_ If a **wallet service** supports both nip44 and nip04 for legacy client apps, it should publish both notification events for each notification - kind 23196 encrypted with NIP-04, and kind 23197 encrypted with NIP-44. It is up to the **client** to decide which event to listen to based on its supported encryption and declared supported encryption schemes of the **wallet service** in the `info` event.

### Error codes
- `RATE_LIMITED`: The client is sending commands too fast. It should retry in a few seconds.
Expand All @@ -105,6 +145,7 @@ The content of notifications is encrypted with [NIP04](04.md), and is a JSON-RPC
- `RESTRICTED`: This public key is not allowed to do this operation.
- `UNAUTHORIZED`: This public key has no wallet connected.
- `INTERNAL`: An internal error.
- `UNSUPPORTED_ENCRYPTION`: The encryption type of the request is not supported by the wallet service.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will this be used? If yes, how will this error be encrypted? 😄

I think it's better to not respond to such requests to reduce complexity...

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch on the encryption point! I'd kind of lost the fact that errors also need to be encrypted. Thinking through this a bit, I'm not sure I love not responding to these events because that's not very debuggable on the client app side and will leave client apps hanging until they time out, so it's not a great ux either. We can't encrypt the error using the client's encryption type because by definition in this case the wallet service doesn't support it.

I think the best behavior here is actually to just encrypt the error content using the wallet service's default encryption scheme. Of course the client won't be able to decrypt it, but at least they'll get a decryption error while debugging, which will tip them in the right direction. There may even be some cases where it was actually just the client's encryption tag which was wrong, and so they will see the error message. For example:

  1. Client forgets to populate the encryption tag in a request or sends something like nip44 or nip44v2 instead of nip44_v2
  2. Wallet service sees this and doesn't understand, so it responds with a nip44 encrypted UNSUPPORTED_ENCRYPTION error
  3. Client app can actually still decrypt this and see the right error type to nudge them to re-check their encryption tags.

Does that make sense to you? If so, I can add some clarification to this PR.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the other hand, building a lot of extra logic to try to catch all the cases where a developer doesn't implement the spec properly is a slippery slope and makes more complicated code to account for all these cases as Adithya mentioned. But it's a good point that developers who cannot see the wallet service logs will find it harder to debug. In Alby Hub the developer can see the logs, but this won't be the case for services where the developer is not the owner of the backend.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After thinking about this more, I still feel pretty strongly that the debugging experience would be pretty painful if servers just ignore these messages entirely. As a client app developer, it's pretty likely that I didn't write the software for my wallet and that I'm not running it locally. It feels like a pretty easy foot-gun to write nip44 instead of nip44_v2 and spend the next several hours confused why your app doesn't work. IMO being defensive here is just good devX.

I do understand your point about not trying to catch every case where a developer doesn't implement the spec correctly, but in this case, even doing nothing is a choice in special handling. Even then you have already looked at the message, seen an invalid encryption scheme, and then decided to do nothing instead of returning an error. I don't think either case is really much extra handling tbh.

- `OTHER`: Other error.

## Nostr Wallet Connect URI
Expand Down Expand Up @@ -499,6 +540,63 @@ Notification:
2. **wallet service** verifies that the author's key is authorized to perform the payment, decrypts the payload and sends the payment.
3. **wallet service** responds to the event by sending an event with kind `23195` and content being a response either containing an error message or a preimage.

## Encryption

The initial version of NWC used [NIP-04](04.md) for encryption which has been deprecated and replaced by [NIP-44](44.md). NIP-44 should always be preferred for encryption, but there may be legacy cases
where the **wallet service** or **client** has not yet migrated to NIP-44. The **wallet service** and **client** should negotiate the encryption method to use based on the `encryption` tag in the `info` event.

The negotiation works as follows.

1. The **wallet service** includes an `encryption` tag in the `info` event. This tag contains a space-separated list of encryption schemes that the **wallet service** supports (eg. `nip44_v2 nip04`)
2. The **client application** includes an `encryption` tag in each request event. This tag contains the encryption scheme which should be used for the request. The **client application** should always prefer nip44 if supported by the **wallet service**.

### Info event

First, the **wallet service** adds an `encryption` tag to its `info` event containing a space-separated list of encryption schemes it supports. For example,
if a wallet service supports nip44, but also allows backwards-compatibility to nip04 client applications, its `encryption` tag in the `info` event might look something like:

```jsonc
{
"kind": 13194,
"tags": [
["encryption", "nip44_v2 nip04"],
// ...
],
"content": "pay_invoice get_balance make_invoice lookup_invoice list_transactions get_info",
// ...
}
```

When a **client application** establishes a connection, it should read the info event and look for the `encryption` tag.

**Absence of this tag implies that the wallet only supports nip04.**

If the `encryption` tag is present, the **client application** will choose optimal encryption supported by both itself, and the **wallet service**, which should always be nip44 if possible.

### Request events

When a **client application** sends a request event, it should include a `encryption` tag with the encryption scheme it is using. The scheme MUST be supported by the **wallet service** as indicated by the info event.
For example, if the client application supports nip44, the request event might look like:

```jsonc
{
"kind": 23194,
"tags": [
["encryption", "nip44_v2"],
// ...
],
// ...
}
```

If the **wallet service** does not support the specified encryption scheme, it will return an `UNSUPPORTED_ENCRYPTION` error. Absence of the `encryption` tag indicates use of nip04 for encryption.

### Notification events

As described above in the [Notifications](#notifications) section, if a **wallet service** supports both nip04 and nip44, it should publish two notification events for each notification - kind 23196 encrypted with NIP-04, and kind 23197 encrypted with NIP-44. If the **wallet service** only supports nip44, it should only publish kind 23197 events.

The **client** should check the `encryption` tag in the `info` event to determine which encryption schemes the **wallet service** supports, and listen to the appropriate notification event. Clients that support both nip04 and nip44 can choose to only listen to 23197 events for simplicity.

## Using a dedicated relay
This NIP does not specify any requirements on the type of relays used. However, if the user is using a custodial service it might make sense to use a relay that is hosted by the custodial service. The relay may then enforce authentication to prevent metadata leaks. Not depending on a 3rd party relay would also improve reliability in this case.

Expand All @@ -513,6 +611,7 @@ This NIP does not specify any requirements on the type of relays used. However,
"created_at": 1713883677,
"kind": 13194,
"tags": [
[ "encryption", "nip44_v2 nip04" ],
[
"notifications",
"payment_received payment_sent"
Expand Down