💬 Christewart commented on issue "Failing to fetch `cfheader` corresponding to block header in `headers` message":
(https://github.com/bitcoin/bitcoin/issues/27085#issuecomment-1879024752)
> Ok, it is called, but you also have to wait for it to complete and return from the RPC, before continuing the test. Currently it looks like it is called, and processes the block events, but at the same time the P2P interface is asked for the filters.
>
> It may be best to only use a single thread for the test logic.

Hmm, you are totally right. You said elsewhere in this thread

>Even when the peer sent the headers message, the block filter index could still be processing the block sign
...

💬 1thales commented on pull request "RFC: Deprecate libconsensus":
(https://github.com/bitcoin/bitcoin/pull/29189#issuecomment-1879025018)
Ping @Davidson-Souza

💬 stickies-v commented on pull request "RFC: Deprecate libconsensus":
(https://github.com/bitcoin/bitcoin/pull/29189#issuecomment-1879026448)
Concept ACK

💬 josibake commented on pull request "wallet, rpc: `FundTransaction` refactor":
(https://github.com/bitcoin/bitcoin/pull/28560#discussion_r1443157721)
Good point; I'll add a check and a comment.

💬 luke-jr commented on issue "Witness scripts being abused to bypass datacarriersize limit (CVE-2023-50428)":
(https://github.com/bitcoin/bitcoin/issues/29187#issuecomment-1879034293)
No, it doesn't contradict it. That overview conveniently leaves out the context of OP_RETURN being the _only_ way tolerated, and the `datacarriersize` limit makes no sense the way you want to spin it.

💬 achow101 commented on pull request "wallet: simplify and batch zap wallet txes process":
(https://github.com/bitcoin/bitcoin/pull/28987#discussion_r1443161309)
In 559529480fedd4181dab76f4452b5998d0cdadc3 "refactor: decouple wallet EraseTxns from ZapSelectTx"

Unnecessary whitespace change.

💬 achow101 commented on pull request "wallet: simplify and batch zap wallet txes process":
(https://github.com/bitcoin/bitcoin/pull/28987#discussion_r1443160561)
In 559529480fedd4181dab76f4452b5998d0cdadc3 "refactor: decouple wallet EraseTxns from ZapSelectTx"

The commit message calls this `EraseTxns`.

💬 achow101 commented on pull request "wallet: simplify and batch zap wallet txes process":
(https://github.com/bitcoin/bitcoin/pull/28987#discussion_r1443164969)
In ff214259de28216a5190c358b786306985f30227 "wallet: migration, remove watch-only transactions atomically"

It seems unnecessary to be making this change here as `ZapSelectTx` is now atomic.

🚀 fanquake merged a pull request: "wallettool: Always be able to dump a wallet's database"
(https://github.com/bitcoin/bitcoin/pull/29117)

💬 TheCharlatan commented on pull request "refactor(tidy): Use C++20 contains method":
(https://github.com/bitcoin/bitcoin/pull/29191#issuecomment-1879038224)
Concept ACK

💬 maflcko commented on issue "Failing to fetch `cfheader` corresponding to block header in `headers` message":
(https://github.com/bitcoin/bitcoin/issues/27085#issuecomment-1879039692)
> I guess the question now is why are we gossiping headers before its possible to serve filterheaders / filters that correspond with them if blockfilterindex=1 is enabled?

There was a previous discussion (I can't find it now), but I think it said that the P2P interface may fail, because it reaches to untrusted peers, so a logic to recover is needed either way. If you want to use a trusted interface, use RPC.

Though, there is also P2P encryption and peer permission flags, so my preference w
...

👍 stickies-v approved a pull request: "RPC/Blockchain: scanblocks: Accept named param for filter_false_positives"
(https://github.com/bitcoin/bitcoin/pull/29184#pullrequestreview-1806497595)
ACK 96ec3b67a7a7f968d002e13d6fc227f69b7f07d7

Also quickly checked that all other options objects properly accept named parameters (although I didn't check every single options parameter)

👍 fanquake approved a pull request: "doc: Rework guix docs after 1.4 release"
(https://github.com/bitcoin/bitcoin/pull/28962#pullrequestreview-1806498886)
ACK fad444f6e1b1ce5c1572c773db4a9a098c7a0b96

✅ fanquake closed an issue: "doc: update Guix install docs for 1.4.0"
(https://github.com/bitcoin/bitcoin/issues/28957)

🚀 fanquake merged a pull request: "doc: Rework guix docs after 1.4 release"
(https://github.com/bitcoin/bitcoin/pull/28962)

💬 josibake commented on pull request "wallet, rpc: `FundTransaction` refactor":
(https://github.com/bitcoin/bitcoin/pull/28560#discussion_r1443173852)
nice! TIL c++20 introduced contains for sets :heart:

💬 Christewart commented on issue "Failing to fetch `cfheader` corresponding to block header in `headers` message":
(https://github.com/bitcoin/bitcoin/issues/27085#issuecomment-1879044563)
> There was a previous discussion (I can't find it now)

Do you recall the venue of where it was discussed? Some PR/issue on github or elsewhere? I may go digging.

In general, should I leave this issue open? It seems like this will not be resolved in the short term (or ever maybe).

✅ fanquake closed a pull request: "CONTRIBUTING: Caution against using AI/LLMs (ChatGPT, Copilot, etc)"
(https://github.com/bitcoin/bitcoin/pull/28175)

💬 fanquake commented on pull request "CONTRIBUTING: Caution against using AI/LLMs (ChatGPT, Copilot, etc)":
(https://github.com/bitcoin/bitcoin/pull/28175#issuecomment-1879046034)
Closing this for now.

💬 hebasto commented on pull request "refactor: C++20: Use std::rotl":
(https://github.com/bitcoin/bitcoin/pull/29085#issuecomment-1879046730)
Concept ACK.