🤔 maflcko requested changes to a pull request: "Docs improvements"
(https://github.com/bitcoin/bitcoin/pull/30337#pullrequestreview-2141087191)
Other changes are also still wrong, see https://github.com/bitcoin/bitcoin/pull/30337#issuecomment-2189405730
Please make sure all changes are correct, then please squash your commits according to https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#squashing-commits
(https://github.com/bitcoin/bitcoin/pull/30337#pullrequestreview-2141087191)
Other changes are also still wrong, see https://github.com/bitcoin/bitcoin/pull/30337#issuecomment-2189405730
Please make sure all changes are correct, then please squash your commits according to https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#squashing-commits
💬 laanwj commented on pull request "chainparams: Add achow101 DNS seeder":
(https://github.com/bitcoin/bitcoin/pull/30007#issuecomment-2191138604)
ACK 2721d64989c2b2114890586b7efd01ab4b062ca6
Adding a DNS seed adds redundancy, so does spreading them over as many top-level domains as possible, so i don't think "this domain might get blocked" is ever an argument against this.
(https://github.com/bitcoin/bitcoin/pull/30007#issuecomment-2191138604)
ACK 2721d64989c2b2114890586b7efd01ab4b062ca6
Adding a DNS seed adds redundancy, so does spreading them over as many top-level domains as possible, so i don't think "this domain might get blocked" is ever an argument against this.
💬 laanwj commented on pull request "rest: don't copy data when sending binary response":
(https://github.com/bitcoin/bitcoin/pull/30321#issuecomment-2191157177)
re-ACK 1556d21599a250297d5f20e5249c970340ab08bc
(https://github.com/bitcoin/bitcoin/pull/30321#issuecomment-2191157177)
re-ACK 1556d21599a250297d5f20e5249c970340ab08bc
🤔 hebasto reviewed a pull request: "depends: build libevent with CMake"
(https://github.com/bitcoin/bitcoin/pull/29835#pullrequestreview-2141182567)
My Guix build:
```
x86_64
d702d02df48bc540da55c47ca7110d122a27ba179ab728fb8bdb6e27589f754c guix-build-f59e9057e2aa/output/aarch64-linux-gnu/SHA256SUMS.part
d806e1994bc873a975714ce75bca87fd3fcec4055e24d1ed0afe99ebc3503288 guix-build-f59e9057e2aa/output/aarch64-linux-gnu/bitcoin-f59e9057e2aa-aarch64-linux-gnu-debug.tar.gz
a9844e6a4d3e86df69b98281c942ac01136b4b115fc6a2504740368c5d441fbd guix-build-f59e9057e2aa/output/aarch64-linux-gnu/bitcoin-f59e9057e2aa-aarch64-linux-gnu.tar.gz
4e3ea3b82
...
(https://github.com/bitcoin/bitcoin/pull/29835#pullrequestreview-2141182567)
My Guix build:
```
x86_64
d702d02df48bc540da55c47ca7110d122a27ba179ab728fb8bdb6e27589f754c guix-build-f59e9057e2aa/output/aarch64-linux-gnu/SHA256SUMS.part
d806e1994bc873a975714ce75bca87fd3fcec4055e24d1ed0afe99ebc3503288 guix-build-f59e9057e2aa/output/aarch64-linux-gnu/bitcoin-f59e9057e2aa-aarch64-linux-gnu-debug.tar.gz
a9844e6a4d3e86df69b98281c942ac01136b4b115fc6a2504740368c5d441fbd guix-build-f59e9057e2aa/output/aarch64-linux-gnu/bitcoin-f59e9057e2aa-aarch64-linux-gnu.tar.gz
4e3ea3b82
...
💬 laanwj commented on pull request "build: Drop redundant `sys/sysctl.h` header check":
(https://github.com/bitcoin/bitcoin/pull/30327#issuecomment-2191181941)
ACK c0b5ea5901d0ed005bca345e5b2de8a502f6af75
verified there is no `HAVE_SYS_SYSCTL_H` in the code, nor ever was, while it was introduced at the same time as `HAVE_SYSCTL`
(https://github.com/bitcoin/bitcoin/pull/30327#issuecomment-2191181941)
ACK c0b5ea5901d0ed005bca345e5b2de8a502f6af75
verified there is no `HAVE_SYS_SYSCTL_H` in the code, nor ever was, while it was introduced at the same time as `HAVE_SYSCTL`
👍 TheCharlatan approved a pull request: "depends: build libevent with CMake"
(https://github.com/bitcoin/bitcoin/pull/29835#pullrequestreview-2141199689)
ACK f59e9057e2aa596b54cf9e85bab35c3ead137547
(https://github.com/bitcoin/bitcoin/pull/29835#pullrequestreview-2141199689)
ACK f59e9057e2aa596b54cf9e85bab35c3ead137547
💬 brunoerg commented on pull request "test: add coverage for `node` field of `getaddednodeinfo` RPC":
(https://github.com/bitcoin/bitcoin/pull/30339#discussion_r1654435108)
`added_node_0` seems weird because it can be confused with nodes[0], I prefer `first_added_node`. I will address it.
(https://github.com/bitcoin/bitcoin/pull/30339#discussion_r1654435108)
`added_node_0` seems weird because it can be confused with nodes[0], I prefer `first_added_node`. I will address it.
💬 brunoerg commented on pull request "test: add coverage for `node` field of `getaddednodeinfo` RPC":
(https://github.com/bitcoin/bitcoin/pull/30339#discussion_r1654436832)
Because I wanted to remove exactly the first added node.
(https://github.com/bitcoin/bitcoin/pull/30339#discussion_r1654436832)
Because I wanted to remove exactly the first added node.
💬 brunoerg commented on pull request "test: add coverage for `node` field of `getaddednodeinfo` RPC":
(https://github.com/bitcoin/bitcoin/pull/30339#discussion_r1654445965)
It seems good to me, I'll address it.
(https://github.com/bitcoin/bitcoin/pull/30339#discussion_r1654445965)
It seems good to me, I'll address it.
💬 brunoerg commented on pull request "test: add coverage for `node` field of `getaddednodeinfo` RPC":
(https://github.com/bitcoin/bitcoin/pull/30339#discussion_r1654446440)
Yes, I can add this verification.
(https://github.com/bitcoin/bitcoin/pull/30339#discussion_r1654446440)
Yes, I can add this verification.
💬 fanquake commented on pull request "contrib: add R(UN)PATH check to ELF symbol-check":
(https://github.com/bitcoin/bitcoin/pull/30312#issuecomment-2191235833)
> performing binary checks after the installation step.
I'll followup with this separately, pre-CMake, given it will be needed there.
(https://github.com/bitcoin/bitcoin/pull/30312#issuecomment-2191235833)
> performing binary checks after the installation step.
I'll followup with this separately, pre-CMake, given it will be needed there.
🚀 fanquake merged a pull request: "contrib: add R(UN)PATH check to ELF symbol-check"
(https://github.com/bitcoin/bitcoin/pull/30312)
(https://github.com/bitcoin/bitcoin/pull/30312)
✅ fanquake closed an issue: "bug: verify-binaries/verify.py incorrectly parses version string; gives error or downloads wrong files"
(https://github.com/bitcoin/bitcoin/issues/30145)
(https://github.com/bitcoin/bitcoin/issues/30145)
🚀 fanquake merged a pull request: "contrib: Fixup verify-binaries OS platform parsing"
(https://github.com/bitcoin/bitcoin/pull/30147)
(https://github.com/bitcoin/bitcoin/pull/30147)
👍 real-or-random approved a pull request: "Update libsecp256k1 subtree to latest master"
(https://github.com/bitcoin/bitcoin/pull/30334#pullrequestreview-2141273911)
utACK cc58e958f341d2759fbabe5c9d8cc557e17d587f
(https://github.com/bitcoin/bitcoin/pull/30334#pullrequestreview-2141273911)
utACK cc58e958f341d2759fbabe5c9d8cc557e17d587f
💬 AngusP commented on pull request "Mining interface followups":
(https://github.com/bitcoin/bitcoin/pull/30335#discussion_r1654485896)
It is also acquired within `miner.testBlockValidity` (and in `miner.createNewBlock`) so maybe is now unnecessary outside the Miner interface?
https://github.com/bitcoin/bitcoin/blob/a9716c53f05082d6d89ebea51a46d4404efb12d7/src/node/interfaces.cpp#L873-L877
(https://github.com/bitcoin/bitcoin/pull/30335#discussion_r1654485896)
It is also acquired within `miner.testBlockValidity` (and in `miner.createNewBlock`) so maybe is now unnecessary outside the Miner interface?
https://github.com/bitcoin/bitcoin/blob/a9716c53f05082d6d89ebea51a46d4404efb12d7/src/node/interfaces.cpp#L873-L877
💬 TheCharlatan commented on pull request "Introduce Mining interface":
(https://github.com/bitcoin/bitcoin/pull/30200#discussion_r1654486212)
Is this lock required?
(https://github.com/bitcoin/bitcoin/pull/30200#discussion_r1654486212)
Is this lock required?
💬 laanwj commented on pull request "contrib: add R(UN)PATH check to ELF symbol-check":
(https://github.com/bitcoin/bitcoin/pull/30312#issuecomment-2191286545)
Post-merge ACK
(https://github.com/bitcoin/bitcoin/pull/30312#issuecomment-2191286545)
Post-merge ACK
👍 Sjors approved a pull request: "Testnet4 including PoW difficulty adjustment fix"
(https://github.com/bitcoin/bitcoin/pull/29775#pullrequestreview-2140918229)
ACK aba504759caa13338aa574a7839d078b989fbf6d
(https://github.com/bitcoin/bitcoin/pull/29775#pullrequestreview-2140918229)
ACK aba504759caa13338aa574a7839d078b989fbf6d
💬 Sjors commented on pull request "Testnet4 including PoW difficulty adjustment fix":
(https://github.com/bitcoin/bitcoin/pull/29775#discussion_r1654259706)
e40f06acfd090797f14abdc1768d7b96ae4eeed8: one advantage of having already had some mining is that we can set `nMinimumChainWork` shortly before the release (if only so people can test the headers pre-sync behavior).
(https://github.com/bitcoin/bitcoin/pull/29775#discussion_r1654259706)
e40f06acfd090797f14abdc1768d7b96ae4eeed8: one advantage of having already had some mining is that we can set `nMinimumChainWork` shortly before the release (if only so people can test the headers pre-sync behavior).