💬 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;`.

💬 davidgumberg commented on pull request "doc: fixup help output for -upnp and -natpmp":
(https://github.com/bitcoin/bitcoin/pull/28874#issuecomment-2045114437)
ACK https://github.com/bitcoin/bitcoin/commit/92f88a962908c49dde99c03a4608e63e4a6eec71

Configured `--with-natpmp` and verified the help messages.

```console
$ bitcoind --help

[...]
-natpmp
Use NAT-PMP to map the listening port (default: 0)
[...]
-upnp
Use UPnP to map the listening port (default: 0)
```

Verified that modifying `bool DEFAULT_UPNP` and `bool DEFAULT_NATPMP` changes the `--help` output.

💬 laanwj commented on pull request "wallet: Implement independent BDB parser":
(https://github.com/bitcoin/bitcoin/pull/26606#discussion_r1557583470)
aside: might be better to not use size_t at all for file offsets, as the type is 32 bit on usual 32 bit platforms

💬 glozow commented on pull request "AcceptMultipleTransactions: Fix workspace not being set as client_maxfeerate failure":
(https://github.com/bitcoin/bitcoin/pull/29735#discussion_r1557583670)
👍 I think this interface is better if the caller knows what the requirements are but can otherwise put whatever args they want

💬 instagibbs commented on pull request "AcceptMultipleTransactions: Fix workspace not being set as client_maxfeerate failure":
(https://github.com/bitcoin/bitcoin/pull/29735#discussion_r1557587565)
moved and added assertion of empty mempool after restart

💬 laanwj commented on pull request "wallet: Implement independent BDB parser":
(https://github.com/bitcoin/bitcoin/pull/26606#discussion_r1557592239)
do we want to sanity check for encryption to be disabled? (didn't know bdb supported db-level encryption in the first place)

👍 fanquake approved a pull request: "build: Change MAC_OSX macro to __APPLE__ in crypto"
(https://github.com/bitcoin/bitcoin/pull/29834#pullrequestreview-1988981666)
ACK a71eadf66bed8d3ea4282c8499f533a8eeed9900