forgejo-admin/nuts
0
0
Fork 0
Code Issues 35 Pull requests 32 Projects Releases Packages Wiki Activity Actions

Cashu v1 – The Great Cleanup #55

Merged
callebtc merged 47 commits from the_great_cleanup_v1 into main 2023-12-18 21:47:00 +00:00
Conversation 49 Commits 47 Files changed 14 +618 -598
callebtc commented 2023-10-03 10:45:35 +00:00 (Migrated from github.com)
Copy link

The Great Cleanup

X

This is a PR that encapsulates a collection of changes to the protocol that will make our lives easier in the future. It cleans up early mistakes that were made as the protocol grew organically and removes implicit assumptions about the funding sources (e.g. Lightning) and currency units (e.g. "sats") used for Cashu mints that are inherent in the current protocol.

New v1 API

We are introducing the v1 API where we clean up many input and output models.

Click to show new endpoints

New endpoints

  • New: GET /v1/keys and GET /v1/keys/{keyset_id} (new Keyset model)
  • New: GET /v1/keysets (new Keysets model)
  • New: POST /v1/swap (new BlindedMessage model)
  • New: POST /v1/melt/quote/bolt11 (replaces GET /checkfees)
  • New: POST /v1/melt/bolt11
  • New: POST /v1/mint/quote/bolt11 (replaces GET /melt)
  • New: POST /v1/mint/bolt11
  • New: POST /v1/check (no changes)
  • New: GET /v1/info (no changes)
  • New: POST /v1/restore (new GetInfoResponse)
Click to show error response

Error responses

Error responses of the mint are now defined as a HTTP 400 response with a JSON body

{
  "detail": "oops",
  "code": 1337
}
Click to show deprecated endpoints

Deprecated endpoints

  • GET /keys, GET /keys/{keyset_id}
  • GET /keysets
  • POST /split
  • GET /mint
  • POST /mint
  • GET /checkfees
  • POST /melt
  • POST /check
  • GET /info
  • POST /restore

NUTs

  • The GET /checkfees endpoint (formerly NUT-03) is now retired
  • The POST /v1/swap endpoint is now in NUT-03 (formerly POST /split NUT-06).
  • NUT-06 is therefore free (suggestion: let's use it for deterministic secrets)

New request and response models

Click to show all changed models

NUT-01

GetKeysResponse

{
  "keysets": [
    {
      "id": <keyset_id_hex_str>,
      "unit": <currency_unit_str>,
      "keys": {
        <amount_int>: <public_key_str>,
        ...
      }
    }
  ]
}

NUT-02

GetKeysetsResponse

{
  "keysets": [
    {
      "id": <keyset_id_hex_str>,
      "unit": <currency_unit_str>,
      "active": <bool>
    },
    ...
  ]
}

NUT-03

PostSwapRequest

{
  "inputs": <Proofs>, <-- RENAMED
  "outputs": <BlindedMessages>,
}

PostSwapResponse

{
  "signatures": <BlindedSignatures> <-- RENAMED
}

NUT-04

PostMintQuoteBolt11Request

{
  "amount": <int>,
  "unit": <str_enum["sat"]>
}

PostMintQuoteBolt11Response

{
  "quote": <str>,
  "request": <str>,
  "paid": <bool>,
  "expiry": <int>
}

PostMintBolt11Request

{
  "quote": <str>,
  "outputs": <Array[BlindedMessage]>
}

PostMintBolt11Response

{
  "signatures": <Array[BlindSignature]> <-- RENAMED
}

NUT-05

PostMeltQuoteBolt11Request

{
  "request": <str>,
  "unit": <str_enum["sat"]>
}

PostMeltQuoteBolt11Response

{
  "quote": <str>,
  "amount": <int>,
  "fee_reserve": <int>,
  "paid": <bool>,
  "expiry": <int>
}

PostMeltBolt11Request

{
  "quote": <str>,
  "inputs": <Array[Proof]>
}

PostMeltBolt11Response

{
  "paid": <bool>,
  "payment_preimage": <str>
}

Quotes

We are also introducing quotes, a general way to register a mint and melt transaction with the mint that will work across different payment methods (bolt11, bolt12, on-chain, ...) and different currency units (sat, msat, usd, ...). Quotes add the ability for Lightning backends to decide the amount and currency of the ecash they need in order to receive or pay a Lightning payment. The flow remains very similar to before and will be illustrated with two examples in the following.

Mint Quotes

To mint ecash (output) via Lightning (input), the wallet first requests a MintQuote for a given amount of sats the wallet wants to mint. The MintQuote has an quote id and includes a Lightning invoice. The user pays the Lightning invoice and then calls /v1/mint referencing the previous quote it corresponds to.

Melt Quotes

To melt ecash (input) and make a Lightning payment (output), the wallet requests a MeltQuote for a given Lightning invoice it likes to pay. In the MeltQuote, the mint tells the wallet how many sats it needs to supply and what the fee reserve is in order for the mint to fulfill this request.

Diagram

image

Hexadecimal keyset IDs

Our keyset IDs are ugly and need special treatment for HTTP (base64 urlsafe). We switch to hexadecimal keyset IDs that are generated much like the previous ones. We also add a version byte as a prefix.

1 - sort public keys by their amount in ascending order
2 - concatenate all public keys to one string
3 - HASH_SHA256 the concatenated public keys
4 - take the first 16 characters of the hex-encoded hash
5 - prefix it with a keyset ID version byte

An example implementation in Python:

def derive_keyset_id(keys: Dict[int, PublicKey]) -> str:
	"""Deterministic derivation keyset_id from set of public keys."""
	sorted_keys = dict(sorted(keys.items()))
	pubkeys_concat = "".join([p.serialize().hex() for _, p in sorted_keys.items()])
	return "00" + hashlib.sha256(pubkeys_concat.encode("utf-8")).hexdigest()[:14]

BlindedMessage now has keyset id field

Outputs (BlindedMessages), now also have a keyset id field, like Proofs (inputs) and BlindSignatures do. With the id, the wallet tells the mint which keyset the client is expecting a signature from during a /v1/swap (created outputs), /v1/melt (change outputs), or /v1/mint (minted outputs). The requested id MUST be from an active keyset (part of the /v1/keys response and the /v1/keysets response). If the wallet uses an id that is not existent or not active (rotated-out of), the mint MUST refuse the transaction.

The BlindedMessage becomes

{
  "amount": int,
  "id": str # <-- NEW!
  "B_": hex_str
}
Click to show additional remarks

Additional remarks

Adding an id fixes a race condition we previously created workarounds for (see https://github.com/cashubtc/cashu-ts/pull/64 for example). Essentially, the mint's keys could have rotated between the wallet sending the outputs to sign to the mint, and the mint responding with a signature. We can now get rid of this code by making it part of the protocol.

Another critical issue that results from the same race condition is deterministic secret derivation: If a wallet deterministically derives secrets for keyset A, sends BlindedMessages to the mint and the mint rotated keys in the mean time, it would respond with BlindedSignatures keyset B. That means the wallet has incremented its deterministic secret derivation counter on the wrong keyset ID.

Standardized secrets

Wallets should use standardized secrets (32 bytes of randomness) in lowercase hex. Closes https://github.com/cashubtc/nuts/issues/54

# The Great Cleanup ![X](https://github.com/cashubtc/nuts/assets/93376500/b2767b06-799a-4987-be67-77c6987d628a) This is a PR that encapsulates a collection of changes to the protocol that will make our lives easier in the future. It cleans up early mistakes that were made as the protocol grew organically and removes implicit assumptions about the funding sources (e.g. Lightning) and currency units (e.g. "sats") used for Cashu mints that are inherent in the current protocol. # New `v1` API We are introducing the `v1` API where we clean up many input and output models. <details> <summary>Click to show new endpoints</summary> ### New endpoints - New: `GET /v1/keys` and `GET /v1/keys/{keyset_id}` (new `Keyset` model) - New: `GET /v1/keysets` (new `Keysets` model) - New: `POST /v1/swap` (new `BlindedMessage` model) - New: `POST /v1/melt/quote/bolt11` (replaces `GET /checkfees`) - New: `POST /v1/melt/bolt11` - New: `POST /v1/mint/quote/bolt11` (replaces `GET /melt`) - New: `POST /v1/mint/bolt11` - New: `POST /v1/check` (no changes) - New: `GET /v1/info` (no changes) - New: `POST /v1/restore` (new `GetInfoResponse`) </details> <details> <summary>Click to show error response</summary> ### Error responses Error responses of the mint are now defined as a HTTP 400 response with a JSON body ```json { "detail": "oops", "code": 1337 } ``` </details> <details> <summary>Click to show deprecated endpoints</summary> ### Deprecated endpoints - `GET /keys`, `GET /keys/{keyset_id}` - `GET /keysets` - `POST /split` - `GET /mint` - `POST /mint` - `GET /checkfees` - `POST /melt` - `POST /check` - `GET /info` - `POST /restore` </details> # NUTs - The `GET /checkfees` endpoint (formerly NUT-03) is now retired - The `POST /v1/swap` endpoint is now in NUT-03 (formerly `POST /split` NUT-06). - NUT-06 is therefore free (suggestion: let's use it for deterministic secrets) # New request and response models <details> <summary>Click to show all changed models</summary> ## NUT-01 `GetKeysResponse` ```json { "keysets": [ { "id": <keyset_id_hex_str>, "unit": <currency_unit_str>, "keys": { <amount_int>: <public_key_str>, ... } } ] } ``` ## NUT-02 `GetKeysetsResponse` ```json { "keysets": [ { "id": <keyset_id_hex_str>, "unit": <currency_unit_str>, "active": <bool> }, ... ] } ``` ## NUT-03 ### `PostSwapRequest` ```json { "inputs": <Proofs>, <-- RENAMED "outputs": <BlindedMessages>, } ``` ### `PostSwapResponse` ```json { "signatures": <BlindedSignatures> <-- RENAMED } ``` ## NUT-04 `PostMintQuoteBolt11Request` ```json { "amount": <int>, "unit": <str_enum["sat"]> } ``` `PostMintQuoteBolt11Response` ```json { "quote": <str>, "request": <str>, "paid": <bool>, "expiry": <int> } ``` `PostMintBolt11Request` ```json { "quote": <str>, "outputs": <Array[BlindedMessage]> } ``` PostMintBolt11Response ```json { "signatures": <Array[BlindSignature]> <-- RENAMED } ``` ## NUT-05 `PostMeltQuoteBolt11Request` ```json { "request": <str>, "unit": <str_enum["sat"]> } ``` `PostMeltQuoteBolt11Response` ```json { "quote": <str>, "amount": <int>, "fee_reserve": <int>, "paid": <bool>, "expiry": <int> } ``` `PostMeltBolt11Request` ```json { "quote": <str>, "inputs": <Array[Proof]> } ``` `PostMeltBolt11Response` ```json { "paid": <bool>, "payment_preimage": <str> } ``` </details> # Quotes We are also introducing `quotes`, a general way to register a mint and melt transaction with the mint that will work across different payment methods (bolt11, bolt12, on-chain, ...) and different currency units (sat, msat, usd, ...). Quotes add the ability for Lightning backends to decide the amount and currency of the ecash they need in order to receive or pay a Lightning payment. The flow remains very similar to before and will be illustrated with two examples in the following. ## Mint Quotes To mint ecash (output) via Lightning (input), the wallet first requests a `MintQuote` for a given amount of sats the wallet wants to mint. The `MintQuote` has an `quote` id and includes a Lightning invoice. The user pays the Lightning invoice and then calls `/v1/mint` referencing the previous `quote` it corresponds to. ## Melt Quotes To melt ecash (input) and make a Lightning payment (output), the wallet requests a `MeltQuote` for a given Lightning invoice it likes to pay. In the `MeltQuote`, the mint tells the wallet how many sats it needs to supply and what the fee reserve is in order for the mint to fulfill this request. ## Diagram <img width="1634" alt="image" src="https://github.com/cashubtc/nuts/assets/93376500/cd1756f2-c087-4058-82c9-ff388fd51e92"> # Hexadecimal keyset IDs Our keyset IDs are ugly and need special treatment for HTTP (base64 urlsafe). We switch to **hexadecimal keyset IDs** that are generated much like the previous ones. We also add a **version byte as a prefix.** ``` 1 - sort public keys by their amount in ascending order 2 - concatenate all public keys to one string 3 - HASH_SHA256 the concatenated public keys 4 - take the first 16 characters of the hex-encoded hash 5 - prefix it with a keyset ID version byte ``` An example implementation in Python: ```python def derive_keyset_id(keys: Dict[int, PublicKey]) -> str: """Deterministic derivation keyset_id from set of public keys.""" sorted_keys = dict(sorted(keys.items())) pubkeys_concat = "".join([p.serialize().hex() for _, p in sorted_keys.items()]) return "00" + hashlib.sha256(pubkeys_concat.encode("utf-8")).hexdigest()[:14] ``` # `BlindedMessage` now has keyset `id` field Outputs (`BlindedMessages`), now also have a keyset `id` field, like `Proofs` (inputs) and `BlindSignatures` do. With the `id`, the wallet tells the mint which keyset the client is expecting a signature from during a `/v1/swap` (created outputs), `/v1/melt` (change outputs), or `/v1/mint` (minted outputs). The requested `id` *MUST* be from an `active` keyset (part of the `/v1/keys` response and the `/v1/keysets` response). If the wallet uses an `id` that is not existent or not active (rotated-out of), the mint *MUST* refuse the transaction. The `BlindedMessage` becomes ```json { "amount": int, "id": str # <-- NEW! "B_": hex_str } ``` <details> <summary>Click to show additional remarks</summary> ### Additional remarks Adding an `id` fixes a race condition we previously created workarounds for (see https://github.com/cashubtc/cashu-ts/pull/64 for example). Essentially, the mint's keys could have rotated between the wallet sending the outputs to sign to the mint, and the mint responding with a signature. We can now get rid of this code by making it part of the protocol. Another critical issue that results from the same race condition is deterministic secret derivation: If a wallet deterministically derives secrets for keyset A, sends `BlindedMessages` to the mint and the mint rotated keys in the mean time, it would respond with `BlindedSignatures` keyset B. That means the wallet has incremented its deterministic secret derivation counter on the wrong keyset ID. </details> ### Standardized secrets Wallets should use standardized secrets (32 bytes of randomness) in lowercase hex. Closes https://github.com/cashubtc/nuts/issues/54
misovan (Migrated from github.com) reviewed 2023-10-03 10:45:35 +00:00
AngusP (Migrated from github.com) reviewed 2023-10-03 10:45:35 +00:00
dipunm (Migrated from github.com) reviewed 2023-10-03 10:45:35 +00:00
ebrakke (Migrated from github.com) reviewed 2023-10-03 10:45:35 +00:00
xphade (Migrated from github.com) reviewed 2023-10-03 10:45:35 +00:00
SuperPhatArrow (Migrated from github.com) reviewed 2023-10-03 10:45:35 +00:00
BilligsterUser (Migrated from github.com) reviewed 2023-10-03 10:45:35 +00:00
moonsettler (Migrated from github.com) reviewed 2023-10-03 10:45:35 +00:00
gandlafbtc (Migrated from github.com) reviewed 2023-10-03 10:45:35 +00:00
dipunm (Migrated from github.com) reviewed 2023-10-03 10:56:12 +00:00
01.md
@ -27,4 +32,4 @@
With curl:
```bash
dipunm (Migrated from github.com) commented 2023-10-03 10:56:12 +00:00
Copy link

👀

👀
dipunm (Migrated from github.com) reviewed 2023-10-03 10:59:09 +00:00
02.md
@ -53,0 +61,4 @@
"unit": "sat",
"active": True
},
{
dipunm (Migrated from github.com) commented 2023-10-03 10:59:09 +00:00
Copy link

This would imply that a single token can have proofs from multiple keysets.

Is there a use case for this? We could reduce the payload if we don't repeat the keyset id in a response.

While we're refactoring you know..... 😅

This would imply that a single token can have proofs from multiple keysets. Is there a use case for this? We could reduce the payload if we don't repeat the keyset id in a response. While we're refactoring you know..... 😅
thesimplekid (Migrated from github.com) reviewed 2023-10-03 19:40:39 +00:00
00.md
@ -128,2 +84,3 @@
##### Example JSON:
### Errors
In case of an error, mints respond with the HTTP status code `400` and include the following data in their response:
thesimplekid (Migrated from github.com) commented 2023-10-03 19:34:48 +00:00
Copy link

Since the old tokens are removed from the nut I think saying "new" is confusing.

Since the old tokens are removed from the nut I think saying "new" is confusing.
03.md
@ -5,3 +4,4 @@
`mandatory`
---
thesimplekid (Migrated from github.com) commented 2023-10-03 19:36:15 +00:00
Copy link

with the requested amount <amount> in satoshis.

Since keyset is being changed to support multiple units will this request always be in satoshis?

> with the requested amount `<amount>` in satoshis. Since keyset is being changed to support multiple units will this request always be in satoshis?
dipunm (Migrated from github.com) reviewed 2023-10-03 20:04:10 +00:00
03.md
@ -5,3 +4,4 @@
`mandatory`
---
dipunm (Migrated from github.com) commented 2023-10-03 20:04:10 +00:00
Copy link

Being able to specify unit is interesting... it can allow denominations of millisats and maybe even (don't shoot me) bits.

Being able to specify unit is interesting... it can allow denominations of millisats and maybe even (don't shoot me) bits.
callebtc (Migrated from github.com) reviewed 2023-10-11 15:17:55 +00:00
02.md
@ -53,0 +61,4 @@
"unit": "sat",
"active": True
},
{
callebtc (Migrated from github.com) commented 2023-10-11 15:17:54 +00:00
Copy link

Yes, this is possible according to the TokenV3 encoding. Some wallets use this to export their entire balance from multiple mints with a single token. Not sure how useful that is, something we might deprecate for a next encoding version.

Yes, this is possible according to the TokenV3 encoding. Some wallets use this to export their entire balance from multiple mints with a single token. Not sure how useful that is, something we might deprecate for a next encoding version.
callebtc (Migrated from github.com) reviewed 2023-10-11 15:31:09 +00:00
03.md
@ -5,3 +4,4 @@
`mandatory`
---
callebtc (Migrated from github.com) commented 2023-10-11 15:31:09 +00:00
Copy link

will this request always be in satoshis?

The new GET /v1/mint allows wallets to specify amount (example: 1234), currency unit (example: usd), and payment method method (example: bolt11) in the request body.

> will this request always be in satoshis? The new `GET /v1/mint` allows wallets to specify `amount` (example: `1234`), currency `unit` (example: `usd`), and payment method `method` (example: `bolt11`) in the request body.
thesimplekid (Migrated from github.com) reviewed 2023-10-12 04:34:47 +00:00
03.md
@ -5,3 +4,4 @@
`mandatory`
---
thesimplekid (Migrated from github.com) commented 2023-10-12 04:34:47 +00:00
Copy link

requested amount <amount> in satoshis.

In that case "in satoshis" should be removed I think.

> requested amount `<amount>` in satoshis. In that case "in satoshis" should be removed I think.
thunderbiscuit (Migrated from github.com) reviewed 2023-10-21 19:52:13 +00:00
thunderbiscuit (Migrated from github.com) left a comment
Copy link

A few overall comments:

  • The PR description states that the new v1 routes are to be used and the old ones are deprecated. Not sure how to handle that elegantly in the NUTs. Should examples for both be given? Or all examples be migrated to the v1? If so, most of those are not yet updated.
  • Breaking changes means we should update the test vectors
  • How should tokens handle v1? Are there corner cases there that could create issues? What if you bundle a token with a ton of inputs, some v1 and some not? Haven't thought about this deeply, just writing down thoughts as they come to mind...

Overall some great improvements in there (also lots of new code to write for us! 😅). I suggest that if there is intention to eventually move forward with a solution to #54, it should be included here as part of this fairly wide-ranging set of breaking changes.

A few overall comments: - The PR description states that the new v1 routes are to be used and the old ones are deprecated. Not sure how to handle that elegantly in the NUTs. Should examples for both be given? Or all examples be migrated to the v1? If so, most of those are not yet updated. - Breaking changes means we should update the test vectors - How should tokens handle v1? Are there corner cases there that could create issues? What if you bundle a token with a ton of inputs, some v1 and some not? Haven't thought about this deeply, just writing down thoughts as they come to mind... Overall some great improvements in there (also lots of new code to write for us! 😅). I suggest that if there is intention to eventually move forward with a solution to #54, it should be included here as part of this fairly wide-ranging set of breaking changes.
00.md
@ -126,3 +84,1 @@
This token format includes information about the mint as well. The field `proofs` is like a V1 token. Additionally, the field `mints` can include an array (list) of multiple mints from which the `proofs` are from. The `url` field is the URL of the mint. `ids` is a list of the keyset IDs belonging to this mint. It is important that all keyset IDs of the `proofs` must be present here to allow a wallet to map each proof to a mint.
##### Example JSON:
### Errors
thunderbiscuit (Migrated from github.com) commented 2023-10-21 18:44:16 +00:00
Copy link

Good call on removing those.

Good call on removing those.
00.md
@ -192,4 +133,4 @@
```json
{
"token": [
thunderbiscuit (Migrated from github.com) commented 2023-10-21 18:51:23 +00:00
Copy link

Not sure where this should be mentioned but I don't want to forget it so I'm commenting here:

Because the keyset IDs are now expressed as hex but the wallets are likely to use them as string identifiers, the capitalization of the hex matters (as opposed to the secrets, where only the underlying bytes matter). We should have somewhere in NUT-02 a line expressing that the keyset ids should always be lowercase (or uppercase if we prefer, but my personal preference is lowercase) when stored/sent over the wire.

Not sure where this should be mentioned but I don't want to forget it so I'm commenting here: Because the keyset IDs are now expressed as hex but the wallets are likely to use them as string identifiers, the capitalization of the hex matters (as opposed to the secrets, where only the underlying bytes matter). We should have somewhere in NUT-02 a line expressing that the keyset ids should always be lowercase (or uppercase if we prefer, but my personal preference is lowercase) when stored/sent over the wire.
01.md
@ -27,4 +32,4 @@
With curl:
```bash
thunderbiscuit (Migrated from github.com) commented 2023-10-21 18:57:09 +00:00
Copy link

May I suggest the spec use a valid keyset json object? This would allow for initial cross-checking of one's understanding (even though at this point you could also test/check using the test vector associated with NUT-01. All that's required here is to remove the ellipsis and calculate the actual keyset ID of this small keyset and replace the current one with it's correct counterpart.

May I suggest the spec use a valid keyset json object? This would allow for initial cross-checking of one's understanding (even though at this point you could also test/check using the test vector associated with NUT-01. All that's required here is to remove the ellipsis and calculate the actual keyset ID of this small keyset and replace the current one with it's correct counterpart.
01.md
thunderbiscuit (Migrated from github.com) commented 2023-10-21 19:00:23 +00:00
Copy link

This PR is called the great cleanup so I'll point this out: the files include links that are not valid, and I think that could be cleaned up here. My preference would be to add links at the bottom as needed in the document above instead of pre-emptively adding an arbitrary number (20) of them, for which many are non-existent NUTs.

This PR is called the great cleanup so I'll point this out: the files include links that are not valid, and I think that could be cleaned up here. My preference would be to add links at the bottom as needed in the document above instead of pre-emptively adding an arbitrary number (20) of them, for which many are non-existent NUTs.
02.md
@ -53,0 +59,4 @@
{
"id": "009a1f293253e41e",
"unit": "sat",
"active": True
thunderbiscuit (Migrated from github.com) commented 2023-10-21 19:11:01 +00:00
Copy link

I don't think the examples in other files are showcasing this version byte (they don't start with 00). Maybe adding a line as to why this version byte is useful might be good. Or you could just state it in the PR maybe so that we have a reference for later as per the thinking behind it.

I don't think the examples in other files are showcasing this version byte (they don't start with `00`). Maybe adding a line as to why this version byte is useful might be good. Or you could just state it in the PR maybe so that we have a reference for later as per the thinking behind it.
thunderbiscuit (Migrated from github.com) commented 2023-10-21 19:16:05 +00:00
Copy link
  1. Are we hashing the string of numbers and letters or the byte array here?
  2. This reminds me that this PR should also udpate the test vectors for the small breaking changes like this one.

My preference would be to hash the concatenated bytes of all public keys. That way it's all bytes until the very end, where you pull the first 8 bytes of the hash and use their hex-encoded format.

1. Are we hashing the string of numbers and letters or the byte array here? 2. This reminds me that this PR should also udpate the test vectors for the small breaking changes like this one. My preference would be to hash the concatenated bytes of all public keys. That way it's all bytes until the very end, where you pull the first 8 bytes of the hash and use their hex-encoded format.
thunderbiscuit (Migrated from github.com) commented 2023-10-21 19:16:58 +00:00
Copy link

Should this be .hexdigest()[:16] instead?

Should this be `.hexdigest()[:16]` instead?
thunderbiscuit (Migrated from github.com) commented 2023-10-21 19:19:52 +00:00
Copy link

Should these use the v1/ paths? Wondering if communication with the mint using the old non-v1 paths are expected to stay with the base64 and only the v1 paths are expected to migrate to the new keyset ids.

Should these use the `v1/` paths? Wondering if communication with the mint using the old non-v1 paths are expected to stay with the base64 and only the v1 paths are expected to migrate to the new keyset ids.
thunderbiscuit (Migrated from github.com) commented 2023-10-21 19:25:12 +00:00
Copy link

Because this is the response to the keys/<keysetid> GET request, I expected this response to not be an array of "keysets" since there can only be one (IIUC). Is there a reason for it to be that way? I'm thinking maybe this would work just as well:

{
  "id": "9bb9d58392cd823e",
  "symbol": "sat",
  "keys": {
    "1": "03a40f20667ed53513075dc51e715ff2046cad64eb68960632269ba7f0210e38bc",
    "2": "03fd4ce5a16b65576145949e6f99f445f8249fee17c606b688b504a849cdc452de",
    "4": "02648eccfa4c026960966276fa5a4cae46ce0fd432211a4f449bf84f13aa5f8303",
    "8": "02fdfd6796bfeac490cbee12f778f867f0a2c68f6508d17c649759ea0dc3547528",
    ...
  }
}
Because this is the response to the `keys/<keysetid>` GET request, I expected this response to not be an array of `"keysets"` since there can only be one (IIUC). Is there a reason for it to be that way? I'm thinking maybe this would work just as well: ```json { "id": "9bb9d58392cd823e", "symbol": "sat", "keys": { "1": "03a40f20667ed53513075dc51e715ff2046cad64eb68960632269ba7f0210e38bc", "2": "03fd4ce5a16b65576145949e6f99f445f8249fee17c606b688b504a849cdc452de", "4": "02648eccfa4c026960966276fa5a4cae46ce0fd432211a4f449bf84f13aa5f8303", "8": "02fdfd6796bfeac490cbee12f778f867f0a2c68f6508d17c649759ea0dc3547528", ... } } ```
02.md
@ -53,0 +61,4 @@
"unit": "sat",
"active": True
},
{
thunderbiscuit (Migrated from github.com) commented 2023-10-21 19:07:59 +00:00
Copy link

I agree that the "multiple mints per token" makes things more complicated than they could be, particularly because the use case for it is unclear. If you export your entire balance from multiple mints in a single token... at this point you're just exporting a data bundle; I think the word token looses its meaning and strength if it is too wide of a definition. Just my 2 cents. But like is stated above, this doesn't need to be solved here of course. Might just be something to think about for future refactorings.

I agree that the "multiple mints per token" makes things more complicated than they could be, particularly because the use case for it is unclear. If you export your entire balance from multiple mints in a single token... at this point you're just exporting a data bundle; I think the word token looses its meaning and strength if it is too wide of a definition. Just my 2 cents. But like is stated above, this doesn't need to be solved here of course. Might just be something to think about for future refactorings.
03.md
@ -12,3 +25,3 @@
## Example
Request of `Alice`:
**Request** of `Alice`:
thunderbiscuit (Migrated from github.com) commented 2023-10-21 19:40:32 +00:00
Copy link

Typo: stealing steal.

Typo: ~stealing~ _steal_.
thunderbiscuit (Migrated from github.com) commented 2023-10-21 19:42:09 +00:00
Copy link

I think this part was forgotten. Did you mean to finish this or remove it?

I think this part was forgotten. Did you mean to finish this or remove it?
03.md
@ -19,1 +32,4 @@
With the data being of the form `PostSwapRequest`:
```json
thunderbiscuit (Migrated from github.com) commented 2023-10-21 19:39:40 +00:00
Copy link

I think it would be good to add an example of the v1 request body.

I think it would be good to add an example of the v1 request body.
04.md
@ -15,2 +14,3 @@
# Mint quote
Request of `Alice`:
To request a mint quote, the wallet of `Alice` makes a `POST /v1/mint/quote/{method}` request where `method` is the payment method requested (here `bolt11`).
thunderbiscuit (Migrated from github.com) commented 2023-10-21 19:42:37 +00:00
Copy link

This might be a v1 path instead of the old one. Or does that change apply to the old requests as well?

This might be a v1 path instead of the old one. Or does that change apply to the old requests as well?
04.md
@ -36,0 +54,4 @@
Response of `Bob`:
```json
thunderbiscuit (Migrated from github.com) commented 2023-10-21 19:44:19 +00:00
Copy link

Again just a reminder that the keyset ids don't have the 00 prefix mentioned in NUT-02.

Again just a reminder that the keyset ids don't have the `00` prefix mentioned in NUT-02.
thunderbiscuit commented 2023-10-27 16:06:56 +00:00 (Migrated from github.com)
Copy link

Great discussions over the dev call the other day. I figured I'd add a small todo list of things that might need to be addressed here before I forget. Note that not all of those might be required, and I'm going off of memory so hopefully not missing anything.

  • Add note on standardized secrets (32 bytes of randomness), most likely transmitted as lowercase hex and why wallet should strive to follow the standard. Closes #54
  • Make sure the /info/ endpoint is future-proof and potentially adds messaging around breaking changes and supported features
  • Consider the idea of using different endpoints for different units (additional NUTs for additional units would define new endpoints). If so, decide how to handle the default bolt11 mint/ and melt/ endpoints and their definition in the NUTs (are they good as is or do they now require their own spec file?)
  • Add unit field to Proof object, requiring a new token format version (cashuB)
  • Update all examples to the v1 routes
  • Address comments from reviewers above
  • Potentially add errors enum with codes
Great discussions over the dev call the other day. I figured I'd add a small todo list of things that might need to be addressed here before I forget. Note that not all of those might be required, and I'm going off of memory so hopefully not missing anything. - [x] Add note on standardized secrets (32 bytes of randomness), most likely transmitted as lowercase hex and why wallet should strive to follow the standard. Closes #54 - [ ] Make sure the `/info/` endpoint is future-proof and potentially adds messaging around breaking changes and supported features - [x] Consider the idea of using different endpoints for different units (additional NUTs for additional units would define new endpoints). If so, decide how to handle the default bolt11 `mint/` and `melt/` endpoints and their definition in the NUTs (are they good as is or do they now require their own spec file?) - [ ] Add `unit` field to `Proof` object, requiring a new token format version (`cashuB`) - [x] Update all examples to the `v1` routes - [ ] Address comments from reviewers above - [ ] Potentially add errors enum with codes
❤️ 1
thesimplekid (Migrated from github.com) reviewed 2023-11-18 11:53:43 +00:00
02.md
@ -53,0 +59,4 @@
{
"id": "009a1f293253e41e",
"unit": "sat",
"active": True
thesimplekid (Migrated from github.com) commented 2023-11-18 11:53:43 +00:00
Copy link

My preference would be to hash the concatenated bytes of all public keys. That way it's all bytes until the very end, where you pull the first 8 bytes of the hash and use their hex-encoded format.

This is my preference as well but the example code below it looks like it is concatenating the string of hex encoded pubkey.

This reminds me that this PR should also udpate the test vectors for the small breaking changes like this one.

+1

> My preference would be to hash the concatenated bytes of all public keys. That way it's all bytes until the very end, where you pull the first 8 bytes of the hash and use their hex-encoded format. This is my preference as well but the example code below it looks like it is concatenating the string of hex encoded pubkey. > This reminds me that this PR should also udpate the test vectors for the small breaking changes like this one. +1
thesimplekid (Migrated from github.com) reviewed 2023-11-18 21:17:22 +00:00
02.md
thesimplekid (Migrated from github.com) commented 2023-11-18 21:17:22 +00:00
Copy link

@thunderbiscuit commented this previously but it was removed when the change to bytes was pushed. Step 4 states to take the first 16 characters of the hex encoded hash but the example implementation seems to take the first 14. I think it step 4 should take the first 14 characters, making the id 16 after the version is added.

@thunderbiscuit commented this previously but it was removed when the change to bytes was pushed. Step 4 states to take the first 16 characters of the hex encoded hash but the example implementation seems to take the first 14. I think it step 4 should take the first 14 characters, making the id 16 after the version is added.
thesimplekid (Migrated from github.com) reviewed 2023-11-18 21:30:45 +00:00
02.md
@ -53,0 +59,4 @@
{
"id": "009a1f293253e41e",
"unit": "sat",
"active": True
thesimplekid (Migrated from github.com) commented 2023-11-18 21:30:45 +00:00
Copy link

This is my understanding as well, and I would prefer this response unless there is a reason for the list.

This is my understanding as well, and I would prefer this response unless there is a reason for the list.
thesimplekid (Migrated from github.com) reviewed 2023-11-19 09:31:06 +00:00
04.md
@ -39,0 +80,4 @@
```bash
curl -X GET http://localhost:3338/v1/mint/quote/bolt11/DSGLX9kevM...
```
thesimplekid (Migrated from github.com) commented 2023-11-19 09:31:06 +00:00
Copy link

Can't comment on the correct line as it wasn't changed in this PR, but on line 77 [NUT-0] should be [NUT-00]

Can't comment on the correct line as it wasn't changed in this PR, but on line 77 [NUT-0] should be [NUT-00]
callebtc commented 2023-11-23 03:07:39 +00:00 (Migrated from github.com)
Copy link

I've completed most of the Todos! I would suggest deferring these remaining suggestions to later since the cleanup in this round focusses more on breaking changes in the API.

  • Make sure the /info/ endpoint is future-proof and potentially adds messaging around breaking changes and supported features

I think these can be added as we go. What we should probably do already now is to indicate which (method, unit) pairs a mint supports.

  • Add unit field to Proof object, requiring a new token format version (cashuB)

We can do it in cashuA and add a unit field in the JSON without breaking the format

  • Potentially add errors enum with codes

I would love this but I'm afraid it needs to be in another PR

Thanks for the detailed comments and many errors you've found!

I've completed most of the Todos! I would suggest deferring these remaining suggestions to later since the cleanup in this round focusses more on breaking changes in the API. > * Make sure the `/info/` endpoint is future-proof and potentially adds messaging around breaking changes and supported features I think these can be added as we go. What we should probably do already now is to indicate which (method, unit) pairs a mint supports. > * [ ] Add `unit` field to `Proof` object, requiring a new token format version (`cashuB`) We can do it in cashuA and add a `unit` field in the JSON without breaking the format > * [ ] Potentially add errors enum with codes I would love this but I'm afraid it needs to be in another PR Thanks for the detailed comments and many errors you've found!
👍 1
thesimplekid (Migrated from github.com) reviewed 2023-11-23 07:15:31 +00:00
01.md
@ -7,3 +7,3 @@
This describes the basic exchange of the public mint keys that the wallet user `Alice` uses to unblind `Bob`'s signature.
This document outlines the exchange of the public keys of the mint `Bob` with the wallet user `Alice`. `Alice` uses the keys to unblind `Bob`'s blind signatures (see [NUT-00][00]).
thesimplekid (Migrated from github.com) commented 2023-11-23 06:30:52 +00:00
Copy link

Should a mint have only on active keyset per unit?

Should a mint have only on active keyset per unit?
02.md
@ -122,4 +149,2 @@
Note that the mint needs to convert the URL-safe id back from `L3zxxRB_I8uE` to `L3zxxRB/I8uE` before it can look up the keys and respond to the request.
[00]: 00.md
thesimplekid (Migrated from github.com) commented 2023-11-23 06:36:23 +00:00
Copy link

I think a test vector for id generation should be included as part of this PR

I think a test vector for id generation should be included as part of this PR
03.md
@ -1,40 +1,85 @@
NUT-03: Request mint
NUT-03: Swap tokens
thesimplekid (Migrated from github.com) commented 2023-11-23 06:43:16 +00:00
Copy link

using them as inputs to invalidates them and request new

invalidates should be invalidate

> using them as inputs to invalidates them and request new invalidates should be invalidate
04.md
@ -3,3 +3,3 @@
`mandatory` `author: calle`
`mandatory`
thesimplekid (Migrated from github.com) commented 2023-11-23 06:48:11 +00:00
Copy link

I agree with limiting this nut to this. However, should a note be added on how this maybe expanded in the future or wait and define that when it happens?

I agree with limiting this nut to this. However, should a note be added on how this maybe expanded in the future or wait and define that when it happens?
05.md
@ -3,3 +3,3 @@
`mandatory` `author: calle`
`mandatory`
thesimplekid (Migrated from github.com) commented 2023-11-23 07:01:50 +00:00
Copy link

Is a call to the check fees endpoint still needed as the fee_reserve is included in the quote?

Is a call to the check fees endpoint still needed as the `fee_reserve` is included in the quote?
08.md
@ -1,7 +1,7 @@
NUT-08: Lightning fee return
thesimplekid (Migrated from github.com) commented 2023-11-23 07:10:44 +00:00
Copy link

I wonder if this should be made more general. I think most payment methods with fees will work in a similar way where the request is over paid and the change is returned as in cashu

I wonder if this should be made more general. I think most payment methods with fees will work in a similar way where the request is over paid and the change is returned as in cashu
09.md
thesimplekid (Migrated from github.com) commented 2023-11-23 07:13:59 +00:00
Copy link

How is this derived?

How is this derived?
moonsettler (Migrated from github.com) reviewed 2023-11-23 15:52:12 +00:00
01.md
@ -7,3 +7,3 @@
This describes the basic exchange of the public mint keys that the wallet user `Alice` uses to unblind `Bob`'s signature.
This document outlines the exchange of the public keys of the mint `Bob` with the wallet user `Alice`. `Alice` uses the keys to unblind `Bob`'s blind signatures (see [NUT-00][00]).
moonsettler (Migrated from github.com) commented 2023-11-23 15:52:12 +00:00
Copy link

yep. more than one just splits the anon set.

special case could be multiple currencies in the future (even with sidechains it's technically a floating exchange rate)

yep. more than one just splits the anon set. special case could be multiple currencies in the future (even with sidechains it's technically a floating exchange rate)
moonsettler commented 2023-11-23 16:01:54 +00:00 (Migrated from github.com)
Copy link

A general observation: in any context that is not obviously a keyset structure naming the keyset_id id is not intuitive at all.
ksid or keyset or keyset_id would probably be more appropriate. the other field names are pretty self explanatory except for the single letter stuff.

A general observation: in any context that is not obviously a keyset structure naming the keyset_id id is not intuitive at all. ksid or keyset or keyset_id would probably be more appropriate. the other field names are pretty self explanatory except for the single letter stuff.
👍 2
thesimplekid (Migrated from github.com) reviewed 2023-11-26 18:54:06 +00:00
00.md
@ -198,3 +139,3 @@
"proofs": List[Proof]
"proofs": Proofs
},
...
thesimplekid (Migrated from github.com) commented 2023-11-26 18:54:06 +00:00
Copy link

Think the example proof secrets here should be updated to the recommended 32 byte hex secret, even though that is not enforced.

Think the example proof secrets here should be updated to the recommended 32 byte hex secret, even though that is not enforced.
thesimplekid (Migrated from github.com) reviewed 2023-11-26 18:58:11 +00:00
03.md
@ -1,40 +1,85 @@
NUT-03: Request mint
NUT-03: Swap tokens
thesimplekid (Migrated from github.com) commented 2023-11-26 18:58:11 +00:00
Copy link

Same comment as above, secret should be the recommended 32 byte hex

Same comment as above, secret should be the recommended 32 byte hex
ngutech21 commented 2023-11-29 13:43:21 +00:00 (Migrated from github.com)
Copy link

Hexadecimal keyset IDs
...
4 - take the first 16 characters of the hex-encoded hash

The descriptions says to take the first 16 characters, but in the python implementation the first 14 characters are used.

>Hexadecimal keyset IDs ... > 4 - take the first 16 characters of the hex-encoded hash The descriptions says to take the first 16 characters, but in the python implementation the first 14 characters are used.
ngutech21 (Migrated from github.com) requested changes 2023-11-29 20:07:10 +00:00
01.md
ngutech21 (Migrated from github.com) commented 2023-11-29 14:25:58 +00:00
Copy link

The example response is missing a pair of curly braces. Keysets returns a list of objects like in the example above (Response GetKeysResponse of Bob:)

The example response is missing a pair of curly braces. Keysets returns a list of objects like in the example above (Response `GetKeysResponse` of `Bob`:)
02.md
@ -13,2 +12,3 @@
## Multiple keysets
A wallet can ask the mint for all active keyset IDs via the `GET /keysets` endpoint. A wallet **CAN** request the list of active keyset IDs from the mint upon startup and, if it does so, **MUST** choose only tokens from its database that have a keyset ID supported by the mint to interact with it.
#### Active keysets
ngutech21 (Migrated from github.com) commented 2023-11-29 20:00:12 +00:00
Copy link

Both endpoints /keys and /keysets are very similar. What was the general idea of having two endpoints that almost return the same data? Now /keys and /keysets both return unit and id and can therefore drift apart. In my opinion it would be easier to just have one endpoint that returns all key related data. If the client is only interested in active keysets, this could be accomplished by a query parameter. Having just one endpoint would be easier to maintain and result in less redundant code.

Both endpoints /keys and /keysets are very similar. What was the general idea of having two endpoints that almost return the same data? Now /keys and /keysets both return `unit` and `id` and can therefore drift apart. In my opinion it would be easier to just have one endpoint that returns all key related data. If the client is only interested in active keysets, this could be accomplished by a query parameter. Having just one endpoint would be easier to maintain and result in less redundant code.
03.md
@ -1,40 +1,85 @@
NUT-03: Request mint
NUT-03: Swap tokens
ngutech21 (Migrated from github.com) commented 2023-11-29 20:06:08 +00:00
Copy link

I think the term split doesn't fit well anymore since we got rid of the amount field. This endpoint should be renamed to /swap, because it is less specific. Not every call to /split is indeed a split, but it transforms the inputs to outputs.

I think the term split doesn't fit well anymore since we got rid of the amount field. This endpoint should be renamed to /swap, because it is less specific. Not every call to /split is indeed a split, but it transforms the inputs to outputs.
04.md
@ -3,3 +3,3 @@
`mandatory` `author: calle`
`mandatory`
ngutech21 (Migrated from github.com) commented 2023-11-27 08:05:01 +00:00
Copy link

typo: The word "fullfil" should be spelled as "fulfill".

typo: The word "fullfil" should be spelled as "fulfill".
thunderbiscuit (Migrated from github.com) reviewed 2023-11-29 21:06:21 +00:00
thunderbiscuit (Migrated from github.com) left a comment
Copy link

Tons of great work in here. I left some more comments. I see you have a few more todo items in your PR description so I don't want to approve it before you're actually done, but this is looking good IMO.

One thing I think doesn't cause problem but I want to make sure I ask: are the v1 routes in any way impacting the way the tokens were before and are now built? As in, I don't think mixing up proofs from old routes and proofs and new one from this newer cashu workflow can cause trouble inside a single cashu token, but can you confirm this to be true?

Lastly, I don't know where this might fit but there is a high-level mental model that took me a few weeks to really grasp I don't know why, and that was the key for me to not get lost in the sauce with all that new jargon/verbiage. Here is how I would write it (again don't know where it fits or if it's even needed, maybe just split into parts at the top of the split/melt/mint files?)

The Cashu protocol defines 3 types of interactions that can happen between a client and a mint, where the client can exchange:

  1. ecash tokens for ecash tokens (called a split operation)
  2. bitcoin for ecash tokens (called a mint operation)
  3. ecash tokens for bitcoin (called a melt operation)
Tons of great work in here. I left some more comments. I see you have a few more todo items in your PR description so I don't want to approve it before you're actually done, but this is looking good IMO. One thing I _think_ doesn't cause problem but I want to make sure I ask: are the v1 routes in any way impacting the way the tokens were before and are now built? As in, I don't _think_ mixing up proofs from old routes and proofs and new one from this newer cashu workflow can cause trouble inside a single cashu token, but can you confirm this to be true? Lastly, I don't know where this might fit but there is a high-level mental model that took me a few weeks to really grasp I don't know why, and that was the key for me to not get lost in the sauce with all that new jargon/verbiage. Here is how I would write it (again don't know where it fits or if it's even needed, maybe just split into parts at the top of the split/melt/mint files?) The Cashu protocol defines 3 types of interactions that can happen between a client and a mint, where the client can exchange: 1. ecash tokens for ecash tokens (called a **split** operation) 2. bitcoin for ecash tokens (called a **mint** operation) 3. ecash tokens for bitcoin (called a **melt** operation)
00.md
@ -54,4 +57,4 @@
```json
{
"amount": int,
"C_": hex_str,
thunderbiscuit (Migrated from github.com) commented 2023-11-29 19:55:38 +00:00
Copy link

I agree with @moonsettler here and would rename to keyset (or keyset_id, but the fact that it's an id is self-explanatory IMO).

I agree with @moonsettler here and would rename to `keyset` (or `keyset_id`, but the fact that it's an id is self-explanatory IMO).
00.md
@ -198,3 +139,3 @@
"proofs": List[Proof]
"proofs": Proofs
},
...
thunderbiscuit (Migrated from github.com) commented 2023-11-29 20:04:30 +00:00
Copy link

Just a note to fix this token once the version is updated to B and the secrets are updated to be hex-formatted.

Just a note to fix this token once the version is updated to `B` and the secrets are updated to be hex-formatted.
00.md
thunderbiscuit (Migrated from github.com) commented 2023-11-29 20:00:57 +00:00
Copy link

I think this line should mention that the current token prefix is B.

I think this line should mention that the current token prefix is `B`.
00.md
thunderbiscuit (Migrated from github.com) commented 2023-11-29 20:01:13 +00:00
Copy link

Same as above. The latest token version is B.

Same as above. The latest token version is `B`.
01.md
@ -7,3 +7,3 @@
This describes the basic exchange of the public mint keys that the wallet user `Alice` uses to unblind `Bob`'s signature.
This document outlines the exchange of the public keys of the mint `Bob` with the wallet user `Alice`. `Alice` uses the keys to unblind `Bob`'s blind signatures (see [NUT-00][00]).
thunderbiscuit (Migrated from github.com) commented 2023-11-29 20:05:55 +00:00
Copy link

Typos:

  1. with his active keysets -> with its active keysets
  2. if the mint will sign promises with. -> if the mint will sign promises with it.
Typos: 1. `with his active keysets` -> `with its active keysets` 2. `if the mint will sign promises with.` -> `if the mint will sign promises with it.`
thunderbiscuit (Migrated from github.com) commented 2023-11-29 20:09:10 +00:00
Copy link

Typo: identified by its keyset id can be computed -> identified by its keyset id, which can be computed

Typo: `identified by its keyset id can be computed` -> `identified by its keyset id, which can be computed`
01.md
@ -42,1 +43,3 @@
...
"keysets": [
{
"id": <keyset_id_hex_str>,
thunderbiscuit (Migrated from github.com) commented 2023-11-29 20:12:28 +00:00
Copy link

What happens if the keyset requested with the GET /v1/keys/{keyset_id} endpoint is not in the mint's old keysets? We might want to have a line on this (with the error returned if there is a specific one?).

What happens if the keyset requested with the `GET /v1/keys/{keyset_id}` endpoint is not in the mint's old keysets? We might want to have a line on this (with the error returned if there is a specific one?).
02.md
@ -88,0 +107,4 @@
def derive_keyset_id(keys: Dict[int, PublicKey]) -> str:
sorted_keys = dict(sorted(keys.items()))
pubkeys_concat = b"".join([p.serialize() for p in sorted_keys.values()])
return "00" + hashlib.sha256(pubkeys_concat).hexdigest()[:14]
thunderbiscuit (Migrated from github.com) commented 2023-11-29 20:16:49 +00:00
Copy link

If you end up changing the name of the field to keyset or keyset_id, reminder to change it here as well.

If you end up changing the name of the field to `keyset` or `keyset_id`, reminder to change it here as well.
05.md
@ -3,3 +3,3 @@
`mandatory` `author: calle`
`mandatory`
thunderbiscuit (Migrated from github.com) commented 2023-11-29 20:43:24 +00:00
Copy link

You refer to a call to the /checkfee endpoint and imply an example but there isn't one anymore. I think an example of the checkfee interaction could be added back.

You refer to a call to the `/checkfee` endpoint and imply an example but there isn't one anymore. I think an example of the checkfee interaction could be added back.
thunderbiscuit (Migrated from github.com) commented 2023-11-29 20:45:41 +00:00
Copy link

There are no pr and proofs fields anymore.

There are no `pr` and `proofs` fields anymore.
ngutech21 (Migrated from github.com) reviewed 2023-12-01 07:31:56 +00:00
05.md
@ -89,1 +139,4 @@
"amount": 8,
"secret": "4f3155acef6481108fcf354f6d06e504ce8b441e617d30c88924991298cdbcad",
"C": "0278ab1c1af35487a5ea903b693e96447b2034d0fd6bac529e753097743bf73ca9",
}
ngutech21 (Migrated from github.com) commented 2023-12-01 07:31:56 +00:00
Copy link

How does the mint return overpaid fees? I think we should add an additional change field to the response like we did before the cleanup

How does the mint return overpaid fees? I think we should add an additional `change` field to the response like we did before the cleanup
ngutech21 (Migrated from github.com) reviewed 2023-12-01 07:34:27 +00:00
05.md
@ -3,3 +3,3 @@
`mandatory` `author: calle`
`mandatory`
ngutech21 (Migrated from github.com) commented 2023-12-01 07:34:27 +00:00
Copy link

The term proof is ambigous in this context, because it doesn't refer to a cashu Proof but a bolt11 payment preimage. payment_preimage might be a better name since this is a specific bolt11 response we can use the lightning terminology.

The term proof is ambigous in this context, because it doesn't refer to a cashu Proof but a bolt11 payment preimage. `payment_preimage` might be a better name since this is a specific bolt11 response we can use the lightning terminology.
ngutech21 (Migrated from github.com) reviewed 2023-12-01 07:34:49 +00:00
05.md
@ -3,3 +3,3 @@
`mandatory` `author: calle`
`mandatory`
ngutech21 (Migrated from github.com) commented 2023-12-01 07:34:49 +00:00
Copy link

typo: bolt11 payment preimage

typo: bolt11 payment preimage
ngutech21 (Migrated from github.com) reviewed 2023-12-01 07:40:27 +00:00
05.md
@ -30,0 +44,4 @@
"expiry": <int>
}
```
Where `quote` is the quote ID, `amount` the amount that needs to be provided, and `fee_reserve` the additional fee reserve that is required. The mint expects `Alice` to include `Proofs` of *at least* `total_amount = amount + fee_reserve`. `paid` indicates whether the request as been paid and `expiry` is the Unix timestamp until which the melt quote is valid.
ngutech21 (Migrated from github.com) commented 2023-12-01 07:40:12 +00:00
Copy link

How long is a quote valid? Can a wallet request a quote wait for 1 day and then call /melt? Or is the mint using the expiry of the bolt11 invoice? I think there should be an expiry field in the reponse of the mint that tells the wallet how long this quote is valid.

How long is a quote valid? Can a wallet request a quote wait for 1 day and then call /melt? Or is the mint using the expiry of the bolt11 invoice? I think there should be an expiry field in the reponse of the mint that tells the wallet how long this quote is valid.
thesimplekid (Migrated from github.com) reviewed 2023-12-04 22:33:08 +00:00
11.md
@ -121,7 +121,7 @@ If the `locktime` is in the past and a tag `refund` is present, the `Proof` is s
thesimplekid (Migrated from github.com) commented 2023-12-04 22:33:08 +00:00
Copy link

field amd

typo: and

> field amd typo: and
thesimplekid (Migrated from github.com) reviewed 2023-12-04 22:52:41 +00:00
12.md
@ -53,26 +53,26 @@ The mint produces these DLEQ proofs when returning `BlindedSignature`'s in the r
thesimplekid (Migrated from github.com) commented 2023-12-04 22:52:41 +00:00
Copy link

Just to stay consistent json

Just to stay consistent `json`
callebtc commented 2023-12-06 15:35:25 +00:00 (Migrated from github.com)
Copy link

All Todos are now closed and with that the PR is officially ready for review (and thanks for all reviews already 🙏)

Rocket emoji!

All Todos are now closed and with that the PR is officially ready for review (and thanks for all reviews already 🙏) Rocket emoji!
🚀 2
thunderbiscuit (Migrated from github.com) approved these changes 2023-12-07 14:27:39 +00:00
thunderbiscuit (Migrated from github.com) left a comment
Copy link

ACK e59f284f0b.

Major improvement of the protocol. I'm happy to see that we have a few implementations that were able to put this into code already, and looking forward to seeing what gets built on top of this. 🚀

ACK e59f284f0b786eb7b239715469f9e0dd9cd4b6ea. Major improvement of the protocol. I'm happy to see that we have a few implementations that were able to put this into code already, and looking forward to seeing what gets built on top of this. 🚀
thunderbiscuit commented 2023-12-07 15:35:50 +00:00 (Migrated from github.com)
Copy link

One little thing: the test vectors should be updated (if not in this PR in a very close follow-up PR).

One little thing: the test vectors should be updated (if not in this PR in a very close follow-up PR).
thesimplekid (Migrated from github.com) approved these changes 2023-12-07 19:58:10 +00:00
thesimplekid (Migrated from github.com) left a comment
Copy link

Looks good to me. Agree with tb that it would be best to include test vectors as it helps to ensure implementations are correct.

ACK e59f284

Looks good to me. Agree with tb that it would be best to include test vectors as it helps to ensure implementations are correct. ACK [e59f284](https://github.com/cashubtc/nuts/pull/55/commits/e59f284f0b786eb7b239715469f9e0dd9cd4b6ea)
00.md
thesimplekid (Migrated from github.com) commented 2023-12-07 19:33:25 +00:00
Copy link

Just to confirm my understanding format A has been changed to include the unit, and there is no version B yet.

Just to confirm my understanding format `A` has been changed to include the unit, and there is no version `B` yet.
01.md
@ -3,3 +3,3 @@
`mandatory` `author: calle`
`mandatory`
thesimplekid (Migrated from github.com) commented 2023-12-07 19:35:12 +00:00
Copy link

Since its not stated that mints cannot. Mints can have multiple keysets for one unit, for example 2 keysets for sat?

Since its not stated that mints cannot. Mints can have multiple keysets for one unit, for example 2 keysets for sat?
thesimplekid (Migrated from github.com) reviewed 2023-12-08 06:44:18 +00:00
05.md
thesimplekid (Migrated from github.com) commented 2023-12-08 06:44:18 +00:00
Copy link

Believe this should be payment_preimage

Believe this should be `payment_preimage`
thesimplekid (Migrated from github.com) reviewed 2023-12-08 06:45:48 +00:00
05.md
@ -93,3 +145,3 @@
```
**Response** `PostMeltResponse` from `Bob`:
Response of `Bob`:
thesimplekid (Migrated from github.com) commented 2023-12-08 06:45:47 +00:00
Copy link

payment_preimage will be null if the payment has failed so should this be optional?

`payment_preimage` will be null if the payment has failed so should this be optional?
thesimplekid (Migrated from github.com) reviewed 2023-12-08 07:08:08 +00:00
08.md
@ -81,0 +114,4 @@
"proof": "c5a1ae1f639e1f4a3872e81500fd028bece7bedc1152f740cba5c3417b748c1b",
"change": [
{
"id": "009a1f293253e41e",
thesimplekid (Migrated from github.com) commented 2023-12-08 07:08:08 +00:00
Copy link

The proof field here should be named the same as it is in NUT-05 since its serves the same function. I believe we decided to go with payment_preimage since it is bolt11 specific.

The proof field here should be named the same as it is in NUT-05 since its serves the same function. I believe we decided to go with payment_preimage since it is bolt11 specific.
ngutech21 (Migrated from github.com) approved these changes 2023-12-08 07:08:41 +00:00
ngutech21 (Migrated from github.com) left a comment
Copy link

ACK e1568fdadc

This is a great improvement of the cashu protocol.

ACK e1568fdadc9e3e99d6a071e1de17a8a79dd658bf This is a great improvement of the cashu protocol.
thunderbiscuit (Migrated from github.com) reviewed 2023-12-08 19:36:17 +00:00
00.md
@ -75,3 +78,3 @@
```
`amount` is the value of the `Proof`, `secret` is the secret message (no encoding standard), `C` is the unblinded signature on `secret` (hex string), `id` is the [keyset id][02] of the mint public keys that signed the token (string).
`amount` is the amount of the `Proof`, `secret` is the secret message (no encoding enforced, 32 byte random hex string recommended to prevent fingerprinting), `C` is the unblinded signature on `secret` (hex string), `id` is the [keyset id][02] of the mint public keys that signed the token (hex string).
thunderbiscuit (Migrated from github.com) commented 2023-12-08 19:36:17 +00:00
Copy link

Super small nit that can totally be addressed in further changes: in the other 2 models you have the order as "amount, id, others", but in this one you have "amount, other, id". The order shouldn't break anyone's code but it feels cleaner to have them all the same (here the id field should precede the C_ field).

Super small nit that can totally be addressed in further changes: in the other 2 models you have the order as "amount, id, others", but in this one you have "amount, other, id". The order shouldn't break anyone's code but it feels cleaner to have them all the same (here the `id` field should precede the `C_` field).
thunderbiscuit (Migrated from github.com) reviewed 2023-12-08 21:18:11 +00:00
00.md
@ -75,3 +78,3 @@
```
`amount` is the value of the `Proof`, `secret` is the secret message (no encoding standard), `C` is the unblinded signature on `secret` (hex string), `id` is the [keyset id][02] of the mint public keys that signed the token (string).
`amount` is the amount of the `Proof`, `secret` is the secret message (no encoding enforced, 32 byte random hex string recommended to prevent fingerprinting), `C` is the unblinded signature on `secret` (hex string), `id` is the [keyset id][02] of the mint public keys that signed the token (hex string).
thunderbiscuit (Migrated from github.com) commented 2023-12-08 21:18:10 +00:00
Copy link

I was wrong, it does break in small ways the tests (not the validity of the tokens but their serialization).

For example note the example at the end of NUT-00 which has the order "id, amount, secret, C" (different from the model for Proof described above):

"proofs": [
    {
        "id": "009a1f293253e41e",
        "amount": 2,
        "secret": "407915bc212be61a77e3e6d2aeb4c727980bda51cd06a6afc29e2861768a7837",
        "C": "02bc9097997d81afb2cc7346b5e4345a9346bd2a506eb7958598a72f0cf85163ea"
    },
    {
        "id": "009a1f293253e41e",
        "amount": 8,
        "secret": "fe15109314e61d7756b0f8ee0f23a624acaa3f4e042f61433c728c7057b931be",
        "C": "029e8e5050b890a7d6c0968db16bc1d5d5fa040ea1de284f6ec69d61299f671059"
    }
]

This means that a library that implemented the order described in the model section:

{
  "amount": int, 
  "id": hex_str,
  "secret": str,
  "C": hex_str,
}

Will not be able to reproduce your resulting serialization in base64 (even though both tokens would be valid).

I suggest 2 things:

  1. the order be "amount, id, secret, C" for both the model and the example.
  2. all models use the "amount, id, others" format.
I was wrong, it does break in small ways the tests (not the validity of the tokens but their serialization). For example note the example at the end of NUT-00 which has the order "id, amount, secret, C" (different from the model for `Proof` described above): ```json "proofs": [ { "id": "009a1f293253e41e", "amount": 2, "secret": "407915bc212be61a77e3e6d2aeb4c727980bda51cd06a6afc29e2861768a7837", "C": "02bc9097997d81afb2cc7346b5e4345a9346bd2a506eb7958598a72f0cf85163ea" }, { "id": "009a1f293253e41e", "amount": 8, "secret": "fe15109314e61d7756b0f8ee0f23a624acaa3f4e042f61433c728c7057b931be", "C": "029e8e5050b890a7d6c0968db16bc1d5d5fa040ea1de284f6ec69d61299f671059" } ] ``` This means that a library that implemented the order described in the model section: ```json { "amount": int, "id": hex_str, "secret": str, "C": hex_str, } ``` Will not be able to reproduce your resulting serialization in base64 (even though both tokens would be valid). I suggest 2 things: 1. the order be "amount, id, secret, C" for both the model and the example. 2. all models use the "amount, id, others" format.
thesimplekid (Migrated from github.com) reviewed 2023-12-11 22:20:52 +00:00
07.md
@ -96,4 +89,1 @@
[18]: 18.md
[19]: 19.md
[20]: 20.md
thesimplekid (Migrated from github.com) commented 2023-12-11 22:20:52 +00:00
Copy link

Can this be used by the mint to fingerprint a wallet? If some wallets are checking by sending the full proof and others are only sending the secret? Should a recommendation be made to do one or the other similar to the ordering of the proofs to avoid fingerprinting?

Can this be used by the mint to fingerprint a wallet? If some wallets are checking by sending the full proof and others are only sending the secret? Should a recommendation be made to do one or the other similar to the ordering of the proofs to avoid fingerprinting?
thesimplekid (Migrated from github.com) reviewed 2023-12-17 18:25:33 +00:00
06.md
@ -84,4 +70,3 @@
`BlindedSignatures` is a list (array) of `BlindedSignature`'s (see [NUT-0][00]).
[00]: 00.md
[01]: 01.md
thesimplekid (Migrated from github.com) commented 2023-12-17 18:25:33 +00:00
Copy link

The trailing comma from 12 should be removed.

The trailing comma from 12 should be removed.
Sign in to join this conversation.
Reviewers
No reviewers
SuperPhatArrow
dipunm
misovan
gandlafbtc
AngusP
ebrakke
BilligsterUser
moonsettler
xphade
thunderbiscuit
thesimplekid
ngutech21
Labels
Clear labels   
breaking change
This issue/PR entails breaking changes

  
bug
Something isn't working

  
documentation
Improvements or additions to documentation

  
enhancement
New feature or request

  
needs discussion
Needs more discussion

  
needs implementation
Needs a reference implementation

  
new nut
A new protocol NUT

  
ready
Ready to merge

  
wallet-only
A change that affects only wallets

No labels
breaking change
bug
documentation
enhancement
needs discussion
needs implementation
new nut
ready
wallet-only
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
forgejo-admin/nuts!55
No description provided.