💬 maflcko commented on pull request "refactor: Print verbose serialize compiler error messages":
(https://github.com/bitcoin/bitcoin/pull/29056#discussion_r1424103755)
Yes, but I guess it is not too useful, so I've dropped the commit for now.
(https://github.com/bitcoin/bitcoin/pull/29056#discussion_r1424103755)
Yes, but I guess it is not too useful, so I've dropped the commit for now.
💬 furszy commented on issue "Benchmark WalletCreateTxUsePresetInputsAndCoinSelection crashes due to #25273":
(https://github.com/bitcoin/bitcoin/issues/29061#issuecomment-1852182222)
The issue is the change output index. Since #25273, needs to be `std::nullopt`, not -1 (which now throws "out of range").
(https://github.com/bitcoin/bitcoin/issues/29061#issuecomment-1852182222)
The issue is the change output index. Since #25273, needs to be `std::nullopt`, not -1 (which now throws "out of range").
💬 furszy commented on issue "Benchmark WalletCreateTxUsePresetInputsAndCoinSelection crashes due to #25273":
(https://github.com/bitcoin/bitcoin/issues/29061#issuecomment-1852187709)
Also, this will happen at the GUI level too. We are still passing -1 at the wallet controller level too.
Will work on both things, add coverage and push the fix in few hours.
(https://github.com/bitcoin/bitcoin/issues/29061#issuecomment-1852187709)
Also, this will happen at the GUI level too. We are still passing -1 at the wallet controller level too.
Will work on both things, add coverage and push the fix in few hours.
💬 instagibbs commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1424116143)
did you want to use `CheckMempoolInvariants` here as well?
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1424116143)
did you want to use `CheckMempoolInvariants` here as well?
🤔 jamesob reviewed a pull request: "fuzz: p2p: Detect peer deadlocks"
(https://github.com/bitcoin/bitcoin/pull/29009#pullrequestreview-1777731296)
ACK 9f265d88253ed464413dea5614fa13dea0d8cfd5 ([`jamesob/ackr/29009.1.maflcko.fuzz_p2p_detect_peer_dea`](https://github.com/jamesob/bitcoin/tree/ackr/29009.1.maflcko.fuzz_p2p_detect_peer_dea))
Using the suggested buggy diff, I was able to reproduce a fuzz timeout locally:
```
SUMMARY: libFuzzer: timeout
================== Job 4 exited with exit code 70 ============
INFO: Running with entropic power schedule (0xFF, 100).
INFO: Seed: 3004856548
INFO: Loaded 1 modules (549836 inline 8-b
...
(https://github.com/bitcoin/bitcoin/pull/29009#pullrequestreview-1777731296)
ACK 9f265d88253ed464413dea5614fa13dea0d8cfd5 ([`jamesob/ackr/29009.1.maflcko.fuzz_p2p_detect_peer_dea`](https://github.com/jamesob/bitcoin/tree/ackr/29009.1.maflcko.fuzz_p2p_detect_peer_dea))
Using the suggested buggy diff, I was able to reproduce a fuzz timeout locally:
```
SUMMARY: libFuzzer: timeout
================== Job 4 exited with exit code 70 ============
INFO: Running with entropic power schedule (0xFF, 100).
INFO: Seed: 3004856548
INFO: Loaded 1 modules (549836 inline 8-b
...
💬 BrandonOdiwuor commented on pull request "doc: explain what the wallet password does":
(https://github.com/bitcoin/bitcoin/pull/28974#discussion_r1424167602)
Fixed
(https://github.com/bitcoin/bitcoin/pull/28974#discussion_r1424167602)
Fixed
💬 glozow commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1424169715)
Taken
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1424169715)
Taken
💬 glozow commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1424176890)
I don't think we should do that; we'd want to look at the impact on all related transaction's descendant scores. This transaction's modified fee might be negative, but it could have high-feerate descendants.
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1424176890)
I don't think we should do that; we'd want to look at the impact on all related transaction's descendant scores. This transaction's modified fee might be negative, but it could have high-feerate descendants.
💬 Sjors commented on pull request "Stratum v2 Template Provider (take 2)":
(https://github.com/bitcoin/bitcoin/pull/28983#issuecomment-1852266713)
Pleased clang-tidy. Fixed memory issue by disabling a broken test, added `Assume` to `noise.cpp` to check the message size. Added a (possibly overkill) destructor to `Sv2Template`.
(https://github.com/bitcoin/bitcoin/pull/28983#issuecomment-1852266713)
Pleased clang-tidy. Fixed memory issue by disabling a broken test, added `Assume` to `noise.cpp` to check the message size. Added a (possibly overkill) destructor to `Sv2Template`.
💬 achow101 commented on pull request "wallet: skip BnB when SFFO is enabled":
(https://github.com/bitcoin/bitcoin/pull/28994#issuecomment-1852286863)
ACK 576bee88fd36e207b7288077626947a1fce0fc33
(https://github.com/bitcoin/bitcoin/pull/28994#issuecomment-1852286863)
ACK 576bee88fd36e207b7288077626947a1fce0fc33
💬 fanquake commented on pull request "depends: fix libmultiprocess build on aarch64":
(https://github.com/bitcoin/bitcoin/pull/28846#issuecomment-1852300861)
Guix build (aarch64):
```bash
1415d765a10d1686e0a37f550aaf83fadad17d2d94c17688ae298ececcca17e2 guix-build-bde8d63b1763/output/aarch64-linux-gnu/SHA256SUMS.part
7e6f42f033469d23300b93c3541babae3d60a7e21615ea3ba08f243131339e5c guix-build-bde8d63b1763/output/aarch64-linux-gnu/bitcoin-bde8d63b1763-aarch64-linux-gnu-debug.tar.gz
3eba206402f4a6f7bcbe73c53f1b95f01944e35b7c3a94cf0cb1daf2db08afc8 guix-build-bde8d63b1763/output/aarch64-linux-gnu/bitcoin-bde8d63b1763-aarch64-linux-gnu.tar.gz
571ba0
...
(https://github.com/bitcoin/bitcoin/pull/28846#issuecomment-1852300861)
Guix build (aarch64):
```bash
1415d765a10d1686e0a37f550aaf83fadad17d2d94c17688ae298ececcca17e2 guix-build-bde8d63b1763/output/aarch64-linux-gnu/SHA256SUMS.part
7e6f42f033469d23300b93c3541babae3d60a7e21615ea3ba08f243131339e5c guix-build-bde8d63b1763/output/aarch64-linux-gnu/bitcoin-bde8d63b1763-aarch64-linux-gnu-debug.tar.gz
3eba206402f4a6f7bcbe73c53f1b95f01944e35b7c3a94cf0cb1daf2db08afc8 guix-build-bde8d63b1763/output/aarch64-linux-gnu/bitcoin-bde8d63b1763-aarch64-linux-gnu.tar.gz
571ba0
...
👍 TheCharlatan approved a pull request: "refactor: Print verbose serialize compiler error messages"
(https://github.com/bitcoin/bitcoin/pull/29056#pullrequestreview-1777851574)
Re-ACK fa45567e030272d34845e6b563a6b626b9eda739
(https://github.com/bitcoin/bitcoin/pull/29056#pullrequestreview-1777851574)
Re-ACK fa45567e030272d34845e6b563a6b626b9eda739
🚀 achow101 merged a pull request: "wallet: skip BnB when SFFO is enabled"
(https://github.com/bitcoin/bitcoin/pull/28994)
(https://github.com/bitcoin/bitcoin/pull/28994)
💬 glozow commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1424215142)
I've nested this part of the function inside `!ancestors.empty()` to make this clearer (and safer imo).
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1424215142)
I've nested this part of the function inside `!ancestors.empty()` to make this clearer (and safer imo).
💬 glozow commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1424216465)
Pulled into a helper function and reused :+1:
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1424216465)
Pulled into a helper function and reused :+1:
💬 glozow commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#issuecomment-1852315761)
Fixed the fuzzer crashes in https://github.com/bitcoin/bitcoin/pull/28948#issuecomment-1828351782 and https://github.com/bitcoin/bitcoin/pull/28948#issuecomment-1835850051 (thanks)
(https://github.com/bitcoin/bitcoin/pull/28948#issuecomment-1852315761)
Fixed the fuzzer crashes in https://github.com/bitcoin/bitcoin/pull/28948#issuecomment-1828351782 and https://github.com/bitcoin/bitcoin/pull/28948#issuecomment-1835850051 (thanks)
💬 fanquake commented on pull request "wallet: skip BnB when SFFO is enabled":
(https://github.com/bitcoin/bitcoin/pull/28994#issuecomment-1852336494)
Added this to #29011 for backporting to 26.x.
(https://github.com/bitcoin/bitcoin/pull/28994#issuecomment-1852336494)
Added this to #29011 for backporting to 26.x.
💬 mzumsande commented on pull request "net, cli: use v2transport for manual/addrfetch connections, add to -netinfo":
(https://github.com/bitcoin/bitcoin/pull/29058#discussion_r1424240319)
thanks, removed the comment.
(https://github.com/bitcoin/bitcoin/pull/29058#discussion_r1424240319)
thanks, removed the comment.
💬 mzumsande commented on pull request "net, cli: use v2transport for manual/addrfetch connections, add to -netinfo":
(https://github.com/bitcoin/bitcoin/pull/29058#issuecomment-1852356884)
> Playing devil's advocate: could there be any noticeable drawbacks of always trying with v2 first? I'm thinking of destination nodes behind firewalls with deep packet inspection which block ips from further connections if they don't follow the protocol (i.e. for bitcoin v1 connections, initiating with a version message). If that's the case, the user would only have the option to disable v2transport in general, or use the `addnode` RPC instead. Not sure how realistic that "i want to connect via
...
(https://github.com/bitcoin/bitcoin/pull/29058#issuecomment-1852356884)
> Playing devil's advocate: could there be any noticeable drawbacks of always trying with v2 first? I'm thinking of destination nodes behind firewalls with deep packet inspection which block ips from further connections if they don't follow the protocol (i.e. for bitcoin v1 connections, initiating with a version message). If that's the case, the user would only have the option to disable v2transport in general, or use the `addnode` RPC instead. Not sure how realistic that "i want to connect via
...
💬 theuni commented on pull request "Replace non-standard CLZ builtins with c++20's bit_width":
(https://github.com/bitcoin/bitcoin/pull/29057#issuecomment-1852375970)
Thanks @theStack and @hebasto for the quick review.
@fanquake sounds good, will push a commit to do the switch once we see some benchmarks.
(https://github.com/bitcoin/bitcoin/pull/29057#issuecomment-1852375970)
Thanks @theStack and @hebasto for the quick review.
@fanquake sounds good, will push a commit to do the switch once we see some benchmarks.