💬 hebasto commented on pull request "cmake: Add application manifests when cross-compiling for Windows":
(https://github.com/bitcoin/bitcoin/pull/32396#discussion_r2079589157)
> > What alternatives are you suggesting for configuring the Developer Command Prompt?
>
> Directly running the command that somebody who owns a Windows computer would run, that doesn't require installing NPM, and then executing somebody else's JavaScript.

Thanks. [Reworked](https://github.com/bitcoin/bitcoin/pull/32396#issuecomment-2862815509).

💬 instagibbs commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#discussion_r2079591061)
I can add a note if I touch the PR again

💬 instagibbs commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#discussion_r2079591354)
Leaving as-is for now

👍 hebasto approved a pull request: "build: let CMake determine the year"
(https://github.com/bitcoin/bitcoin/pull/32445#pullrequestreview-2824945378)
re-ACK dfeac76b951fc2d28e0832b176f3c8ad595697f1.

💬 janb84 commented on pull request "doc: swap "Docker image" for "container image"":
(https://github.com/bitcoin/bitcoin/pull/32444#issuecomment-2862868108)
ACK https://github.com/bitcoin/bitcoin/commit/1372eb09c5d031315bc6cde34f7a15297e96f4cc

💬 hebasto commented on pull request "build: simplify *ifaddr handling":
(https://github.com/bitcoin/bitcoin/pull/32446#issuecomment-2862878161)
Concept ACK. I have considered a similar idea.

💬 Sjors commented on pull request "guix: accomodate migration to codeberg":
(https://github.com/bitcoin/bitcoin/pull/32439#issuecomment-2862895879)
Concept ACK.

💬 hebasto commented on pull request "cmake: Add application manifests when cross-compiling for Windows":
(https://github.com/bitcoin/bitcoin/pull/32396#issuecomment-2862902153)
My Guix build:
```
aarch64
be6aa004944a606e9bbef9129b971960186a533f6572c5fb2e522fabc544c611 guix-build-477b69ad19d7/output/dist-archive/bitcoin-477b69ad19d7.tar.gz
e75f1ed645abf5d086f5e328107ce804e3ebec9ac5b2d0d8a9d037744e2e4859 guix-build-477b69ad19d7/output/x86_64-w64-mingw32/SHA256SUMS.part
526d76c4402086b1f8382d5b461162485efcaaf1ebf98a23dc9fd8b8f7bd62d8 guix-build-477b69ad19d7/output/x86_64-w64-mingw32/bitcoin-477b69ad19d7-win64-codesigning.tar.gz
11029fcb9a8f11c7df44b2dfbda58392d22
...

💬 fanquake commented on pull request "dbwrapper: Bump LevelDB max file size to 32 MiB to avoid system slowdown from high disk cache flush rate":
(https://github.com/bitcoin/bitcoin/pull/30039#issuecomment-2862903991)
Anyone that reviewed here, might be interested in https://github.com/bitcoin-core/leveldb-subtree/pull/52.

👍 laanwj approved a pull request: "build: let CMake determine the year"
(https://github.com/bitcoin/bitcoin/pull/32445#pullrequestreview-2824986604)
Code review ACK dfeac76b951fc2d28e0832b176f3c8ad595697f1

For example:
```sh
$ export SOURCE_DATE_EPOCH=1
$ cmake -B build
$ grep YEAR build/src/*.h
build/src/bitcoin-build-config.h:#define COPYRIGHT_YEAR 1970
```
In the guix build it sets `SOURCE_DATE_EPOCH` based on the last commit
```
SOURCE_DATE_EPOCH="${SOURCE_DATE_EPOCH:-$(git -c log.showSignature=false log --format=%at -1)}"
```
So ostensibly this works out the correct date deterministically.

Should test an actual guix bui
...

👍 theStack approved a pull request: "psbt: add non-default sighash types to PSBTs and unify sighash type match checking"
(https://github.com/bitcoin/bitcoin/pull/31622#pullrequestreview-2824981042)
re-ACK f09e1f943dbd1e5865dece3c35f342e4f0b31b9e :writing_hand: :hash:

💬 theStack commented on pull request "psbt: add non-default sighash types to PSBTs and unify sighash type match checking":
(https://github.com/bitcoin/bitcoin/pull/31622#discussion_r2079628809)
nitty nit: now that the checks are done via `testmempool` above, these two lines could be deleted

💬 fanquake commented on pull request "doc: swap "Docker image" for "container image"":
(https://github.com/bitcoin/bitcoin/pull/32444#issuecomment-2862942029)
Pushed a commit up here: https://github.com/fanquake/core-review/commit/1656c4891dc1ac83b66765ba5a2607815b91d6eb.

🚀 fanquake merged a pull request: "doc: swap "Docker image" for "container image""
(https://github.com/bitcoin/bitcoin/pull/32444)

💬 hebasto commented on pull request "build: simplify *ifaddr handling":
(https://github.com/bitcoin/bitcoin/pull/32446#discussion_r2079675289)
"Define to 1 if '*ifaddrs' are available" implies that "it remains undefined otherwise". To achieve this, I suggest:
```suggestion
/* Define to 1 if '*ifaddrs' are available. */
#cmakedefine HAVE_IFADDRS 1
```

This also allows to replace the following `#if HAVE_IFADDRS` with `#ifdef HAVE_IFADDRS`.

💬 hebasto commented on pull request "Update `secp256k1` subtree to latest master":
(https://github.com/bitcoin/bitcoin/pull/32028#issuecomment-2863028605)
> No movement in #31507, but could rebase & update this with the one new commit?

Sure! Rebased and updated.

💬 laanwj commented on pull request "test: refactor: negate signature-s using libsecp256k1":
(https://github.com/bitcoin/bitcoin/pull/32436#issuecomment-2863033532)
ACK
i think this would make it slightly more clear what is going on with regard to ranges and validity:
```diff
diff --git a/src/test/script_tests.cpp b/src/test/script_tests.cpp
index 33a4555dd595c9e4886f194cb113b905a55bddb3..6f251b094c5b68373f30d1d578088e65d4595a09 100644
--- a/src/test/script_tests.cpp
+++ b/src/test/script_tests.cpp
@@ -148,11 +148,12 @@ void static NegateSignatureS(std::vector<unsigned char>& vchSig) {
while (s.size() < 33) {
s.insert(s.begin(), 0x00)
...

💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#issuecomment-2863039003)
`1c16944a4a...32488cfd6c`: rebase due to conflicts

🤔 rkrux reviewed a pull request: "test: refactor: negate signature-s using libsecp256k1"
(https://github.com/bitcoin/bitcoin/pull/32436#pullrequestreview-2825019929)
Concept ACK bfb7b8876035774eddb200d741aa6a86859069b3

Conceptually, I'm in agreement with the replacement of custom negation code by using the one from secp256k1. Though I was a little surprised to note that git grep of this function doesn't lead to any actual usages of the function besides the tests & `secp256k1_ec_privkey_negate` function that too doesn't seem to be used elsewhere.

```bash
git grep -n "secp256k1_ec_seckey_negate"

src/secp256k1/include/secp256k1.h:699:SECP256K1_API SEC
...

💬 rkrux commented on pull request "test: refactor: negate signature-s using libsecp256k1":
(https://github.com/bitcoin/bitcoin/pull/32436#discussion_r2079651302)
> Note that as the name suggests, secp256k1_ec_seckey_negate is meant to be used for secret keys, but it obviously works in general for scalars modulo the group order.

I can see a call to `secp256k1_scalar_negate` inside `secp256k1_ec_seckey_negate`.

https://github.com/bitcoin/bitcoin/blob/64697529523705f5a411ca893060429afcd5cd47/src/secp256k1/src/secp256k1.c#L622-L635

Nit: However, the current comment makes one wonder why it is (ab)using libsecp256k1, which is answered in the commit m
...