🚀 fanquake merged a pull request: "ci: Avoid && dropping errors"
(https://github.com/bitcoin/bitcoin/pull/32573)
(https://github.com/bitcoin/bitcoin/pull/32573)
💬 laanwj commented on pull request "fs: remove `_POSIX_C_SOURCE` defining":
(https://github.com/bitcoin/bitcoin/pull/32460#issuecomment-2897556590)
> Removing the fallback would make review easier here, because if the code compiles, it is more likely to be correct.
Oof. That is a good point. i hadn't realized that the detection will just make this compile even if `posix_fallocate` doesn't end up used, so it might result in a hidden performance regression instead of just a compile error.
(https://github.com/bitcoin/bitcoin/pull/32460#issuecomment-2897556590)
> Removing the fallback would make review easier here, because if the code compiles, it is more likely to be correct.
Oof. That is a good point. i hadn't realized that the detection will just make this compile even if `posix_fallocate` doesn't end up used, so it might result in a hidden performance regression instead of just a compile error.
💬 ismaelsadeeq commented on pull request "fees: rpc: `estimatesmartfee` now returns a fee rate estimate during low network activity":
(https://github.com/bitcoin/bitcoin/pull/32395#discussion_r2100028491)
Thanks @DrahtBot this is fixed.
(https://github.com/bitcoin/bitcoin/pull/32395#discussion_r2100028491)
Thanks @DrahtBot this is fixed.
💬 laanwj commented on pull request "random: Use modern Windows randomness functions":
(https://github.com/bitcoin/bitcoin/pull/32400#issuecomment-2897593795)
> Yea, on our side, this should be enough to close https://github.com/bitcoin/bitcoin/issues/32391.
Yes, the issue refers to our specific use, so imo this closes it.
From the point of "would our GUIX release run on an OS which actually removed that function", it's different, but given how much breakage that would cause to windows software all over the place it would be idiotic for Microsoft to do that any time soon. i was just curious to see if there were more uses (and if so, where).
(https://github.com/bitcoin/bitcoin/pull/32400#issuecomment-2897593795)
> Yea, on our side, this should be enough to close https://github.com/bitcoin/bitcoin/issues/32391.
Yes, the issue refers to our specific use, so imo this closes it.
From the point of "would our GUIX release run on an OS which actually removed that function", it's different, but given how much breakage that would cause to windows software all over the place it would be idiotic for Microsoft to do that any time soon. i was just curious to see if there were more uses (and if so, where).
💬 janb84 commented on pull request "doc: Remove stale sections in dev notes":
(https://github.com/bitcoin/bitcoin/pull/32572#discussion_r2100039328)
Small suggestion also add in CONTRIBUTING to wait for CI to pass. This creates a nice sort of checklist and is inline with the suggested changes to remove the statement from developer-notes and add it to the readme.
Lines 136-137:
- Push changes to your fork
- Wait for the CI to pass
- Create pull request
(https://github.com/bitcoin/bitcoin/pull/32572#discussion_r2100039328)
Small suggestion also add in CONTRIBUTING to wait for CI to pass. This creates a nice sort of checklist and is inline with the suggested changes to remove the statement from developer-notes and add it to the readme.
Lines 136-137:
- Push changes to your fork
- Wait for the CI to pass
- Create pull request
💬 rkrux commented on issue "Awesome multisig PR labyrinth guide":
(https://github.com/bitcoin/bitcoin/issues/24861#issuecomment-2897605677)
> Alice calls getxpub m/87h/0h/0h on the new wallet
Is this supposed to use the not yet present `gethdkey` RPC instead? Asking because afaik there is no `getxpub` in the core wallet.
(https://github.com/bitcoin/bitcoin/issues/24861#issuecomment-2897605677)
> Alice calls getxpub m/87h/0h/0h on the new wallet
Is this supposed to use the not yet present `gethdkey` RPC instead? Asking because afaik there is no `getxpub` in the core wallet.
💬 vasild commented on pull request "net: make m_nodes_mutex non-recursive":
(https://github.com/bitcoin/bitcoin/pull/32394#issuecomment-2897613233)
`98bba932bf...22d4c57a99`: rebase due to conflicts
(https://github.com/bitcoin/bitcoin/pull/32394#issuecomment-2897613233)
`98bba932bf...22d4c57a99`: rebase due to conflicts
📝 glozow converted_to_draft a pull request: "node: cap `-maxmempool` and `-dbcache` values for 32-bit"
(https://github.com/bitcoin/bitcoin/pull/32530)
32-bit architecture is limited to 4GiB of RAM, so it doesn't make sense to set a too high value.
(https://github.com/bitcoin/bitcoin/pull/32530)
32-bit architecture is limited to 4GiB of RAM, so it doesn't make sense to set a too high value.
👍 laanwj approved a pull request: "random: Use modern Windows randomness functions"
(https://github.com/bitcoin/bitcoin/pull/32400#pullrequestreview-2857356065)
re-ACK 6b4bcc16234575108bb691c15c3532198d9bf98a
`git range-diff 5b8046a6e893b7fad5a93631e6d1e70db31878af..345944bd6cf8be0c8b85b6d3ccb593ce26c598aa 31d3eebfb92ae0521e18225d69be95e78fb02672..6b4bcc16234575108bb691c15c3532198d9bf98a`:
Only difference: rebase for move of `symbol-check.py`
(https://github.com/bitcoin/bitcoin/pull/32400#pullrequestreview-2857356065)
re-ACK 6b4bcc16234575108bb691c15c3532198d9bf98a
`git range-diff 5b8046a6e893b7fad5a93631e6d1e70db31878af..345944bd6cf8be0c8b85b6d3ccb593ce26c598aa 31d3eebfb92ae0521e18225d69be95e78fb02672..6b4bcc16234575108bb691c15c3532198d9bf98a`:
Only difference: rebase for move of `symbol-check.py`
💬 fanquake commented on pull request "Modernize use of UTF-8 in Windows code":
(https://github.com/bitcoin/bitcoin/pull/32380#discussion_r2100060153)
In 1e826bf91932e2d76969ae40a0c0ef85a990cf2f: Can the commit be updated to better explain why this is needed. Looking at mingw-w64, `GetACP` has returned `CP_UTF8` since it was introduced. I'm guessing the same is happening with MSVC (although that's less relevant given it doesn't effect releases)? Given that, and the fact that according to https://github.com/bitcoin/bitcoin/pull/32380/files#r2096044476, we are just going to assume that a user is running a new enough version of Windows, and throw
...
(https://github.com/bitcoin/bitcoin/pull/32380#discussion_r2100060153)
In 1e826bf91932e2d76969ae40a0c0ef85a990cf2f: Can the commit be updated to better explain why this is needed. Looking at mingw-w64, `GetACP` has returned `CP_UTF8` since it was introduced. I'm guessing the same is happening with MSVC (although that's less relevant given it doesn't effect releases)? Given that, and the fact that according to https://github.com/bitcoin/bitcoin/pull/32380/files#r2096044476, we are just going to assume that a user is running a new enough version of Windows, and throw
...
💬 Sjors commented on issue "Awesome multisig PR labyrinth guide":
(https://github.com/bitcoin/bitcoin/issues/24861#issuecomment-2897630755)
@rkrux yes, fixed. HWI has a `getxpub` command, but in Bitcoin Core it would probably be called `gethdkey` (since it might also fetch the extended _private_ key).
(https://github.com/bitcoin/bitcoin/issues/24861#issuecomment-2897630755)
@rkrux yes, fixed. HWI has a `getxpub` command, but in Bitcoin Core it would probably be called `gethdkey` (since it might also fetch the extended _private_ key).
✅ cdecker closed a pull request: "chainparams: Re-add seed.bitcoinstats.com"
(https://github.com/bitcoin/bitcoin/pull/31086)
(https://github.com/bitcoin/bitcoin/pull/31086)
💬 laanwj commented on pull request "Use subprocess library for notifications":
(https://github.com/bitcoin/bitcoin/pull/32566#issuecomment-2897638499)
Rebased for #32567: [ceb1783c1af07f59885f51fdd15e7143fefdd821 → 82aeba60c2b3a166302e10fbf6e3eaf0593fab4d](https://github.com/bitcoin/bitcoin/compare/ceb1783c1af07f59885f51fdd15e7143fefdd821..82aeba60c2b3a166302e10fbf6e3eaf0593fab4d), which dropped one commit.
(https://github.com/bitcoin/bitcoin/pull/32566#issuecomment-2897638499)
Rebased for #32567: [ceb1783c1af07f59885f51fdd15e7143fefdd821 → 82aeba60c2b3a166302e10fbf6e3eaf0593fab4d](https://github.com/bitcoin/bitcoin/compare/ceb1783c1af07f59885f51fdd15e7143fefdd821..82aeba60c2b3a166302e10fbf6e3eaf0593fab4d), which dropped one commit.
📝 hebasto opened a pull request: "subprocess: Handle quoted tokens properly"
(https://github.com/bitcoin/bitcoin/pull/32577)
Fixes https://github.com/bitcoin/bitcoin/issues/32574.
The `subprocess::Popen` constructor has two overloads: https://github.com/bitcoin/bitcoin/blob/7763e86afa045910a14ac9b2cd644927a447370b/src/util/subprocess.h#L938-L941
During the migration from Boost.Process in https://github.com/bitcoin/bitcoin/pull/28981, the second was chosen for two reasons: (1) it minimized changes at the call sites, and (2) it [addressed](https://github.com/bitcoin/bitcoin/pull/28981#issuecomment-1837238921) quot
...
(https://github.com/bitcoin/bitcoin/pull/32577)
Fixes https://github.com/bitcoin/bitcoin/issues/32574.
The `subprocess::Popen` constructor has two overloads: https://github.com/bitcoin/bitcoin/blob/7763e86afa045910a14ac9b2cd644927a447370b/src/util/subprocess.h#L938-L941
During the migration from Boost.Process in https://github.com/bitcoin/bitcoin/pull/28981, the second was chosen for two reasons: (1) it minimized changes at the call sites, and (2) it [addressed](https://github.com/bitcoin/bitcoin/pull/28981#issuecomment-1837238921) quot
...
💬 vasild commented on pull request "i2p: make a time gap between creating transient sessions and using them":
(https://github.com/bitcoin/bitcoin/pull/32065#issuecomment-2897646544)
`859c259092...632eb1a3c9`: rebase due to conflicts
(https://github.com/bitcoin/bitcoin/pull/32065#issuecomment-2897646544)
`859c259092...632eb1a3c9`: rebase due to conflicts
💬 laanwj commented on pull request "doc: Remove stale sections in dev notes":
(https://github.com/bitcoin/bitcoin/pull/32572#discussion_r2100076616)
The reason i added this back in the day is that 64-bit types are awful to handle in a cross-platform way in `printf` family functions. This necessitated use of macros like `PRIx64`, which didn't even work on windows (or something) and actually actively break with tinyformat. So just being able to use `%d` and `%x` is very nice and saves thinking about this.
(https://github.com/bitcoin/bitcoin/pull/32572#discussion_r2100076616)
The reason i added this back in the day is that 64-bit types are awful to handle in a cross-platform way in `printf` family functions. This necessitated use of macros like `PRIx64`, which didn't even work on windows (or something) and actually actively break with tinyformat. So just being able to use `%d` and `%x` is very nice and saves thinking about this.
💬 fanquake commented on pull request "test: Introduce `SUPPRESS_ABORT_MESSAGE` environment variable":
(https://github.com/bitcoin/bitcoin/pull/32409#issuecomment-2897666760)
> This PR adds a way to suppress the abort message box when running test_bitcoin.exe and fuzz.exe built with the debug runtime library on Windows.
Given this isn't being used anywhere in our CI, it's no-longer clear why this is needed (we don't generally add unused code to the project). This also still isn't documented anywhere, so it's not clear how anyone who would need to use this, would know that it exists.
(https://github.com/bitcoin/bitcoin/pull/32409#issuecomment-2897666760)
> This PR adds a way to suppress the abort message box when running test_bitcoin.exe and fuzz.exe built with the debug runtime library on Windows.
Given this isn't being used anywhere in our CI, it's no-longer clear why this is needed (we don't generally add unused code to the project). This also still isn't documented anywhere, so it's not clear how anyone who would need to use this, would know that it exists.
💬 Sjors commented on pull request "wallet: Use `util::Error` throughout `AddWalletDescriptor` instead of returning `nullptr` for some errors":
(https://github.com/bitcoin/bitcoin/pull/32475#issuecomment-2897669129)
utACK 785e1407b0a39fef81a7b25554aab88d4cecd66b
(https://github.com/bitcoin/bitcoin/pull/32475#issuecomment-2897669129)
utACK 785e1407b0a39fef81a7b25554aab88d4cecd66b
💬 vasild commented on pull request "net: replace manual reference counting of CNode with shared_ptr":
(https://github.com/bitcoin/bitcoin/pull/32015#issuecomment-2897671621)
`20657a7c8e...cbd7bd7422`: rebase due to conflicts
(https://github.com/bitcoin/bitcoin/pull/32015#issuecomment-2897671621)
`20657a7c8e...cbd7bd7422`: rebase due to conflicts
💬 fanquake commented on pull request "fs: remove `_POSIX_C_SOURCE` defining":
(https://github.com/bitcoin/bitcoin/pull/32460#issuecomment-2897676179)
I've added another commit (that can be squashed), dropping the fallback.
(https://github.com/bitcoin/bitcoin/pull/32460#issuecomment-2897676179)
I've added another commit (that can be squashed), dropping the fallback.