💬 maflcko commented on pull request "blockstorage: do not flush block to disk if it is already there":
(https://github.com/bitcoin/bitcoin/pull/27039#discussion_r1568304549)
> If we can rely on that to prevent race conditions then this could just be `assert_debug_log`.
`assert_debug_log` is also syncing (it has a timeout argument), so no race should happen. My comment is only about the IO usage. `assert_debug_log` is not using a busy wait, but a sleepy wait. Otherwise they are exactly identical.
However, if `assert_debug_log` is used to sync, my personal preference is to provide the `timeout` argument, so that the code is self-documenting.
I think in this c
...
(https://github.com/bitcoin/bitcoin/pull/27039#discussion_r1568304549)
> If we can rely on that to prevent race conditions then this could just be `assert_debug_log`.
`assert_debug_log` is also syncing (it has a timeout argument), so no race should happen. My comment is only about the IO usage. `assert_debug_log` is not using a busy wait, but a sleepy wait. Otherwise they are exactly identical.
However, if `assert_debug_log` is used to sync, my personal preference is to provide the `timeout` argument, so that the code is self-documenting.
I think in this c
...
💬 stratospher commented on issue "Intermittent issue in p2p_handshake.py", line 75, in run_test self.test_desirable_service_flags(node, [NODE_NETWORK | NODE_WITNESS], not true after 2400.0 seconds":
(https://github.com/bitcoin/bitcoin/issues/29896#issuecomment-2060570489)
looks like this happens in `add_outbound_p2p_connection` when disconnection isn't fully over and we're trying to establish a connection again on same port since `p2p_idx=0`. will open a fix soon.
cc @theStack.
(https://github.com/bitcoin/bitcoin/issues/29896#issuecomment-2060570489)
looks like this happens in `add_outbound_p2p_connection` when disconnection isn't fully over and we're trying to establish a connection again on same port since `p2p_idx=0`. will open a fix soon.
cc @theStack.
💬 fanquake commented on pull request "[RFC] guix: remove xz from deps":
(https://github.com/bitcoin/bitcoin/pull/29895#issuecomment-2060615404)
> And i'm not sure we need to go out of our way to avoid it given how much of the FOSS ecosystem relies on it.
Yea. xz was a way to start a discussion. It could turn out that we actually consolidate everything to xz, and actually drop bzip2 for example.
> possibly easier to backdoor, mirrored tarball.
> and instead use the upstream source control directly
If anything, I'd hope it's less easy to backdoor, because it's just the source code, and not all the bootstrapped garbage we don't n
...
(https://github.com/bitcoin/bitcoin/pull/29895#issuecomment-2060615404)
> And i'm not sure we need to go out of our way to avoid it given how much of the FOSS ecosystem relies on it.
Yea. xz was a way to start a discussion. It could turn out that we actually consolidate everything to xz, and actually drop bzip2 for example.
> possibly easier to backdoor, mirrored tarball.
> and instead use the upstream source control directly
If anything, I'd hope it's less easy to backdoor, because it's just the source code, and not all the bootstrapped garbage we don't n
...
💬 laanwj commented on pull request "[RFC] guix: remove xz from deps":
(https://github.com/bitcoin/bitcoin/pull/29895#issuecomment-2060616166)
> Not sure about the benefit to make extra steps to use a possibly non-standard, possibly easier to backdoor, mirrored tarball.
Mind that we do verify the sha256 hash. The person introducing it has to check, once, that the tarball matches exactly what is in git. i don't think there's a win in making every guix buid do a git checkout.
(https://github.com/bitcoin/bitcoin/pull/29895#issuecomment-2060616166)
> Not sure about the benefit to make extra steps to use a possibly non-standard, possibly easier to backdoor, mirrored tarball.
Mind that we do verify the sha256 hash. The person introducing it has to check, once, that the tarball matches exactly what is in git. i don't think there's a win in making every guix buid do a git checkout.
🚀 fanquake merged a pull request: "guix: remove `gcc-toolchain static` from Windows build"
(https://github.com/bitcoin/bitcoin/pull/29828)
(https://github.com/bitcoin/bitcoin/pull/29828)
🚀 fanquake merged a pull request: "test: Fix failing univalue float test"
(https://github.com/bitcoin/bitcoin/pull/29892)
(https://github.com/bitcoin/bitcoin/pull/29892)
💬 fanquake commented on pull request "test: Fix failing univalue float test":
(https://github.com/bitcoin/bitcoin/pull/29892#issuecomment-2060642724)
Backported to 27.x in #29888.
(https://github.com/bitcoin/bitcoin/pull/29892#issuecomment-2060642724)
Backported to 27.x in #29888.
💬 fanquake commented on pull request "depends: fix mingw-w64 Qt DEBUG=1 build":
(https://github.com/bitcoin/bitcoin/pull/29747#issuecomment-2060643223)
Backported to 27.x in #29888.
(https://github.com/bitcoin/bitcoin/pull/29747#issuecomment-2060643223)
Backported to 27.x in #29888.
💬 fanquake commented on pull request "rpc, bugfix: Enforce maximum value for setmocktime":
(https://github.com/bitcoin/bitcoin/pull/29869#issuecomment-2060643831)
Backported to 27.x in #29888.
(https://github.com/bitcoin/bitcoin/pull/29869#issuecomment-2060643831)
Backported to 27.x in #29888.
💬 fanquake commented on pull request "Change Luke Dashjr seed to dashjr-list-of-p2p-nodes.us":
(https://github.com/bitcoin/bitcoin/pull/29691#issuecomment-2060644226)
Backported to 27.x in #29888.
(https://github.com/bitcoin/bitcoin/pull/29691#issuecomment-2060644226)
Backported to 27.x in #29888.
💬 fanquake commented on pull request "ci: Bump s390x to ubuntu:24.04":
(https://github.com/bitcoin/bitcoin/pull/29856#issuecomment-2060645814)
Backported to 27.x in #29888.
(https://github.com/bitcoin/bitcoin/pull/29856#issuecomment-2060645814)
Backported to 27.x in #29888.
📝 stratospher opened a pull request: "test: Fix intermittent issue in p2p_handshake.py"
(https://github.com/bitcoin/bitcoin/pull/29898)
When establishing outbound connections [`TestNode` --------> `P2PConnection`], `P2PConnection` listens for a single connection from `TestNode` on a [port which is fixed based on `p2p_idx`](https://github.com/bitcoin/bitcoin/blob/312f54278fd972ba3557c6a5b805fd244a063959/test/functional/test_framework/p2p.py#L746).
If we reuse the same port when disconnecting and establishing connections again, we might hit this scenario where:
- disconnection is done on python side for `P2PConnection`
- disc
...
(https://github.com/bitcoin/bitcoin/pull/29898)
When establishing outbound connections [`TestNode` --------> `P2PConnection`], `P2PConnection` listens for a single connection from `TestNode` on a [port which is fixed based on `p2p_idx`](https://github.com/bitcoin/bitcoin/blob/312f54278fd972ba3557c6a5b805fd244a063959/test/functional/test_framework/p2p.py#L746).
If we reuse the same port when disconnecting and establishing connections again, we might hit this scenario where:
- disconnection is done on python side for `P2PConnection`
- disc
...
💬 stratospher commented on issue "Intermittent issue in p2p_handshake.py", line 75, in run_test self.test_desirable_service_flags(node, [NODE_NETWORK | NODE_WITNESS], not true after 2400.0 seconds":
(https://github.com/bitcoin/bitcoin/issues/29896#issuecomment-2060676866)
see #29898.
(https://github.com/bitcoin/bitcoin/issues/29896#issuecomment-2060676866)
see #29898.
💬 maflcko commented on pull request "[RFC] guix: remove xz from deps":
(https://github.com/bitcoin/bitcoin/pull/29895#issuecomment-2060698390)
> The person introducing it has to check, once, that the tarball matches exactly what is in git (and reviewers can verify this)
I think it could make sense to specifically mention this on bumps, going forward. Maybe there could even be a new target written in depends to do the check?
(https://github.com/bitcoin/bitcoin/pull/29895#issuecomment-2060698390)
> The person introducing it has to check, once, that the tarball matches exactly what is in git (and reviewers can verify this)
I think it could make sense to specifically mention this on bumps, going forward. Maybe there could even be a new target written in depends to do the check?
💬 maflcko commented on pull request "[RFC] guix: remove xz from deps":
(https://github.com/bitcoin/bitcoin/pull/29895#issuecomment-2060707521)
> If anything, I'd hope it's less easy to backdoor, because it's just the source code, and not all the bootstrapped garbage we don't need. This is using the tag from the official upstream source control.
Oh, I see. So the motivation is to drop, possibly non-determinisically created, "bootstrapped garbage"? Concept +1 on this.
I am just not sure on dropping `$compression_tool`. I presume guix internally requires it to bootstrap to a point where Bitcoin Core can be compiled, as many guix sou
...
(https://github.com/bitcoin/bitcoin/pull/29895#issuecomment-2060707521)
> If anything, I'd hope it's less easy to backdoor, because it's just the source code, and not all the bootstrapped garbage we don't need. This is using the tag from the official upstream source control.
Oh, I see. So the motivation is to drop, possibly non-determinisically created, "bootstrapped garbage"? Concept +1 on this.
I am just not sure on dropping `$compression_tool`. I presume guix internally requires it to bootstrap to a point where Bitcoin Core can be compiled, as many guix sou
...
💬 Sjors commented on pull request "Silent payment index (for light wallets and consistency check)":
(https://github.com/bitcoin/bitcoin/pull/28241#issuecomment-2060724432)
Rebased. It no longer has a wallet dependency, which is nice.
> The RPC returns an array for each block with one entry per transaction. The entry is empty if there is no tweak. So the ordering is based on the order of transactions in a block.
The middle part here is wrong: when a transaction doesn't have a tweak it's simply skipped, there's no empty element. I could change that, but I don't think it's necessary for anything.
Here's the tweaks I get on a recent signet block:
<details>
...
(https://github.com/bitcoin/bitcoin/pull/28241#issuecomment-2060724432)
Rebased. It no longer has a wallet dependency, which is nice.
> The RPC returns an array for each block with one entry per transaction. The entry is empty if there is no tweak. So the ordering is based on the order of transactions in a block.
The middle part here is wrong: when a transaction doesn't have a tweak it's simply skipped, there's no empty element. I could change that, but I don't think it's necessary for anything.
Here's the tweaks I get on a recent signet block:
<details>
...
💬 Sjors commented on pull request "Silent payment index (for light wallets and consistency check)":
(https://github.com/bitcoin/bitcoin/pull/28241#discussion_r1568462751)
@josibake can you make `GetSilentPaymentTweakDataFromTxInputs` return a `CPub` (or `uint256`) rather than this low level secp thing?
(https://github.com/bitcoin/bitcoin/pull/28241#discussion_r1568462751)
@josibake can you make `GetSilentPaymentTweakDataFromTxInputs` return a `CPub` (or `uint256`) rather than this low level secp thing?
🤔 glozow reviewed a pull request: "test: fix intermittent failure in p2p_compactblocks_hb.py"
(https://github.com/bitcoin/bitcoin/pull/29893#pullrequestreview-2005670115)
ACK 1ae5b208d339fa984d9caf4fab89b0b2ba9cc197
Thanks! I had this failure recently. The reason/fix makes sense to me and I tested that it works using the `sleep`.
(https://github.com/bitcoin/bitcoin/pull/29893#pullrequestreview-2005670115)
ACK 1ae5b208d339fa984d9caf4fab89b0b2ba9cc197
Thanks! I had this failure recently. The reason/fix makes sense to me and I tested that it works using the `sleep`.
✅ glozow closed an issue: "test: Intermittent issue in p2p_compactblocks_hb.py" in relay_block_through assert_equal(status_to, status_from) AssertionError: not([True, False, True, True] == [True, True, True, True])"
(https://github.com/bitcoin/bitcoin/issues/29860)
(https://github.com/bitcoin/bitcoin/issues/29860)
🚀 glozow merged a pull request: "test: fix intermittent failure in p2p_compactblocks_hb.py"
(https://github.com/bitcoin/bitcoin/pull/29893)
(https://github.com/bitcoin/bitcoin/pull/29893)