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

Upstream PRs 1268, 1276, 1267, 1265, 1230, 1279, 1273, 1263, 1231, 1285, 1283, 1205, 1286, 1275, 1234, 1239, 1240, 1284, 1277, 1289, 1270, 1296, 1301, 1299, 1066, 1300, 1292, 1305, 1303, 1133, 1306, 1207, 1304, 1307, 1311, 1309, 1312 #256

Merged

Conversation

jonasnick
Copy link
Contributor

@jonasnick jonasnick commented Jul 24, 2023

[bitcoin-core/secp256k1#1268]: release cleanup: bump version after 0.3.1
[bitcoin-core/secp256k1#1276]: autotools: Don't regenerate Wycheproof header automatically
[bitcoin-core/secp256k1#1267]: doc: clarify process for patch releases
[bitcoin-core/secp256k1#1265]: Remove bits argument from secp256k1_wnaf_const{_xonly}
[bitcoin-core/secp256k1#1230]: Build: allow static or shared but not both
[bitcoin-core/secp256k1#1279]: tests: lint wycheproof's python script
[bitcoin-core/secp256k1#1273]: build: Make SECP_VALGRIND_CHECK preserve CPPFLAGS
[bitcoin-core/secp256k1#1263]: cmake: Make installation optional
[bitcoin-core/secp256k1#1231]: Move SECP256K1_INLINE macro definition out from include/secp256k1.h
[bitcoin-core/secp256k1#1285]: bench: Make sys/time.h a system include
[bitcoin-core/secp256k1#1283]: Get rid of secp256k1_fe_const_b
[bitcoin-core/secp256k1#1205]: field: Improve docs +tests of secp256k1_fe_set_b32
[bitcoin-core/secp256k1#1286]: tests: remove extra semicolon in macro
[bitcoin-core/secp256k1#1275]: build: Fix C4005 "macro redefinition" MSVC warnings in examples
[bitcoin-core/secp256k1#1234]: cmake: Add dev-mode
[bitcoin-core/secp256k1#1239]: cmake: Bugfix and other improvements after bumping CMake up to 3.13
[bitcoin-core/secp256k1#1240]: cmake: Improve and document compiler flag checks
[bitcoin-core/secp256k1#1284]: cmake: Some improvements using PROJECT_IS_TOP_LEVEL variable
[bitcoin-core/secp256k1#1277]: autotools: Clean up after adding Wycheproof
[bitcoin-core/secp256k1#1289]: cmake: Use full signature of add_test() command
[bitcoin-core/secp256k1#1270]: cmake: Fix library ABI versioning
[bitcoin-core/secp256k1#1296]: docs: complete interface description for secp256k1_schnorrsig_sign_custom
[bitcoin-core/secp256k1#1301]: Avoid using bench_verify_data as bench_sign_data; merge them
[bitcoin-core/secp256k1#1299]: Infinity handling: ecmult_const(infinity) works, and group verification
[bitcoin-core/secp256k1#1066]: Abstract out and merge all the magnitude/normalized logic
[bitcoin-core/secp256k1#1300]: Avoid normalize conditional on VERIFY
[bitcoin-core/secp256k1#1292]: refactor: Make 64-bit shift explicit
[bitcoin-core/secp256k1#1305]: Remove unused scratch space from API
[bitcoin-core/secp256k1#1303]: ct: Use more volatile
[bitcoin-core/secp256k1#1133]: schnorrsig: Add test vectors for variable-length messages
[bitcoin-core/secp256k1#1306]: build: Make tests work with external default callbacks
[bitcoin-core/secp256k1#1207]: Split fe_set_b32 into reducing and normalizing variants
[bitcoin-core/secp256k1#1304]: build: Rename arm to arm32 and check if it's really supported
[bitcoin-core/secp256k1#1307]: Mark more assembly outputs as early clobber
[bitcoin-core/secp256k1#1311]: Revert "Remove unused scratch space from API"
[bitcoin-core/secp256k1#1309]: changelog: Catch up
[bitcoin-core/secp256k1#1312]: release: Prepare for 0.3.2

This PR can be recreated with ./contrib/sync-upstream.sh -b sync-upstream range v0.3.2.
Tip: Use git show --remerge-diff to show the changes manually added to the merge commit.

  • remove fe_is_quad_var and use fe_is_square_var instead
  • remove bits argument from ecmult_const
  • secp256k1_fe_set_b32 -> secp256k1_fe_set_b32_limit or secp256k1_fe_set_b32_mod

real-or-random and others added 30 commits February 1, 2023 12:29
Also remove the interface it was attached to since it's no longer needed.
This removal simplifies the next commit.
…r 0.3.1

656c6ea release cleanup: bump version after 0.3.1 (Jonas Nick)

Pull request description:

ACKs for top commit:
  sipa:
    ACK 656c6ea
  real-or-random:
    ACK 656c6ea

Tree-SHA512: da24326ed5feaa6a432522bddd64e6c129455cfe55a9e2decfce8c6039f4ce1a1da64233d17200f45d2c142f5414505b9a9b2ef5d136e047c1dd6cfdde1b560d
Pregenerated files that we distribute should not have dependencies
in Makefile.am. For rationale, see the comments about the precomputed
table files.

See also bitcoin/bitcoin#27445 (comment) .
…roof header automatically

06c67de autotools: Don't regenerate Wycheproof header automatically (Tim Ruffing)

Pull request description:

  This is a hot fix for bitcoin/bitcoin#27445 .

  ---

  Pregenerated files that we distribute should not have dependencies in Makefile.am. For rationale, see the comments about the precomputed table files.

  See also bitcoin/bitcoin#27445 (comment) .

ACKs for top commit:
  hebasto:
    ACK 06c67de
  RandomLattice:
    ACK bitcoin-core/secp256k1@06c67de

Tree-SHA512: fa7f44eaa1c7e42ecba5829ac1b8ae8b5826d1a1551e01c3caf37af780bd5c102c8f54e88520723937f7016d93c67b62a334c7a28b96c4f422a38fcf8e6a1984
This PR lints tests_wycheproof_generate.py according to pylint.
This is a follow-up to PR #1245.

Co-authored-by: Sean Andersen <[email protected]>
…eases

1b6fb55 doc: clarify process for patch releases (Jonas Nick)

Pull request description:

ACKs for top commit:
  real-or-random:
    ACK 1b6fb55

Tree-SHA512: 5c1da34c920f66327b91c1fd11ad2eccbb55c5befdb3ba59138faf921ce83d0e7c62de84f2431b0a63433f1edc0f7f0f025a852a76dd3638e3fd583ca13b83e4
…1_wnaf_const{_xonly}

a575339 Remove bits argument from secp256k1_wnaf_const (always 256) (Pieter Wuille)

Pull request description:

  There is little reason for having the number of bits in the scalar as a parameter, as I don't think there are any (current) use cases for non-256-bit scalars.

ACKs for top commit:
  jonasnick:
    ACK a575339
  real-or-random:
    utACK a575339

Tree-SHA512: 994b1f19b4c513f6d070ed259a5d6f221a0c2450271ec824c5eba1cd0ecace276de391c170285bfeae96aaf8f1e0f7fe6260966ded0336c75c522ab6c56d182c
…not both

ef49a11 build: allow static or shared but not both (Cory Fields)
36b0adf build: remove warning until it's reproducible (Cory Fields)

Pull request description:

  Continuing from here: bitcoin-core/secp256k1#1224 (comment)

  Unfortunately it wasn't really possible to keep a clean diff here because of the nature of the change. I suggest reviewing the lib creation stuff in its entirety, sorry about that :\

  Rather than allowing for shared and static libs to be built at the same time like autotools, this PR switches to the CMake convention of allowing only 1.

  A new `BUILD_SHARED_LIBS` option is added to match CMake convention, as well as a `SECP256K1_DISABLE_SHARED` option which overrides it. That way even projects which have `BUILD_SHARED_LIBS=1` can opt-into a static libsecp in particular.

  Details:

  Two object libraries are created: `secp256k1_asm` and `secp256k1_precomputed_objs`. Some tests/benchmarks use the object libraries directly, some link against the real lib: `secp256k1`.

  Because the objs don't know what they're going to be linked into, they need to be told how to deal with PIC.

  The `DEFINE_SYMBOL` property sets the `DLL_EXPORT` define as necessary (when building a shared lib)

ACKs for top commit:
  hebasto:
    re-ACK ef49a11, only [suggested](bitcoin-core/secp256k1#1230 (review)) changes since my recent [review](bitcoin-core/secp256k1#1230 (review)).
  real-or-random:
    ACK ef49a11

Tree-SHA512: 8870de305176fdb677caff0fdfc6f8c59c0e906489cb72bc9980e551002812685e59e20d731f2a82e33628bdfbb7261eafd6f228038cad3ec83bd74335959600
…ript

35ada3b tests: lint wycheproof's python script (RandomLattice)

Pull request description:

  This PR lints tests_wycheproof_generate.py according to bitcoin's python linting scripts. This is a follow-up to PR #1245.

ACKs for top commit:
  sipa:
    utACK 35ada3b
  real-or-random:
    utACK 35ada3b

Tree-SHA512: ea405060d2e73ff3543626687de5bc5282be923b914bd5c8c53e65df8dca9bea0000c416603095efff29bc7ae43c2081454c4e506db0f6805443d023fbffaf4c
…preserve `CPPFLAGS`

1ecb94e build: Make `SECP_VALGRIND_CHECK` preserve `CPPFLAGS` (Hennadii Stepanov)

Pull request description:

  It was overlooked in #862 and #1027.

ACKs for top commit:
  real-or-random:
    utACK 1ecb94e

Tree-SHA512: 263fc600ce9743e4aad767150f706bf7d4325dabb9c363ed57f08fe38faea94d7d1999804947cffeacbe698bb6d959ee6de3f6e50400050a390ecc0db957e426
Useful for embedding secp256k1 in a subproject.
47ac3d6 cmake: Make installation optional (Anna “CyberTailor”)

Pull request description:

  Useful for embedding secp256k1 in a subproject.

ACKs for top commit:
  theuni:
    ACK 47ac3d6.
  real-or-random:
    utACK 47ac3d6
  hebasto:
    ACK 47ac3d6, tested on Ubuntu 23.04.

Tree-SHA512: 12ac0ba9dc38adf45684055386280b669384b5a4e528a3f6f4470fd0b7f57d64dfed6a8bb9f0a84cacfcb72f509534d71676c5ba37b27297b1a96676eea44e6e
`DESCRIPTION` is available in CMake 3.9+.
`HOMEPAGE_URL` is available in CMake 3.12+.
Available in CMake 3.3+.
real-or-random and others added 24 commits May 11, 2023 18:36
…ariable-length messages

cd54ac7 schnorrsig: Improve docs of schnorrsig_sign_custom (Tim Ruffing)
28687b0 schnorrsig: Add BIP340 varlen test vectors (Tim Ruffing)
97a98be schnorrsig: Refactor test vector code to allow varlen messages (Tim Ruffing)

Pull request description:

ACKs for top commit:
  sipa:
    ACK cd54ac7. I didn't verify the included test vectors match the BIP.
  jonasnick:
    ACK cd54ac7

Tree-SHA512: 268140e239b703aaf79825de2263675a8c31bef999f013ea532b0cd7b80f2d600d78f3872209a93774ba4dbc0a046108e87d151fc4604882c5636876026a0816
…al default callbacks

1907f0f build: Make tests work with external default callbacks (Tim Ruffing)

Pull request description:

ACKs for top commit:
  sipa:
    ACK 1907f0f
  jonasnick:
    ACK 1907f0f

Tree-SHA512: 198598f7bf5292bf5709187f9a40ddf9a0fba93e8b62afb49df2c05b4ef61c394cea43ee07615b51ceea97862228d8ad351fddef13c190cb2e6690943ed63128
… normalizing variants

5b32602 Split fe_set_b32 into reducing and normalizing variants (Pieter Wuille)

Pull request description:

  Follow-up to #1205.

  This splits the `secp256k1_fe_set_b32` function into two variants:
  * `secp256k1_fe_set_b32_mod`, which returns `void`, reduces modulo the curve order, and only promises weakly normalized output.
  * `secp256k1_fe_set_b32_limit`, which returns `int` indicating success/failure, and only promises valid output in case the input is in range (but guarantees it's strongly normalized in this case).

  This removes one of the few cases in the codebase where normalization status depends on runtime values, making it fixed at compile-time instead.

ACKs for top commit:
  real-or-random:
    ACK 5b32602
  jonasnick:
    ACK 5b32602

Tree-SHA512: 4b93502272638c6ecdef4d74afa629e7ee540c0a20b377dccedbe567857b56c4684fad3af4b4293ed7ba35fed4aa5d0beaacdd77a903f44f24e8d87305919b61
In the existing code, the compiler is allowed to allocate the RSI register
for outputs m0, m1, or m2, which are written to before the input in RSI is
read from. Fix this by marking them as early clobber.

Reported by ehoffman2 in bitcoin-core/secp256k1#766
In the field 5x52 asm for x86_64, stack variables are provided as outputs.
The existing inputs are all forcibly allocated to registers, so cannot
coincide, but mark them as early clobber anyway to make this clearer.
…ck if it's really supported

c6bb29b build: Rename `64bit` to `x86_64` (Hennadii Stepanov)
0324645 autotools: Add `SECP_ARM32_ASM_CHECK` macro (Hennadii Stepanov)
ed4ba23 cmake: Add `check_arm32_assembly` function (Hennadii Stepanov)
e5cf4bf build: Rename `arm` to `arm32` (Hennadii Stepanov)

Pull request description:

  Closes bitcoin-core/secp256k1#1034.

  Solves one item in bitcoin-core/secp256k1#1235.

ACKs for top commit:
  real-or-random:
    ACK c6bb29b tested on x86_64 but not on ARM

Tree-SHA512: c3615a18cfa30bb2cc53be18c09ccab08fc800b84444d8c6b333347b4db039a3981da61e7da5086dd9f4472838d7c031d554be9ddc7c435ba906852bba593982
…y clobber

8c9ae37 Add release note (Pieter Wuille)
350b4bd Mark stack variables as early clobber for technical correctness (Pieter Wuille)
0c729ba Bugfix: mark outputs as early clobber in scalar x86_64 asm (Pieter Wuille)

Pull request description:

ACKs for top commit:
  real-or-random:
    ACK 8c9ae37
  jonasnick:
    ACK 8c9ae37

Tree-SHA512: 874d01f5540d14b5188aec25f6441dbc6631f8d3980416040a3e250f1aef75150068415e7a458a9a3fb0d7cbdeb97f5c7e089b187d6d3dd79aa6e45274c241b6
…e from API"

3ad1027 Revert "Remove unused scratch space from API" (Jonas Nick)

Pull request description:

  This reverts commit 712e7f8.

  Removing the scratch space from the API may break bindings to the library.

ACKs for top commit:
  sipa:
    ACK 3ad1027
  real-or-random:
    ACK 3ad1027

Tree-SHA512: ad394c0a2f83fe3a5f400c0e8f2b9bf40037ce4141d4414e6345918f5e6003c61da02a538425a49bdeb5700f5ecb713bd58f5752c0715fb1fcc4950099fdc0e6
697e1cc changelog: Catch up (Tim Ruffing)
76b43f3 changelog: Add entry for #1303 (Tim Ruffing)

Pull request description:

ACKs for top commit:
  sipa:
    ACK 697e1cc
  jonasnick:
    ACK 697e1cc

Tree-SHA512: cfeb513effc69925bdedd3a298b1e2e5bf7709f68b453a5f157c584560b5400c3dc8b9ce87a775281cdea9db7f44e7e1337fbc93563f6efe350fe5defacbc4f6
d490ca2 release: Prepare for 0.3.2 (Tim Ruffing)

Pull request description:

ACKs for top commit:
  sipa:
    ACK d490ca2
  hebasto:
    ACK d490ca2
  jonasnick:
    ACK d490ca2

Tree-SHA512: 0785e9654974b25977dcdb00fe2e91d79a941143d278e315b96238e18c7aedd5814c2534c0aff356d8d4bb456ff8b815bea3657b99243e0a8296bbe635329cfb
@real-or-random
Copy link
Collaborator

For secp256k1_point_load in src/modules/musig/keyagg_impl.h:

Your mechanical translation to secp256k1_fe_set_b32_mod is correct. But I think we should also apply the following upstream patch, which changes secp256k1_fe_set_b32_limit -> secp256k1_fe_set_b32_limit in secp256k1_pubkey_load: bitcoin-core/secp256k1@f165252

I suggest just adding a commit to this PR, or opening a new PR after this one.

(Technically, it would be more consistent to do this when only when we sync bitcoin-core/secp256k1@f165252, but I doubt we'll remember.)

This is similar to the upstream commit "Normalize ge produced from
secp256k1_pubkey_load".
@jonasnick
Copy link
Contributor Author

jonasnick commented Jul 25, 2023

Ok good point. I added a commit that does something similar (can't use ARG_CHECK since I want this function to be of type void - it is assumed that the calling code checks that the input value is initialized).

Copy link
Collaborator

@real-or-random real-or-random left a comment

Choose a reason for hiding this comment

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

tACK e593ed5

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.

9 participants