💬 adam3us commented on pull request "docs: Undeprecate datacarrier and datacarriersize configuration options":
(https://github.com/bitcoin/bitcoin/pull/33453#issuecomment-3339197218)
> @ajtowns wrote:
>
> > For comparison, the difficulty increase from early May to now (given the same hashrate) implies a decrease in revenue of 19.5%, more than 100x as much. I think it makes sense to set defaults to maximise even small amounts of revenue, but that's a "picking up pennies" saving, not a "small miners will be put out of business" situation.
>
> Is the argument that poor compact block reconstruction rates leads to miner centralization overblown then if the loss in revenue d
...

🚀 glozow merged a pull request: "key: use static context for libsecp256k1 calls where applicable"
(https://github.com/bitcoin/bitcoin/pull/33399)

💬 maflcko commented on issue "use -loadblock to load blk*****.dat files, but the blocks in it are not recognized":
(https://github.com/bitcoin/bitcoin/issues/33280#issuecomment-3339296357)
> I don't know if that was discussed when block obfuscation was added (FYI [@maflcko](https://github.com/maflcko)).

I wasn't aware of a use-case to load raw block files without first linearizing them. As suggested, the workaround here would be `-blocksxor=0` for the node that generates block files to import. You could also hack together a trivial python script to do the xor for you, quickly. (The linearize script may also work, if you omit the unneeded block hashes, but I haven't tried this).

🚀 glozow merged a pull request: "test/refactor: use test deque to avoid quadratic iteration"
(https://github.com/bitcoin/bitcoin/pull/33313)

💬 0xB10C commented on pull request "p2p: Use network-dependent timers for inbound inv scheduling":
(https://github.com/bitcoin/bitcoin/pull/33464#issuecomment-3339341600)
Approach ACK.

Nice. I had noticed that we announce INVs to everyone at the same time, but it didn't occur to me that this is a fingerprinting leak. To test this change, I got creative and compared a 30.0 release candidate node with a node running this PR using my [p2p-circle](https://github.com/0xB10C/peer-observer/blob/master/tools/websocket/www/p2p-circle.html) peer-observer tool.

In the middle is our node and around it are our peers. Here, colored by the network which we are connected t
...

💬 RandyMcMillan commented on pull request "rpcconsole: display signet challenge":
(https://github.com/bitcoin-core/gui/pull/896#issuecomment-3339414175)
use HexStr() per [pablomartin4btc](https://github.com/pablomartin4btc) suggestion

📝 purpleKarrot opened a pull request: "CMake: Add dynamic test discovery"
(https://github.com/bitcoin/bitcoin/pull/33483)
Instead of parsing the test names from the source code at configure time, query the list of tests from the test executables at testing time.

This should be upstreamed to CMake: https://gitlab.kitware.com/cmake/cmake/-/issues/26920

Left to do: The `test_bitcoin-qt` executable should be fixed to support the `-function` option documented here: https://doc.qt.io/qt-6/qtest-overview.html#testing-options so that its test cases can be discovered as well.

💬 purpleKarrot commented on pull request "test: Run bench sanity checks in parallel with functional tests":
(https://github.com/bitcoin/bitcoin/pull/33142#issuecomment-3339779236)
Please see https://github.com/bitcoin/bitcoin/pull/33483 for dynamic test discovery.

📝 theStack opened a pull request: "doc: rpc: fix case typo in `finalizepsbt` help (final_scriptwitness)"
(https://github.com/bitcoin/bitcoin/pull/33484)
The lower-case spelling matches the `decodepsbt` result field:
https://github.com/bitcoin/bitcoin/blob/200150beba6601237036bc01ee10f6a0a2246c3d/src/rpc/rawtransaction.cpp#L871
https://github.com/bitcoin/bitcoin/blob/200150beba6601237036bc01ee10f6a0a2246c3d/src/rpc/rawtransaction.cpp#L1253

👍 l0rinc approved a pull request: "doc: rpc: fix case typo in `finalizepsbt` help (final_scriptwitness)"
(https://github.com/bitcoin/bitcoin/pull/33484#pullrequestreview-3273312375)
ACK ff05bebcc4262966b117082a67dc4c63a3f67d2d

The doc seems to be the only mention of `final_scriptWitness`, every other one is indeed `final_scriptwitness` - seems it was added incorrectly in https://github.com/bitcoin/bitcoin/commit/c27fe419efb3b6588c400d764122ffb33375e028#diff-a58e7bb9d9a8a0287c0b7281d99da4e79b6f8c2a5780c24c6d76c14212c48640R1624

💬 l0rinc commented on pull request "validation: ensure assumevalid is always used during reindex":
(https://github.com/bitcoin/bitcoin/pull/31615#discussion_r2383126356)
Not necessarily, but it bothers me that we (or at least I) don't understand what's causing the problem in the first place, the current solution feels like a work-around to me - which is still better than what we had, but it's also arguably more complicated and it doesn't seem to fix the underlying problem of why reindex is missing headers in the first place. And are we sure it's a reindex-only bug or can it happen for reindex-chainstate or IBD as well? I haven't dug too deep into this, but I wan
...

💬 achow101 commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2383164921)
The test failure with `send` was because of sighash types, not that `send` doesn't work for musig. It was failing on the test that changes the sighash type.

💬 achow101 commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2383168777)
The sighash is calculated multiple times during signing outside of musig signing too. I will leave refactoring of this to a followup.

💬 mzumsande commented on pull request "validation: ensure assumevalid is always used during reindex":
(https://github.com/bitcoin/bitcoin/pull/31615#discussion_r2383171731)
> Not necessarily, but it bothers me that we (or at least I) don't understand what's causing the problem in the first place

I tried to describe this in the linked issue, I think the problem is understood. To rephrase it:

Normal IBD has the order 1) Download entire header chain until we have crossed minchainwork 2) Start downloading blocks
=> assumevalid will be applied because we have a header with minchainwork and the assumevalid block in our index

During reindex, we 1) Delete all ex
...

💬 maflcko commented on pull request "CMake: Add dynamic test discovery":
(https://github.com/bitcoin/bitcoin/pull/33483#issuecomment-3340123470)
Seems nice, but in the great picture there is still room for improvement:

* This doesn't address the issue of running cross builds on the target (https://github.com/bitcoin/bitcoin/blob/200150beba6601237036bc01ee10f6a0a2246c3d/.github/workflows/ci.yml#L388)
* The functional tests aren't integrated, so they are run sequentially after the unit tests

I am not going to block this pull, because it is a clear improvement over master, and the other issues should be fixed in separate pulls. Howev
...

💬 achow101 commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2383499341)
Unified.

> Right now, FillSignatureData is only called on new empty SignatureData objects, but not sure if that is an assumption in the code.

That is not an assumption.

💬 achow101 commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2383499483)
Done

💬 achow101 commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2383499646)
Done

💬 Fiach-Dubh commented on something "":
(https://github.com/bitcoin/bitcoin/commit/766561b8297b534dac1ed7ccfc57e03fa5a41043#commitcomment-166657363)
we can't give luke-jr the nuclear launch codes!

🤔 hodlinator reviewed a pull request: "qa: Improvements to debug_assert_log + busy_wait_for_debug_log"
(https://github.com/bitcoin/bitcoin/pull/33423#pullrequestreview-3273948902)
Latest push fixes bug from previous push (https://github.com/bitcoin/bitcoin/pull/33423#discussion_r2379926157), simplifies code (https://github.com/bitcoin/bitcoin/pull/33423#discussion_r2379920746) and adds commit to fix docstring (https://github.com/bitcoin/bitcoin/pull/33423#discussion_r2379958513).

Compared run-times for all functional tests and wasn't able to find one which clearly benefited from this optimization. So I'm open to just replacing `while remaining_expected and remaining_ex
...