💬 fanquake commented on pull request "test: Default timeout factor to 4 under --valgrind":
(https://github.com/bitcoin/bitcoin/pull/27221#issuecomment-1459683010)
I'm still seeing #27208 with this change (fa27cf4cc7c24aa00a66dffabab849d4b3cb1c41). See [here for the full combined log](https://github.com/bitcoin/bitcoin/issues/27208#issuecomment-1459681907):
```bash
...............................................................................................
162/256 - p2p_ibd_stalling.py failed, Duration: 126 s
stdout:
2023-03-08T04:33:24.745000Z TestFramework (INFO): PRNG seed is: 3703385557670057473
2023-03-08T04:33:24.745000Z TestFramework (INF
...
(https://github.com/bitcoin/bitcoin/pull/27221#issuecomment-1459683010)
I'm still seeing #27208 with this change (fa27cf4cc7c24aa00a66dffabab849d4b3cb1c41). See [here for the full combined log](https://github.com/bitcoin/bitcoin/issues/27208#issuecomment-1459681907):
```bash
...............................................................................................
162/256 - p2p_ibd_stalling.py failed, Duration: 126 s
stdout:
2023-03-08T04:33:24.745000Z TestFramework (INFO): PRNG seed is: 3703385557670057473
2023-03-08T04:33:24.745000Z TestFramework (INF
...
🚀 fanquake merged a pull request: "doc: Fixup remove 'omitted...' doc for rpc getrawtransaction when verbose is 2"
(https://github.com/bitcoin/bitcoin/pull/26968)
(https://github.com/bitcoin/bitcoin/pull/26968)
👍 fanquake approved a pull request: "doc: Show how less noisy clang-tidy output can be achieved"
(https://github.com/bitcoin/bitcoin/pull/27205)
ACK 6a29f34ac0afce501af45097916256a9bffe8d19 - seems fine.
(https://github.com/bitcoin/bitcoin/pull/27205)
ACK 6a29f34ac0afce501af45097916256a9bffe8d19 - seems fine.
💬 fanquake commented on pull request "doc: update broken str util reference links on developer-notes":
(https://github.com/bitcoin/bitcoin/pull/27220#discussion_r1129081443)
You could put this on a new line, rather than making this one ridiculously long.
(https://github.com/bitcoin/bitcoin/pull/27220#discussion_r1129081443)
You could put this on a new line, rather than making this one ridiculously long.
💬 MarcoFalke commented on pull request "doc: update broken str util reference links on developer-notes":
(https://github.com/bitcoin/bitcoin/pull/27220#discussion_r1129089643)
ParseInt* is for legacy code only, as well
(https://github.com/bitcoin/bitcoin/pull/27220#discussion_r1129089643)
ParseInt* is for legacy code only, as well
💬 MarcoFalke commented on issue "Issue in `p2p_ibd_stalling.py` under Valgrind":
(https://github.com/bitcoin/bitcoin/issues/27208#issuecomment-1459700854)
Are you running this via the ci system or directly?
(https://github.com/bitcoin/bitcoin/issues/27208#issuecomment-1459700854)
Are you running this via the ci system or directly?
💬 MarcoFalke commented on issue "Issue in `p2p_ibd_stalling.py` under Valgrind":
(https://github.com/bitcoin/bitcoin/issues/27208#issuecomment-1459701790)
Ah yes, CI doesn't use `--valgrind`, so https://github.com/bitcoin/bitcoin/pull/27221 should make no difference.
(https://github.com/bitcoin/bitcoin/issues/27208#issuecomment-1459701790)
Ah yes, CI doesn't use `--valgrind`, so https://github.com/bitcoin/bitcoin/pull/27221 should make no difference.
💬 MarcoFalke commented on issue "Issue in `p2p_ibd_stalling.py` under Valgrind":
(https://github.com/bitcoin/bitcoin/issues/27208#issuecomment-1459705221)
See:
* https://github.com/bitcoin/bitcoin/blob/8d12127a9c19cb218d661a88ab9b6871c9d853b9/ci/test/06_script_b.sh#L23
* https://github.com/bitcoin/bitcoin/blob/8d12127a9c19cb218d661a88ab9b6871c9d853b9/ci/test/00_setup_env.sh#L44
* https://github.com/bitcoin/bitcoin/blob/8d12127a9c19cb218d661a88ab9b6871c9d853b9/ci/test/06_script_b.sh#L36
So I wonder if the timeout factor of 40 is not applied for some reason. (It will be applied if the test is run by the ci system, but if you run it yourself
...
(https://github.com/bitcoin/bitcoin/issues/27208#issuecomment-1459705221)
See:
* https://github.com/bitcoin/bitcoin/blob/8d12127a9c19cb218d661a88ab9b6871c9d853b9/ci/test/06_script_b.sh#L23
* https://github.com/bitcoin/bitcoin/blob/8d12127a9c19cb218d661a88ab9b6871c9d853b9/ci/test/00_setup_env.sh#L44
* https://github.com/bitcoin/bitcoin/blob/8d12127a9c19cb218d661a88ab9b6871c9d853b9/ci/test/06_script_b.sh#L36
So I wonder if the timeout factor of 40 is not applied for some reason. (It will be applied if the test is run by the ci system, but if you run it yourself
...
💬 laohuzhigege commented on issue "Issue in `p2p_ibd_stalling.py` under Valgrind":
(https://github.com/bitcoin/bitcoin/issues/27208#issuecomment-1459728458)
good thankyou
(https://github.com/bitcoin/bitcoin/issues/27208#issuecomment-1459728458)
good thankyou
💬 laohuzhigege commented on issue "Use of a wallet shouldn't be blocked in prune mode ("wallet loading failed... beyond pruned data")":
(https://github.com/bitcoin/bitcoin/issues/27188#issuecomment-1459729315)
thankyou
(https://github.com/bitcoin/bitcoin/issues/27188#issuecomment-1459729315)
thankyou
💬 fanquake commented on issue "Issue in `p2p_ibd_stalling.py` under Valgrind":
(https://github.com/bitcoin/bitcoin/issues/27208#issuecomment-1459747760)
> Are you running this via the ci system
Yea via the CI (native_valgrind.sh). Will look into it.
(https://github.com/bitcoin/bitcoin/issues/27208#issuecomment-1459747760)
> Are you running this via the ci system
Yea via the CI (native_valgrind.sh). Will look into it.
💬 fanquake commented on issue "`combinepsbt` RPC does not work with P2TR inputs":
(https://github.com/bitcoin/bitcoin/issues/27219#issuecomment-1459766555)
Ok. So this is fixed/closed by #23502 (assume the same for one of the alternate PRs).
(https://github.com/bitcoin/bitcoin/issues/27219#issuecomment-1459766555)
Ok. So this is fixed/closed by #23502 (assume the same for one of the alternate PRs).
📝 willcl-ark opened a pull request: "doc: docment json rpc endpoints"
(https://github.com/bitcoin/bitcoin/pull/27225)
fixes #20246
This documents the two JSON-RPC endpoints available, details when they are active, specifies when they can or must be used, and outlines some known behaviour quirks.
(https://github.com/bitcoin/bitcoin/pull/27225)
fixes #20246
This documents the two JSON-RPC endpoints available, details when they are active, specifies when they can or must be used, and outlines some known behaviour quirks.
💬 kristapsk commented on pull request "build: use _FORTIFY_SOURCE=3":
(https://github.com/bitcoin/bitcoin/pull/27027#issuecomment-1459847062)
On my Gentoo system with GCC 11.3.1 and glibc 2.36 this now gives me these warnings for each .cpp file when building Bitcoin Core:
```
CXX bitcoind-bitcoind.o
In file included from /usr/lib/gcc/x86_64-pc-linux-gnu/11/include/g++-v11/x86_64-pc-linux-gnu/bits/os_defines.h:39,
from /usr/lib/gcc/x86_64-pc-linux-gnu/11/include/g++-v11/x86_64-pc-linux-gnu/bits/c++config.h:586,
from /usr/lib/gcc/x86_64-pc-linux-gnu/11/include/g++-v11/bits/stl_algobase.h:59,
...
(https://github.com/bitcoin/bitcoin/pull/27027#issuecomment-1459847062)
On my Gentoo system with GCC 11.3.1 and glibc 2.36 this now gives me these warnings for each .cpp file when building Bitcoin Core:
```
CXX bitcoind-bitcoind.o
In file included from /usr/lib/gcc/x86_64-pc-linux-gnu/11/include/g++-v11/x86_64-pc-linux-gnu/bits/os_defines.h:39,
from /usr/lib/gcc/x86_64-pc-linux-gnu/11/include/g++-v11/x86_64-pc-linux-gnu/bits/c++config.h:586,
from /usr/lib/gcc/x86_64-pc-linux-gnu/11/include/g++-v11/bits/stl_algobase.h:59,
...
💬 vasild commented on pull request "p2p: Diversify automatic outbound connections with respect to networks":
(https://github.com/bitcoin/bitcoin/pull/27213#issuecomment-1459853005)
> I think that scenario would already be problematic ... and then we wouldn't have a peer to evict.
"... then start disconnecting from `m_protected`"
> the math
I think it goes like this: (with 5 networks) the chance of not selecting IPv4 for the first peer is `4/5`, for the first and second peer is `(4/5)^2`. The chance of not selecting IPv4 in all `8` choices is `p=(4/5)^8`. So, the chance of not selecting IPv4 or not selecting IPv6 is `2*p`. The chance of not selecting IPv4, or IPv6
...
(https://github.com/bitcoin/bitcoin/pull/27213#issuecomment-1459853005)
> I think that scenario would already be problematic ... and then we wouldn't have a peer to evict.
"... then start disconnecting from `m_protected`"
> the math
I think it goes like this: (with 5 networks) the chance of not selecting IPv4 for the first peer is `4/5`, for the first and second peer is `(4/5)^2`. The chance of not selecting IPv4 in all `8` choices is `p=(4/5)^8`. So, the chance of not selecting IPv4 or not selecting IPv6 is `2*p`. The chance of not selecting IPv4, or IPv6
...
💬 kristapsk commented on pull request "build: use _FORTIFY_SOURCE=3":
(https://github.com/bitcoin/bitcoin/pull/27027#issuecomment-1459864966)
> This triggered https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=56529 ?
What's there?

(https://github.com/bitcoin/bitcoin/pull/27027#issuecomment-1459864966)
> This triggered https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=56529 ?
What's there?

💬 willcl-ark commented on issue "Small unspent can get removed from OutputGroup for being uneconomical probably leading to later partial spending":
(https://github.com/bitcoin/bitcoin/issues/20287#issuecomment-1459925538)
Even though `OutputGroup::GetPositiveOnlyGroup` has been refactored away, it looks to me that this is still an issue as `GroupOutputs()` is still filtering by `EffectiveValue() >0`:
https://github.com/bitcoin/bitcoin/blob/8d12127a9c19cb218d661a88ab9b6871c9d853b9/src/wallet/spend.cpp#L470-L473
If we remove the dust filter here then we become liable to a dust attack where an attacker could send us many dust outputs which would always be selected, causing our fee requirements to (significantl
...
(https://github.com/bitcoin/bitcoin/issues/20287#issuecomment-1459925538)
Even though `OutputGroup::GetPositiveOnlyGroup` has been refactored away, it looks to me that this is still an issue as `GroupOutputs()` is still filtering by `EffectiveValue() >0`:
https://github.com/bitcoin/bitcoin/blob/8d12127a9c19cb218d661a88ab9b6871c9d853b9/src/wallet/spend.cpp#L470-L473
If we remove the dust filter here then we become liable to a dust attack where an attacker could send us many dust outputs which would always be selected, causing our fee requirements to (significantl
...
💬 brunoerg commented on pull request "addrman: Enable selecting addresses by network":
(https://github.com/bitcoin/bitcoin/pull/27214#discussion_r1129220513)
Yes, if it makes sense for you I'd appreciate your review there btw.
(https://github.com/bitcoin/bitcoin/pull/27214#discussion_r1129220513)
Yes, if it makes sense for you I'd appreciate your review there btw.
👍 brunoerg approved a pull request: "addrman: Enable selecting addresses by network"
(https://github.com/bitcoin/bitcoin/pull/27214)
crACK 25a64a20749f10ce84060f3570ad76d1a4776948
(https://github.com/bitcoin/bitcoin/pull/27214)
crACK 25a64a20749f10ce84060f3570ad76d1a4776948
📝 MarcoFalke opened a pull request: "test: Use self.wait_until over wait_until_helper"
(https://github.com/bitcoin/bitcoin/pull/27226)
`wait_until_helper` is a "private" helper, not intended to be used directly, because it doesn't scale the timeout with the timeout factor. Fix this by replacing it with a call to `self.wait_until`, which does the scaling.
(https://github.com/bitcoin/bitcoin/pull/27226)
`wait_until_helper` is a "private" helper, not intended to be used directly, because it doesn't scale the timeout with the timeout factor. Fix this by replacing it with a call to `self.wait_until`, which does the scaling.