🚀 fanquake merged a pull request: "ci: Rename tasks (previous releases, macOS cross)"
(https://github.com/bitcoin/bitcoin/pull/29218)
(https://github.com/bitcoin/bitcoin/pull/29218)
💬 glozow commented on pull request "test: wallet rescan with reorged parent + IsFromMe child in mempool":
(https://github.com/bitcoin/bitcoin/pull/29179#discussion_r1450164269)
Ah yeah, it's a wallet-specific RPC. I've added a check that the "confirmations" value is 1 on wallet 0 a few lines up, prior to the reorg. Hopefully that also helps make it clear why we're testing this way.
(https://github.com/bitcoin/bitcoin/pull/29179#discussion_r1450164269)
Ah yeah, it's a wallet-specific RPC. I've added a check that the "confirmations" value is 1 on wallet 0 a few lines up, prior to the reorg. Hopefully that also helps make it clear why we're testing this way.
💬 naumenkogs commented on pull request "p2p: Fill reconciliation sets (Erlay)":
(https://github.com/bitcoin/bitcoin/pull/28765#discussion_r1450168792)
`ShouldFanoutTo` looks fitting for benchmarking, and `partial_sort` had no effect.
I took the former suggestion of course.
(https://github.com/bitcoin/bitcoin/pull/28765#discussion_r1450168792)
`ShouldFanoutTo` looks fitting for benchmarking, and `partial_sort` had no effect.
I took the former suggestion of course.
🚀 fanquake merged a pull request: "build: Bump clang minimum supported version to 14"
(https://github.com/bitcoin/bitcoin/pull/29208)
(https://github.com/bitcoin/bitcoin/pull/29208)
👍 stickies-v approved a pull request: "test: wallet rescan with reorged parent + IsFromMe child in mempool"
(https://github.com/bitcoin/bitcoin/pull/29179#pullrequestreview-1817832198)
re-ACK c39e82926a3740642f8f1bb72c6b10b67a6dc724
(https://github.com/bitcoin/bitcoin/pull/29179#pullrequestreview-1817832198)
re-ACK c39e82926a3740642f8f1bb72c6b10b67a6dc724
💬 fanquake commented on pull request "depends: remove dependency on Darwin libtool":
(https://github.com/bitcoin/bitcoin/pull/29232#issuecomment-1888805134)
Guix build (aarch64 & x86_64):
```bash
73062e70b4ec1fa814c0c25f12cbbcfbabfd3a5e82827a3bfeb2f29b30d0b84f guix-build-a28c6bbde0bc/output/aarch64-linux-gnu/SHA256SUMS.part
0965ed91f84129a64b6e0873b5feb182d83cae0dac6d89a51667d5fbe46fc68c guix-build-a28c6bbde0bc/output/aarch64-linux-gnu/bitcoin-a28c6bbde0bc-aarch64-linux-gnu-debug.tar.gz
87ee5bc2103fc10b8e7acbfa776430918538f05fccf024b624f224f73987f745 guix-build-a28c6bbde0bc/output/aarch64-linux-gnu/bitcoin-a28c6bbde0bc-aarch64-linux-gnu.tar.g
...
(https://github.com/bitcoin/bitcoin/pull/29232#issuecomment-1888805134)
Guix build (aarch64 & x86_64):
```bash
73062e70b4ec1fa814c0c25f12cbbcfbabfd3a5e82827a3bfeb2f29b30d0b84f guix-build-a28c6bbde0bc/output/aarch64-linux-gnu/SHA256SUMS.part
0965ed91f84129a64b6e0873b5feb182d83cae0dac6d89a51667d5fbe46fc68c guix-build-a28c6bbde0bc/output/aarch64-linux-gnu/bitcoin-a28c6bbde0bc-aarch64-linux-gnu-debug.tar.gz
87ee5bc2103fc10b8e7acbfa776430918538f05fccf024b624f224f73987f745 guix-build-a28c6bbde0bc/output/aarch64-linux-gnu/bitcoin-a28c6bbde0bc-aarch64-linux-gnu.tar.g
...
🤔 dergoegge reviewed a pull request: "p2p: Allow whitelisting outgoing connections"
(https://github.com/bitcoin/bitcoin/pull/27114#pullrequestreview-1817887891)
I don't see what this would be useful for, could you elaborate on the motivation?
(https://github.com/bitcoin/bitcoin/pull/27114#pullrequestreview-1817887891)
I don't see what this would be useful for, could you elaborate on the motivation?
💬 glozow commented on issue "ci: failure in `wallet_assumeutxo.py --descriptors`":
(https://github.com/bitcoin/bitcoin/issues/29234#issuecomment-1888843388)
https://github.com/bitcoin/bitcoin/actions/runs/7500571397/job/20419528825?pr=29179
(https://github.com/bitcoin/bitcoin/issues/29234#issuecomment-1888843388)
https://github.com/bitcoin/bitcoin/actions/runs/7500571397/job/20419528825?pr=29179
💬 glozow commented on pull request "test: wallet rescan with reorged parent + IsFromMe child in mempool":
(https://github.com/bitcoin/bitcoin/pull/29179#issuecomment-1888844067)
CI failure is unrelated, #29234
(https://github.com/bitcoin/bitcoin/pull/29179#issuecomment-1888844067)
CI failure is unrelated, #29234
⚠️ fanquake opened an issue: "rpc: actually deprecate `rpcuser` & `rpcpass`"
(https://github.com/bitcoin/bitcoin/issues/29240)
The logging to output that `Config options rpcuser and rpcpassword will soon be deprecated.` was added ~8 years ago in #7044. However these options still continue to get new usage today, i.e https://github.com/lightningnetwork/lnd/pull/8354#discussion_r1450166947, and it's easy to forget they are considered deprecated, given they aren't under the normal deprecation mechanism/there aren't more verbose warnings.
If we are going to deprecate these options, now seems like a good time to do so. We
...
(https://github.com/bitcoin/bitcoin/issues/29240)
The logging to output that `Config options rpcuser and rpcpassword will soon be deprecated.` was added ~8 years ago in #7044. However these options still continue to get new usage today, i.e https://github.com/lightningnetwork/lnd/pull/8354#discussion_r1450166947, and it's easy to forget they are considered deprecated, given they aren't under the normal deprecation mechanism/there aren't more verbose warnings.
If we are going to deprecate these options, now seems like a good time to do so. We
...
🤔 glozow reviewed a pull request: "p2p: Allow whitelisting outgoing connections"
(https://github.com/bitcoin/bitcoin/pull/27114#pullrequestreview-1817851701)
+1 Could you update the OP with motivation for this PR? I didn't see much from the linked PRs
(https://github.com/bitcoin/bitcoin/pull/27114#pullrequestreview-1817851701)
+1 Could you update the OP with motivation for this PR? I didn't see much from the linked PRs
💬 glozow commented on pull request "p2p: Allow whitelisting outgoing connections":
(https://github.com/bitcoin/bitcoin/pull/27114#discussion_r1450185018)
concept ack to having a TestFramework-wide setting to whitelist everybody for immediate tx relay. e5593fc9bf48b27227786e7f4145b58c849367a3 could be its own PR, as I don't think there are any instances where we are specifically making outbound connections and need immediate tx relay.
(https://github.com/bitcoin/bitcoin/pull/27114#discussion_r1450185018)
concept ack to having a TestFramework-wide setting to whitelist everybody for immediate tx relay. e5593fc9bf48b27227786e7f4145b58c849367a3 could be its own PR, as I don't think there are any instances where we are specifically making outbound connections and need immediate tx relay.
💬 glozow commented on pull request "p2p: Allow whitelisting outgoing connections":
(https://github.com/bitcoin/bitcoin/pull/27114#discussion_r1450191936)
43614707c4fe9ce91bb90b959574ee4dbf6f7b05
Why is the approach to add "in" and "out" instead of just having specified permissions apply to both? Are there instances where you want to specify different permissions for the same address depending on the connection direction?
(https://github.com/bitcoin/bitcoin/pull/27114#discussion_r1450191936)
43614707c4fe9ce91bb90b959574ee4dbf6f7b05
Why is the approach to add "in" and "out" instead of just having specified permissions apply to both? Are there instances where you want to specify different permissions for the same address depending on the connection direction?
✅ fanquake closed an issue: "`./configure` fails for clang-14 on Ubuntu 23.10"
(https://github.com/bitcoin/bitcoin/issues/29161)
(https://github.com/bitcoin/bitcoin/issues/29161)
💬 fanquake commented on issue "`./configure` fails for clang-14 on Ubuntu 23.10":
(https://github.com/bitcoin/bitcoin/issues/29161#issuecomment-1888920640)
Going to close this as wontfix. If someone is using an operating system that is modern enough to be shipping GCC 13.2 and Clang 16 as the deafult compilers, then I'd suggest using either of those as their compiler. Also given that this is a bug in Clang, but only when coupled with a certain version of libstdc++, it's not easy for us to "fix". An alternative is to try with libc++. For older systems, clang-14 remains supported (for now).
(https://github.com/bitcoin/bitcoin/issues/29161#issuecomment-1888920640)
Going to close this as wontfix. If someone is using an operating system that is modern enough to be shipping GCC 13.2 and Clang 16 as the deafult compilers, then I'd suggest using either of those as their compiler. Also given that this is a bug in Clang, but only when coupled with a certain version of libstdc++, it's not easy for us to "fix". An alternative is to try with libc++. For older systems, clang-14 remains supported (for now).
🤔 glozow reviewed a pull request: "doc: refer to "Node relay options" in policy/README"
(https://github.com/bitcoin/bitcoin/pull/29235#pullrequestreview-1818040779)
lgtm ACK 0d627c4ca8684653ddef3bb4041ad1e129ed3d4d
(https://github.com/bitcoin/bitcoin/pull/29235#pullrequestreview-1818040779)
lgtm ACK 0d627c4ca8684653ddef3bb4041ad1e129ed3d4d
✅ fanquake closed an issue: "`security-check.py` produces false positive result for stack smashing protection"
(https://github.com/bitcoin/bitcoin/issues/29084)
(https://github.com/bitcoin/bitcoin/issues/29084)
💬 fanquake commented on issue "`security-check.py` produces false positive result for stack smashing protection":
(https://github.com/bitcoin/bitcoin/issues/29084#issuecomment-1888967871)
The claim here is that if you change `-fstack-protector-all` to `-fno-stack-protector` in our compile flags, then `check_ELF_Canary` in `security-check.py` should fail during a Guix build.
However that wont happen, because the check looks for the presence of `__stack_chk_fail`, and that will still be in the binary, because libs in depends are compiled with the on-by-default stack-protector flags. This can easily be checked after Guix building with the diff above:
```bash
CXXFLAGS =
...
(https://github.com/bitcoin/bitcoin/issues/29084#issuecomment-1888967871)
The claim here is that if you change `-fstack-protector-all` to `-fno-stack-protector` in our compile flags, then `check_ELF_Canary` in `security-check.py` should fail during a Guix build.
However that wont happen, because the check looks for the presence of `__stack_chk_fail`, and that will still be in the binary, because libs in depends are compiled with the on-by-default stack-protector flags. This can easily be checked after Guix building with the diff above:
```bash
CXXFLAGS =
...
💬 Eunovo commented on pull request "tests: add functional test for miniscript decaying multisig":
(https://github.com/bitcoin/bitcoin/pull/29156#issuecomment-1888973157)
ACK [7f0be89](https://github.com/bitcoin/bitcoin/pull/29156/commits/7f0be8969e721de69fc352f76db5787836803b76)
Adding an example for this to descriptors.md is really helpful but I think users who are unfamiliar with miniscript and descriptors might benefit from the addition of the fragments, `thresh` and `after` to the `Reference` section. Seeing these fragments used in the examples but not included the reference can introduce confusion.
(https://github.com/bitcoin/bitcoin/pull/29156#issuecomment-1888973157)
ACK [7f0be89](https://github.com/bitcoin/bitcoin/pull/29156/commits/7f0be8969e721de69fc352f76db5787836803b76)
Adding an example for this to descriptors.md is really helpful but I think users who are unfamiliar with miniscript and descriptors might benefit from the addition of the fragments, `thresh` and `after` to the `Reference` section. Seeing these fragments used in the examples but not included the reference can introduce confusion.
🚀 glozow merged a pull request: "mempool / rpc: followup to getprioritisedtransactions and delete a mapDeltas entry when delta==0"
(https://github.com/bitcoin/bitcoin/pull/28885)
(https://github.com/bitcoin/bitcoin/pull/28885)