💬 fanquake commented on pull request "ci: A few fixes of `ccache` issues":
(https://github.com/bitcoin/bitcoin/pull/27084#issuecomment-1437320675)
Most of the changes here look straight forward enough, and clearly improve `ccache` usage, but this is blocked on at least an answer to this question: https://github.com/bitcoin/bitcoin/pull/27084#discussion_r1104757809.
(https://github.com/bitcoin/bitcoin/pull/27084#issuecomment-1437320675)
Most of the changes here look straight forward enough, and clearly improve `ccache` usage, but this is blocked on at least an answer to this question: https://github.com/bitcoin/bitcoin/pull/27084#discussion_r1104757809.
📝 fanquake review_request_removed a pull request: "wallet: be able to specify a wallet name and passphrase to migratewallet"
(https://github.com/bitcoin/bitcoin/pull/26595)
`migratewallet` currently operates on wallets that are already loaded, however this is not necessarily required, and in the future, not possible once the legacy wallet is removed. So we need to also be able to give the wallet name to migrate.
Additionally, the passphrase is required when migrating a wallet. Since a wallet may not be loaded when we migrate, and as we currently unload wallets when migrating, we need the passphrase to be given to `migratewallet` in order to migrate encrypted wal
...
(https://github.com/bitcoin/bitcoin/pull/26595)
`migratewallet` currently operates on wallets that are already loaded, however this is not necessarily required, and in the future, not possible once the legacy wallet is removed. So we need to also be able to give the wallet name to migrate.
Additionally, the passphrase is required when migrating a wallet. Since a wallet may not be loaded when we migrate, and as we currently unload wallets when migrating, we need the passphrase to be given to `migratewallet` in order to migrate encrypted wal
...
💬 fanquake commented on pull request "wallet: be able to specify a wallet name and passphrase to migratewallet":
(https://github.com/bitcoin/bitcoin/pull/26595#issuecomment-1437342993)
@pinheadmz review?
(https://github.com/bitcoin/bitcoin/pull/26595#issuecomment-1437342993)
@pinheadmz review?
✅ fanquake closed an issue: "every other change address is unused"
(https://github.com/bitcoin/bitcoin/issues/27051)
(https://github.com/bitcoin/bitcoin/issues/27051)
🚀 fanquake merged a pull request: "wallet: reuse change dest when re-creating TX with avoidpartialspends"
(https://github.com/bitcoin/bitcoin/pull/27053)
(https://github.com/bitcoin/bitcoin/pull/27053)
💬 fanquake commented on pull request "wallet: reuse change dest when re-creating TX with avoidpartialspends":
(https://github.com/bitcoin/bitcoin/pull/27053#issuecomment-1437347735)
Added to #26878 for backporting into 24.x.
(https://github.com/bitcoin/bitcoin/pull/27053#issuecomment-1437347735)
Added to #26878 for backporting into 24.x.
📝 fanquake review_request_removed a pull request: "wallet: reuse change dest when re-creating TX with avoidpartialspends"
(https://github.com/bitcoin/bitcoin/pull/27053)
Closes https://github.com/bitcoin/bitcoin/issues/27051
When the wallet creates a transaction internally, it will also create an alternative that spends using destination groups and see if the fee difference is negligible. If it costs the user the same to send the grouped version, we send it (even if the user has `avoidpartialspends` set to `false` which is default). This patch ensures that the second transaction creation attempt re-uses the change destination selected by the first attempt. Ot
...
(https://github.com/bitcoin/bitcoin/pull/27053)
Closes https://github.com/bitcoin/bitcoin/issues/27051
When the wallet creates a transaction internally, it will also create an alternative that spends using destination groups and see if the fee difference is negligible. If it costs the user the same to send the grouped version, we send it (even if the user has `avoidpartialspends` set to `false` which is default). This patch ensures that the second transaction creation attempt re-uses the change destination selected by the first attempt. Ot
...
📝 fanquake review_request_removed a pull request: "wallet: reuse change dest when re-creating TX with avoidpartialspends"
(https://github.com/bitcoin/bitcoin/pull/27053)
Closes https://github.com/bitcoin/bitcoin/issues/27051
When the wallet creates a transaction internally, it will also create an alternative that spends using destination groups and see if the fee difference is negligible. If it costs the user the same to send the grouped version, we send it (even if the user has `avoidpartialspends` set to `false` which is default). This patch ensures that the second transaction creation attempt re-uses the change destination selected by the first attempt. Ot
...
(https://github.com/bitcoin/bitcoin/pull/27053)
Closes https://github.com/bitcoin/bitcoin/issues/27051
When the wallet creates a transaction internally, it will also create an alternative that spends using destination groups and see if the fee difference is negligible. If it costs the user the same to send the grouped version, we send it (even if the user has `avoidpartialspends` set to `false` which is default). This patch ensures that the second transaction creation attempt re-uses the change destination selected by the first attempt. Ot
...
📝 RandyMcMillan opened a pull request: "build-osx.md: recommend brew install python@3.7 (minimum version)"
(https://github.com/bitcoin/bitcoin/pull/27130)
null
(https://github.com/bitcoin/bitcoin/pull/27130)
null
💬 fanquake commented on pull request "build-osx.md: recommend brew install python@3.7 (minimum version)":
(https://github.com/bitcoin/bitcoin/pull/27130#issuecomment-1437356961)
NACK. There's no need to install an old version of Python.
(https://github.com/bitcoin/bitcoin/pull/27130#issuecomment-1437356961)
NACK. There's no need to install an old version of Python.
💬 fanquake commented on pull request "ci: Cache more stuff in the ci images: msan, iwyu, pip, sdks":
(https://github.com/bitcoin/bitcoin/pull/27028#discussion_r1112210670)
Are these (doing) the same thing? I see they are used in [the same context](https://github.com/bitcoin/bitcoin/pull/27028/commits/fabcf65e46f1b5b5914b9738a87375ad750c5c7f#diff-b088baf93122a28210fe5ebdb9d5c3100813aac183b8362095c2c2d05051d70dR49-R52), immediately after one another. Why the new func? Maybe a comment to explain the difference?
(https://github.com/bitcoin/bitcoin/pull/27028#discussion_r1112210670)
Are these (doing) the same thing? I see they are used in [the same context](https://github.com/bitcoin/bitcoin/pull/27028/commits/fabcf65e46f1b5b5914b9738a87375ad750c5c7f#diff-b088baf93122a28210fe5ebdb9d5c3100813aac183b8362095c2c2d05051d70dR49-R52), immediately after one another. Why the new func? Maybe a comment to explain the difference?
💬 ajtowns commented on pull request "mempool / miner: regularly flush below-minrelayfeerate entries, mine everything in the mempool":
(https://github.com/bitcoin/bitcoin/pull/27018#discussion_r1112211106)
If we want to support miners being able to guarantee the feerate of their block won't be lower than `x`, then we shouldn't be removing/deprecating `-blockmintxfee` ?
I think it makes sense to have "ancestor fee(rate) is at 0sat? we're done here" logic; miners can always modify the fee if the tx is desired anyway...
(https://github.com/bitcoin/bitcoin/pull/27018#discussion_r1112211106)
If we want to support miners being able to guarantee the feerate of their block won't be lower than `x`, then we shouldn't be removing/deprecating `-blockmintxfee` ?
I think it makes sense to have "ancestor fee(rate) is at 0sat? we're done here" logic; miners can always modify the fee if the tx is desired anyway...
💬 fanquake commented on pull request "Support JSON-RPC 2.0 when requested by client":
(https://github.com/bitcoin/bitcoin/pull/27101#issuecomment-1437361943)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/27101#issuecomment-1437361943)
Concept ACK
💬 RandyMcMillan commented on pull request "doc: recommend brew install python@3.7 (minimum version)":
(https://github.com/bitcoin/bitcoin/pull/27130#issuecomment-1437364918)
updated :
```brew install python@3```
https://github.com/bitcoin/bitcoin/blob/b2f2951e51b0be370041ec63527bad8fc0dab3ad/doc/build-osx.md#test-suite-dependencies
(https://github.com/bitcoin/bitcoin/pull/27130#issuecomment-1437364918)
updated :
```brew install python@3```
https://github.com/bitcoin/bitcoin/blob/b2f2951e51b0be370041ec63527bad8fc0dab3ad/doc/build-osx.md#test-suite-dependencies
💬 fanquake commented on pull request "doc: recommend brew install python@3.7 (minimum version)":
(https://github.com/bitcoin/bitcoin/pull/27130#issuecomment-1437365005)
NACK - `python` and `python@3` are the same thing (both aliases for python 3.11, which is the current latest version). We can leave these docs as-is, and/or worry about changing them when Python 4 is (never) released. Thanks.
(https://github.com/bitcoin/bitcoin/pull/27130#issuecomment-1437365005)
NACK - `python` and `python@3` are the same thing (both aliases for python 3.11, which is the current latest version). We can leave these docs as-is, and/or worry about changing them when Python 4 is (never) released. Thanks.
✅ fanquake closed a pull request: "doc: recommend brew install python@3.7 (minimum version)"
(https://github.com/bitcoin/bitcoin/pull/27130)
(https://github.com/bitcoin/bitcoin/pull/27130)
💬 fanquake commented on pull request "github: Switch to yaml issue templates":
(https://github.com/bitcoin/bitcoin/pull/27025#issuecomment-1437374469)
> I suppose one other question is whether having more/mandatory questions will put people off submitting issues,
If people can't be bothered to supply what we think is the minimum required information, then we will likely be unable to debug anything anyways. However, sometimes (fixable/informative) issues do come in the form of a randonly pasted debug log or similar. So if there is still an option for someone to dump something, I think this is ok.
@achow101 @glozow @MarcoFalke @hebasto any
...
(https://github.com/bitcoin/bitcoin/pull/27025#issuecomment-1437374469)
> I suppose one other question is whether having more/mandatory questions will put people off submitting issues,
If people can't be bothered to supply what we think is the minimum required information, then we will likely be unable to debug anything anyways. However, sometimes (fixable/informative) issues do come in the form of a randonly pasted debug log or similar. So if there is still an option for someone to dump something, I think this is ok.
@achow101 @glozow @MarcoFalke @hebasto any
...
💬 jonatack commented on pull request "docs: add ramdisk guide for running tests on OSX":
(https://github.com/bitcoin/bitcoin/pull/27124#discussion_r1111119346)
Unsure, but I thought "macOS" was the naming nowadays.
```suggestion
**macOS**
```
(https://github.com/bitcoin/bitcoin/pull/27124#discussion_r1111119346)
Unsure, but I thought "macOS" was the naming nowadays.
```suggestion
**macOS**
```
💬 jonatack commented on pull request "docs: add ramdisk guide for running tests on OSX":
(https://github.com/bitcoin/bitcoin/pull/27124#discussion_r1111119554)
```suggestion
(`8388608 / 2048 = 4096` or 4 GiB). To run the tests using the RAM disk:
```
(https://github.com/bitcoin/bitcoin/pull/27124#discussion_r1111119554)
```suggestion
(`8388608 / 2048 = 4096` or 4 GiB). To run the tests using the RAM disk:
```
💬 jonatack commented on pull request "docs: add ramdisk guide for running tests on OSX":
(https://github.com/bitcoin/bitcoin/pull/27124#discussion_r1111119386)
Not sure if it's needed to restate the OS here and line 119 above, plus "GiB", and no strong opinion about "ramdisk" but it looks like the most common spelling is "RAM disk" (https://en.wikipedia.org/wiki/RAM_disk).
```suggestion
To create a 4 GiB RAM disk named "ramdisk" at `/Volumes/ramdisk/`:
```
(https://github.com/bitcoin/bitcoin/pull/27124#discussion_r1111119386)
Not sure if it's needed to restate the OS here and line 119 above, plus "GiB", and no strong opinion about "ramdisk" but it looks like the most common spelling is "RAM disk" (https://en.wikipedia.org/wiki/RAM_disk).
```suggestion
To create a 4 GiB RAM disk named "ramdisk" at `/Volumes/ramdisk/`:
```