Bitcoin Core Github
44 subscribers
121K links
Download Telegram
📝 Crypt-iQ opened a pull request: "net: check for empty header before calling FillBlock"
(https://github.com/bitcoin/bitcoin/pull/33296)
This avoids an Assume crash if multiple blocktxn messages are received. The first call to `FillBlock` would make the header empty via `SetNull` and the second call to `FillBlock` would crash [here](https://github.com/bitcoin/bitcoin/blob/689a32197638e92995dd8eb071425715f5fdc3a4/src/net_processing.cpp#L3333) since `LookupBlockIndex` won't find anything. Fix that by checking for an empty header before the Assume.
💬 instagibbs commented on pull request "net: check for empty header before calling FillBlock":
(https://github.com/bitcoin/bitcoin/pull/33296#discussion_r2319776496)
might note that the header is blanked in FillBlock, but the partial block never deleted, which is why this would catch the behavior
👍 ryanofsky approved a pull request: "Add functional test for IPC interface"
(https://github.com/bitcoin/bitcoin/pull/33201#pullrequestreview-3181743977)
Code review ACK 235016f5b78ba9f472b56df0825690307fffc7e6. Nice to have this ability to test the interfaces directly. Left a few comments but none are important
💬 ryanofsky commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2319744962)
In commit "ci: enable IPC tests in CI" (7eb71775dce6f3f2fd0e4b62f439aab86d1b65a0)

Would seem good to drop --break-system-packages here and only add it on the platforms where it's needed following:

https://github.com/bitcoin/bitcoin/blob/689a32197638e92995dd8eb071425715f5fdc3a4/ci/test/00_setup_env_mac_native.sh#L9-L12

It's also not clear what the problem is if --break-system-packages is not used. Does the package fail to install otherwise?
💬 ryanofsky commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2319703609)
In commit "tests: add functional tests for IPC interface" (592e253f76764f108e9c34fa0da32451091b946a)

capnp_dir is listed twice here
💬 ryanofsky commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2319697575)
In commit "tests: add functional tests for IPC interface" (592e253f76764f108e9c34fa0da32451091b946a)

Does this need to say `pip install ./pycapnp`? It seems like on my system without `./` pip ignores the git clone and downloads a wheel instead. Same might apply to the venv instructions below.
💬 ryanofsky commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2319701840)
In commit "tests: add functional tests for IPC interface" (592e253f76764f108e9c34fa0da32451091b946a)

s/there is/there is no/
💬 achow101 commented on pull request "wallet: Identify transactions spending 0-value outputs, and add tests for anchor outputs in a wallet":
(https://github.com/bitcoin/bitcoin/pull/33268#discussion_r2319788315)
The test coverage for this commit is provided in the first commit of the PR. The point of that test is to ensure that behavior doesn't change for commit.
💬 Crypt-iQ commented on pull request "net: check for empty header before calling FillBlock":
(https://github.com/bitcoin/bitcoin/pull/33296#discussion_r2319795093)
Yup good point, will do.
💬 achow101 commented on pull request "wallet: Identify transactions spending 0-value outputs, and add tests for anchor outputs in a wallet":
(https://github.com/bitcoin/bitcoin/pull/33268#discussion_r2319820571)
Done as suggested
💬 achow101 commented on pull request "wallet: Identify transactions spending 0-value outputs, and add tests for anchor outputs in a wallet":
(https://github.com/bitcoin/bitcoin/pull/33268#discussion_r2319820786)
Moved
💬 achow101 commented on pull request "wallet: Identify transactions spending 0-value outputs, and add tests for anchor outputs in a wallet":
(https://github.com/bitcoin/bitcoin/pull/33268#discussion_r2319821210)
Done
💬 achow101 commented on pull request "wallet: Identify transactions spending 0-value outputs, and add tests for anchor outputs in a wallet":
(https://github.com/bitcoin/bitcoin/pull/33268#discussion_r2319821459)
Done
💬 achow101 commented on pull request "wallet: Identify transactions spending 0-value outputs, and add tests for anchor outputs in a wallet":
(https://github.com/bitcoin/bitcoin/pull/33268#discussion_r2319823101)
Done
💬 glozow commented on pull request "Revert compact block cache inefficiencies":
(https://github.com/bitcoin/bitcoin/pull/33253#issuecomment-3250303444)
I suppose it depends on how much of compact block relay latency these changes account for?
💬 romanz commented on pull request "index: store per-block transaction locations for efficient lookups":
(https://github.com/bitcoin/bitcoin/pull/32541#issuecomment-3250323045)
> Are you seeing greater improvements on other setups?

I think that the performance gain will increase when fetching transactions near the end of the block.

For example, when fetching the 5000th transaction of [block #90005](https://mempool.space/block/000000000000000000017bfd05b5fa367a424c4a565a4baf7950d9e8605df8ec) using `ab -k -c 1 -n 100000`[^1], enabled `locationsindex` improves the performance ~19x (2.574ms → 0.136ms).

## `-locationsindex=1`

```
Document Path: /rest/
...
📝 jmoik opened a pull request: "cmake: Inherit WERROR setting for secp256k1 build"
(https://github.com/bitcoin/bitcoin/pull/33297)
This change prevents unexpected CLI output when compiling with -DWERROR=ON (treat warnings as errors). This setting is now properly propagated to the secp256k1 subproject build. Previously, secp256k1 would be built without the -Werror flag even when the main project was configured to treat warnings as errors, creating inconsistent build behavior.
https://github.com/bitcoin/bitcoin/issues/33284
💬 sipa commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2319855566)
Yeah, the import is needed at the top level, I believe.
💬 sipa commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2319857824)
Thanks, included.
💬 sipa commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2319855049)
Done.