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

bip-340: reduce size of randomizers to 128 bit and provide argument #220

Closed
wants to merge 209 commits into from
Closed

bip-340: reduce size of randomizers to 128 bit and provide argument #220

wants to merge 209 commits into from

Conversation

jonasnick
Copy link

@jonasnick jonasnick commented May 22, 2021

This speeds up batch verification in libsecp by up to 9%.

I can open this upstream if people are happy with this PR.

CC @real-or-random

avirgovi and others added 30 commits June 25, 2020 14:07
The current version of the spec requires creator role to initialize empty input fields, but says nothing about output field initialization. At the same time, the following role, updater, "should also add redeemScripts, witnessScripts, and BIP 32 derivation paths to the input and output data if it knows them.", which does not make any sense if the fields were uninitialized. The [current Bitcoin Core implementation does this](https://github.com/bitcoin/bitcoin/blob/a24806c25d7a81a9c436de58eb5778d93abab16b/src/psbt.cpp#L12), and [other PSBT implementations, like rust-bitcoin, follow this practice](https://github.com/rust-bitcoin/rust-bitcoin/blob/master/src/util/psbt/mod.rs#L59)
Co-authored-by: Breno Rodrigues Brito <brenorb@gmail.com>
Co-authored-by: ninjastic <ninjasticdev@protonmail.com>
Co-authored-by: sabotag3x <sabotage.sta@gmail.com>
Co-authored-by: bitmover <67111541+bitmover-studio@users.noreply.github.com>
Co-authored-by: alegotardo <40860228+alegotardo@users.noreply.github.com>
Co-authored-by: kuthullu <kuthullu@gmail.com>
Co-authored-by: Trimegistus <trimegisto@rocketmail.com>
+ Change 'Client support' section to 'Backwards compatibility'
+ Reasons: avoid confusion and according to the format mentioned in the below link:
https://github.com/bitcoin/bips/blob/master/bip-0002.mediawiki#specification
+ Discussed changes in bitcoin#994
Using the same threshold for MUST_SIGNAL as STARTED means that any chain
that would have resulted in activation with lockinontimeout=false will
also result in activation with lockinontimeout=true (and vice-versa).
This reduces the ways in which a consensus split can occur, and avoids
a way in which miners could attempt to discourage users from setting
lockinontimeout=true.
Add Python-HDWallet on Other Implementation.
Non minimal encodings are rejected by the bitcoin-core client because it 
only checks against a specific encoding
Signed-off-by: koushiro <koushiro.cqx@gmail.com>
Add Portuguese wordlist to BIP39
This message, valid between version/verack for peers with version >= 70017,
would allow establishing at the time of connection that no transactions will be
announced/requested between those peers.
Since the sigOpCount calculation was copied from P2SH, and P2SH
restricts the use of CHECKMULTISIG with pushed integers the reference
implementation would not take into account the number of public keys for
17 to 20 keys (not representable with an OP_N) even for P2WSH.
Therefore it fallbacks to accounting for 20 sigops in this case, which
this sentence seemed to mismatch with.

Btcd and Libbitcoin use the same calculation as in Bitcoin Core.

Signed-off-by: Antoine Poinsot <darosior@protonmail.com>
Robert Spigler and others added 23 commits May 15, 2021 00:57
This avoids having to update the BIP with a fresh graph every time there's a
change to libsecp and suggests that the expected speedup depends on the specific
implementation.
BIP 88: Templates for Hierarchical Deterministic derivation paths
BIP340: remove batch speedup graph and link to it instead
BIP-0136: updates data encoding/decoding examples due to bech32m/bech32
@@ -200,7 +200,7 @@ Input:
* The signatures ''sig<sub>1..u</sub>'': ''u'' 64-byte arrays

The algorithm ''BatchVerify(pk<sub>1..u</sub>, m<sub>1..u</sub>, sig<sub>1..u</sub>)'' is defined as:
* Generate ''u-1'' random integers ''a<sub>2...u</sub>'' in the range ''1...n-1''. They are generated deterministically using a [https://en.wikipedia.org/wiki/Cryptographically_secure_pseudorandom_number_generator CSPRNG] seeded by a cryptographic hash of all inputs of the algorithm, i.e. ''seed = seed_hash(pk<sub>1</sub>..pk<sub>u</sub> || m<sub>1</sub>..m<sub>u</sub> || sig<sub>1</sub>..sig<sub>u</sub> )''. A safe choice is to instantiate ''seed_hash'' with SHA256 and use [https://tools.ietf.org/html/rfc8439 ChaCha20] with key ''seed'' as a CSPRNG to generate 256-bit integers, skipping integers not in the range ''1...n-1''.
* Generate ''u-1'' random integers ''a<sub>2...u</sub>'' in the range ''1...n-1''. They are generated deterministically using a [https://en.wikipedia.org/wiki/Cryptographically_secure_pseudorandom_number_generator CSPRNG] seeded by a cryptographic hash of all inputs of the algorithm, i.e. ''seed = seed_hash(pk<sub>1</sub>..pk<sub>u</sub> || m<sub>1</sub>..m<sub>u</sub> || sig<sub>1</sub>..sig<sub>u</sub> )''. A safe choice is to instantiate ''seed_hash'' with SHA256 and use [https://tools.ietf.org/html/rfc8439 ChaCha20] with key ''seed'' as a CSPRNG to generate 128-bit integers.<ref>Randomizers are not required to be larger than 128-bit to achieve the desired security level of 128 bits (see [[bip-0340/batch-randomizers.mediawiki|lemma]]).</ref>

Choose a reason for hiding this comment

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

Updated sentences are good. This still says Generate ''u-1'' random integers ''a<sub>2...u</sub>'' in the range ''1...n-1'' in the first sentence. I mean it's not wrong, not sure if you wanted to keep this on purpose but if yes, then the updated sentences are kind of unexpected to the reader.

I'm not sure what the best thing is. We could simply say 1...2^128-1. That's simple and clean but not sure if this is too restrictive. What if this is harder in your implementation than just generating full scalars.

Nit: In any case, let's change "random" -> "uniformly random"

Here's a suggestion to play with (if you agree with the idea to specify both options):
Generate ''u-1'' uniformly random integers ''a<sub>2...u</sub>'' in the range ''1...n-1'', or alternatively in the range ''1...2^128-1''.<ref>Integers are not required to be larger than 128 bits to achieve the desired security level of 128 bits (see [[bip-0340/batch-randomizers.mediawiki|lemma]]). Shorter integers can speed up computations in optimized implementations.</ref> ...

Next sentence (not your PR but we could fix it here):
At the moment, we don't really say that they must be generated deterministically or not. Maybe:
It is recommended to generate them deterministically ... ?

Another question (again not your PR, sorry :P ) : Do we allow 0...n-1 (or 0...2^128-1)?
Pro: It doesn't matter and it's much easier to implement. If I was given a BIP that says 1..., I might just ignore it and implement 0.... Also, if we write 0..., then it's somewhat cleaner that using 2^128 bit integers work.
Con: If you use a real PRG, you may want to reject 0 because it's dangerous. But it's dangerous anyway to use a real PRG?

I lean towards 0....

Copy link
Author

Choose a reason for hiding this comment

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

not sure if you wanted to keep this on purpose

No, that was an oversight. Good ideas, updated commit with your suggestions.

bip-0340/batch-randomizers.mediawiki Outdated Show resolved Hide resolved
bip-0340/batch-randomizers.mediawiki Outdated Show resolved Hide resolved
This (currently) speeds up batch verification in libsecp256k1 by up to 9%.
@real-or-random
Copy link

@jonasnick Oh there's also #204, which anyway will require touching those sentences again...

Maybe it's better to convert the "generate random integers" paragraph also more to pseudocode-like thing. The text is already complex now and #204 won't make it simpler. I'd volunteer to give it a try when resolving #204.

Having said that, do you think it makes sense to batch some of open issues, i.e., solve them here in this repo and then open a PR to the official BIPs repo? I know I haven't worked on this for a while but maybe now is a good time to focus on this again and then we can solve all or at least most of the changes quickly. None of the issues here should require a lengthy discussion. The discussions already happened, we just need to write down the results.

@jonasnick
Copy link
Author

I'm fine with batching as long as completing the batch doesn't takevery long. We should try avoiding blocking our own implementation work with batching.

@real-or-random
Copy link

Note (sorry for abusing this PR but this is the best place to note):

  • We should also clarify what we except from Verify and Batch Verify: The property that single and batch verification agree i) simply holds for all inputs of batch verification when using fully random multipliers ii) holds for inputs generated by a ppt adversary when using the deterministic method.
  • We should clarify whether the empty batch is allowed. (It should be, e.g., think of empty blocks.)

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.