Bitcoin Core Github
44 subscribers
121K links
Download Telegram
💬 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
💬 TheCharlatan commented on pull request "test: fix pushdata scripts":
(https://github.com/bitcoin/bitcoin/pull/32270#discussion_r2057941914)
Maybe call this just `2byte_push`, since there is no pushdata going on here?