Add a lot of test vectors and improve handling of invalid contributions in reference code #9

jonasnick · 2022-04-21T13:39:41Z

Fixes #5 and #2.

Based on #8.

real-or-random · 2022-04-23T13:24:05Z

Concept ACK on having InvalidContributionErrors

bip-musig2/reference.py

Roasbeef · 2022-04-28T01:16:52Z

I ported the existing test vectors to JSON in this PR: #10

Once this PR is stable/merged, I can update my PR to include those new test vectors.

jonasnick · 2022-04-28T16:23:01Z

Just realized that this doesn't fully address #5 because there are still no tests for nonce generation.

bip-musig2/reference.py

real-or-random

This is a very clean approach, I think.

robot-dreams

Overall looks great—I like the InvalidContributionError approach.

bip-musig2/reference.py

robot-dreams

ACK 8d79d3b

bip-musig2/reference.py

real-or-random · 2022-06-08T09:36:25Z

Initially, I thought that we should document that None means the aggregator is faulty.

But this gets complicated because the guarantees are different. The aggregator is trusted for robustness/identifiable aborts, so the ability to blame aggregator is of limited use, and a malicious aggregator could always claim that someone else is wrong. So having an InvalidContributionError for the aggregator may convey the wrong impression that a honest can identify the disruptive party even when an aggregator is used, and that a malicious aggregator can be identified reliably.

Maybe it would be clearer to simply raise a ValueError instead? This will clearly separate the guaranteed ability to identify disruptive signers (identifiable abort) from other errors, and I think that's cleaner. Yet another alternative is to have a separate exception for the aggregator but this seems overkill to me.

In any case, the BIP text can be made clearer here.

For example, it currently doesn't say anything about blaming the aggregator. If we keep the InvalidContributionError, then we should elaborate in the text.
Also, it currently says "This standard specifies a partial signature verification algorithm to identify disruptive signers. It is incompatible with third-party nonce aggregation because the individual nonce is required for partial verification." That's not entirely true. The aggregator can still identify disruptive parties. We could also refer to ROAST now for a more formal discussion of IA.
"All the aggregator node can do is prevent the signing session from succeeding by sending out incorrect aggregate nonces." It can also send a wrong final sig.

But all of this should be done in a separate PR.

real-or-random

some (micro)-nits

real-or-random · 2022-06-08T09:37:57Z

bip-musig2/reference.py

+#   - InvalidContributionError for indicating that a participant (or the
+#     aggregator) is misbehaving in the protocol


Suggested change

# - InvalidContributionError for indicating that a participant (or the

# aggregator) is misbehaving in the protocol

# - InvalidContributionError for indicating that a signer (or the

# aggregator) is misbehaving in the protocol.

real-or-random · 2022-06-08T09:38:21Z

bip-musig2/reference.py

+# output of a hash function is 0), and can't be manually triggered by any
+# participant


Suggested change

# output of a hash function is 0), and can't be manually triggered by any

# participant

# output of a hash function is 0) and can't be manually triggered by any

# signer.

real-or-random · 2022-06-08T09:38:56Z

bip-musig2/reference.py

+# There are two types of exceptions that can be raised by this implementation:
+#   - ValueError for indicating that an input doesn't conform to some function
+#     precondition (e.g. an input array is the wrong length, a serialized
+#     representation doesn't have the correct format)


Suggested change

# representation doesn't have the correct format)

# representation doesn't have the correct format).

real-or-random · 2022-06-08T09:39:35Z

bip-musig2/reference.py

@@ -336,6 +361,15 @@ def partial_sig_agg(psigs: List[bytes], session_ctx: SessionContext) -> Optional
 def fromhex_all(l):
    return [bytes.fromhex(l_i) for l_i in l]

+# Check that calling `try_fn` raises a `exception`. If `exception` is raised,
+# examine it with`except_fn`.


Suggested change

# examine it with`except_fn`.

# examine it with `except_fn`.

bip-musig2/reference.py

real-or-random · 2022-06-08T09:49:19Z

Do you want to merge this or #20 first? This will affect some of the test vectors here.

Maybe it would be clearer to simply raise a ValueError instead?

Hm on a second thought, I don't know... ValueError is not a good idea either because then signers could crash. Maybe a separate exception is really better?

jonasnick · 2022-06-08T10:27:32Z

Do you want to merge this or #20 first?

I'd like to merge this first, since other PRs (separate tweaking from keyagg) are based on it.

jonasnick · 2022-06-08T11:53:16Z

Fixed nits

real-or-random

ACK 141919e

let's keep the InvalidContributionError, and I can open a PR that improves the explanations

real-or-random reviewed Apr 23, 2022

View reviewed changes

bip-musig2/reference.py Outdated Show resolved Hide resolved

jonasnick force-pushed the test-vectors branch from acb943b to 0df62f6 Compare May 17, 2022 14:11

jonasnick mentioned this pull request May 17, 2022

Include other participants' public nonces in reference tests #14

Closed

real-or-random reviewed May 19, 2022

View reviewed changes

bip-musig2/reference.py Show resolved Hide resolved

real-or-random reviewed May 19, 2022

View reviewed changes

This was referenced May 19, 2022

Separate tweaking from key aggregation #15

Merged

Add algorithmic description of deterministic signing algorithm #11

Merged

robot-dreams mentioned this pull request May 24, 2022

Fix reference code and add NonceGen test vectors #19

Merged

jonasnick mentioned this pull request May 25, 2022

Allow variable length signing #24

Merged

real-or-random mentioned this pull request May 29, 2022

Address AdamISZ's feedback regarding duplicate public keys #25

Merged

robot-dreams reviewed Jun 1, 2022

View reviewed changes

jonasnick force-pushed the test-vectors branch from 0df62f6 to 8d79d3b Compare June 2, 2022 11:31

robot-dreams reviewed Jun 2, 2022

View reviewed changes

bip-musig2/reference.py Outdated Show resolved Hide resolved

jonasnick force-pushed the test-vectors branch from 8d79d3b to e751102 Compare June 3, 2022 14:32

real-or-random reviewed Jun 8, 2022

View reviewed changes

jonasnick force-pushed the test-vectors branch from e751102 to 141919e Compare June 8, 2022 11:52

jonasnick added 7 commits June 8, 2022 11:54

Raise InvalidContributionError in key_agg and add test vectors

20f60b0

Raise InvalidContributionError in nonce_agg and add test vectors

0ec2aef

Raise InvalidContributionError in sign and add test vectors

4c06f31

Add test vectors for partial_sig_verify

ebb6a74

Add test vectors for partial_sig_verify when tweaking

6788ee5

Raise InvalidContributionError in sig_agg and add test vectors

cdc3520

Name nonce_gen test vectors

9e8837f

Update changelog

141919e

real-or-random approved these changes Jun 8, 2022

View reviewed changes

jonasnick merged commit 8f392c9 into musig2 Jun 8, 2022

real-or-random mentioned this pull request Jun 8, 2022

Improve explanation of Identifiable Aborts #26

Closed

real-or-random mentioned this pull request Aug 11, 2022

Don't assert on invalid attacker-controlled input #2

Closed

real-or-random mentioned this pull request Sep 9, 2022

Add blaming to pseudocode #67

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a lot of test vectors and improve handling of invalid contributions in reference code #9

Add a lot of test vectors and improve handling of invalid contributions in reference code #9

jonasnick commented Apr 21, 2022

real-or-random commented Apr 23, 2022

Roasbeef commented Apr 28, 2022

jonasnick commented Apr 28, 2022

real-or-random left a comment

robot-dreams left a comment

robot-dreams left a comment

real-or-random commented Jun 8, 2022 •

edited

Loading

real-or-random left a comment

real-or-random Jun 8, 2022

real-or-random Jun 8, 2022

real-or-random Jun 8, 2022

real-or-random Jun 8, 2022

real-or-random commented Jun 8, 2022

jonasnick commented Jun 8, 2022

jonasnick commented Jun 8, 2022

real-or-random left a comment

		# - InvalidContributionError for indicating that a participant (or the
		# aggregator) is misbehaving in the protocol

		# output of a hash function is 0), and can't be manually triggered by any
		# participant

	# representation doesn't have the correct format)
	# representation doesn't have the correct format).

Add a lot of test vectors and improve handling of invalid contributions in reference code #9

Add a lot of test vectors and improve handling of invalid contributions in reference code #9

Conversation

jonasnick commented Apr 21, 2022

real-or-random commented Apr 23, 2022

Roasbeef commented Apr 28, 2022

jonasnick commented Apr 28, 2022

real-or-random left a comment

Choose a reason for hiding this comment

robot-dreams left a comment

Choose a reason for hiding this comment

robot-dreams left a comment

Choose a reason for hiding this comment

real-or-random commented Jun 8, 2022 • edited Loading

real-or-random left a comment

Choose a reason for hiding this comment

real-or-random Jun 8, 2022

Choose a reason for hiding this comment

real-or-random Jun 8, 2022

Choose a reason for hiding this comment

real-or-random Jun 8, 2022

Choose a reason for hiding this comment

real-or-random Jun 8, 2022

Choose a reason for hiding this comment

real-or-random commented Jun 8, 2022

jonasnick commented Jun 8, 2022

jonasnick commented Jun 8, 2022

real-or-random left a comment

Choose a reason for hiding this comment

real-or-random commented Jun 8, 2022 •

edited

Loading