📝 JeremyRand opened a pull request: "doc: Use more precise anchor link to codesigning docs"
(https://github.com/bitcoin/bitcoin/pull/31387)
The "Codesigning" section is what users presumably are looking for when they follow this link.
(https://github.com/bitcoin/bitcoin/pull/31387)
The "Codesigning" section is what users presumably are looking for when they follow this link.
💬 maflcko commented on pull request "test: Avoid intermittent block download timeout in p2p_ibd_stalling":
(https://github.com/bitcoin/bitcoin/pull/30705#issuecomment-2505414701)
Fixup in https://github.com/bitcoin/bitcoin/pull/31383
(https://github.com/bitcoin/bitcoin/pull/30705#issuecomment-2505414701)
Fixup in https://github.com/bitcoin/bitcoin/pull/31383
💬 rkrux commented on pull request "doc, test: more ephemeral dust follow-ups":
(https://github.com/bitcoin/bitcoin/pull/31371#discussion_r1861638355)
Ah I see, makes sense to me.
(https://github.com/bitcoin/bitcoin/pull/31371#discussion_r1861638355)
Ah I see, makes sense to me.
💬 rkrux commented on pull request "doc, test: more ephemeral dust follow-ups":
(https://github.com/bitcoin/bitcoin/pull/31371#discussion_r1861656216)
Oh interesting, thanks for the clarification. It was seeing `fee_per_output=dust_tx_fee` together that threw me off. Maybe an argument comment here at this call site of `create self transfer multi` can be more clarifying stating that there is only 1 output created initially? That is if the file is retouched.
(https://github.com/bitcoin/bitcoin/pull/31371#discussion_r1861656216)
Oh interesting, thanks for the clarification. It was seeing `fee_per_output=dust_tx_fee` together that threw me off. Maybe an argument comment here at this call site of `create self transfer multi` can be more clarifying stating that there is only 1 output created initially? That is if the file is retouched.
💬 maflcko commented on pull request "ci: detect outbound internet traffic generated while running tests":
(https://github.com/bitcoin/bitcoin/pull/31349#discussion_r1861672332)
I don't think this is sufficient. The CI failure (https://github.com/bitcoin/bitcoin/pull/31349#issuecomment-2499335672) will remain, depending on a vendor default.
Again, my preference would be to explicitly list the required (or removed) caps, instead of relying on a vendor default. Otherwise, it will become harder to run the CI locally, or lead to vendor-lock-in.
(https://github.com/bitcoin/bitcoin/pull/31349#discussion_r1861672332)
I don't think this is sufficient. The CI failure (https://github.com/bitcoin/bitcoin/pull/31349#issuecomment-2499335672) will remain, depending on a vendor default.
Again, my preference would be to explicitly list the required (or removed) caps, instead of relying on a vendor default. Otherwise, it will become harder to run the CI locally, or lead to vendor-lock-in.
💬 maflcko commented on pull request "ci: Update Clang in "tidy" job":
(https://github.com/bitcoin/bitcoin/pull/31306#discussion_r1861679662)
> Looking at the actual usage, `ExpectSuccess` doesn't need to be a variadic template function:
This may or may not be true right now, but the function has been intentionally written the way it is now. This is a pretty standard and common boilerplate code to pass a variable number of args to a constructor.
Changing it would be problematic, because it will need to be touched again in the future to be reverted, if there is a constructor with more than one arg.
Also, your suggestion will f
...
(https://github.com/bitcoin/bitcoin/pull/31306#discussion_r1861679662)
> Looking at the actual usage, `ExpectSuccess` doesn't need to be a variadic template function:
This may or may not be true right now, but the function has been intentionally written the way it is now. This is a pretty standard and common boilerplate code to pass a variable number of args to a constructor.
Changing it would be problematic, because it will need to be touched again in the future to be reverted, if there is a constructor with more than one arg.
Also, your suggestion will f
...
💬 TheCharlatan commented on pull request "kernel: Flush in ChainstateManager destructor":
(https://github.com/bitcoin/bitcoin/pull/31382#issuecomment-2505569501)
> maybe time to introduce a replace mempool method like Carl suggested there?
Did this now, ready for review.
(https://github.com/bitcoin/bitcoin/pull/31382#issuecomment-2505569501)
> maybe time to introduce a replace mempool method like Carl suggested there?
Did this now, ready for review.
👋 TheCharlatan's pull request is ready for review: "kernel: Flush in ChainstateManager destructor"
(https://github.com/bitcoin/bitcoin/pull/31382)
(https://github.com/bitcoin/bitcoin/pull/31382)
💬 Sjors commented on pull request "multiprocess: add bitcoin-mine test program":
(https://github.com/bitcoin/bitcoin/pull/30437#issuecomment-2505597125)
I rebased https://github.com/Sjors/bitcoin/pull/48 to use the latest version of this PR, to figure out what I need for cmake. The only thing I still suggest is:
* add `bitcoin-mine` to the `foreach(target IN ITEMS` list in `cmake/module/Maintenance.cmake`. IIUC that will cause "the Guix script to run symbol-check.py and security-check.py on them". Even though this PR doesn't add bitcoin-mine to the guix build artefacts, it seems better to add it to this list now so we don't forget.
ACK 270
...
(https://github.com/bitcoin/bitcoin/pull/30437#issuecomment-2505597125)
I rebased https://github.com/Sjors/bitcoin/pull/48 to use the latest version of this PR, to figure out what I need for cmake. The only thing I still suggest is:
* add `bitcoin-mine` to the `foreach(target IN ITEMS` list in `cmake/module/Maintenance.cmake`. IIUC that will cause "the Guix script to run symbol-check.py and security-check.py on them". Even though this PR doesn't add bitcoin-mine to the guix build artefacts, it seems better to add it to this list now so we don't forget.
ACK 270
...
💬 fanquake commented on pull request "multiprocess: add bitcoin-mine test program":
(https://github.com/bitcoin/bitcoin/pull/30437#issuecomment-2505607490)
> it seems better to add it to this list now so we don't forget.
Nack. We can add things to Guix and the release process when they are actually part of a release.
(https://github.com/bitcoin/bitcoin/pull/30437#issuecomment-2505607490)
> it seems better to add it to this list now so we don't forget.
Nack. We can add things to Guix and the release process when they are actually part of a release.
🤔 Sjors reviewed a pull request: "multiprocess: Add bitcoin wrapper executable"
(https://github.com/bitcoin/bitcoin/pull/31375#pullrequestreview-2467341532)
Concept ACK
I think it would be more clear to move `build/src/bitcoin-{node,gui}` to `build/src/libexec`, rather than use a different file organization for CMake builds than for installs.
The "Win64 native, VS 2022" job still seems unhappy.
(https://github.com/bitcoin/bitcoin/pull/31375#pullrequestreview-2467341532)
Concept ACK
I think it would be more clear to move `build/src/bitcoin-{node,gui}` to `build/src/libexec`, rather than use a different file organization for CMake builds than for installs.
The "Win64 native, VS 2022" job still seems unhappy.
💬 Sjors commented on pull request "multiprocess: Add bitcoin wrapper executable":
(https://github.com/bitcoin/bitcoin/pull/31375#discussion_r1861814807)
b06c7ad0ae91102fe8cdcac6f86d627aace2219b: this is potentially confusing. If I build without gui, I expect `build/src/bitcoin gui` to fail. Right now it would launch any `bitcoin-qt` in my `$PATH`.
(https://github.com/bitcoin/bitcoin/pull/31375#discussion_r1861814807)
b06c7ad0ae91102fe8cdcac6f86d627aace2219b: this is potentially confusing. If I build without gui, I expect `build/src/bitcoin gui` to fail. Right now it would launch any `bitcoin-qt` in my `$PATH`.
⚠️ Sjors opened an issue: "ARM Windows build"
(https://github.com/bitcoin/bitcoin/issues/31388)
This was brought up recently:
* https://bitcointalk.org/index.php?topic=5517601.0
* https://groups.google.com/d/msgid/bitcoindev/e5b06aaa-1fe9-4c8f-a0ea-db10f8a7e48cn%40googlegroups.com
(https://github.com/bitcoin/bitcoin/issues/31388)
This was brought up recently:
* https://bitcointalk.org/index.php?topic=5517601.0
* https://groups.google.com/d/msgid/bitcoindev/e5b06aaa-1fe9-4c8f-a0ea-db10f8a7e48cn%40googlegroups.com
💬 Sjors commented on issue "ARM Windows build":
(https://github.com/bitcoin/bitcoin/issues/31388#issuecomment-2505742737)
I believe @ZenulAbidin asked on the Bitcoin Dev list?
(https://github.com/bitcoin/bitcoin/issues/31388#issuecomment-2505742737)
I believe @ZenulAbidin asked on the Bitcoin Dev list?
💬 maflcko commented on issue "ARM Windows build":
(https://github.com/bitcoin/bitcoin/issues/31388#issuecomment-2505815571)
I am not sure how easy it would be to ship windows arm release binaries in guix, or if it is even possible today. I guess not, based on https://github.com/Windows-on-ARM-Experiments/mingw-woarm64-build?tab=readme-ov-file#known-issues .
As for the MSVC build, someone would have to go through https://github.com/bitcoin/bitcoin/blob/master/doc/build-windows-msvc.md with a recent ARM64 MSVC, see https://learn.microsoft.com/en-us/visualstudio/install/visual-studio-on-arm-devices?view=vs-2022#get-s
...
(https://github.com/bitcoin/bitcoin/issues/31388#issuecomment-2505815571)
I am not sure how easy it would be to ship windows arm release binaries in guix, or if it is even possible today. I guess not, based on https://github.com/Windows-on-ARM-Experiments/mingw-woarm64-build?tab=readme-ov-file#known-issues .
As for the MSVC build, someone would have to go through https://github.com/bitcoin/bitcoin/blob/master/doc/build-windows-msvc.md with a recent ARM64 MSVC, see https://learn.microsoft.com/en-us/visualstudio/install/visual-studio-on-arm-devices?view=vs-2022#get-s
...
💬 hebasto commented on pull request "multiprocess: Add bitcoin wrapper executable":
(https://github.com/bitcoin/bitcoin/pull/31375#issuecomment-2505819725)
> The "Win64 native, VS 2022" job still seems unhappy.
https://learn.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-3-c4996?view=msvc-170#posix-function-names:
> To turn off deprecation warnings for these functions, define the preprocessor macro `_CRT_NONSTDC_NO_WARNINGS`. You can define this macro at the command line by including the option `/D_CRT_NONSTDC_NO_WARNINGS`.
We have already used this macro:https://github.com/bitcoin/bitcoin/blob/7590e93bc73b3b
...
(https://github.com/bitcoin/bitcoin/pull/31375#issuecomment-2505819725)
> The "Win64 native, VS 2022" job still seems unhappy.
https://learn.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-3-c4996?view=msvc-170#posix-function-names:
> To turn off deprecation warnings for these functions, define the preprocessor macro `_CRT_NONSTDC_NO_WARNINGS`. You can define this macro at the command line by including the option `/D_CRT_NONSTDC_NO_WARNINGS`.
We have already used this macro:https://github.com/bitcoin/bitcoin/blob/7590e93bc73b3b
...
💬 theStack commented on pull request "doc, test: more ephemeral dust follow-ups":
(https://github.com/bitcoin/bitcoin/pull/31371#issuecomment-2505864742)
> Noticed that `mempool_ephemeral_dust.py` takes a significant amount of time to run (around 50s on at least on two of my machines, i.e. greater than 30s), but is near the bottom of `BASE_SCRIPTS` in `test_runner.py`. Moving the test toward the front of the list significantly improved parallel test runtime on each of my machines (e.g. from 131s to 91s, jobs=32). ymmv. I'm happy to submit a separate PR ([tdb3@3c40aac](https://github.com/tdb3/bitcoin/commit/3c40aac84972e0887ecd41161231a08f87d1efa3
...
(https://github.com/bitcoin/bitcoin/pull/31371#issuecomment-2505864742)
> Noticed that `mempool_ephemeral_dust.py` takes a significant amount of time to run (around 50s on at least on two of my machines, i.e. greater than 30s), but is near the bottom of `BASE_SCRIPTS` in `test_runner.py`. Moving the test toward the front of the list significantly improved parallel test runtime on each of my machines (e.g. from 131s to 91s, jobs=32). ymmv. I'm happy to submit a separate PR ([tdb3@3c40aac](https://github.com/tdb3/bitcoin/commit/3c40aac84972e0887ecd41161231a08f87d1efa3
...
💬 TheCharlatan commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-2505923802)
Updated 35f8503285c672e8ee7e98617e236b38d8ce7a7f -> 403c20980ec118f6efdd21d7c25646e20574583b ([kernelApi_6](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_6) -> [kernelApi_7](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_7), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernelApi_6..kernelApi_7))
* Integrate the validation interface into the context. This avoids having to create/destroy and register/deregister a standalone validation interface object and should ma
...
(https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-2505923802)
Updated 35f8503285c672e8ee7e98617e236b38d8ce7a7f -> 403c20980ec118f6efdd21d7c25646e20574583b ([kernelApi_6](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_6) -> [kernelApi_7](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_7), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernelApi_6..kernelApi_7))
* Integrate the validation interface into the context. This avoids having to create/destroy and register/deregister a standalone validation interface object and should ma
...
📝 hebasto opened a pull request: "Remove `src/config` directory"
(https://github.com/bitcoin/bitcoin/pull/31390)
The `src/config` directory has not been used since the migration to CMake, which disables in-source builds.
(https://github.com/bitcoin/bitcoin/pull/31390)
The `src/config` directory has not been used since the migration to CMake, which disables in-source builds.
💬 willcl-ark commented on issue "RFC: support for writing UTXO set dump (`dumptxoutset` RPC) to a named pipe":
(https://github.com/bitcoin/bitcoin/issues/31373#issuecomment-2505993781)
Sounds like a reasonable idea to me.
(https://github.com/bitcoin/bitcoin/issues/31373#issuecomment-2505993781)
Sounds like a reasonable idea to me.