๐ 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)
๐ฌ fanquake commented on pull request "deploy: remove some tools when cross-compiling for macOS":
(https://github.com/bitcoin/bitcoin/pull/29890#issuecomment-2060960613)
Added a commit to decrease the scope of using `-Wl,-headerpad_max_install_names`.
(https://github.com/bitcoin/bitcoin/pull/29890#issuecomment-2060960613)
Added a commit to decrease the scope of using `-Wl,-headerpad_max_install_names`.
๐ฌ fanquake commented on pull request "Reintroduce external signer support for Windows":
(https://github.com/bitcoin/bitcoin/pull/29868#issuecomment-2060962844)
> There are no conflicts with https://github.com/bitcoin/bitcoin/pull/29865.
Looks like there are now?
(https://github.com/bitcoin/bitcoin/pull/29868#issuecomment-2060962844)
> There are no conflicts with https://github.com/bitcoin/bitcoin/pull/29865.
Looks like there are now?
๐ฌ laanwj commented on pull request "ZMQ: Support UNIX domain sockets":
(https://github.com/bitcoin/bitcoin/pull/27679#issuecomment-2061002157)
Looks good to me now
Code review ACK 21d0e6c7b7c7af7f6e54a45829b4fbfba6923b86
(https://github.com/bitcoin/bitcoin/pull/27679#issuecomment-2061002157)
Looks good to me now
Code review ACK 21d0e6c7b7c7af7f6e54a45829b4fbfba6923b86
๐ฌ 0xB10C commented on pull request "tracing: cast block_connected duration to ยตs":
(https://github.com/bitcoin/bitcoin/pull/29877#issuecomment-2061008679)
> How is is possible that this even compiled? I presume tracing just takes any type and value and passes it along as raw bytes, so `std::chrono` being type-safe does not help here?
>
> Edit: If yes, one could consider if adding a static_assert to ban std::chrono (and possibly other types) from tracing?
In this case, casting with e.g. `int64_t{time_5 - time_start}` would have failed to compile too. Maybe being explicit with the type we expect here, as we do with e.g. the utxocache tracepoin
...
(https://github.com/bitcoin/bitcoin/pull/29877#issuecomment-2061008679)
> How is is possible that this even compiled? I presume tracing just takes any type and value and passes it along as raw bytes, so `std::chrono` being type-safe does not help here?
>
> Edit: If yes, one could consider if adding a static_assert to ban std::chrono (and possibly other types) from tracing?
In this case, casting with e.g. `int64_t{time_5 - time_start}` would have failed to compile too. Maybe being explicit with the type we expect here, as we do with e.g. the utxocache tracepoin
...