💬 vasild commented on issue "Error: Cannot resolve -bind address: 'bitcoind:8334=onion'":
(https://github.com/bitcoin/bitcoin/issues/26484#issuecomment-3415416644)
@dayer3, so Tor daemon is running on another machine and you want to specifically use `-bind=hostname:8334=onion` in `bitcoind` config and not `-bind=a.b.c.d:8334=onion`? I am not sure if that is possible. This comment contains some workaround:
https://github.com/bitcoin/bitcoin/issues/26484#issuecomment-1311427984
(https://github.com/bitcoin/bitcoin/issues/26484#issuecomment-3415416644)
@dayer3, so Tor daemon is running on another machine and you want to specifically use `-bind=hostname:8334=onion` in `bitcoind` config and not `-bind=a.b.c.d:8334=onion`? I am not sure if that is possible. This comment contains some workaround:
https://github.com/bitcoin/bitcoin/issues/26484#issuecomment-1311427984
💬 instagibbs commented on pull request "doc: correct topology requirements in submitpackage helptext":
(https://github.com/bitcoin/bitcoin/pull/33630#discussion_r2439936432)
maybe?
```suggestion
"The package must consist of either: exactly 1 transaction, or 1 transaction with its unconfirmed parents. None of the parents may depend on each other. Not all in-mempool parents need to be present.\n"
```
(https://github.com/bitcoin/bitcoin/pull/33630#discussion_r2439936432)
maybe?
```suggestion
"The package must consist of either: exactly 1 transaction, or 1 transaction with its unconfirmed parents. None of the parents may depend on each other. Not all in-mempool parents need to be present.\n"
```
✅ waketraindev closed a pull request: "rpc, test: fix "internal" label check in importdescriptors and extend test"
(https://github.com/bitcoin/bitcoin/pull/33614)
(https://github.com/bitcoin/bitcoin/pull/33614)
💬 glozow commented on pull request "doc: correct topology requirements in submitpackage helptext":
(https://github.com/bitcoin/bitcoin/pull/33630#discussion_r2440039778)
Incorporated this as (some, all, or none of)
(https://github.com/bitcoin/bitcoin/pull/33630#discussion_r2440039778)
Incorporated this as (some, all, or none of)
💬 glozow commented on pull request "doc: correct topology requirements in submitpackage helptext":
(https://github.com/bitcoin/bitcoin/pull/33630#discussion_r2440040161)
Took this, but reworded a little bit.
(https://github.com/bitcoin/bitcoin/pull/33630#discussion_r2440040161)
Took this, but reworded a little bit.
💬 glozow commented on pull request "doc: correct topology requirements in submitpackage helptext":
(https://github.com/bitcoin/bitcoin/pull/33630#issuecomment-3415634149)
Thanks for the reviews! Re-pushed to address comments.
(https://github.com/bitcoin/bitcoin/pull/33630#issuecomment-3415634149)
Thanks for the reviews! Re-pushed to address comments.
💬 ismaelsadeeq commented on pull request "kernel: Introduce C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-3415649155)
re-ACK a755e00a13541b3b5a707cf385f1cbec0449c6a9 per https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-3409324684
(https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-3415649155)
re-ACK a755e00a13541b3b5a707cf385f1cbec0449c6a9 per https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-3409324684
💬 ismaelsadeeq commented on pull request "kernel: Introduce C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2440053480)
@TheCharlatan per https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-3409324684 you replace mentioned you add an assertion?
It seems not? it's okay with me though since the handle check for nullptr and throw.
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2440053480)
@TheCharlatan per https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-3409324684 you replace mentioned you add an assertion?
It seems not? it's okay with me though since the handle check for nullptr and throw.
🤔 janb84 reviewed a pull request: "doc: correct topology requirements in submitpackage helptext"
(https://github.com/bitcoin/bitcoin/pull/33630#pullrequestreview-3350421480)
ACK 3d222825642bfb052ce40cbc1c69318a0d8835bf
PR updates helptext of submitpackage to be up 2 date with the reent changes in #31385 . New help text reflects changes in #31385 and is clear.
Thanks for incorporating my nit!
(https://github.com/bitcoin/bitcoin/pull/33630#pullrequestreview-3350421480)
ACK 3d222825642bfb052ce40cbc1c69318a0d8835bf
PR updates helptext of submitpackage to be up 2 date with the reent changes in #31385 . New help text reflects changes in #31385 and is clear.
Thanks for incorporating my nit!
👍 instagibbs approved a pull request: "doc: correct topology requirements in submitpackage helptext"
(https://github.com/bitcoin/bitcoin/pull/33630#pullrequestreview-3350431854)
ACK 3d222825642bfb052ce40cbc1c69318a0d8835bf
(https://github.com/bitcoin/bitcoin/pull/33630#pullrequestreview-3350431854)
ACK 3d222825642bfb052ce40cbc1c69318a0d8835bf
🚀 fanquake merged a pull request: "doc: correct topology requirements in submitpackage helptext"
(https://github.com/bitcoin/bitcoin/pull/33630)
(https://github.com/bitcoin/bitcoin/pull/33630)
💬 fanquake commented on pull request "doc: correct topology requirements in submitpackage helptext":
(https://github.com/bitcoin/bitcoin/pull/33630#issuecomment-3415826380)
Backported to `30.x` in #33609.
(https://github.com/bitcoin/bitcoin/pull/33630#issuecomment-3415826380)
Backported to `30.x` in #33609.
🤔 rkrux reviewed a pull request: "wallet: Expand MuSig test coverage and follow-ups"
(https://github.com/bitcoin/bitcoin/pull/33636#pullrequestreview-3350082420)
Concept ACK 73a5d9315340942f030e6586c640b9240498f8ac
Adds more functional tests that are preferred.
I also like the last commit 73a5d9315340942f030e6586c640b9240498f8ac quite a lot, thanks for adding it.
(https://github.com/bitcoin/bitcoin/pull/33636#pullrequestreview-3350082420)
Concept ACK 73a5d9315340942f030e6586c640b9240498f8ac
Adds more functional tests that are preferred.
I also like the last commit 73a5d9315340942f030e6586c640b9240498f8ac quite a lot, thanks for adding it.
💬 rkrux commented on pull request "wallet: Expand MuSig test coverage and follow-ups":
(https://github.com/bitcoin/bitcoin/pull/33636#discussion_r2439794359)
In c7c9dc2d8634bc79e76783be113f8d42cee63c6e "test: Add musig failure scenarios"
`Miniscript` pattern or `Multipath` pattern?
(https://github.com/bitcoin/bitcoin/pull/33636#discussion_r2439794359)
In c7c9dc2d8634bc79e76783be113f8d42cee63c6e "test: Add musig failure scenarios"
`Miniscript` pattern or `Multipath` pattern?
💬 rkrux commented on pull request "wallet: Expand MuSig test coverage and follow-ups":
(https://github.com/bitcoin/bitcoin/pull/33636#discussion_r2440002764)
In https://github.com/bitcoin/bitcoin/commit/c7c9dc2d8634bc79e76783be113f8d42cee63c6e "test: Add musig failure scenarios"
Nit: can rename this function to `test_success_case` now.
(https://github.com/bitcoin/bitcoin/pull/33636#discussion_r2440002764)
In https://github.com/bitcoin/bitcoin/commit/c7c9dc2d8634bc79e76783be113f8d42cee63c6e "test: Add musig failure scenarios"
Nit: can rename this function to `test_success_case` now.
💬 rkrux commented on pull request "wallet: Expand MuSig test coverage and follow-ups":
(https://github.com/bitcoin/bitcoin/pull/33636#discussion_r2440229322)
In https://github.com/bitcoin/bitcoin/commit/c7c9dc2d8634bc79e76783be113f8d42cee63c6e "test: Add musig failure scenarios"
I noticed some redundancy in both the success and failure cases and I tried to refactor this file based on the following points. Please find full diff below and consider extracting out commonalities.
1. Separated the loop in the `do_test` func so that wallet creation and expectation calculations are not together.
2. Because of the above separation, extracted `create_wa
...
(https://github.com/bitcoin/bitcoin/pull/33636#discussion_r2440229322)
In https://github.com/bitcoin/bitcoin/commit/c7c9dc2d8634bc79e76783be113f8d42cee63c6e "test: Add musig failure scenarios"
I noticed some redundancy in both the success and failure cases and I tried to refactor this file based on the following points. Please find full diff below and consider extracting out commonalities.
1. Separated the loop in the `do_test` func so that wallet creation and expectation calculations are not together.
2. Because of the above separation, extracted `create_wa
...
💬 rkrux commented on pull request "wallet: Expand MuSig test coverage and follow-ups":
(https://github.com/bitcoin/bitcoin/pull/33636#discussion_r2439990998)
In c7c9dc2d8634bc79e76783be113f8d42cee63c6e "test: Add musig failure scenarios"
Nit: extra blank line
(https://github.com/bitcoin/bitcoin/pull/33636#discussion_r2439990998)
In c7c9dc2d8634bc79e76783be113f8d42cee63c6e "test: Add musig failure scenarios"
Nit: extra blank line
💬 rkrux commented on pull request "wallet: Expand MuSig test coverage and follow-ups":
(https://github.com/bitcoin/bitcoin/pull/33636#discussion_r2440231846)
In https://github.com/bitcoin/bitcoin/commit/c7c9dc2d8634bc79e76783be113f8d42cee63c6e "test: Add musig failure scenarios"
I don't think this comment is necessary because the tests pass even if the failure cases run first.
(https://github.com/bitcoin/bitcoin/pull/33636#discussion_r2440231846)
In https://github.com/bitcoin/bitcoin/commit/c7c9dc2d8634bc79e76783be113f8d42cee63c6e "test: Add musig failure scenarios"
I don't think this comment is necessary because the tests pass even if the failure cases run first.
💬 fanquake commented on pull request "multiprocess: Don't require bitcoin -m argument when IPC options are used":
(https://github.com/bitcoin/bitcoin/pull/33229#issuecomment-3415896069)
Backported to `30.x` in #33609.
(https://github.com/bitcoin/bitcoin/pull/33229#issuecomment-3415896069)
Backported to `30.x` in #33609.
💬 sipa commented on pull request "refactor: optimize block index comparisons (1.4-7.7x faster)":
(https://github.com/bitcoin/bitcoin/pull/33637#discussion_r2440314209)
Mind using `bench.batch(n * n).units("cmp")` or so here, to put the output units in perspective?
(https://github.com/bitcoin/bitcoin/pull/33637#discussion_r2440314209)
Mind using `bench.batch(n * n).units("cmp")` or so here, to put the output units in perspective?