💬 ajtowns commented on pull request "p2p: protect addnode peers during IBD":
(https://github.com/bitcoin/bitcoin/pull/32051#issuecomment-2825824998)
> I think the main conceptual question is what should happen in the case of a stalling situation where the addnode peer is actually slow and causing congestion?

IMHO: It could be nice if we could "score" peers during IBD, and scale the number of blocks we request from them at once according to that score. In that case, automatic peers with low scores would get dropped, and manual peers with low scores would just not get any blocks requested.

💬 jonatack commented on pull request "p2p: protect addnode peers during IBD":
(https://github.com/bitcoin/bitcoin/pull/32051#issuecomment-2825834909)
Appreciate the thoughtful feedback. I'll look at an update.

📝 nervana21 opened a pull request: "doc: Add missing top-level description to pruneblockchain RPC"
(https://github.com/bitcoin/bitcoin/pull/32333)
Previously, the `pruneblockchain` RPC help output included only the method signature and arguments, with no top-level description explaining its purpose or constraints.

This PR adds a top-level description, improving documentation consistency and alerting users to the potential impacts of using the command.

🤔 hodlinator reviewed a pull request: "[IBD] multi-byte block obfuscation"
(https://github.com/bitcoin/bitcoin/pull/31144#pullrequestreview-2788618744)
Got some mixed messaging here since you told me repeatedly in private not to investigate failures I reported (https://github.com/bitcoin/bitcoin/pull/31144#issuecomment-2821410082) on my previous branch (https://github.com/bitcoin/bitcoin/pull/31144#pullrequestreview-2783071854) but then chided me in public (https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2056174501). Anyways, the previous branch was not the sliced bread I believed it to be, looking back.

---

New branch: https://
...

💬 hodlinator commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2056830517)
In 2a50ec0d17b12f2adb1bf8872592b898810e9ac4:
`key` is no longer used.

💬 hodlinator commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2057656754)
Inside a ctor which initializes `m_obfuscation{0}` through the header, you have:
```C++
...
{
std::vector<unsigned char> obfuscate_key_vector(Obfuscation::SIZE_BYTES, '\000');
...
if (...) {
... (modify obfuscate_key_vector)
}
m_obfuscation = obfuscate_key_vector;
}
}
```
Do you feel a need to `assert(m_obfuscation == 0)` at the beginning of the block because the ctor has ~30 preceeding lines? Can't think of another rea
...

🤔 hodlinator reviewed a pull request: "refactor: Update `XOnlyPubKey::GetKeyIDs()` to return a pair of pubkeys"
(https://github.com/bitcoin/bitcoin/pull/32332#pullrequestreview-2790021380)
Was nerd-sniped by this PR decreasing heap usage. Otherwise these kinds of refactorings are a bit frowned upon.

💬 hodlinator commented on pull request "refactor: Update `XOnlyPubKey::GetKeyIDs()` to return a pair of pubkeys":
(https://github.com/bitcoin/bitcoin/pull/32332#discussion_r2057721143)
If you instead return `std::array<CKeyID, 2>` the data is still on the stack and you don't need to touch signingprovider.h.

💬 hodlinator commented on pull request "refactor: Update `XOnlyPubKey::GetKeyIDs()` to return a pair of pubkeys":
(https://github.com/bitcoin/bitcoin/pull/32332#discussion_r2057718472)
New code should use `snake_case` for variables according to developer-notes.md.

💬 Sjors commented on pull request "miner: timelock the coinbase to the mined block's height":
(https://github.com/bitcoin/bitcoin/pull/32155#issuecomment-2826681887)
Failure in `interface_usdt_coinselection` seems to be an instance of #32322.

👍 TheCharlatan approved a pull request: "miner: timelock the coinbase to the mined block's height"
(https://github.com/bitcoin/bitcoin/pull/32155#pullrequestreview-2790092196)
Re-ACK 3904fcd6680dce7a9a0f5f91c5a3069b9fc81782

⚠️ maflcko opened an issue: "intermittent issue in feature_bip68_sequence.py: sequence_value = utxos[j]["confirmations"] IndexError: list index out of range"
(https://github.com/bitcoin/bitcoin/issues/32334)
https://cirrus-ci.com/task/5337593233014784?logs=ci#L3671

```
test 2025-04-23T20:13:04.883000Z TestFramework (ERROR): Unexpected exception caught during testing
Traceback (most recent call last):
File "/ci_container_base/test/functional/test_framework/test_framework.py", line 182, in main
self.run_test()
File "/ci_container_base/ci/scratch/build-
...

📝 maflcko opened a pull request: "ci: Temporarily disable failing bpf checks"
(https://github.com/bitcoin/bitcoin/pull/32335)
The ` interface_usdt_coinselection.py` seems to consistently fail under Linux kernel 6.11.

I don't have a VM with that kernel right now to test, and https://github.com/bitcoin/bitcoin/issues/32322 is open since two days, so I don't think anyone else has either.

So temporarily disable the failing tests, to allow for more time while unbreaking the CI.

💬 hodlinator commented on pull request "net: replace manual reference counting of CNode with shared_ptr":
(https://github.com/bitcoin/bitcoin/pull/32015#discussion_r2057787797)
> If we start extracting the raw pointers from `unique_ptr` and managing them separately and manually ref-counting then maybe it is better to just use `shared_ptr`.

I think doing ref-counting for snapshots is fine, but agree raw pointers in this codebase are ambiguous. It would be one thing if we started over with strict policies for `shared_ptr`/`unique_ptr`/raw pointers. Then one could consistently use raw pointer to always mean a non-owning "view-pointer" (https://github.com/bitcoin/bitcoi
...

💬 maflcko commented on issue "ci: failure in interface_usdt_coinselection.py":
(https://github.com/bitcoin/bitcoin/issues/32322#issuecomment-2826714257)
Done in [32335](https://github.com/bitcoin/bitcoin/pull/32335) to buy more time to test/fix/workaround this in the meantime.

💬 TheCharlatan commented on pull request "ci: Temporarily disable failing bpf checks":
(https://github.com/bitcoin/bitcoin/pull/32335#issuecomment-2826730778)
Isn't the problem just a compiler warning that can be suppressed with `-Wduplicate-decl-specifier` alongside the existing suppression?

💬 maflcko commented on pull request "ci: Temporarily disable failing bpf checks":
(https://github.com/bitcoin/bitcoin/pull/32335#issuecomment-2826745242)
Yes, I suspect so too, but I can't try locally right now. I can push to a new pull request and test via CI, if people don't mind.

📝 maflcko opened a pull request: "test: Suppress upstream `-Wduplicate-decl-specifier` in bpfcc"
(https://github.com/bitcoin/bitcoin/pull/32336)
(draft)

👍 laanwj approved a pull request: "test: Suppress upstream `-Wduplicate-decl-specifier` in bpfcc"
(https://github.com/bitcoin/bitcoin/pull/32336#pullrequestreview-2790382441)
Code review ACK facb9b327b9da39ce1e09ed56199be9efb19b5b8

💬 kehl-gopher commented on something "":
(https://github.com/bitcoin/bitcoin/commit/edffb50b984ff1c6a4dfdc4ebdb84ca776eb0666#commitcomment-155808830)
wow