💬 tdb3 commented on pull request "rpc: getorphantxs follow-up":
(https://github.com/bitcoin/bitcoin/pull/31043#discussion_r1790043575)
Yeah, if we can, that would be simpler/preference.
Would need the overhead (message sending, processing, orphanage addition, etc.) to always take less than a second, right? Otherwise there might be intermittent test failure.
If it's always under a second, then maybe something like `setmocktime(int(time.time()))` to force time to start at the beginning of a second might work?
Locally, I very roughly checked runtime and it was consistently under 105ms (not sure how accurate logger is tho
...
(https://github.com/bitcoin/bitcoin/pull/31043#discussion_r1790043575)
Yeah, if we can, that would be simpler/preference.
Would need the overhead (message sending, processing, orphanage addition, etc.) to always take less than a second, right? Otherwise there might be intermittent test failure.
If it's always under a second, then maybe something like `setmocktime(int(time.time()))` to force time to start at the beginning of a second might work?
Locally, I very roughly checked runtime and it was consistently under 105ms (not sure how accurate logger is tho
...
💬 dergoegge commented on issue "build: RFC Coverage build type":
(https://github.com/bitcoin/bitcoin/issues/31047#issuecomment-2396661558)
I haven't tried after the transition to CMake but I always had trouble with the build in coverage build mode (e.g. `make cov`). Usually there was problem with path prefixes or a `lcov` version mismatch. The coverage reports themselves were always hard to read and I could not find any documentation on how to read them, for example:
How is branch coverage supposed to be int
...
(https://github.com/bitcoin/bitcoin/issues/31047#issuecomment-2396661558)
I haven't tried after the transition to CMake but I always had trouble with the build in coverage build mode (e.g. `make cov`). Usually there was problem with path prefixes or a `lcov` version mismatch. The coverage reports themselves were always hard to read and I could not find any documentation on how to read them, for example:
How is branch coverage supposed to be int
...
💬 vasild commented on issue "build: RFC Coverage build type":
(https://github.com/bitcoin/bitcoin/issues/31047#issuecomment-2396670484)
For me it seems best to use each compiler's native workflow. That is: gcov+lcov if using GCC and the [source based coverage](https://github.com/hebasto/bitcoin/issues/341#issuecomment-2316704163) if using Clang.
Would be nice if that is integrated into the build system so it boils down to just `cmake --i-want-coverage`, but if not, then some good instructions in `doc/` would be fine as well.
(https://github.com/bitcoin/bitcoin/issues/31047#issuecomment-2396670484)
For me it seems best to use each compiler's native workflow. That is: gcov+lcov if using GCC and the [source based coverage](https://github.com/hebasto/bitcoin/issues/341#issuecomment-2316704163) if using Clang.
Would be nice if that is integrated into the build system so it boils down to just `cmake --i-want-coverage`, but if not, then some good instructions in `doc/` would be fine as well.
💬 maflcko commented on pull request "rpc: getorphantxs follow-up":
(https://github.com/bitcoin/bitcoin/pull/31043#discussion_r1790049818)
I think with mocktime, you don't have to do benchmarks. In most cases it should work on any system, or not at all.
Mocktime applies to most times, so there is no requirement that something happens in less than 105ms.
You should be able to test this by adding a `time.sleep(1.1)` and observing that the test still passes.
(https://github.com/bitcoin/bitcoin/pull/31043#discussion_r1790049818)
I think with mocktime, you don't have to do benchmarks. In most cases it should work on any system, or not at all.
Mocktime applies to most times, so there is no requirement that something happens in less than 105ms.
You should be able to test this by adding a `time.sleep(1.1)` and observing that the test still passes.
💬 fanquake commented on issue "build: RFC Coverage build type":
(https://github.com/bitcoin/bitcoin/issues/31047#issuecomment-2396679207)
> For me it seems best to use each compiler's native workflow.
Sure, anyone can still use whatever they like. However our build system doesn't need to (and shouldn't have to) handle all possible combinations of compiler + tooling. Providing a single convenience build type that is the same as what is used by the majority of the developers seems fine, however I don't think we need to natively support anything outside of that, given it's unlikely to be tested/used, or work properly, and just fu
...
(https://github.com/bitcoin/bitcoin/issues/31047#issuecomment-2396679207)
> For me it seems best to use each compiler's native workflow.
Sure, anyone can still use whatever they like. However our build system doesn't need to (and shouldn't have to) handle all possible combinations of compiler + tooling. Providing a single convenience build type that is the same as what is used by the majority of the developers seems fine, however I don't think we need to natively support anything outside of that, given it's unlikely to be tested/used, or work properly, and just fu
...
💬 vasild commented on issue "Listen on random port by default (not 8333)":
(https://github.com/bitcoin/bitcoin/issues/31036#issuecomment-2396705062)
Alright, looks like getting the DNS seeds to propagate ports would be really nice to have or an outright blocker for this.
Currently, about 7% of my IPv4/IPv6 addrman entries use non 8333 port.
```sh
$ bitcoin-cli getnodeaddresses 0 | jq -r 'map(select((.network == "ipv6" or .network == "ipv4") and .port != 8333)) | length'
$ bitcoin-cli getnodeaddresses 0 | jq -r 'map(select(.network == "ipv6" or .network == "ipv4")) | length'
```
(https://github.com/bitcoin/bitcoin/issues/31036#issuecomment-2396705062)
Alright, looks like getting the DNS seeds to propagate ports would be really nice to have or an outright blocker for this.
Currently, about 7% of my IPv4/IPv6 addrman entries use non 8333 port.
```sh
$ bitcoin-cli getnodeaddresses 0 | jq -r 'map(select((.network == "ipv6" or .network == "ipv4") and .port != 8333)) | length'
$ bitcoin-cli getnodeaddresses 0 | jq -r 'map(select(.network == "ipv6" or .network == "ipv4")) | length'
```
💬 tdb3 commented on pull request "rpc: getorphantxs follow-up":
(https://github.com/bitcoin/bitcoin/pull/31043#discussion_r1790087981)
Thanks.
The following passed. Will push.
```diff
diff --git a/test/functional/rpc_orphans.py b/test/functional/rpc_orphans.py
index 207647f548..825657a1ed 100755
--- a/test/functional/rpc_orphans.py
+++ b/test/functional/rpc_orphans.py
@@ -96,11 +96,15 @@ class OrphanRPCsTest(BitcoinTestFramework):
tx_child_2 = self.wallet.create_self_transfer(utxo_to_spend=tx_parent_2["new_utxo"])
peer_1 = node.add_p2p_connection(P2PInterface())
peer_2 = node.add_p2p_co
...
(https://github.com/bitcoin/bitcoin/pull/31043#discussion_r1790087981)
Thanks.
The following passed. Will push.
```diff
diff --git a/test/functional/rpc_orphans.py b/test/functional/rpc_orphans.py
index 207647f548..825657a1ed 100755
--- a/test/functional/rpc_orphans.py
+++ b/test/functional/rpc_orphans.py
@@ -96,11 +96,15 @@ class OrphanRPCsTest(BitcoinTestFramework):
tx_child_2 = self.wallet.create_self_transfer(utxo_to_spend=tx_parent_2["new_utxo"])
peer_1 = node.add_p2p_connection(P2PInterface())
peer_2 = node.add_p2p_co
...
💬 tdb3 commented on pull request "rpc: getorphantxs follow-up":
(https://github.com/bitcoin/bitcoin/pull/31043#issuecomment-2396730642)
Pushed to ae17f9050e0c2a6fd31cd9f5d2f3ad8066f02cc9 to replace `assert_approx` with `assert_equal` (using `setmocktime`)
(https://github.com/bitcoin/bitcoin/pull/31043#issuecomment-2396730642)
Pushed to ae17f9050e0c2a6fd31cd9f5d2f3ad8066f02cc9 to replace `assert_approx` with `assert_equal` (using `setmocktime`)
👍 tdb3 approved a pull request: "ci: set a ctest test timeout of 1200 (20 minutes)"
(https://github.com/bitcoin/bitcoin/pull/31026#pullrequestreview-2351756890)
re ACK 56aad83307e46983a397236bd0959e634207f83e
(https://github.com/bitcoin/bitcoin/pull/31026#pullrequestreview-2351756890)
re ACK 56aad83307e46983a397236bd0959e634207f83e
💬 maflcko commented on pull request "Windows bitcoind stall debugging [NOMERGE, DRAFT]":
(https://github.com/bitcoin/bitcoin/pull/30956#discussion_r1790111622)
Any reason to filter to only mempool_?
(https://github.com/bitcoin/bitcoin/pull/30956#discussion_r1790111622)
Any reason to filter to only mempool_?
💬 maflcko commented on issue "ci: ConnectionRefusedError: [WinError 10061] No connection could be made because the target machine actively refused it":
(https://github.com/bitcoin/bitcoin/issues/30390#issuecomment-2396787199)
https://github.com/bitcoin/bitcoin/actions/runs/11164991068/job/31035622913?pr=30982#step:12:242
(https://github.com/bitcoin/bitcoin/issues/30390#issuecomment-2396787199)
https://github.com/bitcoin/bitcoin/actions/runs/11164991068/job/31035622913?pr=30982#step:12:242
💬 Sjors commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2396818224)
It's nice to keep macOS 11 support, but I don't have a (easy) way to test it, so I wouldn't put too much effort in it.
(https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2396818224)
It's nice to keep macOS 11 support, but I don't have a (easy) way to test it, so I wouldn't put too much effort in it.
💬 maflcko commented on pull request "Add -pausebackgroundsync startup option":
(https://github.com/bitcoin/bitcoin/pull/31023#issuecomment-2396823190)
I tend to agree with https://github.com/bitcoin/bitcoin/pull/31023#issuecomment-2391695605. Adding a test-only option in this way won't be useful to benchmark previous releases (and compare them), and seems more work than just using a test-only datadir dump.
> The main use case I have in mind is some who doesn't want to do background validation.
There are reasons to do the background validation, see https://github.com/bitcoin/bitcoin/pull/28616#issuecomment-2022485737 . If you are adding a
...
(https://github.com/bitcoin/bitcoin/pull/31023#issuecomment-2396823190)
I tend to agree with https://github.com/bitcoin/bitcoin/pull/31023#issuecomment-2391695605. Adding a test-only option in this way won't be useful to benchmark previous releases (and compare them), and seems more work than just using a test-only datadir dump.
> The main use case I have in mind is some who doesn't want to do background validation.
There are reasons to do the background validation, see https://github.com/bitcoin/bitcoin/pull/28616#issuecomment-2022485737 . If you are adding a
...
🚀 fanquake merged a pull request: "ci: set a ctest test timeout of 1200 (20 minutes)"
(https://github.com/bitcoin/bitcoin/pull/31026)
(https://github.com/bitcoin/bitcoin/pull/31026)
💬 maflcko commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2396830039)
IIUC macOS 11 does not receive security patches, so it may be better to remove it either way, see https://github.com/bitcoin/bitcoin/pull/30997#discussion_r1789897798
(https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2396830039)
IIUC macOS 11 does not receive security patches, so it may be better to remove it either way, see https://github.com/bitcoin/bitcoin/pull/30997#discussion_r1789897798
💬 Sjors commented on pull request "docs: Add instructions on how to self-sign bitcoin-core binaries for macOS":
(https://github.com/bitcoin/bitcoin/pull/30982#discussion_r1790160328)
This might be a bit confusing, because you don't need to self-sign the main drag & drop release.
(https://github.com/bitcoin/bitcoin/pull/30982#discussion_r1790160328)
This might be a bit confusing, because you don't need to self-sign the main drag & drop release.
💬 brunoerg commented on pull request "net: fuzz: bypass network magic and checksum validation":
(https://github.com/bitcoin/bitcoin/pull/31012#issuecomment-2396845482)
> Copy and pasting the command from the docs doesn't work:
Just fixed it. Can you check it?
(https://github.com/bitcoin/bitcoin/pull/31012#issuecomment-2396845482)
> Copy and pasting the command from the docs doesn't work:
Just fixed it. Can you check it?
💬 ismaelsadeeq commented on issue "[Testnet] Insufficient data or no feerate found":
(https://github.com/bitcoin/bitcoin/issues/31032#issuecomment-2396883367)
> I'd like to think that this is a problem caused by not having enough data, but that doesn't seem to be the case for me.
> I have two Bitcoin testnet nodes running (let's call them A and B).
> Sometimes node A throws the error "Insufficient data or no feerate found," while node B returns a feerate without any issue.
> Other times, both nodes A and B encounter the same error.
This behavior is inherent to the fee estimator, which collects data and decays it over time. As a result, you
...
(https://github.com/bitcoin/bitcoin/issues/31032#issuecomment-2396883367)
> I'd like to think that this is a problem caused by not having enough data, but that doesn't seem to be the case for me.
> I have two Bitcoin testnet nodes running (let's call them A and B).
> Sometimes node A throws the error "Insufficient data or no feerate found," while node B returns a feerate without any issue.
> Other times, both nodes A and B encounter the same error.
This behavior is inherent to the fee estimator, which collects data and decays it over time. As a result, you
...
💬 kevkevinpal commented on pull request "test: Assert that when we add the max orphan amount that we cannot add anymore and that a random orphan gets dropped":
(https://github.com/bitcoin/bitcoin/pull/31040#issuecomment-2396883806)
> Concept ACK Thanks, this is useful! I'm uncertain if this check is better suited for `p2p_orphan_handling` or `rpc_getorphantxs` (or for a `feature_orphanage` or `mempool_orphanage`). If `p2p_orphan_handling` is the preferred location, I'm happy to include your commit (preserving you as author) in #31037.
>
> https://github.com/bitcoin/bitcoin/blob/master/test/functional/README.md#naming-guidelines
>
> Can also check unit tests (https://github.com/bitcoin/bitcoin/blob/master/src/test/orp
...
(https://github.com/bitcoin/bitcoin/pull/31040#issuecomment-2396883806)
> Concept ACK Thanks, this is useful! I'm uncertain if this check is better suited for `p2p_orphan_handling` or `rpc_getorphantxs` (or for a `feature_orphanage` or `mempool_orphanage`). If `p2p_orphan_handling` is the preferred location, I'm happy to include your commit (preserving you as author) in #31037.
>
> https://github.com/bitcoin/bitcoin/blob/master/test/functional/README.md#naming-guidelines
>
> Can also check unit tests (https://github.com/bitcoin/bitcoin/blob/master/src/test/orp
...
💬 TheCharlatan commented on pull request "kernel: Move block tree db open to block manager":
(https://github.com/bitcoin/bitcoin/pull/30965#discussion_r1790201191)
See #31046
(https://github.com/bitcoin/bitcoin/pull/30965#discussion_r1790201191)
See #31046