💬 maflcko commented on pull request "wallet, rpc: deprecate settxfee":
(https://github.com/bitcoin/bitcoin/pull/31278#issuecomment-2639357946)
> > * I expect some users not using Bitcoin Core's fee estimation, but an external one. This means that the fee estimates will need to be fed to Bitcoin Core. One way to do it is via the global RPC, another one is via the per-transaction RPC(s).
>
> If a user relies on an external fee estimation service, setting a static wallet-wide fee rate makes little sense. Fees are dynamic

I understand that fees are dynamic. But it seems reasonable to (let's say) call `settxfee` every minute to update
...

💬 maflcko commented on pull request "rpc, logging: return "verificationprogress" of 1 when up to date":
(https://github.com/bitcoin/bitcoin/pull/31177#issuecomment-2639383254)
On a second thought, the GUI uses the derivative (progress increase per hour), so whenever progress decreases, the GUI will display a negative increase. Then, likely the same people that complain about the progress not being 1 will complain about the increase being negative (or zero).

I guess this scenario can already happen post-minchainwork-ibd, but it will probably happen more often when progress is just based on available headers, given that it is faster to download headers than to verify
...

📝 hebasto opened a pull request: "Prepare "Open Transifex translations for v29.0" release step"
(https://github.com/bitcoin/bitcoin/pull/31809)
This PR follows our [Release Process](https://github.com/bitcoin/bitcoin/blob/864386a7444fb5cf16613956ce8bf335f51b67d5/doc/release-process.md).

It is required to open Transifex translations for v29.0, as scheduled in https://github.com/bitcoin/bitcoin/issues/31029.

The previous similar PR: https://github.com/bitcoin/bitcoin/pull/30548.

**Notes for reviewers:**

1. This is the first release process conducted after migrating the build system to CMake. This revealed a bug, which is fixed
...

💬 rkrux commented on pull request "Implement BIP 370 PSBTv2":
(https://github.com/bitcoin/bitcoin/pull/21283#issuecomment-2639461217)
Definite Concept ACK

This one is a biggie, I should get around to reviewing this PR sometime later this month and will leave a review then.

💬 hebasto commented on pull request "Prepare "Open Transifex translations for v29.0" release step":
(https://github.com/bitcoin/bitcoin/pull/31809#issuecomment-2639472097)
cc @stickies-v @pablomartin4btc @johnny9 @jarolrod as regular reviewers of similar previous PRs.

cc @l0rinc as the author of https://github.com/bitcoin/bitcoin/pull/31731.

cc @glozow as the 29.0 release manager.

💬 hebasto commented on pull request "doc: update translation generation cmake example":
(https://github.com/bitcoin/bitcoin/pull/31731#discussion_r1944521310)
You might consider referencing https://github.com/bitcoin/bitcoin/pull/31809 for the actual process of regenerating a translation source file.

Another important point to amend in this doc is that the `translate` target generates the `src/qt/locale/bitcoin_en.xlf` file, which is the translation source for the Transifex:https://github.com/bitcoin/bitcoin/blob/d6c229d8bd4a6203a7255c140aa35c59fb20378b/.tx/config#L6

💬 polespinasa commented on pull request "rpc, logging: return "verificationprogress" of 1 when up to date":
(https://github.com/bitcoin/bitcoin/pull/31177#issuecomment-2639568292)
I'm not sure I understand your proposal correctly. You're proposing this offset on the original function version or the "new" one?

Would it get to 1.0 if we set a constant offset on all blocks?

I think this idea is kinda similar to this one: https://github.com/bitcoin/bitcoin/pull/31177#discussion_r1943113886 but in this case I was thinking of applying it when the block time is far behind the local time.

💬 fanquake commented on issue "test: 32-bit Clang `ipc_test` failure at `-O0`":
(https://github.com/bitcoin/bitcoin/issues/31772#issuecomment-2639590386)
> Other than that, printing a warning when mixing two compilers when compiling with depends may be useful?

Why only depends though? If the issue is generally mixing compilers/flags, then using Clang + system libs on any (gcc-based) Linux system (potentially) has the same problem. It's currently the case that we use Clang + (GCC built) system libs in at least the ASAN,fuzz,valgrind CIs, and Clang + GCC depends libs in the 32-bit job.

💬 jsarenik commented on issue "Fee Estimation via Fee rate Forecasters tracking issue":
(https://github.com/bitcoin/bitcoin/issues/30392#issuecomment-2639597574)
Just few related notes, a compilation from what I have seen so far. HTH, may be biased and YMMV:

Yes, getting a transaction into a block (confirmed) is a future event so on all my nodes I do `ln -nsf /dev/null ~/.bitcoin/fee_estimates.dat` and never call the RPCs guessing "smart fee estimation" from historical values.

Setting a rational initial fee allows for simple (if not m-of-n multisig where `n>=m>1`) RBF replacements to increment the fee just by subtracting the `vsize` sats from one (or m
...

💬 hebasto commented on pull request "kernel: Avoid duplicating symbols in the kernel and downstream users":
(https://github.com/bitcoin/bitcoin/pull/31807#issuecomment-2639600305)
> tl;dr: Certain symbols when defined in headers may end up existing in both the shared lib and the application using it, leading to subtle bugs. More info can be found [here](https://www.mail-archive.com/cfe-commits@lists.llvm.org/msg503669.html).
>
> The solution is generally to avoid defining static/inline vars in headers if they aren't meant to be shared with downstream kernel users.

I don't think these concerns are relevant to shared kernel library users, as https://github.com/llvm/ll
...

💬 maflcko commented on pull request "rpc, logging: return "verificationprogress" of 1 when up to date":
(https://github.com/bitcoin/bitcoin/pull/31177#issuecomment-2639659890)
Just a simple one-line patch to offset `GetBlockTime` with a constant number of hours on top of the code in current master. I don't think there needs to be any special casing?

💬 hebasto commented on pull request "kernel: Avoid duplicating symbols in the kernel and downstream users":
(https://github.com/bitcoin/bitcoin/pull/31807#issuecomment-2639768171)
> But still, with a c++17-style `inline` variable in a header, it's not clear to me that all linkers are guaranteed to do the right thing in all cases (I admit I might be totally wrong on that), and I would at least like some way to be warned about these so that we can investigate.

I do believe that this case is covered by the [ODR](https://en.cppreference.com/w/cpp/language/definition):
> There can be more than one definition in a program of each of the following: ... inline variable ...
>
...

💬 Sjors commented on pull request "Double check all block rules in `ConnectBlock`, not only `CheckBlock`":
(https://github.com/bitcoin/bitcoin/pull/31792#discussion_r1944480843)
ee05eb440310a1b9d6def8a26442433dffd1a122: this seems nonsensical to me, but it's pre-existing behavior. If you run `-reindex(-chainstate)` on a node with a broken clock, it seems perfectly fine to me that it's going to stop validation with an error. The user can then just fix their clock and continue.

Additionally, what if the users clock was too far in the future and they accidentally accepted a block they shouldn't have. It seems fine that during a reindex, after the user fixed their clock,
...

💬 Sjors commented on pull request "Double check all block rules in `ConnectBlock`, not only `CheckBlock`":
(https://github.com/bitcoin/bitcoin/pull/31792#discussion_r1944473646)
ee05eb440310a1b9d6def8a26442433dffd1a122: dropping the "Fully solving" part of this remark seems too optimistic. When you upgrade a node after a soft-fork activates, we don't roll back and recheck.

The only thing this commit fixes are `-reindex(-chainstate)` and (manually called) `verifychain` scenarios.

💬 Sjors commented on pull request "multiprocess: Add libmultiprocess git subtree":
(https://github.com/bitcoin/bitcoin/pull/31741#issuecomment-2639794378)
@ryanofsky if you need to retouch can you also rebase? That makes it easier for my followup PRs since `00_setup_env_i686_multiprocess.sh` changed in master.

💬 Sjors commented on pull request "Double check all block rules in `ConnectBlock`, not only `CheckBlock`":
(https://github.com/bitcoin/bitcoin/pull/31792#discussion_r1944710838)
cc @sdaftuar who wrote this comment in #11737

💬 vasild commented on pull request "Split CConnman":
(https://github.com/bitcoin/bitcoin/pull/30988#discussion_r1944720234)
Changed. This is now `SockMan::Id` and in `net.h`: `using NodeId = SockMan::Id;`. In SockMan comments change the word "node" to "connection". This caused a lot of mechanical renames within this PR, but it looks better now, thanks!

`SockMan::GetNewNodeId()` renamed to
`SockMan::GetNewId()`

`SockMan::EventIOLoopCompletedForNode()` renamed to
`SockMan::EventIOLoopCompletedForOne()`

`SockMan::EventIOLoopCompletedForAllPeers()` renamed to
`SockMan::EventIOLoopCompletedForAll()`

`SockMa
...

💬 Sjors commented on pull request "Double check all block rules in `ConnectBlock`, not only `CheckBlock`":
(https://github.com/bitcoin/bitcoin/pull/31792#issuecomment-2639841215)
Some historical context. The 2 hour future check used to be in `CheckBlockHeader`, but was moved in #8068 (Compact Blocks) in order to "prevent yet more includes and the crazy-looking GetAdjustedTime call from within the GetBlock stuff.": https://github.com/bitcoin/bitcoin/pull/8068/files#r66835690

> In order to avoid checking the merkle tree twice (which is incredibly expensive), I had to call CheckBlock, which required three #includes (including main.h) to blockencodings.cpp, despite them b
...

💬 ryanofsky commented on issue "test: 32-bit Clang `ipc_test` failure at `-O0`":
(https://github.com/bitcoin/bitcoin/issues/31772#issuecomment-2639854388)
> Why only depends though? If the issue is generally mixing compilers/flags, then using Clang + system libs on any (gcc-based) Linux system (potentially) has the same problem.

Yes this issue is not specific to depends, and could theoretically could happen if, for example an ubuntu library package was compiled with gcc and exposed a function with an empty struct parameter, and you tried to use the library with clang.

The point of having an ABI is to prevent issues like this and allow different
...

💬 arejula27 commented on pull request "test: expect that files may disappear from /proc/PID/fd/":
(https://github.com/bitcoin/bitcoin/pull/31614#issuecomment-2639920298)
ACK [`b2e9fdc`](https://github.com/bitcoin/bitcoin/pull/31614/commits/b2e9fdc00f5c40c241a37739f7b73b74c2181e39)

The issue is that the process of getting the list of files in the directory and then reading each one is not atomic. This can lead to race conditions. The proposed solution doesn’t fully fix the problem but avoids its consequences, preventing the test from failing when it shouldn’t.

Following @theuni’s idea, I tried exploring other solutions using `scandir`:
```pyhton
for item
...