🚀 achow101 merged a pull request: "RPC: Fix fund transaction crash when at 0-value, 0-fee"
(https://github.com/bitcoin/bitcoin/pull/27271)

💬 martinus commented on pull request "Add pool based memory resource":
(https://github.com/bitcoin/bitcoin/pull/25325#issuecomment-1479975523)
@jamesob interesting that it didn't see a bigger speedup, but I guess it depends on a lot of other factors as well. How fast is your harddisk, and how much RAM does your computer have?

📝 ryanofsky opened a pull request: "init: Error if ignored bitcoin.conf file is found"
(https://github.com/bitcoin/bitcoin/pull/27302)
Show an error on startup if a bitcoin datadir that is being used contains a `bitcoin.conf` file that is ignored. There are two cases where this could happen:

- One case reported in [#27246 (comment)](https://github.com/bitcoin/bitcoin/issues/27246#issuecomment-1470006043) happens when a `bitcoin.conf` file in the default datadir (e.g. `$HOME/.bitcoin/bitcoin.conf`) has a `datadir=/path` line that sets different datadir containing a second `bitcoin.conf` file. Currently the second `bitcoin.con
...

💬 martinus commented on pull request "Add pool based memory resource":
(https://github.com/bitcoin/bitcoin/pull/25325#discussion_r1145197139)
That here would be `BOOST_TEST(block != b)`, because since `b` now has to come from the `::operator new` and not from the freelist

💬 martinus commented on pull request "Add pool based memory resource":
(https://github.com/bitcoin/bitcoin/pull/25325#discussion_r1145200479)
With the above change, if I'm not mistaken I don't think it's safe to check for `BOOST_TEST(block != b)` here too. This too calls `::operator new`, and because the previous memory was deallocated already, it might give out the same block of memory, depending on however malloc is implemented

👍 pinheadmz approved a pull request: "blockstorage: Adjust fastprune limit if block exceeds blockfile size"
(https://github.com/bitcoin/bitcoin/pull/27191)
ACK e930814fdb9261f180d5c436cefe52e9cf38fd67

I also wrote a test for this that freezes (infinite loop) on master but passes on this branch. Take it or leave it, I just wanted to see the mechanism work before acking:

https://github.com/pinheadmz/bitcoin/commit/c576efa6dc7c32981d3b2fb42c9c8573b8c2e293

<details><summary>Show Signature</summary>

```
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

ACK e930814fdb9261f180d5c436cefe52e9cf38fd67
-----BEGIN PGP SIGNATURE-----

iQIzBAEB
...

💬 achow101 commented on pull request "wallet, tests: Expand and test when the blank wallet flag should be un/set":
(https://github.com/bitcoin/bitcoin/pull/25634#issuecomment-1480012074)
It turns out that encrypting a blank legacy wallet also does not unset the blank flag nor does it generate a new seed.

The latest push removes the descriptor wallet changes and adds tests for the behavior when encrypting.

🚀 fanquake merged a pull request: "test: Remove unused Check* default constructors"
(https://github.com/bitcoin/bitcoin/pull/27297)

💬 fanquake commented on pull request "ci: Use TSan new runtime (llvm-16, take 3)":
(https://github.com/bitcoin/bitcoin/pull/27298#issuecomment-1480024555)
Looks like just explicitly installing `libclang-rt-16-dev` will solve this. This branch works for me, for running the TSAN CI job on x86_64: https://github.com/fanquake/bitcoin/tree/27298_plus_libclang:
```bash
ALL | ✓ Passed | 3493 s (accumulated)
Runtime: 991 s

Stop and remove CI container by ID
48aee56b02c33313b240f21c0b9924c68bfa9fcaaa16d0557adcd31b1efb73be

real 32m28.067s
```

Still testing on aarch64.

💬 achow101 commented on pull request "wallet: Implement independent BDB parser":
(https://github.com/bitcoin/bitcoin/pull/26606#issuecomment-1480027322)
> Have you looked into a fuzz target for this parser?

I haven't. If someone would like to write one, I'd be happy to add it.

💬 pinheadmz commented on pull request "Support JSON-RPC 2.0 when requested by client":
(https://github.com/bitcoin/bitcoin/pull/27101#discussion_r1145223127)
Hm yeah good point, a few error codes like this [are specified](https://www.jsonrpc.org/specification#error_object). One problem with this is that our application error `RPC_INVALID_PARAMETER` (`-8`) is coded ~186 times in RPC functions everywhere. I suppose we could re-map those error codes back to the jsonrpc 2.0 spec in `JSONRPCExecOne()`? Only when the request indicates jsonrpc 2.0

💬 pinheadmz commented on pull request "Support JSON-RPC 2.0 when requested by client":
(https://github.com/bitcoin/bitcoin/pull/27101#discussion_r1145229348)
In the tests at least it was originally `version: "1.1"` as seen in [authproxy.py](https://github.com/bitcoin/bitcoin/pull/27101/files#diff-6e2d6b181408a11452bcf83c79b299ba195524165e8a46e746b9d712961cc6fb) which is actually [in the 1.1 spec](https://www.jsonrpc.org/historical/json-rpc-1-1-wd.html#ProcedureCall). I'm not sure if `"jsonrpc": "1.0"` was part of an older spec or if thats a mistake [here](https://github.com/bitcoin/bitcoin/pull/27101/files#diff-fc0f86b6c3bb23b0e983bcf79d7546d1c9eaa15
...

💬 fanquake commented on pull request "ci: Use TSan new runtime (llvm-16, take 3)":
(https://github.com/bitcoin/bitcoin/pull/27298#issuecomment-1480041089)
Ran into a failure (with the above branch, and NO_BDB=1), on aarch64:
```bash
2023-03-22T18:02:11.462544Z (mocktime: 2020-08-31T15:34:12Z) [txindex] [util/thread.cpp:20] [TraceThread] txindex thread start
test/txindex_tests.cpp(38): fatal error: in "txindex_tests/txindex_initial_sync": critical check time_start + timeout_ms > GetTimeMillis() has failed
LLVMSymbolizer: error reading file: No such file or directory
make[3]: Leaving directory '/home/fedora/ci_scratch/ci/scratch/build/bitcoin-a
...

💬 ryanofsky commented on pull request "refactor: Extract util/fs from util/system":
(https://github.com/bitcoin/bitcoin/pull/27254#discussion_r1145230536)
In commit "refactor: Extract util/fs_helpers from util/system" (59c02e68f75f7b727162d2ea36bf3ed99fabf035)

Anybody have advice on easiest way to review changes like this when blocks of code are moving but also changing slightly at the same time?

I find it pretty easy to review blocks of moved code in git with the `--color-moved` option, but once code is moved and reformatted at the same time (the `{` at the end of this line was previously on its own line) it becomes very tedious to find the
...

💬 pinheadmz commented on pull request "Support JSON-RPC 2.0 when requested by client":
(https://github.com/bitcoin/bitcoin/pull/27101#discussion_r1145232671)
aw geez. Well, ok so it looks like `disconnectnode` is the only RPC that throws the (correct?) error code for invalid params, while the rest throw `RPC_INVALID_PARAMETER` (`-8`). Maybe it does make sense to use a scripted diff to replace all the `-8`s with `-32602`. Nice catch!

💬 pinheadmz commented on pull request "Support JSON-RPC 2.0 when requested by client":
(https://github.com/bitcoin/bitcoin/pull/27101#discussion_r1145235420)
Not sure if `RPC_INTERNAL_ERROR` (`-32603`) is totally correct for estimating fee from a blocksonly node. It's more of a bad request ...

💬 martinus commented on pull request "Add pool based memory resource":
(https://github.com/bitcoin/bitcoin/pull/25325#discussion_r1145236261)
After having a closer look, you are right that currently allocating & freeing 0 bytes does not work with the resource. But I think I can't allow allocating 0 bytes with the resource at all, because when allocating multple 0 bytes it would always give out the same pointer, and the standard says that this is not allowed: https://cplusplus.github.io/LWG/issue9

So I think the best solution is to just add a `bytes > 0` condition to `IsFreeListUsable`, so that `::operator new` will be used in that
...

💬 achow101 commented on pull request "bumpfee: allow send coins back to yourself":
(https://github.com/bitcoin/bitcoin/pull/27195#issuecomment-1480050513)
ACK c260d3fbf4cf8be07176840f39125abcb3d59c1b

👍 ryanofsky approved a pull request: "refactor: Extract util/fs from util/system"
(https://github.com/bitcoin/bitcoin/pull/27254)
Code review ACK 3ceb8dde48fc12f4f16372f661a4cccf7104e194

💬 pinheadmz commented on pull request "Support JSON-RPC 2.0 when requested by client":
(https://github.com/bitcoin/bitcoin/pull/27101#discussion_r1145246067)
This error is caused by a failure to parse JSON in `ReadBody()` so theoretically we also can't read the `"jsonrpc": "2.0"` pair either. I agree with you though, the HTTP transport is fine and should be 200, because the error is coming from the json parser. I guess the options are:
1. always send 200 [here](https://github.com/bitcoin/bitcoin/blob/master/src/httprpc.cpp#L77) and kinda break backwards compat
2. attempt to parse the `"jsonrpc"` string and only send 200 if we can detect the "2.0" v
...

💬 pinheadmz commented on pull request "Support JSON-RPC 2.0 when requested by client":
(https://github.com/bitcoin/bitcoin/pull/27101#discussion_r1145250554)
yep thanks, will update