Bitcoin Core Github
44 subscribers
121K links
Download Telegram
📝 juanmigdr opened a pull request: "Add rpcestimateconservativefees"
(https://github.com/bitcoin/bitcoin/pull/32329)
### Summary

This PR introduces a new configuration option: `-rpcestimateconservativefees`. When enabled, it forces the default behavior of the `estimatesmartfee` RPC to use **conservative mode** fee estimation.

### Motivation

Bitcoin Core v28.0 changed the default fee estimation mode for `estimatesmartfee` from **conservative** to **economical** ([[#30275](https://github.com/bitcoin/bitcoin/pull/30275)](https://github.com/bitcoin/bitcoin/pull/30275)). While this change reduces fee over-
...
💬 rkrux commented on pull request "test: fix pushdata scripts":
(https://github.com/bitcoin/bitcoin/pull/32270#discussion_r2055789608)
This is fine but seeing `pushdata1` in `tutorial/nonminpushdata1` later in the spender comment seems unnecessary then.
💬 rkrux commented on pull request "test: fix pushdata scripts":
(https://github.com/bitcoin/bitcoin/pull/32270#discussion_r2055856082)
I don't have a firm grasp on script execution yet and there are few things going on inside the "test_spenders" as well that I have not gone through in its entirety. Are following the ways these 3 tapscript scenarios executed?

1. During script execution, the empty string witness data is dropped because of "OP_DROP" and the 2 byte data is left on the stack. I'm assuming this 2 byte data counts as 1 stack item (because the stack is a vector of vector) and that's why script execution doesn't lead
...
💬 Sjors commented on pull request "refactor: inline `UndoWriteToDisk` and `WriteBlockToDisk` to reduce serialization calls":
(https://github.com/bitcoin/bitcoin/pull/31490#issuecomment-2824016734)
This PR renamed UndoReadFromDisk to ReadBlockUndo. Hopefully this comment makes this easier to find.
💬 hodlinator commented on pull request "qa: Verify clean shutdown on startup failure":
(https://github.com/bitcoin/bitcoin/pull/30660#discussion_r2055884036)
Another objection to your approach as a general rule:
If one of the lines in the happy path is modified to call a function that raises exceptions under some circumstances, we run the risk of accidentally catching it by related (parent)type through a pre-existing `except`/`catch`-block, and treating it differently than we had if we intentionally designed the code.

Side note: I'm in favor of newer languages like Rust/Zig **not** supporting exceptions.
💬 hodlinator commented on pull request "qa: Verify clean shutdown on startup failure":
(https://github.com/bitcoin/bitcoin/pull/30660#discussion_r2055895552)
The forwarding of `self.options` to the child process is purely to obey the expectations of someone running feature_framework_startup_failures.py or test_runner.py with those arguments on the CLI/CI.
💬 l0rinc commented on pull request "refactor: inline `UndoWriteToDisk` and `WriteBlockToDisk` to reduce serialization calls":
(https://github.com/bitcoin/bitcoin/pull/31490#issuecomment-2824055131)
yes, it was a leftover from the renames, suggested in https://github.com/bitcoin/bitcoin/pull/31490#pullrequestreview-2509562827

Did the rename cause any problems?
💬 hebasto commented on issue "PIE+LTO causes Bitcoin-Qt to segfault at startup":
(https://github.com/bitcoin-core/gui/issues/867#issuecomment-2824064676)
Is anyone aware of other distros that provide shared Qt libraries built with `-reduce-relocations`?

(asking because I'm not quite familiar with Gentoo)

Fedora, Debian/Ubuntu, and Arch do not have such an issue.
💬 l0rinc commented on pull request "qa: Verify clean shutdown on startup failure":
(https://github.com/bitcoin/bitcoin/pull/30660#discussion_r2055902833)
Not sure I understand, so is it dead code? Would the above exceptions be triggered on CI? In either case I find it confusing.
💬 ismaelsadeeq commented on pull request "Add rpcestimateconservativefees":
(https://github.com/bitcoin/bitcoin/pull/32329#issuecomment-2824083941)


> Bitcoin Core v28.0 changed the default fee estimation mode for `estimatesmartfee` from *conservative* to *economical* ([[#30275](https://github.com/bitcoin/bitcoin/pull/30275)](https://github.com/bitcoin/bitcoin/pull/30275)). While this change reduces fee overestimation for most users, it may not be ideal in all environments.

You can easily opt in to the `conservative` mode by passing a parameter to `estimatesmartfee`.

> In my case, I maintain a long-running node (since v24.0) with m
...
💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2055915118)
> constructing a malleated transaction wasn't as straightforward as I thought

There is `malleate_tx()` Python function in the tests. It is used by the newly added `p2p_private_broadcast.py`. It creates an invalid witness. Did you create a valid one? If yes, then maybe you want to contribute to that function, to make it possible to create valid or invalid.
💬 hodlinator commented on pull request "qa: Verify clean shutdown on startup failure":
(https://github.com/bitcoin/bitcoin/pull/30660#discussion_r2055920502)
It is sometimes useful to debug tests by looking at what is left behind on the filesystem. In those cases one could invoke the parent with "--nocleanup". Not forwarding it to child processes means they would not leave files behind.
💬 hebasto commented on issue "PIE+LTO causes Bitcoin-Qt to segfault at startup":
(https://github.com/bitcoin-core/gui/issues/867#issuecomment-2824097531)
Is anyone aware of other distros that provide shared Qt libraries built with -reduce-relocations?

(asking because I'm not quite familiar with Gentoo)

Fedora, Debian/Ubuntu, and Arch do not have such an issue.
💬 thesamesam commented on issue "PIE+LTO causes Bitcoin-Qt to segfault at startup":
(https://github.com/bitcoin-core/gui/issues/867#issuecomment-2824099055)
Fedora, Debian/Ubuntu, opensuse all disable it. Arch disables it but only for Qt 5 AFAICT.
💬 l0rinc commented on pull request "qa: Verify clean shutdown on startup failure":
(https://github.com/bitcoin/bitcoin/pull/30660#discussion_r2055927034)
Sure, but why isn't it used currently? The dead code part is what bothers me
💬 hebasto commented on issue "PIE+LTO causes Bitcoin-Qt to segfault at startup":
(https://github.com/bitcoin-core/gui/issues/867#issuecomment-2824107785)
> Arch disables it but only for Qt 5 AFAICT (has it enabled for Qt 6).

Building on Arch with `-flto` does not expose any runtime issues (obviously, using Qt 6).
💬 Sjors commented on pull request "Silent payment index (for light wallets and consistency check)":
(https://github.com/bitcoin/bitcoin/pull/28241#issuecomment-2824109607)
Opened a PR on my own fork for this: https://github.com/Sjors/bitcoin/pull/86

It's a new branch based on this one, rebased on the latest #28122, and fixes conflicts with #28358, #31483, #31490 and the introduction of CMake.
💬 Sjors commented on pull request "Silent payment index (for light wallets and consistency check)":
(https://github.com/bitcoin/bitcoin/pull/28241#discussion_r2055934963)
I'll fix this in https://github.com/Sjors/bitcoin/pull/86
💬 maflcko commented on issue "ci: failure in interface_usdt_coinselection.py":
(https://github.com/bitcoin/bitcoin/issues/32322#issuecomment-2824123571)
> Seems like a one-off event for now and not much that can be done about it?

No, this happens consistently with the new kernel.

I suspect anyone can reproduce this with a new kernel locally as well.

My suggestion would be to fix this or work around it in some way temporarily, until the kernel takes the patch (one was submitted iirc).

If it isn't possible to fix or work around, the test should be disabled temporarily:

```diff
ci: Temporarily disable failing bpf checks

diff --git a/.gith
...