Bitcoin Core Github
44 subscribers
120K links
Download Telegram
💬 HowHsu commented on pull request "index: fix wrong assert of current_tip == m_best_block_index":
(https://github.com/bitcoin/bitcoin/pull/32878#issuecomment-3038295873)
> Can you write a test that triggers this assert on master branch?

Sure, I'll do that.
💬 b-l-u-e commented on issue "Internal bug detected: FinalizeAndExtractPSBT(psbtx_copy, mtx)":
(https://github.com/bitcoin/bitcoin/issues/32849#issuecomment-3038398123)
Hi @ekrembal,

I've analyzed this issue. The "Internal bug detected" error occurs because `descriptorprocesspsbt` uses `PSBTInputSigned()` which only checks for signature **presence**, not **validity**.

### Root Cause
In `src/rpc/rawtransaction.cpp` around lines 2092-2094:
```cpp
bool complete = true;
for (const auto& input : psbtx.inputs) {
complete &= PSBTInputSigned(input); // Only checks presence, not validity
}
```

When `complete` is `true`, the code calls `CHECK_NONFATAL(FinalizeA
...
📝 maflcko opened a pull request: "ci: Avoid cd into build dir"
(https://github.com/bitcoin/bitcoin/pull/32880)
Changing into the build dir is confusing and brittle, because the following commands implicitly assume it. So they could break on unrelated changes.

The changes are required for stuff like:

* cmake presets (see https://github.com/bitcoin/bitcoin/pull/30871#issuecomment-2344031208)
* meta ci tests (like https://github.com/bitcoin/bitcoin/pull/32874)

So remove the `cd` and just make the build dir explicit.
💬 ismaelsadeeq commented on pull request "fees: prevent redundant estimates flushes":
(https://github.com/bitcoin/bitcoin/pull/32748#discussion_r2187355851)
Taken, also please add an explanation.

I also note a behavior change introduced by this. It is possible that we may track some unconfirmed transactions and, in rare cases, see some unconfirmed transactions get evicted. However, since we have no confirmations yet, if we shut down in this state, we will lose that data and have to start from scratch. To mitigate this, we need a way to detect when we are in such a state.

I see two possible approaches:

1. Brute-force check : Go through all t
...
📝 maflcko opened a pull request: "Turn rpcauth.py test into functional test"
(https://github.com/bitcoin/bitcoin/pull/32881)
Currently the `rpcauth-test.py` is problematic, because:

* The boilerplate for the test runner is duplicate or inconsistent with the other (functional) tests. Specifically `ConfigParser`.
* The cmake/ci behavior is brittle and can silently fail, as explained in https://github.com/bitcoin/bitcoin/issues/31476.
* Outside of ctest, this single test has to be run manually and separately, which is easy to forget.
* If the test is manually called, it runs single threaded, when it could just run
...
💬 maflcko commented on pull request "cmake: Improve Python robustness and test usability":
(https://github.com/bitcoin/bitcoin/pull/31233#discussion_r2187385272)
> the correct fix would be to just remove the cmake stuff here and move this test to the python test runner, so that it is run like all other python unit/util/functional tests.

Fixed in https://github.com/bitcoin/bitcoin/pull/32881



> (Actually, while testing, a missing python is now a hard error already when `-DBUILD_GUI=ON`. So I don't think the wording "refactor" accurately represents the changes here.)

To clarify, either the changes here in this pull need to be reverted/fixed up,
...
📝 HowHsu opened a pull request: "index: remove unnecessary locater cleaning in BaseIndex::Init()"
(https://github.com/bitcoin/bitcoin/pull/32882)
BaseIndex::DB::ReadBestBlock() already cleans locater in failure path, remove duplicate code in the caller and change ReadBestBlock to return void.
tdb3 closed a pull request: "rpc: add `relevant_blocks` to `scanblocks status`"
(https://github.com/bitcoin/bitcoin/pull/30713)
💬 romanz commented on pull request "index: store per-block transaction locations for efficient lookups":
(https://github.com/bitcoin/bitcoin/pull/32541#issuecomment-3039513232)
Added a short description in `doc/REST-interface.md` and `doc/files.md`, and updated release notes in https://github.com/bitcoin/bitcoin/pull/32541/commits/c695d134683d52bce9e499a5848e4c4c7951155c.
Please let me know if there are any additional open issues :)
🤔 l0rinc reviewed a pull request: "[IBD] multi-byte block obfuscation"
(https://github.com/bitcoin/bitcoin/pull/31144#pullrequestreview-2987741921)
Thanks @maflcko for the review, addressed your nits in the latest push.

The [final diff](https://github.com/bitcoin/bitcoin/compare/f5c6a09d3593955a1966b359c76c60c223896c8f..e036ba4ddaeb24a2361c356f7f270ee60bfff984) is mostly the same, since the comments were mostly about intermediary commits.

I have exploded the big vector-to-uint64 commit into 3, these two were split out:
* refactor: move `util::Xor` to `Obfuscation().Xor`
* refactor: encapsulate `std::vector<std::byte>` keys into `Obfuscati
...
💬 l0rinc commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2185852047)
> adding const before std::span doesn't really say much

This is an intermediary commit, meant to lay the groundwork for more risky changes later.
It may not make a lot of sense on its own, even though it does prohibits argument reassignments such as:
```C++
auto apply_random_xor_chunks{[&](std::span<std::byte> target, const std::span<std::byte, sizeof(uint64_t)> obfuscation) {
std::array<std::byte, sizeof(uint64_t)> key_bytes{};
obfuscation = std::span<std::byte, sizeof(uint64_t)
...
💬 l0rinc commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2185875881)
I'm not exactly sure I understood, but added a comment to `write_offset` to clarify that it's meant to check that we can start obfuscation from any offset
💬 l0rinc commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2185698665)
I can't say that I don't disagree with none of that!

----

I tried to encode the meaning into a check (i.e. that the obfuscated value should differ from the original, unless obfuscation is turned off), but it got indeed quite weird, thanks :)

Maybe there's a middle-ground that documents the expectation better:
```C++
BOOST_CHECK_NE(original != roundtrip, all_zeros);
```
or even better (which would eliminate the need for the comment as well)
```C++
const bool key_all_zeros{std::rang
...
💬 l0rinc commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2185892222)
was a leftover from the previous `1_MiB` - removed, thanks.
💬 l0rinc commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2185693472)
Yeah, this was done to separate the formatting from code change being done later - but I also dislike doing it in a former commit where we don't see yet why the alignment won't be off in a few more commits. And the comment is meaningless, so I've removed it, thanks.
💬 l0rinc commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2185692178)
the `key_offset_bytes % SIZE_BYTES` part wouldn't be tested if we just used `key_offset{m_rng.randrange(8U)}` - can you suggest a better comment to make that obvious?
💬 l0rinc commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2185864960)
Kept the inner const, removed the outer
💬 l0rinc commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2185893737)
Fine, reverted.
💬 l0rinc commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2185940231)
> Also, the line is touched later anyway

That's why low-risk changes such as the whitespace formatting was done in a separate commit. But I've reverted it an now the optimization commit does the formatting as well.
💬 l0rinc commented on pull request "mempool: Avoid needless vtx iteration during IBD":
(https://github.com/bitcoin/bitcoin/pull/32827#discussion_r2187619161)
@furszy and @ismaelsadeeq have opined on it in the [IRC meeting](https://bitcoin-irc.chaincode.com/bitcoin-core-dev/2025-07-03):
> [18:38](https://bitcoin-irc.chaincode.com/bitcoin-core-dev/2025-07-03#1132853) <furszy> l0rinc: I just had a quick talk with abubakarsadiq and It seems we still need to send the signal just to update the best seen height inside the fee estimation class, but we can skip some of the calculations on the event processing side (all the stats objects are initialized but c
...