💬 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
...

💬 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?

💬 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.

✅ fanquake closed a pull request: "RPC/Wallet: Add "use_txids" to output of getaddressinfo"
(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

💬 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?

💬 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?

💬 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.

💬 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"

📝 MarcoFalke opened a pull request: "refactor: Add [[nodiscard]] where ignoring a Result return type is an error "
(https://github.com/bitcoin/bitcoin/pull/27774)
Only add it for those where it is an error to ignore. Also, fix the gcc compile warning https://github.com/bitcoin/bitcoin/pull/25977#issuecomment-1564350880. Also, fix iwyu for touched header files.

💬 hebasto commented on pull request "ci, iwyu: Update mappings":
(https://github.com/bitcoin/bitcoin/pull/27710#discussion_r1209239987)
I've skipped non-test Boost stuff intentionally to avoid potential conflicts with other (more important) PRs.

💬 MarcoFalke commented on pull request "ci, iwyu: Update mappings":
(https://github.com/bitcoin/bitcoin/pull/27710#discussion_r1209243376)
`contrib/devtools/iwyu/bitcoin.core.imp` isn't modified by anyone else, so shouldn't cause any (silent) merge conflicts?

💬 joostjager commented on pull request "Make non-standard tx acceptance a peer option.":
(https://github.com/bitcoin/bitcoin/pull/27772#issuecomment-1567067889)
>Rather than open more PRs that are duplicative of other recently rejected PRs, it would be more helpful to directly address the feedback that was previously given.

This PR was intended as a draft. The goal was not to propose any changes right away, but rather to illustrate (also for myself) the touch points in the codebase. I wouldn't consider this PR a duplicate though. It only relaxes tx acceptance on the rpc level, and leaves the p2p restrictions in place - contrary to #27578. Nevertheles
...

🚀 fanquake merged a pull request: "contrib/init: Better systemd integration"
(https://github.com/bitcoin/bitcoin/pull/25975)

💬 hebasto commented on pull request "ci, iwyu: Update mappings":
(https://github.com/bitcoin/bitcoin/pull/27710#issuecomment-1567109899)
Addressed @MarcoFalke's https://github.com/bitcoin/bitcoin/pull/27710#discussion_r1209217111.

💬 hebasto commented on pull request "ci, iwyu: Update mappings":
(https://github.com/bitcoin/bitcoin/pull/27710#discussion_r1209296024)
> If you want to look for missing ones, there should be at least signals2? See [api.cirrus-ci.com/v1/task/4955079116062720/logs/ci.log](https://api.cirrus-ci.com/v1/task/4955079116062720/logs/ci.log) with search string "+#include <boos"

Thanks! [Done](https://github.com/bitcoin/bitcoin/pull/27710#issuecomment-1567109899).

> `contrib/devtools/iwyu/bitcoin.core.imp` isn't modified by anyone else, so shouldn't cause any (silent) merge conflicts?

You're right. I meant actual adding/removing
...

💬 TheCharlatan commented on pull request "refactor: Add [[nodiscard]] where ignoring a Result return type is an error":
(https://github.com/bitcoin/bitcoin/pull/27774#issuecomment-1567112224)
> Only add it for those where it is an error to ignore.

How did you select these?

💬 joostjager commented on issue "Allow accepting non-standard transactions on mainnet via local rpc":
(https://github.com/bitcoin/bitcoin/issues/27768#issuecomment-1567115600)
@sdaftuar wrote in https://github.com/bitcoin/bitcoin/pull/27772#issuecomment-1566232510:

> Are you unaware that some of the DoS attacks that are prevented by our standardness checks include situations where a block might be mined that would take a very long time to validate, a cost that would be felt by the whole network? See for instance [https://bitcointalk.org/?topic=140078%7CCVE-2013-2292]]](https://bitcointalk.org/?topic=140078%7CCVE-2013-2292%5D%5D). It is absurd that this issue would
...

💬 MarcoFalke commented on pull request "refactor: Add [[nodiscard]] where ignoring a Result return type is an error":
(https://github.com/bitcoin/bitcoin/pull/27774#issuecomment-1567120817)
> How did you select these?

I asked my brain if it thought that ignoring the return type would lead to errors in later code.

💬 MarcoFalke commented on pull request "Make non-standard tx acceptance a peer option.":
(https://github.com/bitcoin/bitcoin/pull/27772#discussion_r1209308134)
Looks like you just switch this from true to false here, without making it an option. NACK on making this a silent footgun for all users.