💬 hebasto commented on pull request "Fix broken navigation link to files documentation in README_doxygen.md":
(https://github.com/bitcoin-core/gui/pull/878#issuecomment-3007988414)
It works fine as is in https://doxygen.bitcoincore.org/.
In the future, please open non-GUI-specific PRs in the main [repo](https://github.com/bitcoin/bitcoin).
(https://github.com/bitcoin-core/gui/pull/878#issuecomment-3007988414)
It works fine as is in https://doxygen.bitcoincore.org/.
In the future, please open non-GUI-specific PRs in the main [repo](https://github.com/bitcoin/bitcoin).
✅ hebasto closed a pull request: "Fix broken navigation link to files documentation in README_doxygen.md"
(https://github.com/bitcoin-core/gui/pull/878)
(https://github.com/bitcoin-core/gui/pull/878)
💬 fanquake commented on pull request "test: fix catchup loop in outbound eviction functional test":
(https://github.com/bitcoin/bitcoin/pull/32742#issuecomment-3007993519)
Backported to 29.x in #32810.
(https://github.com/bitcoin/bitcoin/pull/32742#issuecomment-3007993519)
Backported to 29.x in #32810.
💬 m3dwards commented on pull request "test: enabling wallet migration functional test on windows":
(https://github.com/bitcoin/bitcoin/pull/32219#issuecomment-3008004914)
@maflcko addressed nits and force pushed
(https://github.com/bitcoin/bitcoin/pull/32219#issuecomment-3008004914)
@maflcko addressed nits and force pushed
💬 josibake commented on pull request "[29.x] More backports":
(https://github.com/bitcoin/bitcoin/pull/32810#issuecomment-3008071033)
ACK [e96d360](https://github.com/bitcoin/bitcoin/commit/e96d360767e7dd2e3fc215c48384fadd3f2da82d)
Verified that the correct commits are being pulled in and the release notes.
(https://github.com/bitcoin/bitcoin/pull/32810#issuecomment-3008071033)
ACK [e96d360](https://github.com/bitcoin/bitcoin/commit/e96d360767e7dd2e3fc215c48384fadd3f2da82d)
Verified that the correct commits are being pulled in and the release notes.
🤔 fanquake reviewed a pull request: "cmake: Use `HINTS` instead of `PATHS` in `find_*` commands"
(https://github.com/bitcoin/bitcoin/pull/32805#pullrequestreview-2961702133)
ACK ead44687483e9c936ba970de890c01d5e7ad3485
(https://github.com/bitcoin/bitcoin/pull/32805#pullrequestreview-2961702133)
ACK ead44687483e9c936ba970de890c01d5e7ad3485
🚀 fanquake merged a pull request: "cmake: Use `HINTS` instead of `PATHS` in `find_*` commands"
(https://github.com/bitcoin/bitcoin/pull/32805)
(https://github.com/bitcoin/bitcoin/pull/32805)
💬 fanquake commented on pull request "Fix build on macOS when `qt@6` is installed":
(https://github.com/bitcoin/bitcoin/pull/32804#issuecomment-3008113882)
Do we need this open? Rebased it's the same as #32814.
(https://github.com/bitcoin/bitcoin/pull/32804#issuecomment-3008113882)
Do we need this open? Rebased it's the same as #32814.
💬 fanquake commented on pull request "cmake: Use `HINTS` instead of `PATHS` in `find_*` commands":
(https://github.com/bitcoin/bitcoin/pull/32805#issuecomment-3008115336)
Backported to 29.x in #32810.
(https://github.com/bitcoin/bitcoin/pull/32805#issuecomment-3008115336)
Backported to 29.x in #32810.
💬 Eunovo commented on pull request "refactor: CFeeRate encapsulates FeeFrac internally":
(https://github.com/bitcoin/bitcoin/pull/32750#discussion_r2168890446)
@polespinasa now that the code has been updated to use `uint32`, I think you can mark this as resolved
(https://github.com/bitcoin/bitcoin/pull/32750#discussion_r2168890446)
@polespinasa now that the code has been updated to use `uint32`, I think you can mark this as resolved
💬 Eunovo commented on pull request "refactor: CFeeRate encapsulates FeeFrac internally":
(https://github.com/bitcoin/bitcoin/pull/32750#discussion_r2168896663)
https://github.com/bitcoin/bitcoin/pull/32750/commits/990010f49ca97479f9fe46e288ed0f0fbb0fde94: Can `nSize` still be less than zero? Converting from `uint32` to `int64` should be safe
(https://github.com/bitcoin/bitcoin/pull/32750#discussion_r2168896663)
https://github.com/bitcoin/bitcoin/pull/32750/commits/990010f49ca97479f9fe46e288ed0f0fbb0fde94: Can `nSize` still be less than zero? Converting from `uint32` to `int64` should be safe
🤔 janb84 reviewed a pull request: "bitcoin-cli: Add -ipcconnect option"
(https://github.com/bitcoin/bitcoin/pull/32297#pullrequestreview-2961926007)
ACK 9ffe57f81b2a6abd161ae010f07916b05e57191d
PR adds functionality to allow bitcoin-cli to connect to the node via IPC instead of TCP (if enabled) as part of the Multiprocess project
nit maybe fix grammar issues found by drahtbot https://github.com/bitcoin/bitcoin/pull/32297#issuecomment-2813015247 ?
- code review ✅
- build & tested ✅
- tested communication between cli and node binaries ✅
(https://github.com/bitcoin/bitcoin/pull/32297#pullrequestreview-2961926007)
ACK 9ffe57f81b2a6abd161ae010f07916b05e57191d
PR adds functionality to allow bitcoin-cli to connect to the node via IPC instead of TCP (if enabled) as part of the Multiprocess project
nit maybe fix grammar issues found by drahtbot https://github.com/bitcoin/bitcoin/pull/32297#issuecomment-2813015247 ?
- code review ✅
- build & tested ✅
- tested communication between cli and node binaries ✅
💬 janb84 commented on pull request "bitcoin-cli: Add -ipcconnect option":
(https://github.com/bitcoin/bitcoin/pull/32297#discussion_r2168951779)
```suggestion
std::unique_ptr<interfaces::Ipc> m_ipc;
```
```suggestion
private:
std::unique_ptr<interfaces::Ipc> m_ipc;
```
NIT can be made private ?
(https://github.com/bitcoin/bitcoin/pull/32297#discussion_r2168951779)
```suggestion
std::unique_ptr<interfaces::Ipc> m_ipc;
```
```suggestion
private:
std::unique_ptr<interfaces::Ipc> m_ipc;
```
NIT can be made private ?
💬 Crypt-iQ commented on pull request "log: Mitigate disk filling attacks by rate limiting LogPrintf, LogInfo, LogWarning, LogError":
(https://github.com/bitcoin/bitcoin/pull/32604#discussion_r2168960313)
I didn't validate it, but I can. I think it is important that the new and old versions of `MemUsage` are roughly the same.
(https://github.com/bitcoin/bitcoin/pull/32604#discussion_r2168960313)
I didn't validate it, but I can. I think it is important that the new and old versions of `MemUsage` are roughly the same.
⚠️ sipa opened an issue: "`dumptxoutset` rollback feature does not take forks into account"
(https://github.com/bitcoin/bitcoin/issues/32817)
In the `dumptxoutset` RPC, when the rollback feature is used with a height H, it will issue an `InvalidateBlock` on the active-chain block at height H+1. However, if an inactive branch of the chain exists that forks off at height H or below, and extends to block H+1 or later (or has equivalent PoW to that), `InvalidateBlock` will reorg there, and dump will be for the tip of that branch, rather than the height-H ancestor of the main chain.
(https://github.com/bitcoin/bitcoin/issues/32817)
In the `dumptxoutset` RPC, when the rollback feature is used with a height H, it will issue an `InvalidateBlock` on the active-chain block at height H+1. However, if an inactive branch of the chain exists that forks off at height H or below, and extends to block H+1 or later (or has equivalent PoW to that), `InvalidateBlock` will reorg there, and dump will be for the tip of that branch, rather than the height-H ancestor of the main chain.
💬 Crypt-iQ commented on pull request "log: Mitigate disk filling attacks by rate limiting LogPrintf, LogInfo, LogWarning, LogError":
(https://github.com/bitcoin/bitcoin/pull/32604#discussion_r2168974305)
Sweet, thanks for doing this! I was curious about any performance hits.
(https://github.com/bitcoin/bitcoin/pull/32604#discussion_r2168974305)
Sweet, thanks for doing this! I was curious about any performance hits.
💬 hebasto commented on pull request "Fix build on macOS when `qt@6` is installed":
(https://github.com/bitcoin/bitcoin/pull/32804#issuecomment-3008363545)
> Do we need this open? Rebased it's the same as #32814.
To make testing this branch easier?
(https://github.com/bitcoin/bitcoin/pull/32804#issuecomment-3008363545)
> Do we need this open? Rebased it's the same as #32814.
To make testing this branch easier?
💬 musaHaruna commented on pull request "rpc: Distinguish between vsize and sigop adjusted mempool vsize":
(https://github.com/bitcoin/bitcoin/pull/32800#discussion_r2169017125)
The symbols are "⌈⌉" ceiling function, can also be written as ceil(weight/4) fixed it in the PR decription for easy readablity
(https://github.com/bitcoin/bitcoin/pull/32800#discussion_r2169017125)
The symbols are "⌈⌉" ceiling function, can also be written as ceil(weight/4) fixed it in the PR decription for easy readablity
💬 musaHaruna commented on pull request "rpc: Distinguish between vsize and sigop adjusted mempool vsize":
(https://github.com/bitcoin/bitcoin/pull/32800#discussion_r2169033043)
Yes I agree. Removed.
(https://github.com/bitcoin/bitcoin/pull/32800#discussion_r2169033043)
Yes I agree. Removed.
💬 musaHaruna commented on pull request "rpc: Distinguish between vsize and sigop adjusted mempool vsize":
(https://github.com/bitcoin/bitcoin/pull/32800#discussion_r2169035477)
Thanks for the review!!! I have added `[Feerates and vsize](feerates-and-vsize.md)` to the README.md to make the list exhaustive
(https://github.com/bitcoin/bitcoin/pull/32800#discussion_r2169035477)
Thanks for the review!!! I have added `[Feerates and vsize](feerates-and-vsize.md)` to the README.md to make the list exhaustive
💬 fanquake commented on pull request "cmake: Explicitly specify `Boost_ROOT` for Homebrew's package":
(https://github.com/bitcoin/bitcoin/pull/32814#issuecomment-3008445000)
cc @purpleKarrot
(https://github.com/bitcoin/bitcoin/pull/32814#issuecomment-3008445000)
cc @purpleKarrot