💬 Eunovo commented on pull request "validation: ensure assumevalid is always used during reindex":
(https://github.com/bitcoin/bitcoin/pull/31615#discussion_r2092562248)
Will do
(https://github.com/bitcoin/bitcoin/pull/31615#discussion_r2092562248)
Will do
🚀 fanquake merged a pull request: "cmake: Add application manifests when cross-compiling for Windows"
(https://github.com/bitcoin/bitcoin/pull/32396)
(https://github.com/bitcoin/bitcoin/pull/32396)
💬 maflcko commented on pull request "ci: Enable feature_init and wallet_reorgsrestore in valgrind task":
(https://github.com/bitcoin/bitcoin/pull/32519#issuecomment-2886010555)
If someone wants to quickly test this outside the CI system a minimal reproducer would be:
```
───────┬────────────────────────────────────────────────────────────────────────
│ File: /tmp/a.py
───────┼────────────────────────────────────────────────────────────────────────
1 │ import subprocess
2 │ import time
3 │
4 │ process = subprocess.Popen(["bash", "/tmp/a.sh"])
5 │ print("Wait for the first print to happen ...")
6 │ time.sleep(0.1)
7
...
(https://github.com/bitcoin/bitcoin/pull/32519#issuecomment-2886010555)
If someone wants to quickly test this outside the CI system a minimal reproducer would be:
```
───────┬────────────────────────────────────────────────────────────────────────
│ File: /tmp/a.py
───────┼────────────────────────────────────────────────────────────────────────
1 │ import subprocess
2 │ import time
3 │
4 │ process = subprocess.Popen(["bash", "/tmp/a.sh"])
5 │ print("Wait for the first print to happen ...")
6 │ time.sleep(0.1)
7
...
💬 fanquake commented on pull request "depends: bump to latest config.guess and config.sub":
(https://github.com/bitcoin/bitcoin/pull/32505#discussion_r2092584341)
I guess this will allow someone to try and build depends with an LLVM libc target. i.e:
```bash
# master
make -C depends/ HOST=aarch64-unknown-linux-llvm
make: Entering directory '/root/ci_scratch/depends'
Invalid configuration 'aarch64-unknown-linux-llvm': OS 'llvm' not recognized
```
```bash
# this branch
make -C depends/ HOST=aarch64-unknown-linux-llvm
make: Entering directory '/root/ci_scratch/depends'
Extracting native_qt..
```
(https://github.com/bitcoin/bitcoin/pull/32505#discussion_r2092584341)
I guess this will allow someone to try and build depends with an LLVM libc target. i.e:
```bash
# master
make -C depends/ HOST=aarch64-unknown-linux-llvm
make: Entering directory '/root/ci_scratch/depends'
Invalid configuration 'aarch64-unknown-linux-llvm': OS 'llvm' not recognized
```
```bash
# this branch
make -C depends/ HOST=aarch64-unknown-linux-llvm
make: Entering directory '/root/ci_scratch/depends'
Extracting native_qt..
```
👍 fanquake approved a pull request: "depends: bump to latest config.guess and config.sub"
(https://github.com/bitcoin/bitcoin/pull/32505#pullrequestreview-2845902109)
ACK 486bc91790721a37047762c1ea893c29746b4358
(https://github.com/bitcoin/bitcoin/pull/32505#pullrequestreview-2845902109)
ACK 486bc91790721a37047762c1ea893c29746b4358
💬 davidgumberg commented on issue "Restrict RPCs that make server-side files":
(https://github.com/bitcoin/bitcoin/issues/20866#issuecomment-2886024029)
Related: https://github.com/bitcoin/bitcoin/issues/32274
(https://github.com/bitcoin/bitcoin/issues/20866#issuecomment-2886024029)
Related: https://github.com/bitcoin/bitcoin/issues/32274
💬 Eunovo commented on pull request "validation: ensure assumevalid is always used during reindex":
(https://github.com/bitcoin/bitcoin/pull/31615#discussion_r2092592761)
Are you referring to the 100 blocks mined after the first 2100 blocks?
(https://github.com/bitcoin/bitcoin/pull/31615#discussion_r2092592761)
Are you referring to the 100 blocks mined after the first 2100 blocks?
🚀 fanquake merged a pull request: "init: drop `-upnp`"
(https://github.com/bitcoin/bitcoin/pull/32500)
(https://github.com/bitcoin/bitcoin/pull/32500)
🚀 fanquake merged a pull request: "depends: bump to latest config.guess and config.sub"
(https://github.com/bitcoin/bitcoin/pull/32505)
(https://github.com/bitcoin/bitcoin/pull/32505)
💬 vasild commented on pull request "net: improve the interface around FindNode() and avoid a recursive mutex lock":
(https://github.com/bitcoin/bitcoin/pull/32326#issuecomment-2886037713)
`4f635d100d...8668b0c64d`: address https://github.com/bitcoin/bitcoin/pull/32326#discussion_r2072443916
(https://github.com/bitcoin/bitcoin/pull/32326#issuecomment-2886037713)
`4f635d100d...8668b0c64d`: address https://github.com/bitcoin/bitcoin/pull/32326#discussion_r2072443916
💬 vasild commented on pull request "net: improve the interface around FindNode() and avoid a recursive mutex lock":
(https://github.com/bitcoin/bitcoin/pull/32326#discussion_r2092602639)
Changed to
```cpp
bool AlreadyConnectedToAddressPort(const std::string& addr_port);
bool AlreadyConnectedToAddressPort(const CService& addr_port);
bool AlreadyConnectedToAddress(const CNetAddr& addr);
```
(https://github.com/bitcoin/bitcoin/pull/32326#discussion_r2092602639)
Changed to
```cpp
bool AlreadyConnectedToAddressPort(const std::string& addr_port);
bool AlreadyConnectedToAddressPort(const CService& addr_port);
bool AlreadyConnectedToAddress(const CNetAddr& addr);
```
👍 willcl-ark approved a pull request: "Update minisketch subtree"
(https://github.com/bitcoin/bitcoin/pull/32485#pullrequestreview-2845934886)
ACK 46b533dfe6fc6c956ac178896d9caf1d59b73d9f
The changes look correct vs upstream.
Apologies for my git-ignorance, but why are there two commits with the same changes? Is that how a subtree update normally looks?
(https://github.com/bitcoin/bitcoin/pull/32485#pullrequestreview-2845934886)
ACK 46b533dfe6fc6c956ac178896d9caf1d59b73d9f
The changes look correct vs upstream.
Apologies for my git-ignorance, but why are there two commits with the same changes? Is that how a subtree update normally looks?
👍 hodlinator approved a pull request: "net: improve the interface around FindNode() and avoid a recursive mutex lock"
(https://github.com/bitcoin/bitcoin/pull/32326#pullrequestreview-2845962709)
re-ACK 8668b0c64d59d4dd7207bf8347b23b8b50aeb7c3
Just changed prefixes from `Is-` -> `Already-` and nicely simplified message in initial commit as per https://github.com/bitcoin/bitcoin/pull/32326#discussion_r2072443916.
(https://github.com/bitcoin/bitcoin/pull/32326#pullrequestreview-2845962709)
re-ACK 8668b0c64d59d4dd7207bf8347b23b8b50aeb7c3
Just changed prefixes from `Is-` -> `Already-` and nicely simplified message in initial commit as per https://github.com/bitcoin/bitcoin/pull/32326#discussion_r2072443916.
💬 fanquake commented on pull request "cmake: Add application manifests when cross-compiling for Windows":
(https://github.com/bitcoin/bitcoin/pull/32396#issuecomment-2886076162)
Guix Build
```bash
14f6c52455cfd4f871e97271938e906137d0a0eac8adb381cf22775e71e9662a guix-build-8f4fed7ec700/output/aarch64-linux-gnu/SHA256SUMS.part
793ebdb24d8956a0829c59074e3a5748b0914aa34f076956a6d0b4ef09274187 guix-build-8f4fed7ec700/output/aarch64-linux-gnu/bitcoin-8f4fed7ec700-aarch64-linux-gnu-debug.tar.gz
6bc3f991fce9e2f3dbd9e8aafd3f5a3957f92fdef478c1b95af2e5badfaddc30 guix-build-8f4fed7ec700/output/aarch64-linux-gnu/bitcoin-8f4fed7ec700-aarch64-linux-gnu.tar.gz
e475ede22a88ba1ca
...
(https://github.com/bitcoin/bitcoin/pull/32396#issuecomment-2886076162)
Guix Build
```bash
14f6c52455cfd4f871e97271938e906137d0a0eac8adb381cf22775e71e9662a guix-build-8f4fed7ec700/output/aarch64-linux-gnu/SHA256SUMS.part
793ebdb24d8956a0829c59074e3a5748b0914aa34f076956a6d0b4ef09274187 guix-build-8f4fed7ec700/output/aarch64-linux-gnu/bitcoin-8f4fed7ec700-aarch64-linux-gnu-debug.tar.gz
6bc3f991fce9e2f3dbd9e8aafd3f5a3957f92fdef478c1b95af2e5badfaddc30 guix-build-8f4fed7ec700/output/aarch64-linux-gnu/bitcoin-8f4fed7ec700-aarch64-linux-gnu.tar.gz
e475ede22a88ba1ca
...
🤔 rkrux reviewed a pull request: "refactor: bdb removals"
(https://github.com/bitcoin/bitcoin/pull/32511#pullrequestreview-2845971700)
crACK fafee85358397289aa4c6b799d2603a5d89e83a2
The functions in the last 3 commits do look like good helper functions but I don't mind their removal as they are not used besides tests, making it easier to navigate the codebase that I prefer.
An alternative could be to move some of them under tests directory. But fine with current state of the PR too.
(https://github.com/bitcoin/bitcoin/pull/32511#pullrequestreview-2845971700)
crACK fafee85358397289aa4c6b799d2603a5d89e83a2
The functions in the last 3 commits do look like good helper functions but I don't mind their removal as they are not used besides tests, making it easier to navigate the codebase that I prefer.
An alternative could be to move some of them under tests directory. But fine with current state of the PR too.
💬 maflcko commented on pull request "Update minisketch subtree":
(https://github.com/bitcoin/bitcoin/pull/32485#issuecomment-2886088208)
> Apologies for my git-ignorance, but why are there two commits with the same changes? Is that how a subtree update normally looks?
If you want to preserve the subtree, the only way to update is via a merge (I think), so you need one commit for the subtree changes itself and then a merge commit to merge them into this repo.
(https://github.com/bitcoin/bitcoin/pull/32485#issuecomment-2886088208)
> Apologies for my git-ignorance, but why are there two commits with the same changes? Is that how a subtree update normally looks?
If you want to preserve the subtree, the only way to update is via a merge (I think), so you need one commit for the subtree changes itself and then a merge commit to merge them into this repo.
🚀 fanquake merged a pull request: "refactor: reenable `implicit-integer-sign-change` check for `serialize.h`"
(https://github.com/bitcoin/bitcoin/pull/32296)
(https://github.com/bitcoin/bitcoin/pull/32296)
💬 maflcko commented on pull request "Update minisketch subtree":
(https://github.com/bitcoin/bitcoin/pull/32485#issuecomment-2886091020)
lgtm ACK 46b533dfe6fc6c956ac178896d9caf1d59b73d9f
The changes look like the merged upstream changes. I haven't run the subtree check locally to confirm.
(https://github.com/bitcoin/bitcoin/pull/32485#issuecomment-2886091020)
lgtm ACK 46b533dfe6fc6c956ac178896d9caf1d59b73d9f
The changes look like the merged upstream changes. I haven't run the subtree check locally to confirm.
💬 maflcko commented on pull request "refactor: bdb removals":
(https://github.com/bitcoin/bitcoin/pull/32511#issuecomment-2886105442)
> The functions in the last 3 commits do look like good helper functions but I don't mind their removal as they are not used besides tests, making it easier to navigate the codebase that I prefer.
> An alternative could be to move some of them under tests directory. But fine with current state of the PR too.
It is hard for me to see a use case for the functions, even in tests. The tests only used them to generate dummy values, but those are trivially created inline. Generally, when it comes
...
(https://github.com/bitcoin/bitcoin/pull/32511#issuecomment-2886105442)
> The functions in the last 3 commits do look like good helper functions but I don't mind their removal as they are not used besides tests, making it easier to navigate the codebase that I prefer.
> An alternative could be to move some of them under tests directory. But fine with current state of the PR too.
It is hard for me to see a use case for the functions, even in tests. The tests only used them to generate dummy values, but those are trivially created inline. Generally, when it comes
...
💬 fanquake commented on pull request "Update minisketch subtree":
(https://github.com/bitcoin/bitcoin/pull/32485#issuecomment-2886105544)
For reference, the command used here is `git subtree pull --prefix src/minisketch https://github.com/bitcoin-core/minisketch.git master --squash`.
(https://github.com/bitcoin/bitcoin/pull/32485#issuecomment-2886105544)
For reference, the command used here is `git subtree pull --prefix src/minisketch https://github.com/bitcoin-core/minisketch.git master --squash`.