📝 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

💬 laanwj commented on pull request "wallet: Implement independent BDB parser":
(https://github.com/bitcoin/bitcoin/pull/26606#discussion_r1557522115)
Just a note for other reviewers: This comment mentioning "our system" was slightly confusing to me: our serialization framework always serializes in LE, so if we see BTREE_MAGIC_OE it means the file is in big-endian. Which means we always need to swap it, independent of system's endianness.

💬 laanwj commented on pull request "wallet: Implement independent BDB parser":
(https://github.com/bitcoin/bitcoin/pull/26606#discussion_r1557537019)
i don't think importing `uint160` is necessary here--the actual value isnt used, we're just using it to consume some bytes

🚀 glozow merged a pull request: "refactor: Use typesafe Wtxid in compact block encodings"
(https://github.com/bitcoin/bitcoin/pull/29752)

💬 laanwj commented on pull request "wallet: Implement independent BDB parser":
(https://github.com/bitcoin/bitcoin/pull/26606#issuecomment-2045074692)
Reference for reviewers:
- `db_page.h` from berkeleydb 4.8.30: https://gist.github.com/laanwj/2b998d6db8d9df384a30871035675aec

💬 fanquake commented on pull request "build: Change MAC_OSX macro to __APPLE__ in crypto":
(https://github.com/bitcoin/bitcoin/pull/29834#issuecomment-2045089673)
Guix Build (aarch64):
```bash
bdbdd644be801bd72472a94e878076406e63e8bc39c55f5a6e3bc28e23a72c81 guix-build-a71eadf66bed/output/aarch64-linux-gnu/SHA256SUMS.part
cf1eb27df84f25149a743be33f037eac960c1a943da2750b9030315593df4192 guix-build-a71eadf66bed/output/aarch64-linux-gnu/bitcoin-a71eadf66bed-aarch64-linux-gnu-debug.tar.gz
732f794906af544c3c0e41ff3cdc8fc0253fbef96d5572998286ad19ecbc5a9e guix-build-a71eadf66bed/output/aarch64-linux-gnu/bitcoin-a71eadf66bed-aarch64-linux-gnu.tar.gz
a50f89
...

💬 murchandamus commented on pull request "wallet: Implement independent BDB parser":
(https://github.com/bitcoin/bitcoin/pull/26606#discussion_r1557578207)
calculation of two 32 bit integers, you need to use a cast

💬 glozow commented on pull request "AcceptMultipleTransactions: Fix workspace not being set as client_maxfeerate failure":
(https://github.com/bitcoin/bitcoin/pull/29735#discussion_r1557579240)
This should be in the extra args _above_ to avoid writing mempool.dat.

Currently the test is fine, but this is because the new datacarriersize causes all the transactions to be rejected.

💬 sipa commented on pull request "wallet: Implement independent BDB parser":
(https://github.com/bitcoin/bitcoin/pull/26606#discussion_r1557581834)
You probably want `size_t pos = size_t{page_num} * page_size;`.