💬 maflcko commented on issue "Spurious (?) valgrind failure for p2p_compactblocks.py":
(https://github.com/bitcoin/bitcoin/issues/27741#issuecomment-2472823441)
Steps to reproduce on a fresh Ubuntu 24.04:
```
export DEBIAN_FRONTEND=noninteractive && apt update && apt install curl wget htop git vim ccache -y && git clone https://github.com/bitcoin/bitcoin.git --depth=1 ./b-c && cd b-c && apt install build-essential cmake pkg-config python3-zmq libzmq3-dev libevent-dev libboost-dev libsqlite3-dev libdb++-dev clang llvm valgrind -y && cmake -B ./bld-cmake -DBUILD_GUI=OFF -DBUILD_FUZZ_BINARY=OFF -DBUILD_BENCH=OFF -DWITH_ZMQ=OFF
...
(https://github.com/bitcoin/bitcoin/issues/27741#issuecomment-2472823441)
Steps to reproduce on a fresh Ubuntu 24.04:
```
export DEBIAN_FRONTEND=noninteractive && apt update && apt install curl wget htop git vim ccache -y && git clone https://github.com/bitcoin/bitcoin.git --depth=1 ./b-c && cd b-c && apt install build-essential cmake pkg-config python3-zmq libzmq3-dev libevent-dev libboost-dev libsqlite3-dev libdb++-dev clang llvm valgrind -y && cmake -B ./bld-cmake -DBUILD_GUI=OFF -DBUILD_FUZZ_BINARY=OFF -DBUILD_BENCH=OFF -DWITH_ZMQ=OFF
...
💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r1839750202)
By "for listening" you mean "for learning new transactions and blocks", right? So, in that setup the node is not listening (not accepting inbound connections), has only one connection to a known honest node and broadcasts its own transactions using private broadcast? That seems like an useful setup that would be nice to have support for. But the current semantic of `-connect` is "only connect to the specified node(s)" which is at odds with "connect to random nodes for transaction broadcast". I d
...
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r1839750202)
By "for listening" you mean "for learning new transactions and blocks", right? So, in that setup the node is not listening (not accepting inbound connections), has only one connection to a known honest node and broadcasts its own transactions using private broadcast? That seems like an useful setup that would be nice to have support for. But the current semantic of `-connect` is "only connect to the specified node(s)" which is at odds with "connect to random nodes for transaction broadcast". I d
...
⚠️ Sandra-Amina-Boss opened an issue: "Peue"
(https://github.com/bitcoin/bitcoin/issues/31281)
Bitcoin on Coinbase: https://www.coinbase.com/price/bitcoin?utm_campaign=rt_i_m_w_m_acq_ugc_soc_0_asset&utm_source=ugc&utm_platform=Android
(https://github.com/bitcoin/bitcoin/issues/31281)
Bitcoin on Coinbase: https://www.coinbase.com/price/bitcoin?utm_campaign=rt_i_m_w_m_acq_ugc_soc_0_asset&utm_source=ugc&utm_platform=Android
✅ fanquake closed an issue: "Peue"
(https://github.com/bitcoin/bitcoin/issues/31281)
(https://github.com/bitcoin/bitcoin/issues/31281)
:lock: fanquake locked an issue: "Peue"
(https://github.com/bitcoin/bitcoin/issues/31281)
(https://github.com/bitcoin/bitcoin/issues/31281)
📝 maflcko opened a pull request: " refactor: Make node_id a const& in RemoveBlockRequest "
(https://github.com/bitcoin/bitcoin/pull/31282)
Currently, `valgrind` is not usable on a default build with GCC. Specifically, `p2p_compactblocks.py --valgrind` gives a false-positive in `RemoveBlockRequest` when comparing `node_id` with `from_peer`. According to the upstream bug report, this happens because both symbols are on the stack and the compiler can more aggressively optimize the compare (order). See https://bugs.kde.org/show_bug.cgi?id=472329#c7
It is possible to work around this bug by pulling at least one value from the stack.
...
(https://github.com/bitcoin/bitcoin/pull/31282)
Currently, `valgrind` is not usable on a default build with GCC. Specifically, `p2p_compactblocks.py --valgrind` gives a false-positive in `RemoveBlockRequest` when comparing `node_id` with `from_peer`. According to the upstream bug report, this happens because both symbols are on the stack and the compiler can more aggressively optimize the compare (order). See https://bugs.kde.org/show_bug.cgi?id=472329#c7
It is possible to work around this bug by pulling at least one value from the stack.
...
🚀 fanquake merged a pull request: "doc: mention `descriptorprocesspsbt` in psbt.md"
(https://github.com/bitcoin/bitcoin/pull/31277)
(https://github.com/bitcoin/bitcoin/pull/31277)
🤔 Sjors reviewed a pull request: " refactor: Make node_id a const& in RemoveBlockRequest "
(https://github.com/bitcoin/bitcoin/pull/31282#pullrequestreview-2432510141)
Concept ACK
Only studied the main one line change.
(https://github.com/bitcoin/bitcoin/pull/31282#pullrequestreview-2432510141)
Concept ACK
Only studied the main one line change.
💬 Sjors commented on pull request " refactor: Make node_id a const& in RemoveBlockRequest ":
(https://github.com/bitcoin/bitcoin/pull/31282#discussion_r1839882117)
fa91206b3c570dc17ee18565ade04067a88e4ef8: this seems fine to me. The code is nicer and happens to fix a false positive.
(https://github.com/bitcoin/bitcoin/pull/31282#discussion_r1839882117)
fa91206b3c570dc17ee18565ade04067a88e4ef8: this seems fine to me. The code is nicer and happens to fix a false positive.
💬 fanquake commented on pull request "guix: scope pkg-config to Linux only":
(https://github.com/bitcoin/bitcoin/pull/31276#issuecomment-2473092574)
Guix build (aarch64):
```bash
02cdb4ec345e0e4b2f3876dded48a468c864fe58515ae7e4d7f9a8c1c4911337 guix-build-7edc54b086ac/output/aarch64-linux-gnu/SHA256SUMS.part
83d376375b1d5309108acc9e02711138c826d38fca0a73e7cc6048b21f3c4ab1 guix-build-7edc54b086ac/output/aarch64-linux-gnu/bitcoin-7edc54b086ac-aarch64-linux-gnu-debug.tar.gz
25677323242badfd9be8c5fdbc71ecf9b663239842deb244bbca4bc90a311d4d guix-build-7edc54b086ac/output/aarch64-linux-gnu/bitcoin-7edc54b086ac-aarch64-linux-gnu.tar.gz
de94c7
...
(https://github.com/bitcoin/bitcoin/pull/31276#issuecomment-2473092574)
Guix build (aarch64):
```bash
02cdb4ec345e0e4b2f3876dded48a468c864fe58515ae7e4d7f9a8c1c4911337 guix-build-7edc54b086ac/output/aarch64-linux-gnu/SHA256SUMS.part
83d376375b1d5309108acc9e02711138c826d38fca0a73e7cc6048b21f3c4ab1 guix-build-7edc54b086ac/output/aarch64-linux-gnu/bitcoin-7edc54b086ac-aarch64-linux-gnu-debug.tar.gz
25677323242badfd9be8c5fdbc71ecf9b663239842deb244bbca4bc90a311d4d guix-build-7edc54b086ac/output/aarch64-linux-gnu/bitcoin-7edc54b086ac-aarch64-linux-gnu.tar.gz
de94c7
...
💬 fanquake commented on pull request "guix: scope pkg-config to Linux only":
(https://github.com/bitcoin/bitcoin/pull/31276#issuecomment-2473152977)
Dropped another patch and pruned a Qt libtool archive.
(https://github.com/bitcoin/bitcoin/pull/31276#issuecomment-2473152977)
Dropped another patch and pruned a Qt libtool archive.
💬 fjahr commented on pull request "test: Introduce ensure_for helper":
(https://github.com/bitcoin/bitcoin/pull/30893#discussion_r1840013926)
reverted it
(https://github.com/bitcoin/bitcoin/pull/30893#discussion_r1840013926)
reverted it
💬 fjahr commented on pull request "test: Introduce ensure_for helper":
(https://github.com/bitcoin/bitcoin/pull/30893#discussion_r1840014484)
done
(https://github.com/bitcoin/bitcoin/pull/30893#discussion_r1840014484)
done
💬 fjahr commented on pull request "test: Introduce ensure_for helper":
(https://github.com/bitcoin/bitcoin/pull/30893#discussion_r1840014778)
done
(https://github.com/bitcoin/bitcoin/pull/30893#discussion_r1840014778)
done
💬 fjahr commented on pull request "test: Introduce ensure_for helper":
(https://github.com/bitcoin/bitcoin/pull/30893#issuecomment-2473228711)
Addressed latest feedback from @maflcko
(https://github.com/bitcoin/bitcoin/pull/30893#issuecomment-2473228711)
Addressed latest feedback from @maflcko
👍 vasild approved a pull request: "ci: skip Github CI on branch pushes for forks"
(https://github.com/bitcoin/bitcoin/pull/30487#pullrequestreview-2432802018)
ACK 8610bcef9d030013f9e36cffe0c58dd2cfe85d66
(https://github.com/bitcoin/bitcoin/pull/30487#pullrequestreview-2432802018)
ACK 8610bcef9d030013f9e36cffe0c58dd2cfe85d66
👍 vasild approved a pull request: "addrman: cap the `max_pct` to not exceed the maximum number of addresses"
(https://github.com/bitcoin/bitcoin/pull/31235#pullrequestreview-2432819520)
ACK 9c5775c331e02dab06c78ecbb58488542d16dda7
(https://github.com/bitcoin/bitcoin/pull/31235#pullrequestreview-2432819520)
ACK 9c5775c331e02dab06c78ecbb58488542d16dda7
💬 polespinasa commented on pull request "rpc: print P2WSH witScript in getrawtransaction":
(https://github.com/bitcoin/bitcoin/pull/31252#discussion_r1840096537)
I agree, it needs to be changed in one more place apart from this line but yes I will apply it.
Thanks!
(https://github.com/bitcoin/bitcoin/pull/31252#discussion_r1840096537)
I agree, it needs to be changed in one more place apart from this line but yes I will apply it.
Thanks!
💬 naumenkogs commented on pull request "Package validation: accept packages of size 1":
(https://github.com/bitcoin/bitcoin/pull/31096#discussion_r1840103944)
Expanding on this thought, I sketched a two-step improvement in handling single-package tx wrt code readability. Wanna [take a look](https://github.com/naumenkogs/bitcoin/commits/2024-10-submitpackage-singleton-v1/)?
I certainly might be missing something.
(https://github.com/bitcoin/bitcoin/pull/31096#discussion_r1840103944)
Expanding on this thought, I sketched a two-step improvement in handling single-package tx wrt code readability. Wanna [take a look](https://github.com/naumenkogs/bitcoin/commits/2024-10-submitpackage-singleton-v1/)?
I certainly might be missing something.
👍 rkrux approved a pull request: "test: Rework wallet_migration.py to use previous releases"
(https://github.com/bitcoin/bitcoin/pull/31248#pullrequestreview-2432399968)
tACK a76ad56a80d9c9a60352bb98b363131e359a383b
Certainly useful because it reflects how the end users would do wallet migration after the upcoming release.
Make is successful but around 15 functional tests are failing in my system that seem unrelated to this change because they are failing in master as well. All of the failures are due to timeouts, probably an issue with my setup.
Asked few questions and left suggestions. Found few really good tests in this file in the end!
(https://github.com/bitcoin/bitcoin/pull/31248#pullrequestreview-2432399968)
tACK a76ad56a80d9c9a60352bb98b363131e359a383b
Certainly useful because it reflects how the end users would do wallet migration after the upcoming release.
Make is successful but around 15 functional tests are failing in my system that seem unrelated to this change because they are failing in master as well. All of the failures are due to timeouts, probably an issue with my setup.
Asked few questions and left suggestions. Found few really good tests in this file in the end!