Code Review

Code review #

Open PRs #

flowchart LR secp979(secp/#979) click secp979 "https://github.com/bitcoin-core/secp256k1/pull/979" secp1118(secp/#1118) click secp1118 "https://github.com/bitcoin-core/secp256k1/pull/1118" secp1129(secp/#1129) click secp1129 "https://github.com/bitcoin-core/secp256k1/pull/1129" secp979 --> secp1129 secp1118 --> secp1129 core26153(#26153) click core26153 "https://github.com/bitcoin/bitcoin/pull/26153" core26153 --> core25361 core25361(#25361) click core25361 "https://github.com/bitcoin/bitcoin/pull/25361" core23233(#23233) click core23233 "https://github.com/bitcoin/bitcoin/pull/23233" core25361 --> core23233 core23432(#23432) click core23432 "https://github.com/bitcoin/bitcoin/pull/23432" secp1129 --> core23432 core23561(#23561) click core23561 "https://github.com/bitcoin/bitcoin/pull/23561" core25361 --> core23561 secp1129 --> core23561 core24545(#24545) click core24545 "https://github.com/bitcoin/bitcoin/pull/24545" core23233 --> core24545 core23432 --> core24545 core23561 --> core24545 core24005(#24005) click core24005 "https://github.com/bitcoin/bitcoin/pull/24005" core23322(#23322) click core23322 "https://github.com/bitcoin/bitcoin/pull/23322" core23441(#23441) click core23441 "https://github.com/bitcoin/bitcoin/pull/23441" core25361 --> core23441 core24748(#24748) click core24748 "https://github.com/bitcoin/bitcoin/pull/24748" core24545 --> core24748 core24005 --> core24748

  • Oct 2021: #23233 (supersedes #18242). Depends on #25361. Implements V2TransportSerializer and V2TransportDeserializer.
  • Sep 2021: secp/#979 implements jacobi symbols to find quadratic residues.
  • Oct 2021: #23322. Differential fuzz testing for Poly1305.
  • Nov 2021: #23432 depends on secp/#1129 and adds elligator-swift encode/decode capability to CPubkey.
  • Nov 2021: #23561 depends on #25361, secp/#1129 and implements ECDH and HKDF key derivation for BIP324.
  • Nov 2021: #23441 depends on #20962 introduces differential fuzz testing for [email protected] authenticated encryption.
  • Jan 2022: #24005 adds python implementation of Elligator-squared for use in the integration test framework.
  • Mar 2022: #24545 brings together all branches and enables BIP324. Reviewers and enthusiasts can now run BIP324 v2 nodes supporting encrypted transport on mainnet.
  • Apr 2022: #24748 builds on #24005 and #24545. Adds test framework capability for v2 transport and uses it to create integration tests for BIP324 v2 nodes.
  • Jun 2022: #25361 (supersedes #20962), depends on #26153 adds a two-layered BIP324 cipher suite with RFC8439 being used in an inner layer to provide strong confidentiality and security guarantees(security analysis) and best-effort encryption of the message length using a forward secure version of ChaCha20, FSChaCha20.
  • July 2022: secp/#1118 depends on secp/#979, implements x-only ecmult_const version with x specified as n/d.
  • July 2022: secp/#1129 depends on secp/#979, secp/#1118 and implements an Elligator-swift module.

Merged PRs #

  • Aug 2018: #14047 adds CKey::Negate() and HKDF_HMAC_L32.
    • TODO: CKey::Negate() may not be needed anymore.
  • Mar 2019: #15512 Instead of bringing in [email protected], this PR leverages the existing ChaCha20 in Bitcoin Core. The existing implementation is only used as a Deterministic Random Bit Generator (DRBG) for Pseudo-RNG purposes. So all that’s needed to be added is XOR the keystream with the plaintext to product ciphertext.
  • Mar 2019: #15519 adds a Poly1305 MAC implementation.
  • Mar 2019: #15649 leverages ChaCha20 from #15512 and Poly1305 from #15519 to compose the [email protected] authenticated encryption cipher suite.
  • Jun 2019: #16202 refactors net.{h, cpp} and net_processing.{h, cpp} to abstract out transport serialization and deserialization. Sets the stage for #18242
  • Aug 2019:#16562 continues the refactor from #16202 and creates a V1TransportSerializer presumably paving the way for a v2 serializer.
  • Dec 2019: #17771 adds fuzz tests for V1TransportDeserializer introduced in #16202.
  • Jun 2020: #19296 adds fuzz tests for ChaCha20 and Poly1305.
  • May 2021: #22029 improves fuzz test coverage for V1TransportSerializer as requested in #16562.
  • Jun 2021: #22331 fixes inconsistency between K1/K2 terminology between code and BIP.
    • Oct 2021: #23271 fixes K1/K2 inconsistency in comments
  • Aug 2021: #22704 Differential fuzzing bitcoin core ChaCha20 against the original DJB implementation.

Closed unmerged (for archaeology) #

  • Aug 2018: #14032 (superseded by mutiple). Proof-of-concept.
  • Aug 2018: #14049 (superseded by #23561). Enables libsecp256k1 ECDH.
  • Aug 2018: #14050 (superseded by #15512, #15519 and #15649). Add [email protected]
  • May 2019: #14046 (superseded by #16202) refactors message parsing in CNetMessage. PR approach deemed too complex.
  • Mar 2020: #18242 (superseded by #23233) implements V2TransportSerializer and V2TransportDeserializer.
  • Dec 2021: #23900 (superseded by #24545) adds RPC:addnode parameter to advertise a manual peer connection as supportive of BIP324 v2 encrypted transport using NODE_P2P_V2.
  • Jan 2021: #20962 (superseded by #25361) depends on #25354 adds a two-layered BIP324 cipher suite with RFC8439 being used in an inner layer to provide strong confidentiality and security guarantees along with a proof and best-effort encryption of the message length using a forward secure version of ChaCha20, FSChaCha20.
    • August 2021: dhruv inherited the PR.
    • Prior to Jun 2022, this PR added forward secrecy by re-keying the cipher suite every 4064 bytes per a well-received suggestion by sipa from Nov 2020.
  • Sep 2021: secp256k1/#982 depends on secp256k1/#979 and implements an Elligator-squared module. Elligator-squared was replaced with elligator-squared in the design as it is nearly 2x faster.
  • Jun 2022: #25354 reduces wasted pseudorandom bytes from partially used ChaCha20 blocks. Unfortunately has to un-do #22704 as our implementation now deviates from the reference implementation by Daniel J. Bernstein. Was abandoned in Sep 2022 in favor of #26153 due to faster benchmarks and more coupled ChaCha20 changes.