Bitcoin Core Github
44 subscribers
121K links
Download Telegram
πŸ’¬ l0rinc commented on pull request "validation: periodically flush dbcache during reindex-chainstate":
(https://github.com/bitcoin/bitcoin/pull/32414#issuecomment-2872303093)
> I'll retry it with 100 minutes to see if it would fix the situation.

With 100 mins it's **7%** slower instead of **11%** - still quite bad, but we shouldn't have to choose between performance and persistence, so let's find ways to reduce that. And IBD is the usecase that matters most anyway.

<details>
<summary>Details</summary>

```bash
COMMITS="59d3e4ed34eb55cb40928d524cb0bd5e183ed85a cf75a7b599fe459935b7aa4776e91526fcfb0fcc"; \
STOP_HEIGHT=888888; DBCACHE=45000; \
CC=gcc; CXX=g+
...
πŸ‘ theStack approved a pull request: "cmake: Allow `WITH_DBUS` on all Unix-like systems"
(https://github.com/bitcoin/bitcoin/pull/32469#pullrequestreview-2833096553)
tACK 5b7ed460c7c181f1fd34a927a09aed36284083cb

Tested with OpenBSD 7.7, xfce4.20.0 and the [dbus-1.16.2p0v0](https://openports.pl/path/x11/dbus) package installed that configuring the build with `-DBUILD_GUI=ON` enables "DBus (GUI)" and that DBus is indeed used for notifications (added a [debug message at the proper place](https://github.com/bitcoin/bitcoin/blob/59e09e0fb7b4cf8f8db97c2f81a8f6f69fe6cacf/src/qt/notificator.cpp#L225) just to be sure).
πŸ’¬ vasild commented on pull request "rpc: Undeprecate rpcuser/rpcpassword, store all credentials hashed in memory":
(https://github.com/bitcoin/bitcoin/pull/32423#discussion_r2084600466)
Thanks! Another option, if this gets frowned upon, with less changes, is to leave `GenerateAuthCookie()` alone and have two separate variables in `InitRPCAuthentication()`. In the `else` branch set them like `user = gArgs.GetArg("-rpcuser", "");` and `pass = gArgs.GetArg("-rpcpassword", "");` and in the `if` branch where `GenerateAuthCookie()` is called - use a temporary variable `user_colon_pass` and parse it immediately into the two `user` and `pass` variables.

Anyway, I think the current o
...
πŸ‘ vasild approved a pull request: "rpc: Undeprecate rpcuser/rpcpassword, store all credentials hashed in memory"
(https://github.com/bitcoin/bitcoin/pull/32423#pullrequestreview-2833098998)
ACK e49a7274a2141dcb9e188bc4b45c2d7b928ccecd
βœ… fanquake closed an issue: "WITH_DBUS shouldn't be limited to Linux"
(https://github.com/bitcoin/bitcoin/issues/32464)
πŸš€ fanquake merged a pull request: "cmake: Allow `WITH_DBUS` on all Unix-like systems"
(https://github.com/bitcoin/bitcoin/pull/32469)
πŸ’¬ instagibbs commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#issuecomment-2872570256)
@k98kurz It's not generally true, that buffer is limited to 100 txns total, with no ratelimiting whatsoever. Please take the time to do some background reading: https://delvingbitcoin.org/t/stats-on-compact-block-reconstructions/1052
πŸ“ Eunovo opened a pull request: "Fix listdescriptors true fails with 'Can't get descriptor string' in non-watch-only descriptor wallet"
(https://github.com/bitcoin/bitcoin/pull/32471)
This PR updates the `listdescriptors true` RPC to show public keys instead of private keys when the private keys aren't available.

In non-watch-only wallets, it's possible to import descriptors as long as at least one private key is included. It's important that users can still view these descriptors when they need to create a backupβ€”even if some private keys are missing ([#32078 (comment)](https://github.com/bitcoin/bitcoin/issues/32078#issuecomment-2781428475)). This change makes it possibl
...
πŸ’¬ fanquake commented on pull request "cmake: Allow `WITH_DBUS` on all Unix-like systems":
(https://github.com/bitcoin/bitcoin/pull/32469#issuecomment-2872589894)
Backported to `29.x` in #32292.
πŸ’¬ Sjors commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#issuecomment-2872602791)
re-ACK c164064c266c518588d9d9175f9b14140ee751b6

Since my last review it adds `MAX_SCRIPT_ELEMENT_SIZE` to the test and uses a larger payload for it.
πŸ’¬ fanquake commented on pull request "Fix listdescriptors true fails with 'Can't get descriptor string' in non-watch-only descriptor wallet":
(https://github.com/bitcoin/bitcoin/pull/32471#issuecomment-2872611218)
```bash
/Users/runner/work/bitcoin/bitcoin/src/test/fuzz/miniscript.cpp:1040:42: error: no matching member function for call to 'ToString'
std::optional<std::string> str{node->ToString(parser_ctx)};
~~~~~~^~~~~~~~
/Users/runner/work/bitcoin/bitcoin/src/script/miniscript.h:830:17: note: candidate function template not viable: requires 2 arguments, but 1 was provided
std::string ToString(const CTx& ctx, bool& has_priv_key) const {
^
...
πŸ‘ theStack approved a pull request: "test: test MAX_SCRIPT_SIZE for block validity"
(https://github.com/bitcoin/bitcoin/pull/32304#pullrequestreview-2833275888)
ACK b1ea542ae651cec433910d8c727abc41f17a7678

nit: if you have to retouch, could add a log message (`self.log.info`)
πŸ‘ vasild approved a pull request: "common: Close non-std fds before exec in RunCommandJSON"
(https://github.com/bitcoin/bitcoin/pull/32343#pullrequestreview-2833238669)
ACK a0eed55398f882d9390e50582b10272d18f2b836
πŸ’¬ vasild commented on pull request "common: Close non-std fds before exec in RunCommandJSON":
(https://github.com/bitcoin/bitcoin/pull/32343#discussion_r2084684177)
Can `close()` be called right away at line `1319` and avoid the `fds_to_close` variable and the second `for` loop?
πŸ’¬ vasild commented on pull request "common: Close non-std fds before exec in RunCommandJSON":
(https://github.com/bitcoin/bitcoin/pull/32343#discussion_r2084710474)
Nothing prevents another thread from opening a new file after all these `close()` calls have completed and before the `execvp()` call below. So I guess this is best effort attempt?

A sure way would be to use `O_CLOEXEC` when opening a file: https://www.man7.org/linux/man-pages/man2/open.2.html.
πŸ“ Eunovo converted_to_draft a pull request: "Fix listdescriptors true fails with 'Can't get descriptor string' in non-watch-only descriptor wallet"
(https://github.com/bitcoin/bitcoin/pull/32471)
This PR updates the `listdescriptors true` RPC to show public keys instead of private keys when the private keys aren't available.

In non-watch-only wallets, it's possible to import descriptors as long as at least one private key is included. It's important that users can still view these descriptors when they need to create a backupβ€”even if some private keys are missing ([#32078 (comment)](https://github.com/bitcoin/bitcoin/issues/32078#issuecomment-2781428475)). This change makes it possibl
...
πŸ’¬ Eunovo commented on pull request "Fix listdescriptors true fails with 'Can't get descriptor string' in non-watch-only descriptor wallet":
(https://github.com/bitcoin/bitcoin/pull/32471#issuecomment-2872665572)
Putting draft while I fix some issues with the fuzz tests
πŸ’¬ 1ma commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#issuecomment-2872668421)
Concept NACK, for the same reasons stated in #32359 and #28130
πŸ‘ ajtowns approved a pull request: "policy: uncap datacarrier by default"
(https://github.com/bitcoin/bitcoin/pull/32406#pullrequestreview-2832143503)
ACK c164064c266c518588d9d9175f9b14140ee751b6

I believe this PR makes the following changes:

* allows multiple data carrier outputs in a single transaction (removing `multi-op-return` standardness failures)
* puts multiple data carrier outputs under the same limit (giving a new `datacarrier` standardness failure when the limit is exceeded instead of `scriptpubkey`)
* increases the limit's default from 83 to 100,000, making the limit irrelevant
* deprecates the two datacarrier setting
...
πŸ’¬ ajtowns commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#discussion_r2084189058)
I'd probably have phrased this more like:

> ### Updated Settings
>
> - `-datacarriersize` is increased to 100,000 which effectively uncaps the limit (as the maximum transaction size limit will be hit first). It can be overridden with `-datacarriersize=83` to revert to the limit enforced in previous versions. Both `-datacarrier` and `-datacarriersize` options have been marked as deprecated and are expected to be removed in a future release. (#32406)
> - Multiple data carrier (`OP_RETURN`) o
...