📝 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
...
(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
...
(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!
(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.
(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.
(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)
(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
(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)
(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
(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.
(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.
(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!
(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.
(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.
(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.
(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?
(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.
(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 :).
(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 :).
👍 willcl-ark approved a pull request: "validation: assumeutxo params mainnet"
(https://github.com/bitcoin/bitcoin/pull/28553#pullrequestreview-2236651580)
tACK 1610643c8b37a9f674b236cfa79abf8f8aaf1410
Checked that the snapshot was created and had the same `sha256sum`
<details>
<summary>Details</summary>
```log
$ ./contrib/devtools/utxo_snapshot.sh 840000 snapshot.dat ./src/bitcoin-cli
Do you want to disable network activity (setnetworkactive false) before running invalidateblock? (Y/n): y
Disabling network activity
false
Rewinding chain back to height 840000 (by invalidating 00000000000000000001b48a75d5a3077913f3f441eb7e08c13c43f768
...
(https://github.com/bitcoin/bitcoin/pull/28553#pullrequestreview-2236651580)
tACK 1610643c8b37a9f674b236cfa79abf8f8aaf1410
Checked that the snapshot was created and had the same `sha256sum`
<details>
<summary>Details</summary>
```log
$ ./contrib/devtools/utxo_snapshot.sh 840000 snapshot.dat ./src/bitcoin-cli
Do you want to disable network activity (setnetworkactive false) before running invalidateblock? (Y/n): y
Disabling network activity
false
Rewinding chain back to height 840000 (by invalidating 00000000000000000001b48a75d5a3077913f3f441eb7e08c13c43f768
...
💬 ryanofsky commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-2287225308)
> I think for most functions here it could be feasible to have more concise error codes without too much effort, but I feel like I have to detach from this a bit before being able to come up with an alternative.
Thanks, I think I'd need to look at this more to give concrete suggestions, but I'd hope most functions would just return a simple success or failure status, with a descriptive error message in the case of failure. When functions need to return more complicated information or can fail
...
(https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-2287225308)
> I think for most functions here it could be feasible to have more concise error codes without too much effort, but I feel like I have to detach from this a bit before being able to come up with an alternative.
Thanks, I think I'd need to look at this more to give concrete suggestions, but I'd hope most functions would just return a simple success or failure status, with a descriptive error message in the case of failure. When functions need to return more complicated information or can fail
...