🤔 Safay99 reviewed a pull request: "test: Drop `x` modifier in `fsbridge::fopen` call for MinGW builds"
(https://github.com/bitcoin/bitcoin/pull/29357#pullrequestreview-2259263018)
1. بیبی

💬 Safay99 commented on pull request "test: Drop `x` modifier in `fsbridge::fopen` call for MinGW builds":
(https://github.com/bitcoin/bitcoin/pull/29357#issuecomment-2308840524)
[
/](url)

📝 tdb3 opened a pull request: "rpc: add revelant_blocks to scanblocks status"
(https://github.com/bitcoin/bitcoin/pull/30713)
Currently, after starting a scan with `scanblocks` the user must wait until the scan is complete to see the associated/relevant block hashes.
This updates `scanblocks status` results to provide the `relevant_blocks` found so far.
This enables earlier subsequent lookup through matching blocks (e.g. to run `getdescriptoractivity` in PR #30708)

Leaving this one as a (rough) draft for now. If enough concept ACK/NACK is received, then can look into adding tests (into `rpc_scanblocks.py`. `-dep
...

💬 tdb3 commented on pull request "rpc: add getdescriptoractivity":
(https://github.com/bitcoin/bitcoin/pull/30708#issuecomment-2308842097)
> One possibility (that is probably more valuable) is to add an incremental `relevant_blocks` key to `scanblocks status` output, so that the client can begin calling `getdescriptoractivity` progressively as results roll in but before the entire scan is finished.

Great idea. Created an initial draft PR #30713

💬 Albroheem commented on issue "Release Schedule for 28.0":
(https://github.com/bitcoin/bitcoin/issues/29891#issuecomment-2308888456)
tar xf bitcoin-osx-unsigned.tar.gz
./detached-sig-create.sh /path/to/codesign.p12
Enter the keychain password and authorize the signature
signature-osx.tar.gz will be created

💬 hebasto commented on pull request "fuzz: Add missing fuzz targets to cmake build":
(https://github.com/bitcoin/bitcoin/pull/30712#issuecomment-2308897782)
Looks correct.

💬 TheCharlatan commented on pull request "init: fixes file descriptor accounting":
(https://github.com/bitcoin/bitcoin/pull/30065#discussion_r1730004982)
Nit (clang-format): Space after closing parenthesis.

🤔 TheCharlatan reviewed a pull request: "init: fixes file descriptor accounting"
(https://github.com/bitcoin/bitcoin/pull/30065#pullrequestreview-2258747309)
This looks good, but I think these few, simple things should be addressed.

💬 TheCharlatan commented on pull request "init: fixes file descriptor accounting":
(https://github.com/bitcoin/bitcoin/pull/30065#discussion_r1730402391)
nit: `static constexpr`

💬 TheCharlatan commented on pull request "init: fixes file descriptor accounting":
(https://github.com/bitcoin/bitcoin/pull/30065#discussion_r1730408130)
I think this should be de-globalized and renamed like I've done here: https://github.com/TheCharlatan/bitcoin/commit/a2c048322c67436e02a3a9607400f65f10dd6564

The `avaialble_fds` global variable there are also kind of silly, since it is only there to be logged at a later point in time, but I don't feel strongly if it should go or not either way.

💬 TheCharlatan commented on pull request "init: fixes file descriptor accounting":
(https://github.com/bitcoin/bitcoin/pull/30065#discussion_r1730402947)
Why did you not put `MAX_ADDNODE_CONNECTIONS` in here?

💬 pablomartin4btc commented on pull request "Update about logo icon (colour) to denote the chain type of the QT instance in About/ Help Message Window/ Dialog":
(https://github.com/bitcoin-core/gui/pull/762#issuecomment-2308962681)
Updates:
- Addressed @hebasto's feedback.
- Fixed the [error message window inconsistency](https://github.com/bitcoin-core/gui/pull/762#issuecomment-2276259183) detected by @hebasto [above](https://github.com/bitcoin-core/gui/pull/762#pullrequestreview-2219281894).

<details>
<summary>Now running <code>qt</code> from the command line with <code>bitcoin-qt.exe -signet -about</code> (or similar command with an invalid argument) will show the correct chain type icon at the top left of the tit
...

📝 theStack opened a pull request: "test: fix `TestShell` initialization (late follow-up for #30463)"
(https://github.com/bitcoin/bitcoin/pull/30714)
Creating a `TestShell` instance as stated in the [docs](https://github.com/bitcoin/bitcoin/blob/master/test/functional/test-shell.md) currently fails on master:
```
$ python3
Python 3.10.13 (main, Mar 15 2024, 07:36:23) [Clang 16.0.6 ] on openbsd7
Type "help", "copyright", "credits" or "license" for more information.
>>> import sys
>>> sys.path.insert(0, "/home/thestack/bitcoin/test/functional")
>>> from test_framework.test_shell import TestShell
>>> test = TestShell().setup(num_nodes=2,
...

👍 tdb3 approved a pull request: "http: set TCP_NODELAY when creating HTTP server"
(https://github.com/bitcoin/bitcoin/pull/30675#pullrequestreview-2259359677)
ACK 03d49d0f25ab5660524d5ddd171de677a808b984

Nice speedup! Saw a 28x speedup for a full block.


Ran the test on a full block (`ab -k -c 1 -n 100 http://localhost:8332/rest/block/00000000000000000000b2ffb308ce769ad1c1b239c78553c4e3f931a74d5569.bin`).

27.1 release:
```
Requests per second: 5.08 [#/sec] (mean)
Time per request: 196.684 [ms] (mean)
```

master (6d546336e800f7b8990fececab6bc08413f28690):

```
Requests per second: 5.28 [#/sec] (mean)
Time per request:
...

💬 theStack commented on pull request "test: fix `TestShell` initialization (late follow-up for #30463)":
(https://github.com/bitcoin/bitcoin/pull/30714#issuecomment-2308964530)
cc @hebasto, maybe you have a better idea how to solve this; the current approach works but feels a bit hacky :)

💬 fjahr commented on pull request "refactor: Improve assumeutxo state representation":
(https://github.com/bitcoin/bitcoin/pull/30214#issuecomment-2308980712)
Thanks for addressing my feedback @ryanofsky ! I hope we can merge #29553 soon and then I will get back to this with another full review. But it does look very good to me already.

> I'm especially interested if you have ideas on splitting up documentation and code changes into separate PRs.

I'm unsure if it's really needed because I find clean refactoring commits like those here can be reviewed quicker than other changes, so I would be fine to keep it as is just based on volume of the chan
...

🤔 i-am-yuvi reviewed a pull request: "build: Introduce CMake-based build system"
(https://github.com/bitcoin/bitcoin/pull/30454#pullrequestreview-2259369009)
tACK

Tested and Built successfully on:

- MacOS 14.6.1 without GUI and Wallet
- Ubuntu 24.04 without GUI and Wallet

Also ran tests with `ctest`.

💬 jamesob commented on pull request "rpc: add `revelant_blocks` to `scanblocks status`":
(https://github.com/bitcoin/bitcoin/pull/30713#issuecomment-2309028680)
Concept ACK. This'd be useful, especially in conjunction with https://github.com/bitcoin/bitcoin/pull/30708.

💬 furszy commented on pull request "wallet: Write best block to disk before backup":
(https://github.com/bitcoin/bitcoin/pull/30678#discussion_r1730380590)
The idea of moving this to `Flush()` sounded promising to me too but I don't think we can/should do it after some code walking. At least not without doing some extra restructuring.

The shutdown procedure stops the wallet, closing the db connection, before the wallet is destructed (which is when the flushing occurs). So any db write during `Flush` should cause a segfault due to the db handler access.

Also, moving this to `Flush()` would write the same locator twice during shutdown. And this
...

💬 furszy commented on pull request "Wallet, Bugfix: Lock Wallet Context mutex Before Adding/Removing Settings":
(https://github.com/bitcoin/bitcoin/pull/30697#issuecomment-2309041765)
Take this one: https://github.com/furszy/bitcoin-core/commit/b017949d83c1c47e6aea708ac89f0d81cead1064.
Have removed the `OptionModel` changes because those require further changes to the `Node` interface and implementation.