Bitcoin Core Github
44 subscribers
121K links
Download Telegram
🤔 furszy reviewed a pull request: "Migrate legacy wallets that are not loaded"
(https://github.com/bitcoin-core/gui/pull/824#pullrequestreview-2236408618)
ACK 0240454120a88915396b0eb68d9b5bee73e52fed
💬 TheCharlatan commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-2286994610)
> I guess another thing I'd like to know is if this is the initial C API, and the implementation is around 3000 lines, and it doesn't handle "transactions, block headers, coins cache, utxo set, meta data, and the mempool", how much bigger do you think it will get if it does cover most of the things you would like it to cover? Like is this 20%, 30%, or 50% of the expected size?

I think a fair comparison would be comparing the amount of code "glue" required, e.g. the size of the `bitcoinkernel.
...
📝 justinvforvendetta opened a pull request: "remove repeated word in note"
(https://github.com/bitcoin/bitcoin/pull/30651)
<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that improv
...
📝 fanquake locked a pull request: "removes repeated words, fix grammar"
(https://github.com/bitcoin/bitcoin/pull/30650)
<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that improv
...
💬 LarryRuane commented on pull request "log: expand BCLog::LogFlags (categories) to 64 bits":
(https://github.com/bitcoin/bitcoin/pull/26619#issuecomment-2287023896)
> I think would just write `CategoryMask{1}` everywhere instead of having a separate constant

I like that suggestion, I just force-pushed it, thanks!
📝 fjahr opened a pull request: "rpc: Set best header in reconsiderblock"
(https://github.com/bitcoin/bitcoin/pull/30652)
The `reconsiderblock` RPC resets the blocks flags and then calls `ActivateBestChain()`. But this doesn't set `m_best_header` which leaves us in an inconsistent state until something else triggers a change, like connecting a new block. But, until then, `getblockchaininfo`, for example, reports having less headers than blocks.

I found this while working on a more extensive tests for #29553.
💬 fanquake commented on pull request "rpc: Set best header in reconsiderblock":
(https://github.com/bitcoin/bitcoin/pull/30652#issuecomment-2287062693)
See also #26245.
💬 mzumsande commented on pull request "rpc: Set best header in reconsiderblock":
(https://github.com/bitcoin/bitcoin/pull/30652#issuecomment-2287068247)
It's a duplicate of #29913 (but see also my last comment there why the problem is harder than it seems)
👍 stickies-v approved a pull request: "Update translation source file for v28.0 string freeze"
(https://github.com/bitcoin-core/gui/pull/833#pullrequestreview-2236511012)
ACK 9edd8f300ca27cfabc310cbe015dc14160d24f1a
fjahr closed a pull request: "rpc: Set best header in reconsiderblock"
(https://github.com/bitcoin/bitcoin/pull/30652)
💬 fjahr commented on pull request "rpc: Set best header in reconsiderblock":
(https://github.com/bitcoin/bitcoin/pull/30652#issuecomment-2287095138)
@mzumsande thanks
💬 fjahr commented on pull request "bugfix: update chainman best_header after block reconsideration":
(https://github.com/bitcoin/bitcoin/pull/29913#issuecomment-2287099485)
Concept ACK

@furszy Are you still working on this? I need a fix for this to do better testing in #29553.
💬 mzumsande commented on pull request "bugfix: update chainman best_header after block reconsideration":
(https://github.com/bitcoin/bitcoin/pull/29913#issuecomment-2287106023)
I had a branch with a different solution that involves iterating over the entire block index (which is not super-efficient, but if it only is necessary in certain cases such as invalidateblock it may be ok?!): https://github.com/mzumsande/bitcoin/tree/202404_invalidblock
I think I'll clean that branch up in the next days and PR it as an alternative.
💬 fjahr commented on pull request "bugfix: update chainman best_header after block reconsideration":
(https://github.com/bitcoin/bitcoin/pull/29913#issuecomment-2287110142)
> I think I'll clean that branch up in the next days and PR it as an alternative.

That would be great, thanks!
🤔 hebasto reviewed a pull request: "Migrate legacy wallets that are not loaded"
(https://github.com/bitcoin-core/gui/pull/824#pullrequestreview-2236548045)
Almost ACK fd087d4667b94051d00617189dcecc4f88d849ce.
💬 hebasto commented on pull request "Migrate legacy wallets that are not loaded":
(https://github.com/bitcoin-core/gui/pull/824#discussion_r1715909769)
> Fixed during rebase.

It still needs to be fixed.
💬 furszy commented on pull request "bugfix: update chainman best_header after block reconsideration":
(https://github.com/bitcoin/bitcoin/pull/29913#issuecomment-2287134402)
IIRC, we have #30207 containing @mzumsande's fix branch and my intention was to leave this open while was reviewing the other one but.. it seems I never pushed my review there.
💬 fjahr commented on pull request "bugfix: update chainman best_header after block reconsideration":
(https://github.com/bitcoin/bitcoin/pull/29913#issuecomment-2287143652)
#30207 isn't marked as fixing #26245 and also misses test coverage in `rpc_invalidateblock` so I think it's something different than the branch @mzumsande mentions?
💬 mzumsande commented on pull request "bugfix: update chainman best_header after block reconsideration":
(https://github.com/bitcoin/bitcoin/pull/29913#issuecomment-2287147028)
#30207 doesn't contain a fix for the `m_best_header` issue, it just cautions against using `m_best_header` for anything important (see first commit) and fixes some other related bugs.
💬 furszy commented on pull request "bugfix: update chainman best_header after block reconsideration":
(https://github.com/bitcoin/bitcoin/pull/29913#issuecomment-2287173535)
Ok, just remembered it. #30207 shares part of the fix branch, and thats why I moved there first.
@mzumsande, guess that your upcoming PR will continue containing the same shared commit "validation: mark blocks between m_best_header and invalid block as BLOCK_FAILED_CHILD" ?

So in any case, we could move forward with both PRs #30207 and the new one now that there is some review power :).