Bitcoin Core Github
44 subscribers
120K links
Download Telegram
💬 ryanofsky commented on pull request "index: improve initialization and pruning violation check":
(https://github.com/bitcoin/bitcoin/pull/27607#issuecomment-1553414023)
Thanks! So the problem is that `m_synced` is set to true before `CustomInit` code runs, instead of after. It looks like this bug was introduced in bef4e405f3de2718dfee279a9abff4daf016da26 from #25494
💬 ryanofsky commented on pull request "indexes: Stop using node internal types":
(https://github.com/bitcoin/bitcoin/pull/25494#discussion_r1198123860)
In commit "indexes, refactor: Remove CBlockIndex* uses in index Init methods" (bef4e405f3de2718dfee279a9abff4daf016da26)

Calling the `CustomInit()` function after the `Init()` function here seems to cause a race condition, noticed by @furszy and described in https://github.com/bitcoin/bitcoin/pull/27607#issuecomment-1553400917. The problem is that `Init()` can set `m_synced` to true, which enables `BlockConnected` callbacks. But if `BlockConnected` callback runs before `CustomInit` finishes t
...
💬 TheCharlatan commented on pull request "kernel: Remove interface_ui, util/system from kernel library":
(https://github.com/bitcoin/bitcoin/pull/27636#issuecomment-1553426376)
Rebased 5a01a63ac8267debf152c8757e24f300e18ae379 -> d96c82a76775b1a41c098e6af60130fbdbba9975 ([chainstateRmKernelUiInterface_9](https://github.com/TheCharlatan/bitcoin/commits/chainstateRmKernelUiInterface_9) -> [chainstateRmKernelUiInterface_10](https://github.com/TheCharlatan/bitcoin/commits/chainstateRmKernelUiInterface_10), [compare](https://github.com/TheCharlatan/bitcoin/compare/chainstateRmKernelUiInterface_9..chainstateRmKernelUiInterface_10)).
* Squashed the last commit
💬 furszy commented on pull request "index: improve initialization and pruning violation check":
(https://github.com/bitcoin/bitcoin/pull/27607#issuecomment-1553427942)
> Thanks! So the problem is that `m_synced` is set to true before `CustomInit` code runs, instead of after. It looks like this bug was introduced in [bef4e40](https://github.com/bitcoin/bitcoin/commit/bef4e405f3de2718dfee279a9abff4daf016da26) from #25494

Yeah!, that is more or less what I wrote in the description "Fixed a small race, where we set the index `m_synced` flag (which enables `BlockConnected` events) before calling to the child class init function".

Happy to write it differently
...
💬 pinheadmz commented on pull request "Fee estimation: avoid serving stale fee estimate":
(https://github.com/bitcoin/bitcoin/pull/27622#discussion_r1198122182)
could also simplify a bit:

```cpp
if (est_file.IsNull() || !Write(est_file))
LogPrintf("Failed to write fee estimates to %s. Continue anyway.\n", fs::PathToString(m_estimation_filepath));
else
LogPrintf("Flushed fee estimates to fee_estimates.dat.\n");
```
🤔 pinheadmz reviewed a pull request: "Fee estimation: avoid serving stale fee estimate"
(https://github.com/bitcoin/bitcoin/pull/27622#pullrequestreview-1433103480)
concept ACK

This should solve the stale fee issue, the test is clever and effective. I ensured the test fails without the patch and also checked the test failed when the mtime was set < 12 hours. Reviewed code and left some notes / questions
💬 pinheadmz commented on pull request "Fee estimation: avoid serving stale fee estimate":
(https://github.com/bitcoin/bitcoin/pull/27622#discussion_r1198123417)
I don't actually know for sure but couldn't this just be ...?

```cpp
static constexpr std::chrono::hours FEE_FLUSH_INTERVAL{1};
```
💬 pinheadmz commented on pull request "Fee estimation: avoid serving stale fee estimate":
(https://github.com/bitcoin/bitcoin/pull/27622#discussion_r1198071636)
Mine as well use this constant here for the filename?

https://github.com/bitcoin/bitcoin/blob/6cc136bbd36f859a16e469bb5c016d06c19bcd50/src/policy/fees_args.cpp#L10
💬 pinheadmz commented on pull request "Fee estimation: avoid serving stale fee estimate":
(https://github.com/bitcoin/bitcoin/pull/27622#discussion_r1198131760)
I think you can use `self.restart_node(0)` here
💬 ryanofsky commented on pull request "index: improve initialization and pruning violation check":
(https://github.com/bitcoin/bitcoin/pull/27607#issuecomment-1553447313)
> Happy to write it differently if it is not good enough. Maybe should change "child class init function" for "CustomInit"?. (in the commit description it's explained with more details anyway)

Yes if CustomInit was mentioned I probably would have understood better. I would say this PR fixes a race condition where an index's `CustomAppend` method might be called before its `CustomInit` method, causing the index to try to update itself before it is initialized. And that the PR fixes the proble
...
💬 hebasto commented on pull request "Debug Console implementation of generate method":
(https://github.com/bitcoin-core/gui/pull/692#issuecomment-1553447916)
The `help generate` command returns:
```
generate

has been replaced by the -generate cli option. Refer to -help for more information.
```
💬 pinheadmz commented on pull request "Add wallet method to detect if a key is "active"":
(https://github.com/bitcoin/bitcoin/pull/27216#issuecomment-1553450191)
> Needs rebase

done thanks
💬 MarcoFalke commented on pull request "Fee estimation: avoid serving stale fee estimate":
(https://github.com/bitcoin/bitcoin/pull/27622#discussion_r1198150716)
It could also be `constexpr auto FEE_FLUSH_INTERVAL{1h};`
⚠️ Omermohemmed74 opened an issue: "DSE"
(https://github.com/bitcoin/bitcoin/issues/27701)
pinheadmz closed an issue: "DSE"
(https://github.com/bitcoin/bitcoin/issues/27701)
💬 furszy commented on pull request "index: improve initialization and pruning violation check":
(https://github.com/bitcoin/bitcoin/pull/27607#issuecomment-1553480215)
sounds good. Description updated, thanks.
hebasto closed an issue: "Watch-only, balance not "available" for psbt"
(https://github.com/bitcoin-core/gui/issues/83)
💬 hebasto commented on issue "guiutil.cpp: formatNiceTimeOffset":
(https://github.com/bitcoin-core/gui/issues/728#issuecomment-1553504397)
Closing for now.

> Please post a screen shot - for clarity.

Can be reopened when more details (a screenshot) will be provided.

> that tool tip can be improved

It is still not clear what is wrong with the current implementation.
hebasto closed an issue: "guiutil.cpp: formatNiceTimeOffset"
(https://github.com/bitcoin-core/gui/issues/728)
💬 hebasto commented on pull request "Make network graph slider easier to use":
(https://github.com/bitcoin-core/gui/pull/483#issuecomment-1553508354)
> There hasn't been much activity lately. What is the status here?

Closing due to lack of interest.