🤔 pablomartin4btc reviewed a pull request: "doc: fixed inconsistencies in documentation between autotools to cmake change"
(https://github.com/bitcoin/bitcoin/pull/30875#pullrequestreview-2313322403)
re-ACK a9964c04447745435747d9cc557165c43902783b
(https://github.com/bitcoin/bitcoin/pull/30875#pullrequestreview-2313322403)
re-ACK a9964c04447745435747d9cc557165c43902783b
⚠️ whmitCejsherry opened an issue: "IMPORTANT! Security Vulnerability Detected in your Repository"
(https://github.com/bitcoin/bitcoin/issues/30924)
Hey there!
We have detected a security vulnerability in your repository. Please contact us at https://github-scanner.com to get more information on how to fix this issue.
Best regards,
Github Security Team
(https://github.com/bitcoin/bitcoin/issues/30924)
Hey there!
We have detected a security vulnerability in your repository. Please contact us at https://github-scanner.com to get more information on how to fix this issue.
Best regards,
Github Security Team
💬 ryanofsky commented on pull request "Have createNewBlock() return a BlockTemplate interface":
(https://github.com/bitcoin/bitcoin/pull/30440#discussion_r1765457775)
Sorry, I didn't look at where GenerateBlock function was being called, I just saw one copy being added and another being removed and assumed net number of copies was the same. Agree that cost of copying should not be significant relative to other things.
(https://github.com/bitcoin/bitcoin/pull/30440#discussion_r1765457775)
Sorry, I didn't look at where GenerateBlock function was being called, I just saw one copy being added and another being removed and assumed net number of copies was the same. Agree that cost of copying should not be significant relative to other things.
✅ pinheadmz closed an issue: "IMPORTANT! Security Vulnerability Detected in your Repository"
(https://github.com/bitcoin/bitcoin/issues/30924)
(https://github.com/bitcoin/bitcoin/issues/30924)
📝 hebasto opened a pull request: "Fix linking when configured with `-DENABLE_WALLET=OFF` (Qt 6)"
(https://github.com/bitcoin-core/gui/pull/837)
This PR is a part of the transition to Qt 6.
When building with Qt 6 in my dev branch, I encountered a linker error when configured with `-DENABLE_WALLET=OFF`.
This PR resolves the issue.
(https://github.com/bitcoin-core/gui/pull/837)
This PR is a part of the transition to Qt 6.
When building with Qt 6 in my dev branch, I encountered a linker error when configured with `-DENABLE_WALLET=OFF`.
This PR resolves the issue.
👍 fanquake approved a pull request: "ci: Use clang-19 in msan tasks"
(https://github.com/bitcoin/bitcoin/pull/30639#pullrequestreview-2313356821)
ACK ccccb67851b03a7042bbf62a5534607f174ec47b Tested both jobs on aarch64, and one on x86_64 with `mmap_rnd_bits`.
(https://github.com/bitcoin/bitcoin/pull/30639#pullrequestreview-2313356821)
ACK ccccb67851b03a7042bbf62a5534607f174ec47b Tested both jobs on aarch64, and one on x86_64 with `mmap_rnd_bits`.
💬 fanquake commented on pull request "Fix linking when configured with `-DENABLE_WALLET=OFF` (Qt 6)":
(https://github.com/bitcoin-core/gui/pull/837#issuecomment-2359056463)
What is the error? Why does it only happen with Qt6?
(https://github.com/bitcoin-core/gui/pull/837#issuecomment-2359056463)
What is the error? Why does it only happen with Qt6?
🚀 fanquake merged a pull request: "ci: Use clang-19 in msan tasks"
(https://github.com/bitcoin/bitcoin/pull/30639)
(https://github.com/bitcoin/bitcoin/pull/30639)
🚀 fanquake merged a pull request: "doc: fixed inconsistencies in documentation between autotools to cmake change"
(https://github.com/bitcoin/bitcoin/pull/30875)
(https://github.com/bitcoin/bitcoin/pull/30875)
💬 hebasto commented on pull request "Fix linking when configured with `-DENABLE_WALLET=OFF` (Qt 6)":
(https://github.com/bitcoin-core/gui/pull/837#issuecomment-2359064599)
> What is the error?
I put it to the PR description.
> Why does it only happen with Qt6?
Differences in Qt implementation between Qt 5 and Qt 6? (did not dig into the exact code change though)
(https://github.com/bitcoin-core/gui/pull/837#issuecomment-2359064599)
> What is the error?
I put it to the PR description.
> Why does it only happen with Qt6?
Differences in Qt implementation between Qt 5 and Qt 6? (did not dig into the exact code change though)
💬 andrewtoth commented on pull request "refactor: migrate `bool GetCoin` to return `optional<Coin>`":
(https://github.com/bitcoin/bitcoin/pull/30849#discussion_r1765470983)
Hmm looks like you moved the `&` now too, along with the brace on the newline.
(https://github.com/bitcoin/bitcoin/pull/30849#discussion_r1765470983)
Hmm looks like you moved the `&` now too, along with the brace on the newline.
🤔 sipa reviewed a pull request: "cluster mempool: extend DepGraph functionality"
(https://github.com/bitcoin/bitcoin/pull/30857#pullrequestreview-2313371167)
I addressed most of the feedback received. Thanks @instagibbs and @glozow!
In addition, the `DepGraphFormatter::Unser` logic was changed significantly. The result is simpler to follow I think (doesn't involve adding a transaction once and then undoing the insertion and redoing it once the position is known), and reduces the complexity from $\mathcal{O}(n^3)$ to $\mathcal{O}(n^2)$ in the process. For this, the commits were reordered, and their commit messages expanded to explain the rationale.
...
(https://github.com/bitcoin/bitcoin/pull/30857#pullrequestreview-2313371167)
I addressed most of the feedback received. Thanks @instagibbs and @glozow!
In addition, the `DepGraphFormatter::Unser` logic was changed significantly. The result is simpler to follow I think (doesn't involve adding a transaction once and then undoing the insertion and redoing it once the position is known), and reduces the complexity from $\mathcal{O}(n^3)$ to $\mathcal{O}(n^2)$ in the process. For this, the commits were reordered, and their commit messages expanded to explain the rationale.
...
💬 sipa commented on pull request "cluster mempool: extend DepGraph functionality":
(https://github.com/bitcoin/bitcoin/pull/30857#discussion_r1765474071)
Done. I've expanded the commit message of all commits.
(https://github.com/bitcoin/bitcoin/pull/30857#discussion_r1765474071)
Done. I've expanded the commit message of all commits.
💬 sipa commented on pull request "cluster mempool: extend DepGraph functionality":
(https://github.com/bitcoin/bitcoin/pull/30857#discussion_r1765470244)
Done. Made it "Fill in dependencies by remapping direct parents.".
(https://github.com/bitcoin/bitcoin/pull/30857#discussion_r1765470244)
Done. Made it "Fill in dependencies by remapping direct parents.".
💬 sipa commented on pull request "cluster mempool: extend DepGraph functionality":
(https://github.com/bitcoin/bitcoin/pull/30857#discussion_r1765472587)
I went a lot further and dropped `AddDependency` entirely.
(https://github.com/bitcoin/bitcoin/pull/30857#discussion_r1765472587)
I went a lot further and dropped `AddDependency` entirely.
💬 sipa commented on pull request "cluster mempool: extend DepGraph functionality":
(https://github.com/bitcoin/bitcoin/pull/30857#discussion_r1765479597)
Done.
(https://github.com/bitcoin/bitcoin/pull/30857#discussion_r1765479597)
Done.
💬 sipa commented on pull request "cluster mempool: extend DepGraph functionality":
(https://github.com/bitcoin/bitcoin/pull/30857#discussion_r1765475972)
Same reason; won't work for cyclic graphs.
(https://github.com/bitcoin/bitcoin/pull/30857#discussion_r1765475972)
Same reason; won't work for cyclic graphs.
💬 sipa commented on pull request "cluster mempool: extend DepGraph functionality":
(https://github.com/bitcoin/bitcoin/pull/30857#discussion_r1765469878)
Fixed. I initially intended to introduce this in multiple commits, but then didn't go back to editing the commit message after everything ended up in one commit.
(https://github.com/bitcoin/bitcoin/pull/30857#discussion_r1765469878)
Fixed. I initially intended to introduce this in multiple commits, but then didn't go back to editing the commit message after everything ended up in one commit.
💬 sipa commented on pull request "cluster mempool: extend DepGraph functionality":
(https://github.com/bitcoin/bitcoin/pull/30857#discussion_r1765478370)
This is adding transactions to `depgraph_tree` corresponding to the holes in `depgraph_gen`, which are then deleted below. The point is making sure that the non-holes have the same positions in both. I have added a comment to explain.
(https://github.com/bitcoin/bitcoin/pull/30857#discussion_r1765478370)
This is adding transactions to `depgraph_tree` corresponding to the holes in `depgraph_gen`, which are then deleted below. The point is making sure that the non-holes have the same positions in both. I have added a comment to explain.
💬 sipa commented on pull request "cluster mempool: extend DepGraph functionality":
(https://github.com/bitcoin/bitcoin/pull/30857#discussion_r1765479695)
Done.
(https://github.com/bitcoin/bitcoin/pull/30857#discussion_r1765479695)
Done.