Bitcoin Core Github
44 subscribers
120K links
Download Telegram
🚀 fanquake merged a pull request: "Drop Windows Socket dependency for `randomenv.cpp`"
(https://github.com/bitcoin/bitcoin/pull/29786)
fanquake closed an issue: "ci: Enable bpfcc-tools"
(https://github.com/bitcoin/bitcoin/issues/29804)
🚀 fanquake merged a pull request: "Revert "ci: Temporarily disable bpfcc-tools""
(https://github.com/bitcoin/bitcoin/pull/29832)
🚀 fanquake merged a pull request: "fuzz: Some `test/fuzz/test_runner.py` improvements"
(https://github.com/bitcoin/bitcoin/pull/29821)
💬 maflcko commented on issue "ci: failure in `rpc_scanblocks.py`":
(https://github.com/bitcoin/bitcoin/issues/29831#issuecomment-2044502164)
Is this a race where a validation event is lost?

```
node0 2024-04-08T13:15:11.629578Z [httpworker.0] [validationinterface.cpp:212] [BlockConnected] [validation] Enqueuing BlockConnected: block hash=2194b76c425a2423728356cb8b9c82ad89068bcddfe8dc287c93732fb3354b59 block height=201
...
node0 2024-04-08T13:15:11.629801Z [scheduler] [validationinterface.cpp:212] [operator()] [validation] BlockConnected: block hash=2194b76c425a2423728356cb8b9c82ad89068bcddfe8dc287c93732fb3354b59 block heigh
...
🤔 ajtowns reviewed a pull request: "fees: Remove CLIENT_VERSION serialization"
(https://github.com/bitcoin/bitcoin/pull/29702#pullrequestreview-1988526952)
Concept ACK
💬 ajtowns commented on pull request "fees: Remove CLIENT_VERSION serialization":
(https://github.com/bitcoin/bitcoin/pull/29702#discussion_r1557291850)
`static` ? Move it to the top of the file?

Could consider defining three values:

```
static constexpr int FEE_FILE_MIN_READ_VERSION = 149900;
static constexpr int FEE_FILE_MAX_READ_VERSION = 279900;
static constexpr int FEE_FILE_WRITE_VERSION = 149900;
```
💬 ajtowns commented on pull request "fees: Remove CLIENT_VERSION serialization":
(https://github.com/bitcoin/bitcoin/pull/29702#discussion_r1557296093)
Storing `nVersionThatWrote` seems somewhat useful for manually debugging a corrupt fee data file, and figuring out how the bug occurred. If we're just storing a constant, then the value isn't going to be useful, so it seems better to just write `(int)0` instead -- that's the amount of information it will convey... In any event, "version that wrote the file" is not accurate anymore if we're making it something different from `CLIENT_VERSION`.
📝 paplorinc opened a pull request: "build: Change MAC_OSX macro to __APPLE__ in crypto package"
(https://github.com/bitcoin/bitcoin/pull/29834)
Split out from https://github.com/bitcoin/bitcoin/pull/29450#issuecomment-2044405345 to avoid the uncertainties and simplify review.
💬 paplorinc commented on pull request "build: replace custom `MAC_OSX` macro with standard `__APPLE__` for consistent macOS detection":
(https://github.com/bitcoin/bitcoin/pull/29450#issuecomment-2044548648)
@theuni, I've split it out the crypto package to https://github.com/bitcoin/bitcoin/pull/29834, thanks for checking it!
💬 furszy commented on issue "ci: failure in `rpc_scanblocks.py`":
(https://github.com/bitcoin/bitcoin/issues/29831#issuecomment-2044591551)
Quick check, the index seems to not have processed the block by the time `scanblocks()` (line 48) is called. The `getindexinfo()['synced']` value is set to true after the index initial sync. It does not tell us anything about tip scanning.
So, we can change line 46 check to verify the latest block height instead of the `synced` value. And that should fix the issue:
```diff
diff --git a/test/functional/rpc_scanblocks.py b/test/functional/rpc_scanblocks.py
--- a/test/functional/rpc_scanblocks.
...
💬 maflcko commented on issue "ci: failure in `rpc_scanblocks.py`":
(https://github.com/bitcoin/bitcoin/issues/29831#issuecomment-2044604484)
@furszy Are you sure? The event at block height = 201 does not exist anymore, so I don't think it will be processed at all.

To summarize my excerpts from above:

* Block 201 is mined, the event is put on the scheduler, and the event is consumed
* The background index build is completing for height 200, and the index will start to listen to events?
* The block 201 event is already consumed, so wont make it to the index
* What would happen if a block 202 event was fired at some point?

B
...
💬 theStack commented on pull request "test: p2p: add test for rejected tx request logic (`m_recent_rejects` filter)":
(https://github.com/bitcoin/bitcoin/pull/29827#discussion_r1557383102)
Oh good point, my intention was indeed to hit the TX_RECONSIDERABLE error, wasn't aware that there is another "min relay fee not met" check before (in the future, after #29496 is in, I could simply create a v3 tx, right?). Will change to cause the "mempool min fee not met" error, probably building on #29735.
💬 furszy commented on issue "ci: failure in `rpc_scanblocks.py`":
(https://github.com/bitcoin/bitcoin/issues/29831#issuecomment-2044654765)
> @furszy Are you sure? The event at block height = 201 does not exist anymore, so I don't think it will be processed at all.

Hmm ok, what I said could be another issue.. nvm. The index will probably stop syncing if this happens.. throwing a "WARNING: Block %s does not connect to an ancestor of..". Will do some tests.
👍 theuni approved a pull request: "build: Change MAC_OSX macro to __APPLE__ in crypto package"
(https://github.com/bitcoin/bitcoin/pull/29834#pullrequestreview-1988681854)
ACK a71eadf66bed8d3ea4282c8499f533a8eeed9900

I whipped up a minimal test-case locally to verify that this code at least compiles using the iOS sdk. Not that we support iOS, but I wanted to prove to myself that generically "APPLE" really is what we mean here.

```c++
#include <sys/types.h>
#include <sys/sysctl.h>
int main()
{
int val = 0;
int have_arm_shani;
size_t len = sizeof(val);
if (sysctlbyname("hw.optional.arm.FEAT_SHA256", &val, &len, nullptr, 0) == 0) {

...
💬 laanwj commented on pull request "Remove the legacy wallet and BDB dependency":
(https://github.com/bitcoin/bitcoin/pull/28710#discussion_r1557399432)
OR'ed, i guess?
💬 paplorinc commented on pull request "refactor: Optimize IsSpace function for common non-whitespace characters":
(https://github.com/bitcoin/bitcoin/pull/29602#issuecomment-2044682508)
Thanks for the review @laanwj. The gain is small, but measurable:
> make && ./src/bench/bench_bitcoin --filter=HexParse --min-time=10000


`Before:`
```
| ns/base16 | base16/s | err% | total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
| 0.68 | 1,461,990,359.30 | 0.0% | 10.83 | `HexParse`
| 0.68 | 1,461,442,745.57 | 0.1% | 10.83 | `HexParse`
| 0.68
...
📝 fanquake opened a pull request: "[WIP] libevent @ master + use CMake"
(https://github.com/bitcoin/bitcoin/pull/29835)
The last libevent release was ~4 years ago. This means there are a lot of changes missing from the CMake build system we'd like to switch too and use. This PR pulls libevent @ master and converts it to use CMake in depends. Depending on the outcome, we could possibly pull a (minified) patch set into depends.
👋 murchandamus's pull request is ready for review: "Refactor BnB tests"
(https://github.com/bitcoin/bitcoin/pull/29532)
💬 murchandamus commented on pull request "Refactor BnB tests":
(https://github.com/bitcoin/bitcoin/pull/29532#issuecomment-2044975612)
Pinging @furszy, @achow101, @S3RK, as discussed