🚀 fanquake merged a pull request: "doc: Suggest only necessary Qt packages for installation on OpenBSD"
(https://github.com/bitcoin/bitcoin/pull/29947)
(https://github.com/bitcoin/bitcoin/pull/29947)
🚀 fanquake merged a pull request: "doc: suggest only necessary Qt packages for installation on FreeBSD"
(https://github.com/bitcoin/bitcoin/pull/29932)
(https://github.com/bitcoin/bitcoin/pull/29932)
✅ fanquake closed an issue: "fuzz, rpc: Internal bug in `finalizepsbt` "CHECK_NONFATAL(last - first == 32)" "
(https://github.com/bitcoin/bitcoin/issues/29851)
(https://github.com/bitcoin/bitcoin/issues/29851)
🚀 fanquake merged a pull request: "sign: don't assume we are parsing a sane TapMiniscript"
(https://github.com/bitcoin/bitcoin/pull/29853)
(https://github.com/bitcoin/bitcoin/pull/29853)
✅ hebasto closed a pull request: "ci: Workaround Microsoft mirror issue for GitHub Action"
(https://github.com/bitcoin/bitcoin/pull/29951)
(https://github.com/bitcoin/bitcoin/pull/29951)
💬 hebasto commented on pull request "ci: Workaround Microsoft mirror issue for GitHub Action":
(https://github.com/bitcoin/bitcoin/pull/29951#issuecomment-2074923286)
https://github.com/microsoft/linux-package-repositories/issues/130
Closing.
(https://github.com/bitcoin/bitcoin/pull/29951#issuecomment-2074923286)
https://github.com/microsoft/linux-package-repositories/issues/130
Closing.
💬 fanquake commented on pull request "sign: don't assume we are parsing a sane TapMiniscript":
(https://github.com/bitcoin/bitcoin/pull/29853#issuecomment-2074931006)
Backported to 27.x in #29888.
(https://github.com/bitcoin/bitcoin/pull/29853#issuecomment-2074931006)
Backported to 27.x in #29888.
⚠️ maflcko opened an issue: "Require thread_local"
(https://github.com/bitcoin/bitcoin/issues/29952)
After dbfca4a815d2dbef69f3b634c24b875bc1d22afc from the 23.x release (and fe1b3256888bd0e70d0c9655f565e139ec87b606 from 25.x), I think the `--disable-threadlocal` can now be dropped, as those commits removed the need for it?
cc @hebasto @fanquake
(https://github.com/bitcoin/bitcoin/issues/29952)
After dbfca4a815d2dbef69f3b634c24b875bc1d22afc from the 23.x release (and fe1b3256888bd0e70d0c9655f565e139ec87b606 from 25.x), I think the `--disable-threadlocal` can now be dropped, as those commits removed the need for it?
cc @hebasto @fanquake
💬 hernanmarino commented on pull request "test: Assumeutxo: snapshots with less work should not be loaded":
(https://github.com/bitcoin/bitcoin/pull/29428#issuecomment-2074934165)
> Please delete the hidden comment in the pull description, because this will end up in the merge commit.
Oh, sorry that. Deleted.
(https://github.com/bitcoin/bitcoin/pull/29428#issuecomment-2074934165)
> Please delete the hidden comment in the pull description, because this will end up in the merge commit.
Oh, sorry that. Deleted.
💬 fanquake commented on issue "Require thread_local":
(https://github.com/bitcoin/bitcoin/issues/29952#issuecomment-2074938316)
Possibly. Someone will first need to test it on FreeBSD and mingw-w64. Given we still assume those implementations to be broken.
(https://github.com/bitcoin/bitcoin/issues/29952#issuecomment-2074938316)
Possibly. Someone will first need to test it on FreeBSD and mingw-w64. Given we still assume those implementations to be broken.
✅ fanquake closed an issue: "SEE ALSO section for all man pages"
(https://github.com/bitcoin/bitcoin/issues/29558)
(https://github.com/bitcoin/bitcoin/issues/29558)
🚀 fanquake merged a pull request: "contrib: list other binaries in manpage output"
(https://github.com/bitcoin/bitcoin/pull/29585)
(https://github.com/bitcoin/bitcoin/pull/29585)
💬 maflcko commented on issue "Require thread_local":
(https://github.com/bitcoin/bitcoin/issues/29952#issuecomment-2074959708)
I couldn't get wine running locally, so my testing failed. But given that we require a C++20 compiler, I'd be surprised if there is still a platform out there that hasn't fully implemented C++11 at this point.
I presume, at least for the mingw issue, the existing unit tests should detect the issue, as I presume the `RenameEnMasse` unit test is equivalent to the C++ shared in the linked gist?
(https://github.com/bitcoin/bitcoin/issues/29952#issuecomment-2074959708)
I couldn't get wine running locally, so my testing failed. But given that we require a C++20 compiler, I'd be surprised if there is still a platform out there that hasn't fully implemented C++11 at this point.
I presume, at least for the mingw issue, the existing unit tests should detect the issue, as I presume the `RenameEnMasse` unit test is equivalent to the C++ shared in the linked gist?
🤔 kevkevinpal reviewed a pull request: "test: Run framework unit tests in parallel"
(https://github.com/bitcoin/bitcoin/pull/29771#pullrequestreview-2019885446)
Approach ACK f19f0a2e5af6c2a64900f1f229e21b6f1668bd3d
I still need to run this on my local machine
(https://github.com/bitcoin/bitcoin/pull/29771#pullrequestreview-2019885446)
Approach ACK f19f0a2e5af6c2a64900f1f229e21b6f1668bd3d
I still need to run this on my local machine
💬 kevkevinpal commented on pull request "test: Run framework unit tests in parallel":
(https://github.com/bitcoin/bitcoin/pull/29771#discussion_r1577898651)
is there any reason we picked this location in `BASE_SCRIPTS`, I would think we would want to run `TEST_FRAMEWORK_UNIT_TESTS` earlier on even if they are running in parallel
since if the unit tests fail other tests will probably fail as well.
I think making it the first script in `BASE_SCRIPTS` would make sense to me and then making an exception in regards to run times since other tests deepened on this.
(https://github.com/bitcoin/bitcoin/pull/29771#discussion_r1577898651)
is there any reason we picked this location in `BASE_SCRIPTS`, I would think we would want to run `TEST_FRAMEWORK_UNIT_TESTS` earlier on even if they are running in parallel
since if the unit tests fail other tests will probably fail as well.
I think making it the first script in `BASE_SCRIPTS` would make sense to me and then making an exception in regards to run times since other tests deepened on this.
👍 ryanofsky approved a pull request: "validation: improve performance of CheckBlockIndex"
(https://github.com/bitcoin/bitcoin/pull/28339#pullrequestreview-2019819063)
Code review ACK 3c3895ccfa108e10f4a0b1a78f64ffedade1fd11
Left another suggestion, but this looks good in its current form.
(https://github.com/bitcoin/bitcoin/pull/28339#pullrequestreview-2019819063)
Code review ACK 3c3895ccfa108e10f4a0b1a78f64ffedade1fd11
Left another suggestion, but this looks good in its current form.
💬 ryanofsky commented on pull request "validation: improve performance of CheckBlockIndex":
(https://github.com/bitcoin/bitcoin/pull/28339#discussion_r1577858964)
In commit "validation: improve performance of CheckBlockIndex" (7183bc90483cc9361cfffb157b91fd4245ab77e0)
This might be a little simpler and clearer since it consolidates the best header chain cases:
```diff
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -5292,15 +5292,14 @@ void ChainstateManager::CheckBlockIndex()
pindex = range.first->second;
nHeight++;
continue;
- } else if (best_hdr_chain.Contains(pindex) && nHeight < best_hdr
...
(https://github.com/bitcoin/bitcoin/pull/28339#discussion_r1577858964)
In commit "validation: improve performance of CheckBlockIndex" (7183bc90483cc9361cfffb157b91fd4245ab77e0)
This might be a little simpler and clearer since it consolidates the best header chain cases:
```diff
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -5292,15 +5292,14 @@ void ChainstateManager::CheckBlockIndex()
pindex = range.first->second;
nHeight++;
continue;
- } else if (best_hdr_chain.Contains(pindex) && nHeight < best_hdr
...
💬 ryanofsky commented on pull request "validation: improve performance of CheckBlockIndex":
(https://github.com/bitcoin/bitcoin/pull/28339#discussion_r1577887301)
In commit "validation: improve performance of CheckBlockIndex" (7183bc90483cc9361cfffb157b91fd4245ab77e0)
Note: this assert is a little different than what I suggested https://github.com/bitcoin/bitcoin/pull/28339#discussion_r1569423672 because it is not enforcing that if the parent block is the best header, then pindex must be null. So the statement that there will not be a best block if the parent block is the best header is not checked by the assert.
But I think this is fine. The commen
...
(https://github.com/bitcoin/bitcoin/pull/28339#discussion_r1577887301)
In commit "validation: improve performance of CheckBlockIndex" (7183bc90483cc9361cfffb157b91fd4245ab77e0)
Note: this assert is a little different than what I suggested https://github.com/bitcoin/bitcoin/pull/28339#discussion_r1569423672 because it is not enforcing that if the parent block is the best header, then pindex must be null. So the statement that there will not be a best block if the parent block is the best header is not checked by the assert.
But I think this is fine. The commen
...
💬 maflcko commented on pull request "ci: disable `_FORTIFY_SOURCE` with MSAN":
(https://github.com/bitcoin/bitcoin/pull/29837#issuecomment-2075003021)
lgtm ACK 08ff17d1420a3d1c14c6b1a5436678fbb1dd9cbc
(https://github.com/bitcoin/bitcoin/pull/29837#issuecomment-2075003021)
lgtm ACK 08ff17d1420a3d1c14c6b1a5436678fbb1dd9cbc
💬 glozow commented on pull request "p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1577936862)
I don't think that is much simpler. Also I think this is easier to delete if/when we don't try orphans from different peers in the future.
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1577936862)
I don't think that is much simpler. Also I think this is easier to delete if/when we don't try orphans from different peers in the future.