👍 hebasto approved a pull request: "ci: Add missing set -e to 01_base_install.sh"
(https://github.com/bitcoin/bitcoin/pull/27739#pullrequestreview-1449096752)
ACK fa12558d21aa03c22407a1458a0345d8a7ab6a4b
(https://github.com/bitcoin/bitcoin/pull/27739#pullrequestreview-1449096752)
ACK fa12558d21aa03c22407a1458a0345d8a7ab6a4b
💬 hebasto commented on pull request "ci, iwyu: Update mappings":
(https://github.com/bitcoin/bitcoin/pull/27710#issuecomment-1566747695)
CI failure seems unrelated.
(https://github.com/bitcoin/bitcoin/pull/27710#issuecomment-1566747695)
CI failure seems unrelated.
👍 hebasto approved a pull request: "kernel: Remove util/system from kernel library, interface_ui from validation."
(https://github.com/bitcoin/bitcoin/pull/27636#pullrequestreview-1449106502)
re-ACK 7d3b35004b039f2bd606bb46a540de7babdbc41e, only last two commits dropped since my [recent](https://github.com/bitcoin/bitcoin/pull/27636#pullrequestreview-1435394620) review.
(https://github.com/bitcoin/bitcoin/pull/27636#pullrequestreview-1449106502)
re-ACK 7d3b35004b039f2bd606bb46a540de7babdbc41e, only last two commits dropped since my [recent](https://github.com/bitcoin/bitcoin/pull/27636#pullrequestreview-1435394620) review.
💬 TheCharlatan commented on pull request "ci: Add missing set -e to 01_base_install.sh":
(https://github.com/bitcoin/bitcoin/pull/27739#issuecomment-1566766689)
Re https://github.com/bitcoin/bitcoin/pull/27739#issuecomment-1564529747
> Thanks, the last commit should fix your bug.
Thank you for digging into this, seems like everything is working as it should now.
ACK [fa12558](https://github.com/bitcoin/bitcoin/pull/27739/commits/fa12558d21aa03c22407a1458a0345d8a7ab6a4b)
(https://github.com/bitcoin/bitcoin/pull/27739#issuecomment-1566766689)
Re https://github.com/bitcoin/bitcoin/pull/27739#issuecomment-1564529747
> Thanks, the last commit should fix your bug.
Thank you for digging into this, seems like everything is working as it should now.
ACK [fa12558](https://github.com/bitcoin/bitcoin/pull/27739/commits/fa12558d21aa03c22407a1458a0345d8a7ab6a4b)
👍 real-or-random approved a pull request: "contrib/init: Better systemd integration"
(https://github.com/bitcoin/bitcoin/pull/25975#pullrequestreview-1449124089)
ACK 689a65d878638ae67b07f111dd77ea3c624e4f58 tested this service file with 25.0
(https://github.com/bitcoin/bitcoin/pull/25975#pullrequestreview-1449124089)
ACK 689a65d878638ae67b07f111dd77ea3c624e4f58 tested this service file with 25.0
💬 ismaelsadeeq commented on issue "25.0 RC Testing Guide Feedback":
(https://github.com/bitcoin/bitcoin/issues/27736#issuecomment-1566768353)
All comments and suggestions have been addressed, Thank you everyone for the feedback.
(https://github.com/bitcoin/bitcoin/issues/27736#issuecomment-1566768353)
All comments and suggestions have been addressed, Thank you everyone for the feedback.
💬 real-or-random commented on pull request "Introduce secp256k1 module with field and group classes to test framework":
(https://github.com/bitcoin/bitcoin/pull/26222#discussion_r1209080001)
Perhaps the comment should be explicit about the behavior when `v >= FE.SIZE`
(https://github.com/bitcoin/bitcoin/pull/26222#discussion_r1209080001)
Perhaps the comment should be explicit about the behavior when `v >= FE.SIZE`
👋 fanquake's pull request is ready for review: "lint: stop ignoring LIEF imports"
(https://github.com/bitcoin/bitcoin/pull/27507)
(https://github.com/bitcoin/bitcoin/pull/27507)
💬 fanquake commented on pull request "lint: stop ignoring LIEF imports":
(https://github.com/bitcoin/bitcoin/pull/27507#issuecomment-1566839915)
Rebased and updated to use LIEF 0.13.1.
(https://github.com/bitcoin/bitcoin/pull/27507#issuecomment-1566839915)
Rebased and updated to use LIEF 0.13.1.
🚀 fanquake merged a pull request: "ci: Add missing set -e to 01_base_install.sh"
(https://github.com/bitcoin/bitcoin/pull/27739)
(https://github.com/bitcoin/bitcoin/pull/27739)
🚀 fanquake merged a pull request: "test: Throw error when -signetchallenge is non-hex"
(https://github.com/bitcoin/bitcoin/pull/27765)
(https://github.com/bitcoin/bitcoin/pull/27765)
💬 fanquake commented on pull request "Fix rpc_getblockfrompeer intermittency by introducing 'getblockfileinfo' RPC command":
(https://github.com/bitcoin/bitcoin/pull/27770#issuecomment-1566869276)
> I started there but.. do we really care?
We definitely care enough to investigate further, before adding a new RPC command to fix an intermittent issue in a functional test.
Looks like @theStack has done that investigation here: https://github.com/bitcoin/bitcoin/issues/27749#issuecomment-1566354933, and @mzumsande has suggested a potential alternative fix: https://github.com/bitcoin/bitcoin/issues/27749#issuecomment-1566554075:
> add a sync_blocks call between node0 and node2 before le
...
(https://github.com/bitcoin/bitcoin/pull/27770#issuecomment-1566869276)
> I started there but.. do we really care?
We definitely care enough to investigate further, before adding a new RPC command to fix an intermittent issue in a functional test.
Looks like @theStack has done that investigation here: https://github.com/bitcoin/bitcoin/issues/27749#issuecomment-1566354933, and @mzumsande has suggested a potential alternative fix: https://github.com/bitcoin/bitcoin/issues/27749#issuecomment-1566554075:
> add a sync_blocks call between node0 and node2 before le
...
💬 fanquake commented on pull request "util: generalize accounting of system-allocated memory in pool resource":
(https://github.com/bitcoin/bitcoin/pull/27748#discussion_r1209144572)
Not sure what value this comment adds. Also don't see a reason to mention a version of glibc that we don't support?
(https://github.com/bitcoin/bitcoin/pull/27748#discussion_r1209144572)
Not sure what value this comment adds. Also don't see a reason to mention a version of glibc that we don't support?
💬 fanquake commented on pull request "RPC/Wallet: Add "use_txids" to output of getaddressinfo":
(https://github.com/bitcoin/bitcoin/pull/22693#issuecomment-1566876405)
Can be reopened if/when work is continued.
(https://github.com/bitcoin/bitcoin/pull/22693#issuecomment-1566876405)
Can be reopened if/when work is continued.
✅ fanquake closed a pull request: "RPC/Wallet: Add "use_txids" to output of getaddressinfo"
(https://github.com/bitcoin/bitcoin/pull/22693)
(https://github.com/bitcoin/bitcoin/pull/22693)
👍 MarcoFalke approved a pull request: "ci, iwyu: Update mappings"
(https://github.com/bitcoin/bitcoin/pull/27710#pullrequestreview-1449308791)
lgtm ACK
(https://github.com/bitcoin/bitcoin/pull/27710#pullrequestreview-1449308791)
lgtm ACK
💬 MarcoFalke commented on pull request "ci, iwyu: Update mappings":
(https://github.com/bitcoin/bitcoin/pull/27710#discussion_r1209195309)
Would it make sense to upstream those?
(https://github.com/bitcoin/bitcoin/pull/27710#discussion_r1209195309)
Would it make sense to upstream those?
💬 fanquake commented on pull request "ci, iwyu: Update mappings":
(https://github.com/bitcoin/bitcoin/pull/27710#discussion_r1209196694)
Was wondering the same about `boost-1.75-all.imp` and `qt5_11.imp`, given they are clearly for older versions. Are we going to upstream updates, or just wait until the mappings break?
(https://github.com/bitcoin/bitcoin/pull/27710#discussion_r1209196694)
Was wondering the same about `boost-1.75-all.imp` and `qt5_11.imp`, given they are clearly for older versions. Are we going to upstream updates, or just wait until the mappings break?
💬 hebasto commented on pull request "ci, iwyu: Update mappings":
(https://github.com/bitcoin/bitcoin/pull/27710#discussion_r1209200447)
Yes, I did consider upstreaming these change. It seems make sense to do it after removing all Boost and Qt related warnings for our code, to be sure that the changes are complete.
(https://github.com/bitcoin/bitcoin/pull/27710#discussion_r1209200447)
Yes, I did consider upstreaming these change. It seems make sense to do it after removing all Boost and Qt related warnings for our code, to be sure that the changes are complete.
💬 MarcoFalke commented on pull request "ci, iwyu: Update mappings":
(https://github.com/bitcoin/bitcoin/pull/27710#discussion_r1209217111)
If you want to look for missing ones, there should be at least signals2? See https://api.cirrus-ci.com/v1/task/4955079116062720/logs/ci.log with search string "+#include <boos"
(https://github.com/bitcoin/bitcoin/pull/27710#discussion_r1209217111)
If you want to look for missing ones, there should be at least signals2? See https://api.cirrus-ci.com/v1/task/4955079116062720/logs/ci.log with search string "+#include <boos"