👍 andrewtoth approved a pull request: "init: don't delete PID file if it was not generated"
(https://github.com/bitcoin/bitcoin/pull/28946#pullrequestreview-1762959130)
ACK 8f6ab318635d823a7e6ad9020ac17b8d2dd0e91b
(https://github.com/bitcoin/bitcoin/pull/28946#pullrequestreview-1762959130)
ACK 8f6ab318635d823a7e6ad9020ac17b8d2dd0e91b
💬 achow101 commented on pull request "wallet: skip BnB when SFFO is enabled":
(https://github.com/bitcoin/bitcoin/pull/28994#discussion_r1414228551)
For unit tests, having the information in a struct is fine. However I'm not sure that it is really useful information to return in the RPC. IIRC my suggestion for searching the debug log is for functional tests since those already have an `assert_debug_log` helper.
(https://github.com/bitcoin/bitcoin/pull/28994#discussion_r1414228551)
For unit tests, having the information in a struct is fine. However I'm not sure that it is really useful information to return in the RPC. IIRC my suggestion for searching the debug log is for functional tests since those already have an `assert_debug_log` helper.
💬 fanquake commented on pull request "build: use macOS 14 SDK (Xcode 15.0)":
(https://github.com/bitcoin/bitcoin/pull/28622#discussion_r1414229876)
From having another look now, Clang still uses `-mlinker-version`, in it's decision making about whether to use `-platform_version` (by default), which it does if the linker version is > 520. If we are passing `-platform_version` ourselves, then maybe we can drop `-mlinker-version`, but I would have to look again, to see if there are other side effects, and this is probably ok to leave for now, and revisit when we do the lld switch, as I assume more flags are going to change there as well.
(https://github.com/bitcoin/bitcoin/pull/28622#discussion_r1414229876)
From having another look now, Clang still uses `-mlinker-version`, in it's decision making about whether to use `-platform_version` (by default), which it does if the linker version is > 520. If we are passing `-platform_version` ourselves, then maybe we can drop `-mlinker-version`, but I would have to look again, to see if there are other side effects, and this is probably ok to leave for now, and revisit when we do the lld switch, as I assume more flags are going to change there as well.
💬 maflcko commented on pull request "build: use macOS 14 SDK (Xcode 15.0)":
(https://github.com/bitcoin/bitcoin/pull/28622#issuecomment-1839131640)
https://cirrus-ci.com/task/4855730595954688?logs=ci#L8068
```
configure:33916: error: External signing is not supported for this Boost version
(https://github.com/bitcoin/bitcoin/pull/28622#issuecomment-1839131640)
https://cirrus-ci.com/task/4855730595954688?logs=ci#L8068
```
configure:33916: error: External signing is not supported for this Boost version
⚠️ FU3X opened an issue: "are there any plans to deal with fees?"
(https://github.com/bitcoin/bitcoin/issues/28995)
like a future update or eta?
(https://github.com/bitcoin/bitcoin/issues/28995)
like a future update or eta?
💬 fanquake commented on pull request "build: use macOS 14 SDK (Xcode 15.0)":
(https://github.com/bitcoin/bitcoin/pull/28622#issuecomment-1839158062)
> https://cirrus-ci.com/task/4855730595954688?logs=ci#L8068
> configure:33916: error: External signing is not supported for this Boost version
Nice. It's this issue https://github.com/boostorg/process/issues/342 & change https://github.com/boostorg/process/pull/343 from upstream Boost Process.
(https://github.com/bitcoin/bitcoin/pull/28622#issuecomment-1839158062)
> https://cirrus-ci.com/task/4855730595954688?logs=ci#L8068
> configure:33916: error: External signing is not supported for this Boost version
Nice. It's this issue https://github.com/boostorg/process/issues/342 & change https://github.com/boostorg/process/pull/343 from upstream Boost Process.
✅ fanquake closed an issue: "are there any plans to deal with fees?"
(https://github.com/bitcoin/bitcoin/issues/28995)
(https://github.com/bitcoin/bitcoin/issues/28995)
💬 ryanofsky commented on issue "`capnp` fails when cross-compiling":
(https://github.com/bitcoin/bitcoin/issues/28993#issuecomment-1839184118)
I'm struggling to get darwin build working locally, but I will try docker to reproduce this. Do you know if this might be a recent regression, or when was the last time it might have worked?
(https://github.com/bitcoin/bitcoin/issues/28993#issuecomment-1839184118)
I'm struggling to get darwin build working locally, but I will try docker to reproduce this. Do you know if this might be a recent regression, or when was the last time it might have worked?
📝 theStack opened a pull request: "test: maxuploadtarget: check for mempool msg disconnect if limit is reached, improve existing test coverage"
(https://github.com/bitcoin/bitcoin/pull/28996)
This PR improves existing and adds new test coverage for the `-maxuploadtarget` mechanism (feature_maxuploadtarget.py) in the following ways, one commit each:
* verify the uploadtarget state via the `getnettotals` RPC (`uploadtarget` result field):
https://github.com/bitcoin/bitcoin/blob/160d23677ad799cf9b493eaa923b2ac080c3fb8e/src/rpc/net.cpp#L581-L582
Note that reaching the total limit (`target_reached` == True) always implies that the historical blocks serving limits is also reached (`serv
...
(https://github.com/bitcoin/bitcoin/pull/28996)
This PR improves existing and adds new test coverage for the `-maxuploadtarget` mechanism (feature_maxuploadtarget.py) in the following ways, one commit each:
* verify the uploadtarget state via the `getnettotals` RPC (`uploadtarget` result field):
https://github.com/bitcoin/bitcoin/blob/160d23677ad799cf9b493eaa923b2ac080c3fb8e/src/rpc/net.cpp#L581-L582
Note that reaching the total limit (`target_reached` == True) always implies that the historical blocks serving limits is also reached (`serv
...
🤔 mzumsande reviewed a pull request: "net: Continuous ASMap health check"
(https://github.com/bitcoin/bitcoin/pull/27581#pullrequestreview-1763108020)
ACK 3ea54e5db7d53da5afa321e1800c29aa269dd3b3
(https://github.com/bitcoin/bitcoin/pull/27581#pullrequestreview-1763108020)
ACK 3ea54e5db7d53da5afa321e1800c29aa269dd3b3
🤔 jonatack reviewed a pull request: "rpc: encryptwallet help, mention HD seed rotation and backup requirement"
(https://github.com/bitcoin/bitcoin/pull/28980#pullrequestreview-1763137265)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/28980#pullrequestreview-1763137265)
Concept ACK
💬 jonatack commented on pull request "rpc: encryptwallet help, mention HD seed rotation and backup requirement":
(https://github.com/bitcoin/bitcoin/pull/28980#discussion_r1414333653)
s/file, containing/file that contains/
Suggest:
"Therefore, it is crucial to make a fresh backup of the newly generated wallet file."
(https://github.com/bitcoin/bitcoin/pull/28980#discussion_r1414333653)
s/file, containing/file that contains/
Suggest:
"Therefore, it is crucial to make a fresh backup of the newly generated wallet file."
💬 jonatack commented on pull request "rpc: encryptwallet help, mention HD seed rotation and backup requirement":
(https://github.com/bitcoin/bitcoin/pull/28980#discussion_r1414328971)
"backup" is a noun spelled as a single word, while "back up" is a verb spelled as 2 words
```suggestion
"Please exercise caution and ensure that you securely back up the updated wallet file to safeguard your funds and wallet information.\n",
```
(https://github.com/bitcoin/bitcoin/pull/28980#discussion_r1414328971)
"backup" is a noun spelled as a single word, while "back up" is a verb spelled as 2 words
```suggestion
"Please exercise caution and ensure that you securely back up the updated wallet file to safeguard your funds and wallet information.\n",
```
👍 romanz approved a pull request: "init: don't delete PID file if it was not generated"
(https://github.com/bitcoin/bitcoin/pull/28946#pullrequestreview-1763165176)
ACK https://github.com/bitcoin/bitcoin/commit/8f6ab318635d823a7e6ad9020ac17b8d2dd0e91b
(https://github.com/bitcoin/bitcoin/pull/28946#pullrequestreview-1763165176)
ACK https://github.com/bitcoin/bitcoin/commit/8f6ab318635d823a7e6ad9020ac17b8d2dd0e91b
👋 instagibbs's pull request is ready for review: "Cluster size 2 package rbf"
(https://github.com/bitcoin/bitcoin/pull/28984)
(https://github.com/bitcoin/bitcoin/pull/28984)
💬 murchandamus commented on pull request "wallet: Add CoinGrinder coin selection algorithm":
(https://github.com/bitcoin/bitcoin/pull/27877#issuecomment-1839309807)
> The PR description should say that we don't use hardcoded fee rate value, but rather multiple of long-term-fee-rate. LTFR is an existing user configurable parameter. It's also a good fit for the purpose of determining when to be more "aggressive" with coin selection, because it captures forward looking fee market expectations.
Thanks, I’ve amended the description.
(https://github.com/bitcoin/bitcoin/pull/27877#issuecomment-1839309807)
> The PR description should say that we don't use hardcoded fee rate value, but rather multiple of long-term-fee-rate. LTFR is an existing user configurable parameter. It's also a good fit for the purpose of determining when to be more "aggressive" with coin selection, because it captures forward looking fee market expectations.
Thanks, I’ve amended the description.
💬 instagibbs commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1414385368)
this can't be hit yet; post-v3 it can. I can remove this, or leave a note. It makes some package rbfs more useful
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1414385368)
this can't be hit yet; post-v3 it can. I can remove this, or leave a note. It makes some package rbfs more useful
💬 achow101 commented on pull request "init: don't delete PID file if it was not generated":
(https://github.com/bitcoin/bitcoin/pull/28946#issuecomment-1839330822)
ACK 8f6ab318635d823a7e6ad9020ac17b8d2dd0e91b
(https://github.com/bitcoin/bitcoin/pull/28946#issuecomment-1839330822)
ACK 8f6ab318635d823a7e6ad9020ac17b8d2dd0e91b
🚀 achow101 merged a pull request: "init: don't delete PID file if it was not generated"
(https://github.com/bitcoin/bitcoin/pull/28946)
(https://github.com/bitcoin/bitcoin/pull/28946)
📝 instagibbs opened a pull request: "fuzz: txorphan check wtxids using GenTxid::Wtxid not GenTxid::Txid"
(https://github.com/bitcoin/bitcoin/pull/28997)
Fixes the bugs in the fuzz test with no more changes as an alternative to https://github.com/bitcoin/bitcoin/pull/28658
(https://github.com/bitcoin/bitcoin/pull/28997)
Fixes the bugs in the fuzz test with no more changes as an alternative to https://github.com/bitcoin/bitcoin/pull/28658