Support for NUT-12 #200

lollerfirst · 2024-10-29T13:25:01Z

Fixes: #156

Changes

added optional dleq and dleqValid fields to Proof. dleq holds a SerializedDLEQ proof, while dleqValid, if present, signals whether it has been correctly verified.
CashuWallet.meltProofs and CashuWallet.createSwapPayload strip the dleq field off of each Proof, if there is any. (No privacy leak).
CashuWallet.construct_proofs now unpacks and verifies DLEQ proofs returned by the Mint, if any. It will create Proof with:
- dleqValid == true if the DLEQ is present and verified
- dleqValid == undefined if DLEQ not present
- dleqValid == false if DLEQ present and verification failed.
added boolean option includeDleq to CashuWallet.send:
- if includeDleq == true, makes sure the selected send proofs each have a defined dleq field;
- if includeDleq == false, strip off the dleq field from send;
- if includeDleq == undefined, do not intervene.
added boolean option requireDleq to CashuWallet.receive:
- if requireDleq == true, verifies each proof's dleq for correctness.
- if requireDleq == false || requireDleq == undefined, does not verify dleq.
added private helper function CashuWallet.requireDLEQ. Separates the logic for requiring valid dleqs from a token from the function receive.
added numberToHexPadded64 to the utilities. This function takes in a bigint and returns an a hex string prefixed to 64 characters with '0'.
tests

PR Tasks

Open PR
run npm run test --> no failing unit tests
run npm run format

callebtc · 2024-10-30T00:13:32Z

src/CashuWallet.ts

+			if (verifyDLEQ) {
+				if (dleq == undefined) {
+					throw new Error('DLEQ verification required, but none found');
+				}
+				if (!verifyDLEQProof_reblind(secret, dleq, proof.C, A)) {
+					throw new Error('DLEQ verification failed');
+				}
+			}


Suggested change

if (verifyDLEQ) {

if (dleq == undefined) {

throw new Error('DLEQ verification required, but none found');

}

if (!verifyDLEQProof_reblind(secret, dleq, proof.C, A)) {

throw new Error('DLEQ verification failed');

}

}

if (verifyDLEQ) {

if (dleq == undefined) {

throw new Error('DLEQ verification required, but none found');

}

if (!verifyDLEQProof_reblind(secret, dleq, proof.C, A)) {

throw new Error('DLEQ verification failed');

}

}

It might be dangerous to throw an error here, we might still want to keep the proofs, even if the verification failed. I have no good solution for this case though.

It might be dangerous to throw an error here

Agreed.

What if cashu-ts just constructs the dleq, then its up to the implementer to verify and handle failures?

The other idea I had was have some sort of flag either on the Proof or in the return value of constructProofs that indicates the verification failed, but that gets gross:

/** * represents a single Cashu proof. */ export type Proof = { ... /** * DLEQ proof */ dleq?: SerializedDLEQ; /** * Is the DLEQ proof valid */ dleqValid?: boolan; };

I really like @gudnuf 's idea

What do you think @callebtc

nostr-tools uses a Symbol to signal verified events: https://github.com/nbd-wtf/nostr-tools/blob/master/core.ts

@Egge21M What changes from a normal boolean flag?

Symbols are a bit "safer" than regular properties. The are not enumerable and have to be accessed very purposefully. Also, because they are not enumerable they will be excluded by default by methods like JSON.stringify

… returned a DLEQ and it was verified correctly.

lollerfirst · 2024-10-30T18:35:07Z

Unfortunately it seems we need to fix cashu-crypto-ts's package.json again...

…ase.

lollerfirst · 2024-11-06T18:38:05Z

So the integration tests with Nutshell work fine. I might try and other ones better cover cases where DLEQ is not verified correctly.

I couldn't create normal tests (in wallet.test.ts) because the mocked keyset does not provide the corresponding private keys so I can't create the dleq for the mocked proof.

Egge21M · 2024-11-07T10:02:33Z

Great stuff! LMK when this is ready for review

lollerfirst · 2024-11-07T11:38:21Z

FYI I tried using a symbol for dleqValid before but got some error.

Egge21M

Great work and a fantastic start! I added some comments, but i think we are mostly there

Egge21M · 2024-11-07T16:09:19Z

src/CashuWallet.ts

 		}
 	): Promise<SendResponse> {
+		if (options?.includeDleq ?? false) {


Why use nullish coalescing here?

src/CashuWallet.ts

Egge21M · 2024-11-07T16:12:47Z

src/CashuWallet.ts

+			let { keep, send } = await this.swap(amount, sendProofs, options);
+			keep = keepProofsSelect.concat(keep);
+
+			// strip dleq if explicitly told so


does !options?.includeDleq have two meanings in this case? Ignoring proofs without dleqs when true and stripping dleqs when false ? Maybe we should not overload this option to be more clear.

Yea that's what I intended and what made most sense to me.

Okay I see. I think most people will assume that not-passing an optional flag is equal to setting the boolean to false. Diverging from that needs to be documented very clearly I believe. However I would love to get some implementers thoughts on this. @callebtc @gudnuf could you leave your 2 sats on this?

Could you maybe explain the usecase to me? WWhy would someone decide to strip DLEQs when sending to another wallet?

I think it just clicked for me... sorry for all the confusion. includeDleq signals wether or not a token sent should include DLEQs for all included proofs, THEREFORE:

if (options?.includeDleq) -> Filter proofs for proofs with DLEQ, do not strip them
if (!options?.includeDleq) This is not an explicit false, simply NOT an explicit true -> Include all proofs, strip all DLEQs

src/CashuWallet.ts

src/model/BlindedSignature.ts

Egge21M · 2024-11-07T16:28:42Z

src/utils.ts

@@ -292,7 +321,15 @@ export function handleTokens(token: string): Token {
 					secret: p.s,
 					C: bytesToHex(p.c),
 					amount: p.a,
-					id: bytesToHex(t.i)
+					id: bytesToHex(t.i),
+					dleq:


We can avoid dleq:undefined being present on every token without dleqs by doing something like

proofs.push({ secret: p.s, C: bytesToHex(p.c), amount: p.a, id: bytesToHex(t.i), ...(p.d && { dleq: { e: bytesToHex(p.d.e), s: bytesToHex(p.d.s), r: bytesToHex(p.d.r) } as SerializedDLEQ }) });

What do you think?

We could also use an if-statement, as it might be more readable

I tried using this elegant solution in the place of almost all other ugly dleq == undefined ? undefined : ... check it out.

Egge21M · 2024-11-07T16:42:16Z

src/CashuWallet.ts

+	 * @param token The token subject to the verification
+	 * @param keys The Mint's keyset to be used for verification
+	 */
+	private requireDLEQ(token: Token, keys: MintKeys) {


I might be missing something here, but it seems like this function does not rely on any of the instance state. Therefore I would like to move it to utils.ts and export it to the public API.

Functions that do not return anything, but instead throw are hard to work with IMHO. Would you not agree that it would be clearer to rename this to hasValidDleq, make it take a SINGLE proof, and make it return true is all pass and false if dleq are either missing or invalid?

We could then use this util to build filters like proofs.filter(p => hasValidDleq(p, keys) or a util to replace requireDleq with a better best-case running time like

if (proofs.some(p => !hasValidDelq(p, keys))) { throw new Error("Proofs contain invalid DLEQ") }

You are right.

Doing this right now. Should I let hasValidDleq throw in case there is no matching key from the keyset proof.id for a particular proof amount? Or should I just return false.

I think it makes sense to throw in this case. false would indicate an invalid DLEQ, while in this case it's more than just that.

added a test for hasValidDleq as well.

Egge21M · 2024-11-12T20:23:33Z

src/CashuWallet.ts

+			let { keep, send } = await this.swap(amount, sendProofs, options);
+			keep = keepProofsSelect.concat(keep);
+
+			// strip dleq if explicitly told so


Could you maybe explain the usecase to me? WWhy would someone decide to strip DLEQs when sending to another wallet?

Egge21M · 2024-11-12T20:25:27Z

src/model/BlindedSignature.ts

+			id: this.id,
+			amount: this.amount,
+			C_: this.C_.toHex(true),
+			dleq:


Can we use the spread syntax here too?

Yes. Also I am responding here to your other comment about "why would someone strip the DLEQs when sending to Carol". One might try to get a smaller size cashuB token for example. In general if it's not required by Carol why include it?

Egge21M

LGTM!

lollerfirst added 4 commits October 29, 2024 07:39

dleq

30445e1

less tab

d794f2c

package.json

40b5468

fix

c7ac6d9

callebtc reviewed Oct 30, 2024

View reviewed changes

lollerfirst added 2 commits October 30, 2024 17:53

Merge remote-tracking branch 'upstream/development' into dleq

bb8aa49

add optional dleqValid to Proof type. It will be true if the mint…

102908f

… returned a DLEQ and it was verified correctly.

lollerfirst added 11 commits October 30, 2024 20:15

include dleq in encoded V4 token if provided

e4a59f6

handleTokens include dleq if present.

72f4211

fix tests accordingly

e7c6501

Carol receive from Alice

2379ed5

strip DLEQs when melting, swapping

36a643e

npm format

87686d7

Merge remote-tracking branch 'upstream/development' into dleq

a5f9dd2

remove unused imports + update cashu/crypto dependency to latest rele…

f8e66e7

…ase.

integration tests for verify and includeDleq in token + some fixes

6b8c7c7

sure-fire pad to 64 characters for bigint

8c12a87

check pubkey is not undefined

5cb0d07

lollerfirst added 2 commits November 7, 2024 08:37

fix dleq strip

8aea25f

test not enough funds when dleq missing and includeDleq true

582172f

Egge21M linked an issue Nov 7, 2024 that may be closed by this pull request

[Feature request] Support for DLEQ proofs #156

Open

lollerfirst added 2 commits November 7, 2024 11:22

npm run format

2bfb5a0

test receive with invalid dleq

8e7a58d

lollerfirst marked this pull request as ready for review November 7, 2024 11:38

Egge21M requested changes Nov 7, 2024

View reviewed changes

requested changes

c70b2c6

lollerfirst added 4 commits November 8, 2024 17:03

clean tests

d2da842

test for hasValidDleq in utils tests.

266c8aa

npm run format

efc73ec

test no matching key

f26bd15

lollerfirst requested a review from Egge21M November 11, 2024 14:41

Egge21M reviewed Nov 12, 2024

View reviewed changes

lollerfirst and others added 3 commits November 13, 2024 10:37

use new syntax for serializedDLEQ

06fdaab

abstracted strip util + changed conditions

af68d57

use stripDleq everywhere

31d3832

Egge21M approved these changes Nov 13, 2024

View reviewed changes

Egge21M merged commit 90dc186 into cashubtc:development Nov 13, 2024
1 check passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support for NUT-12 #200

Support for NUT-12 #200

lollerfirst commented Oct 29, 2024 •

edited

Loading

callebtc Oct 30, 2024

gudnuf Oct 30, 2024

lollerfirst Oct 30, 2024

lollerfirst Oct 30, 2024

Egge21M Oct 30, 2024

lollerfirst Oct 30, 2024

Egge21M Oct 30, 2024

lollerfirst commented Oct 30, 2024

lollerfirst commented Nov 6, 2024 •

edited

Loading

Egge21M commented Nov 7, 2024

lollerfirst commented Nov 7, 2024

Egge21M left a comment

Egge21M Nov 7, 2024

Egge21M Nov 7, 2024

lollerfirst Nov 8, 2024

Egge21M Nov 8, 2024

Egge21M Nov 12, 2024

Egge21M Nov 12, 2024 •

edited

Loading

Egge21M Nov 7, 2024

Egge21M Nov 7, 2024

lollerfirst Nov 8, 2024 •

edited

Loading

Egge21M Nov 7, 2024

lollerfirst Nov 8, 2024

lollerfirst Nov 8, 2024 •

edited

Loading

Egge21M Nov 8, 2024

lollerfirst Nov 8, 2024 •

edited

Loading

Egge21M Nov 12, 2024

Egge21M Nov 12, 2024

lollerfirst Nov 13, 2024

Egge21M left a comment

Support for NUT-12 #200

Support for NUT-12 #200

Conversation

lollerfirst commented Oct 29, 2024 • edited Loading

Fixes: #156

Changes

PR Tasks

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lollerfirst commented Oct 30, 2024

lollerfirst commented Nov 6, 2024 • edited Loading

Egge21M commented Nov 7, 2024

lollerfirst commented Nov 7, 2024

Egge21M left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Egge21M Nov 12, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lollerfirst Nov 8, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lollerfirst Nov 8, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lollerfirst Nov 8, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Egge21M left a comment

Choose a reason for hiding this comment

lollerfirst commented Oct 29, 2024 •

edited

Loading

lollerfirst commented Nov 6, 2024 •

edited

Loading

Egge21M Nov 12, 2024 •

edited

Loading

lollerfirst Nov 8, 2024 •

edited

Loading

lollerfirst Nov 8, 2024 •

edited

Loading

lollerfirst Nov 8, 2024 •

edited

Loading