π¬ furszy commented on pull request "p2p: make block download logic aware of limited peers threshold":
(https://github.com/bitcoin/bitcoin/pull/28120#discussion_r1380849706)
Thx, done as suggested.
(https://github.com/bitcoin/bitcoin/pull/28120#discussion_r1380849706)
Thx, done as suggested.
π¬ furszy commented on pull request "p2p: make block download logic aware of limited peers threshold":
(https://github.com/bitcoin/bitcoin/pull/28120#discussion_r1380849886)
Thx, done as suggested.
(https://github.com/bitcoin/bitcoin/pull/28120#discussion_r1380849886)
Thx, done as suggested.
π€ furszy reviewed a pull request: "p2p: make block download logic aware of limited peers threshold"
(https://github.com/bitcoin/bitcoin/pull/28120#pullrequestreview-1711359884)
Updated per feedback, thanks for the review!
Fixed the two blocks buffer extension (now is reduction). Small [diff](https://github.com/bitcoin/bitcoin/compare/c1612ea1f050b3daede6a181464bbbce6e254b2d..9eba981808a56eee61a20b8378447bd0972ad589).
(https://github.com/bitcoin/bitcoin/pull/28120#pullrequestreview-1711359884)
Updated per feedback, thanks for the review!
Fixed the two blocks buffer extension (now is reduction). Small [diff](https://github.com/bitcoin/bitcoin/compare/c1612ea1f050b3daede6a181464bbbce6e254b2d..9eba981808a56eee61a20b8378447bd0972ad589).
π¬ ariard commented on pull request "refactors for subpackage evaluation":
(https://github.com/bitcoin/bitcoin/pull/28758#discussion_r1380874443)
βas unconfirmed transactions are not allowed to have no inputs at consensus validation (see CheckTransaction)"
(https://github.com/bitcoin/bitcoin/pull/28758#discussion_r1380874443)
βas unconfirmed transactions are not allowed to have no inputs at consensus validation (see CheckTransaction)"
π¬ ariard commented on pull request "refactors for subpackage evaluation":
(https://github.com/bitcoin/bitcoin/pull/28758#discussion_r1380876016)
I donβt know what is mean by consistency of a transaction with oneself (e.g if txouts are under 0 or over `MAX_MONEY`) or youβre only implying βdoes not spend same prevout multiple timesβ.
(https://github.com/bitcoin/bitcoin/pull/28758#discussion_r1380876016)
I donβt know what is mean by consistency of a transaction with oneself (e.g if txouts are under 0 or over `MAX_MONEY`) or youβre only implying βdoes not spend same prevout multiple timesβ.
π¬ kashifs commented on pull request "net: improves addnode / m_added_nodes logic":
(https://github.com/bitcoin/bitcoin/pull/28155#issuecomment-1791691848)
tACK [0420f9](https://github.com/bitcoin/bitcoin/commit/0420f99f429ce2382057e101859067f40de47be0)
(https://github.com/bitcoin/bitcoin/pull/28155#issuecomment-1791691848)
tACK [0420f9](https://github.com/bitcoin/bitcoin/commit/0420f99f429ce2382057e101859067f40de47be0)
β
maflcko closed an issue: "qa: Intermittent failure in `feature_fee_estimation.py`"
(https://github.com/bitcoin/bitcoin/issues/23165)
(https://github.com/bitcoin/bitcoin/issues/23165)
π¬ vasild commented on pull request "Avoid returning references to mutex guarded members":
(https://github.com/bitcoin/bitcoin/pull/28774#issuecomment-1792011287)
@maflcko, a mutex is required by readers to avoid writers modifying the blob while the read is happening, resulting in a torn read. The underlying variable is not readonly (ie declared as `const`), I explored converting the variables to `const` and only setting them at the constructor, but that didn't seem viable.
(https://github.com/bitcoin/bitcoin/pull/28774#issuecomment-1792011287)
@maflcko, a mutex is required by readers to avoid writers modifying the blob while the read is happening, resulting in a torn read. The underlying variable is not readonly (ie declared as `const`), I explored converting the variables to `const` and only setting them at the constructor, but that didn't seem viable.
π¬ hebasto commented on pull request "build: Patch Qt to handle minimum macOS version properly":
(https://github.com/bitcoin/bitcoin/pull/28775#issuecomment-1792023227)
> In that case, maybe it would be easier to set something like `-DOS_ACTIVITY_OBJECT_API=1` for the Qt build, and not have to patch any source?
I think it will hit
```
#error Please change your minimum OS requirements because OS_ACTIVITY_OBJECT_API is not available
```
> Native builds (using the latest Xcode) are not broken in this way, and the minimum version handling shouldn't differ between the two?
I've verified the native build on macOS 14.1 without depends. Both macros are defi
...
(https://github.com/bitcoin/bitcoin/pull/28775#issuecomment-1792023227)
> In that case, maybe it would be easier to set something like `-DOS_ACTIVITY_OBJECT_API=1` for the Qt build, and not have to patch any source?
I think it will hit
```
#error Please change your minimum OS requirements because OS_ACTIVITY_OBJECT_API is not available
```
> Native builds (using the latest Xcode) are not broken in this way, and the minimum version handling shouldn't differ between the two?
I've verified the native build on macOS 14.1 without depends. Both macros are defi
...
π¬ maflcko commented on pull request "Avoid returning references to mutex guarded members":
(https://github.com/bitcoin/bitcoin/pull/28774#issuecomment-1792035522)
No write will happen to the datadir cache after the first write, and no reader will have the reference before the first write, no? I am trying to say that the current code is fine. Otherwise, it would be good to add steps to reproduce UB.
No objection to changing the code, but if this is worth it to change, it should be done for all places in the whole codebase, not just some places.
(https://github.com/bitcoin/bitcoin/pull/28774#issuecomment-1792035522)
No write will happen to the datadir cache after the first write, and no reader will have the reference before the first write, no? I am trying to say that the current code is fine. Otherwise, it would be good to add steps to reproduce UB.
No objection to changing the code, but if this is worth it to change, it should be done for all places in the whole codebase, not just some places.
π hebasto converted_to_draft a pull request: "build: Patch Qt to handle minimum macOS version properly"
(https://github.com/bitcoin/bitcoin/pull/28775)
This PR is:
- required to [switch](https://github.com/bitcoin/bitcoin/pull/28622) to macOS 14 SDK (Xcode 15).
- an alternative to https://github.com/bitcoin/bitcoin/pull/28732.
(https://github.com/bitcoin/bitcoin/pull/28775)
This PR is:
- required to [switch](https://github.com/bitcoin/bitcoin/pull/28622) to macOS 14 SDK (Xcode 15).
- an alternative to https://github.com/bitcoin/bitcoin/pull/28732.
π BrandonOdiwuor opened a pull request: "gui: add used balance to overview page"
(https://github.com/bitcoin-core/gui/pull/775)
**Second part** of solving https://github.com/bitcoin-core/gui/issues/769
Add used balance to the overview page for wallets with the avoid_reuse flag enabled
### Prerequsite:
- **Part one**: https://github.com/bitcoin/bitcoin/pull/28776
overview page when avoid_reuse is enabled

overview page when avoid_reuse is not enabled

overview page when avoid_reuse is not enabled
![Screenshot from 2023-11-03
...
π¬ BrandonOdiwuor commented on issue "`used` balance should be shown on overview page":
(https://github.com/bitcoin-core/gui/issues/769#issuecomment-1792061121)
Created two PRs on this issue:
- Part one: https://github.com/bitcoin/bitcoin/pull/28776
- Part two: https://github.com/bitcoin-core/gui/pull/775
(https://github.com/bitcoin-core/gui/issues/769#issuecomment-1792061121)
Created two PRs on this issue:
- Part one: https://github.com/bitcoin/bitcoin/pull/28776
- Part two: https://github.com/bitcoin-core/gui/pull/775
π¬ glozow commented on pull request "refactors for subpackage evaluation":
(https://github.com/bitcoin/bitcoin/pull/28758#discussion_r1381323292)
I don't think it's that helpful to list more things this function *doesn't* check.
(https://github.com/bitcoin/bitcoin/pull/28758#discussion_r1381323292)
I don't think it's that helpful to list more things this function *doesn't* check.
π hebasto's pull request is ready for review: "build: Patch Qt to handle minimum macOS version properly"
(https://github.com/bitcoin/bitcoin/pull/28775)
(https://github.com/bitcoin/bitcoin/pull/28775)
π¬ maflcko commented on pull request "refactor: Remove unused SER_DISK, SER_NETWORK, SER_GETHASH":
(https://github.com/bitcoin/bitcoin/pull/28451#issuecomment-1792178673)
Please review https://github.com/bitcoin/bitcoin/pull/28438 (Use serialization parameters for CTransaction by ajtowns) first.
(https://github.com/bitcoin/bitcoin/pull/28451#issuecomment-1792178673)
Please review https://github.com/bitcoin/bitcoin/pull/28438 (Use serialization parameters for CTransaction by ajtowns) first.
π¬ hebasto commented on pull request "build: use macOS 14 SDK (Xcode 15.0)":
(https://github.com/bitcoin/bitcoin/pull/28622#issuecomment-1792179529)
> > The following patch fixes the Qt build for me:
>
> We wont be taking that patch, but if the problem is that qt fails to respect `-mmacosx-version-min` for some reason, I would assume that should make it fairly obvious where the problem is.
Apparently, direct tests show that `env -u C_INCLUDE_PATH -u CPLUS_INCLUDE_PATH -u OBJC_INCLUDE_PATH -u OBJCPLUS_INCLUDE_PATH -u CPATH -u LIBRARY_PATH /home/hebasto/git/bitcoin/depends/x86_64-apple-darwin/native/bin/clang++ --target=x86_64-apple-darw
...
(https://github.com/bitcoin/bitcoin/pull/28622#issuecomment-1792179529)
> > The following patch fixes the Qt build for me:
>
> We wont be taking that patch, but if the problem is that qt fails to respect `-mmacosx-version-min` for some reason, I would assume that should make it fairly obvious where the problem is.
Apparently, direct tests show that `env -u C_INCLUDE_PATH -u CPLUS_INCLUDE_PATH -u OBJC_INCLUDE_PATH -u OBJCPLUS_INCLUDE_PATH -u CPATH -u LIBRARY_PATH /home/hebasto/git/bitcoin/depends/x86_64-apple-darwin/native/bin/clang++ --target=x86_64-apple-darw
...
π¬ glozow commented on pull request "MiniMiner changes for package linearization":
(https://github.com/bitcoin/bitcoin/pull/28762#discussion_r1381447100)
added a refactor commit to pull fee amounts into constants
(https://github.com/bitcoin/bitcoin/pull/28762#discussion_r1381447100)
added a refactor commit to pull fee amounts into constants
π¬ glozow commented on pull request "MiniMiner changes for package linearization":
(https://github.com/bitcoin/bitcoin/pull/28762#discussion_r1381447239)
done
(https://github.com/bitcoin/bitcoin/pull/28762#discussion_r1381447239)
done
π¬ glozow commented on pull request "MiniMiner changes for package linearization":
(https://github.com/bitcoin/bitcoin/pull/28762#discussion_r1381447982)
Done, and updated the expected boost includes for the linter. Unsure if it's preferred to omit those...?
(https://github.com/bitcoin/bitcoin/pull/28762#discussion_r1381447982)
Done, and updated the expected boost includes for the linter. Unsure if it's preferred to omit those...?