💬 vasild commented on pull request "multiprocess: Add bitcoin wrapper executable":
(https://github.com/bitcoin/bitcoin/pull/31375#discussion_r1963537775)
`tx` is missing and `test` is part of the internal commands, so:
```suggestion
{gui,daemon,rpc,wallet,tx,help}
```
(https://github.com/bitcoin/bitcoin/pull/31375#discussion_r1963537775)
`tx` is missing and `test` is part of the internal commands, so:
```suggestion
{gui,daemon,rpc,wallet,tx,help}
```
💬 vasild commented on pull request "multiprocess: Add bitcoin wrapper executable":
(https://github.com/bitcoin/bitcoin/pull/31375#discussion_r1963523131)
nit: unused include
(https://github.com/bitcoin/bitcoin/pull/31375#discussion_r1963523131)
nit: unused include
💬 vasild commented on pull request "multiprocess: Add bitcoin wrapper executable":
(https://github.com/bitcoin/bitcoin/pull/31375#discussion_r1963558283)
This prints "bitcoin gui". One might get the impression that the command is "bitcoin gui", whereas it is just "gui". For comparison:
```
$ git help
usage: git ... <command> [<args>]
...
clone Clone a repository into a new directory
init Create an empty Git repository or reinitialize an existing one
...
```
So maybe:
```suggestion
gui [ARGS] Start GUI, equivalent to running 'bitcoin-qt [ARGS]' or 'bitcoin-gui [ARGS]'.
```
(https://github.com/bitcoin/bitcoin/pull/31375#discussion_r1963558283)
This prints "bitcoin gui". One might get the impression that the command is "bitcoin gui", whereas it is just "gui". For comparison:
```
$ git help
usage: git ... <command> [<args>]
...
clone Clone a repository into a new directory
init Create an empty Git repository or reinitialize an existing one
...
```
So maybe:
```suggestion
gui [ARGS] Start GUI, equivalent to running 'bitcoin-qt [ARGS]' or 'bitcoin-gui [ARGS]'.
```
💬 fanquake commented on pull request "WIP: Permit Combiner to strip bip32_deriv information":
(https://github.com/bitcoin/bitcoin/pull/30341#issuecomment-2671829633)
> Looking for approach (N)ACK.
Maybe @achow101 ? Otherwise given no interest in this in 8 months, maybe we should just close.
(https://github.com/bitcoin/bitcoin/pull/30341#issuecomment-2671829633)
> Looking for approach (N)ACK.
Maybe @achow101 ? Otherwise given no interest in this in 8 months, maybe we should just close.
💬 fanquake commented on pull request "Wallet: (Refactor) GetBalance to calculate used balance":
(https://github.com/bitcoin/bitcoin/pull/29062#issuecomment-2671832453)
Moved to draft, given this doesn't seem to be active. @BrandonOdiwuor are you still working on this?
(https://github.com/bitcoin/bitcoin/pull/29062#issuecomment-2671832453)
Moved to draft, given this doesn't seem to be active. @BrandonOdiwuor are you still working on this?
📝 fanquake converted_to_draft a pull request: "Wallet: (Refactor) GetBalance to calculate used balance"
(https://github.com/bitcoin/bitcoin/pull/29062)
Add `GetFullBalance()` to compute `used` balance in `getbalances RPC` and on GUI [GUI PR #775](https://github.com/bitcoin-core/gui/pull/775)
_Check https://github.com/bitcoin/bitcoin/pull/28776#discussion_r1407875236 by @achow101_
> However, I think it would be better to move all of this into `GetBalance` itself and just have it always compute the used balance for us rather than having the caller do this extra computation.
### Update:
- Compute `used` balance from `GetBalance(...)`
(https://github.com/bitcoin/bitcoin/pull/29062)
Add `GetFullBalance()` to compute `used` balance in `getbalances RPC` and on GUI [GUI PR #775](https://github.com/bitcoin-core/gui/pull/775)
_Check https://github.com/bitcoin/bitcoin/pull/28776#discussion_r1407875236 by @achow101_
> However, I think it would be better to move all of this into `GetBalance` itself and just have it always compute the used balance for us rather than having the caller do this extra computation.
### Update:
- Compute `used` balance from `GetBalance(...)`
✅ fanquake closed a pull request: "rpc, cli: add getbalances#total, and use it for -getinfo"
(https://github.com/bitcoin/bitcoin/pull/31353)
(https://github.com/bitcoin/bitcoin/pull/31353)
💬 fanquake commented on pull request "rpc, cli: add getbalances#total, and use it for -getinfo":
(https://github.com/bitcoin/bitcoin/pull/31353#issuecomment-2671835110)
Closing for now, due to no/lack of followup. Please leave a comment, if you want this reopened.
(https://github.com/bitcoin/bitcoin/pull/31353#issuecomment-2671835110)
Closing for now, due to no/lack of followup. Please leave a comment, if you want this reopened.
💬 0xB10C commented on issue "ci: Intermittent failure "Could not resolve host: github.com"":
(https://github.com/bitcoin/bitcoin/issues/31889#issuecomment-2671844275)
Can confirm that reverting fc50bee9780c3d20ae391fa82b2a488e87459219 (with 7c54d9fb75bb7a630f56990beff48b302a8d2b41) resolves `Could not resolve host: github.com` on my runners
(https://github.com/bitcoin/bitcoin/issues/31889#issuecomment-2671844275)
Can confirm that reverting fc50bee9780c3d20ae391fa82b2a488e87459219 (with 7c54d9fb75bb7a630f56990beff48b302a8d2b41) resolves `Could not resolve host: github.com` on my runners
💬 vasild commented on pull request "multiprocess: Add bitcoin wrapper executable":
(https://github.com/bitcoin/bitcoin/pull/31375#discussion_r1963804202)
Ok, you can mark this as resolved if not already (I don't see if it is marked as resolved or not). I will followup some related discussion at https://github.com/bitcoin/bitcoin/pull/31375#discussion_r1952943101
(https://github.com/bitcoin/bitcoin/pull/31375#discussion_r1963804202)
Ok, you can mark this as resolved if not already (I don't see if it is marked as resolved or not). I will followup some related discussion at https://github.com/bitcoin/bitcoin/pull/31375#discussion_r1952943101
💬 darosior commented on pull request "qa: fix an off-by-one in utxo snapshot fuzz target and sanity check its snapshot data":
(https://github.com/bitcoin/bitcoin/pull/31910#issuecomment-2671849282)
Just pushed an update to have the sanity check in the fuzz target initialization instead. Despite the previous version being technically correct it was so for [non-obvious reasons](https://github.com/bitcoin/bitcoin/pull/31910#discussion_r1963724337) and it makes sense to have the sanity check next to where the value will be used.
You can check it serves its intended purpose with the following diff for instance:
```diff
diff --git a/src/test/util/mining.cpp b/src/test/util/mining.cpp
index
...
(https://github.com/bitcoin/bitcoin/pull/31910#issuecomment-2671849282)
Just pushed an update to have the sanity check in the fuzz target initialization instead. Despite the previous version being technically correct it was so for [non-obvious reasons](https://github.com/bitcoin/bitcoin/pull/31910#discussion_r1963724337) and it makes sense to have the sanity check next to where the value will be used.
You can check it serves its intended purpose with the following diff for instance:
```diff
diff --git a/src/test/util/mining.cpp b/src/test/util/mining.cpp
index
...
✅ fanquake closed a pull request: "Add `contrib/justfile` containing useful development workflow commands."
(https://github.com/bitcoin/bitcoin/pull/31292)
(https://github.com/bitcoin/bitcoin/pull/31292)
💬 fanquake commented on pull request "Add `contrib/justfile` containing useful development workflow commands.":
(https://github.com/bitcoin/bitcoin/pull/31292#issuecomment-2671849593)
Seems like there isn't yet agreement to add this here, so closing for now. Discussion can continue.
(https://github.com/bitcoin/bitcoin/pull/31292#issuecomment-2671849593)
Seems like there isn't yet agreement to add this here, so closing for now. Discussion can continue.
💬 fanquake commented on pull request "Wallet: "listreceivedby*" fix":
(https://github.com/bitcoin/bitcoin/pull/30972#issuecomment-2671852366)
Maybe @achow101 or @furszy want to review here?
(https://github.com/bitcoin/bitcoin/pull/30972#issuecomment-2671852366)
Maybe @achow101 or @furszy want to review here?
💬 l0rinc commented on pull request "test: cover base[32|58|64] with symmetric roundtrip fuzz (and padding) tests":
(https://github.com/bitcoin/bitcoin/pull/30746#discussion_r1963808690)
> how could a fuzz engine work around the checksum?
I was wondering the same, wasn't aware of the `qa-assets` project. I will contribute a few values after I'll understand how it works exactly, thanks for bringing it to my attention!
> However, given the removed unit test, I am not sure if the coverage is identical.
You mean https://github.com/bitcoin/bitcoin/pull/30746/files#diff-98e64689e3af8a62a1ad571f8a6fa93e4955eb73a16db94c4df5adb0e7d0282dL85-L99?
I meant to move it over to the f
...
(https://github.com/bitcoin/bitcoin/pull/30746#discussion_r1963808690)
> how could a fuzz engine work around the checksum?
I was wondering the same, wasn't aware of the `qa-assets` project. I will contribute a few values after I'll understand how it works exactly, thanks for bringing it to my attention!
> However, given the removed unit test, I am not sure if the coverage is identical.
You mean https://github.com/bitcoin/bitcoin/pull/30746/files#diff-98e64689e3af8a62a1ad571f8a6fa93e4955eb73a16db94c4df5adb0e7d0282dL85-L99?
I meant to move it over to the f
...
💬 fanquake commented on pull request "doc: navigate section links":
(https://github.com/bitcoin/bitcoin/pull/31670#discussion_r1963812607)
> I would be better to fix (or report) it upstream.
Yea I think so.
(https://github.com/bitcoin/bitcoin/pull/31670#discussion_r1963812607)
> I would be better to fix (or report) it upstream.
Yea I think so.
✅ fanquake closed a pull request: "doc: navigate section links"
(https://github.com/bitcoin/bitcoin/pull/31670)
(https://github.com/bitcoin/bitcoin/pull/31670)
💬 sipa commented on pull request "p2p: improve TxOrphanage denial of service bounds and increase -maxorphantxs":
(https://github.com/bitcoin/bitcoin/pull/31829#issuecomment-2671888414)
Sad to see this slip, but given the amount of changes and discoveries that necessitated them even in just the last week, it's probably the right decision.
(https://github.com/bitcoin/bitcoin/pull/31829#issuecomment-2671888414)
Sad to see this slip, but given the amount of changes and discoveries that necessitated them even in just the last week, it's probably the right decision.
💬 achow101 commented on pull request "guix: Notarize MacOS app bundle and codesign all MacOS and Windows binaries":
(https://github.com/bitcoin/bitcoin/pull/31407#issuecomment-2671912435)
> for [096525e](https://github.com/bitcoin/bitcoin/commit/096525e92cc2f5a4318bae13cedd2cf36b928d5f) did you only notarize the arm binaries and not x86?
Yes
> One more thing worth trying might be to notarize the tar.gz archive.
Only binaries are notarized, not archives.
> An earlier Apple Forum thread also suggests that the way to notarize these binaries is to put the contents of the tar.gz archive into a zip and then upload
That's what we're doing.
(https://github.com/bitcoin/bitcoin/pull/31407#issuecomment-2671912435)
> for [096525e](https://github.com/bitcoin/bitcoin/commit/096525e92cc2f5a4318bae13cedd2cf36b928d5f) did you only notarize the arm binaries and not x86?
Yes
> One more thing worth trying might be to notarize the tar.gz archive.
Only binaries are notarized, not archives.
> An earlier Apple Forum thread also suggests that the way to notarize these binaries is to put the contents of the tar.gz archive into a zip and then upload
That's what we're doing.
💬 glozow commented on pull request "p2p: improve TxOrphanage denial of service bounds and increase -maxorphantxs":
(https://github.com/bitcoin/bitcoin/pull/31829#issuecomment-2671912617)
I'm sad too! Seeing the stats from https://delvingbitcoin.org/t/stats-on-orphanage-overflows/1421 made this more pressing in my opinion, but it's not a regression. I think we can still try to consider small, obviously safe changes for v29, but this feels too big. I don't want to risk creating new DoS problems.
(https://github.com/bitcoin/bitcoin/pull/31829#issuecomment-2671912617)
I'm sad too! Seeing the stats from https://delvingbitcoin.org/t/stats-on-orphanage-overflows/1421 made this more pressing in my opinion, but it's not a regression. I think we can still try to consider small, obviously safe changes for v29, but this feels too big. I don't want to risk creating new DoS problems.