💬 Sjors commented on pull request "wallet: skip R-value signature grinding for external signers":
(https://github.com/bitcoin/bitcoin/pull/26032#issuecomment-1439968821)
I considered switching to @S3RK's approach of adding extra argument to `DummySignInput` rather than expanding `CCoinControl`. The result is here: https://github.com/Sjors/bitcoin/tree/2023/02/external-signer-feerate-leave-ccoincontrol-alone
The downside of this is approach is that I had to sprinkle `const bool can_grind_r = !wallet->IsWalletFlagSet(WALLET_FLAG_EXTERNAL_SIGNER);` in several different places. This is fine right now, but at some point we might have some external signers that do
...
(https://github.com/bitcoin/bitcoin/pull/26032#issuecomment-1439968821)
I considered switching to @S3RK's approach of adding extra argument to `DummySignInput` rather than expanding `CCoinControl`. The result is here: https://github.com/Sjors/bitcoin/tree/2023/02/external-signer-feerate-leave-ccoincontrol-alone
The downside of this is approach is that I had to sprinkle `const bool can_grind_r = !wallet->IsWalletFlagSet(WALLET_FLAG_EXTERNAL_SIGNER);` in several different places. This is fine right now, but at some point we might have some external signers that do
...
💬 brunoerg commented on pull request "rpc: Add test-only RPC getaddrmaninfo for new/tried table address count":
(https://github.com/bitcoin/bitcoin/pull/26988#issuecomment-1440011972)
```sh
"network" : "str", (string) The network (ipv4, ipv6, onion, i2p, cjdns)
```
Instead of being a string, wouldn't make sense it to be an array? E.g. I want to get new/tried table address count for ipv5 and ipv6 together.
(https://github.com/bitcoin/bitcoin/pull/26988#issuecomment-1440011972)
```sh
"network" : "str", (string) The network (ipv4, ipv6, onion, i2p, cjdns)
```
Instead of being a string, wouldn't make sense it to be an array? E.g. I want to get new/tried table address count for ipv5 and ipv6 together.
👍 theStack approved a pull request: "test: Raise PRNG seed log to INFO"
(https://github.com/bitcoin/bitcoin/pull/27137)
ACK 4d84eaec82e7b5a450d47cd30e5936a717035f77
(https://github.com/bitcoin/bitcoin/pull/27137)
ACK 4d84eaec82e7b5a450d47cd30e5936a717035f77
📝 roconnor-blockstream opened a pull request: "test: Replace 0xC0 constant"
(https://github.com/bitcoin/bitcoin/pull/27143)
Instead it should be the named constant `LEAF_VERSION_TAPSCRIPT`.
(https://github.com/bitcoin/bitcoin/pull/27143)
Instead it should be the named constant `LEAF_VERSION_TAPSCRIPT`.
💬 instagibbs commented on pull request "test: Replace 0xC0 constant":
(https://github.com/bitcoin/bitcoin/pull/27143#issuecomment-1440091525)
Since we're already here, we could replace the rest of the instances of this variable too?
(https://github.com/bitcoin/bitcoin/pull/27143#issuecomment-1440091525)
Since we're already here, we could replace the rest of the instances of this variable too?
💬 roconnor-blockstream commented on pull request "test: Replace 0xC0 constant":
(https://github.com/bitcoin/bitcoin/pull/27143#issuecomment-1440101100)
I am not aware of other instances.
(https://github.com/bitcoin/bitcoin/pull/27143#issuecomment-1440101100)
I am not aware of other instances.
👍 john-moffett approved a pull request: "wallet: be able to specify a wallet name and passphrase to migratewallet"
(https://github.com/bitcoin/bitcoin/pull/26595)
reACK 9486509be65f09174a0cb50a337cac58a0c09de4
(https://github.com/bitcoin/bitcoin/pull/26595)
reACK 9486509be65f09174a0cb50a337cac58a0c09de4
💬 instagibbs commented on pull request "test: Replace 0xC0 constant":
(https://github.com/bitcoin/bitcoin/pull/27143#issuecomment-1440153890)
here and `script_lists` a few lines below it https://github.com/bitcoin/bitcoin/blob/master/test/functional/feature_taproot.py#L1548
(https://github.com/bitcoin/bitcoin/pull/27143#issuecomment-1440153890)
here and `script_lists` a few lines below it https://github.com/bitcoin/bitcoin/blob/master/test/functional/feature_taproot.py#L1548
💬 pinheadmz commented on issue "Bech32 address generation in regtest does not have the prefix `tb`":
(https://github.com/bitcoin/bitcoin/issues/12314#issuecomment-1440185562)
also bcoin: https://github.com/bcoin-org/bcoin/blob/57f59569116a1dede3d7d97b595a34809aaace5a/lib/protocol/networks.js#L867
(https://github.com/bitcoin/bitcoin/issues/12314#issuecomment-1440185562)
also bcoin: https://github.com/bcoin-org/bcoin/blob/57f59569116a1dede3d7d97b595a34809aaace5a/lib/protocol/networks.js#L867
💬 roconnor-blockstream commented on pull request "test: Replace 0xC0 constant":
(https://github.com/bitcoin/bitcoin/pull/27143#issuecomment-1440187848)
*lol* I'm embarrassed to say, I was only looking at Elements code.
(https://github.com/bitcoin/bitcoin/pull/27143#issuecomment-1440187848)
*lol* I'm embarrassed to say, I was only looking at Elements code.
💬 hebasto commented on pull request "clang-tidy: Add more `performance-*` checks and related fixes":
(https://github.com/bitcoin/bitcoin/pull/26642#issuecomment-1440196497)
Updated 2cdb84ca4c96cef03946b14ec46a7720f3ece098 -> 7db868b49b60de371d01cda1f8e5faaa0e18f822 ([pr26642.09](https://github.com/hebasto/bitcoin/commits/pr26642.09) -> [pr26642.10](https://github.com/hebasto/bitcoin/commits/pr26642.10)):
- rebased
- addressed @martinus's https://github.com/bitcoin/bitcoin/pull/26642#discussion_r1108879800
---
@martinus [wrote](https://github.com/bitcoin/bitcoin/pull/26642#discussion_r1108879800):
> I ran clang-tidy on this PR, and found a few more warnin
...
(https://github.com/bitcoin/bitcoin/pull/26642#issuecomment-1440196497)
Updated 2cdb84ca4c96cef03946b14ec46a7720f3ece098 -> 7db868b49b60de371d01cda1f8e5faaa0e18f822 ([pr26642.09](https://github.com/hebasto/bitcoin/commits/pr26642.09) -> [pr26642.10](https://github.com/hebasto/bitcoin/commits/pr26642.10)):
- rebased
- addressed @martinus's https://github.com/bitcoin/bitcoin/pull/26642#discussion_r1108879800
---
@martinus [wrote](https://github.com/bitcoin/bitcoin/pull/26642#discussion_r1108879800):
> I ran clang-tidy on this PR, and found a few more warnin
...
💬 Sjors commented on pull request "bumpfee: Allow the user to choose which output is change":
(https://github.com/bitcoin/bitcoin/pull/26467#issuecomment-1440197785)
utACK 4167843e0729994954317e2a4ac8a96a453bad79. The latest rebase takes the new `outputs` argument into account from #25344. It ensures they can't be combined, so that's good.
(https://github.com/bitcoin/bitcoin/pull/26467#issuecomment-1440197785)
utACK 4167843e0729994954317e2a4ac8a96a453bad79. The latest rebase takes the new `outputs` argument into account from #25344. It ensures they can't be combined, so that's good.
💬 hebasto commented on pull request "clang-tidy: Add more `performance-*` checks and related fixes":
(https://github.com/bitcoin/bitcoin/pull/26642#discussion_r1114448106)
Thanks! [Fixed](https://github.com/bitcoin/bitcoin/pull/26642#issuecomment-1440196497).
(https://github.com/bitcoin/bitcoin/pull/26642#discussion_r1114448106)
Thanks! [Fixed](https://github.com/bitcoin/bitcoin/pull/26642#issuecomment-1440196497).
💬 hebasto commented on pull request "clang-tidy: Add more `performance-*` checks and related fixes":
(https://github.com/bitcoin/bitcoin/pull/26642#discussion_r1114449588)
> I wonder why clang-tidy doesn't see these
So do I :)
(https://github.com/bitcoin/bitcoin/pull/26642#discussion_r1114449588)
> I wonder why clang-tidy doesn't see these
So do I :)
👍 pinheadmz approved a pull request: "wallet: be able to specify a wallet name and passphrase to migratewallet"
(https://github.com/bitcoin/bitcoin/pull/26595)
ACK 9486509be65f09174a0cb50a337cac58a0c09de4
reviewed code, ran tests, tested on regtest with cli commands
<details><summary>Show Signature</summary>
```
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256
ACK 9486509be65f09174a0cb50a337cac58a0c09de4
-----BEGIN PGP SIGNATURE-----
iQIzBAEBCAAdFiEE5hdzzW4BBA4vG9eM5+KYS2KJyToFAmP2L3gACgkQ5+KYS2KJ
yToaJA/9GCc9Y84y5GfTPsmAUUYV/jFKr5Z9lDIATQca+Ou323CQ76CF00IQiyBB
ECtPlFJSPxaaKac4k5QmwT4BA4EgzcTKKVhjCVlorHyJQYk5yYBD5xOk2G0fPthS
VA3Ckmi
...
(https://github.com/bitcoin/bitcoin/pull/26595)
ACK 9486509be65f09174a0cb50a337cac58a0c09de4
reviewed code, ran tests, tested on regtest with cli commands
<details><summary>Show Signature</summary>
```
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256
ACK 9486509be65f09174a0cb50a337cac58a0c09de4
-----BEGIN PGP SIGNATURE-----
iQIzBAEBCAAdFiEE5hdzzW4BBA4vG9eM5+KYS2KJyToFAmP2L3gACgkQ5+KYS2KJ
yToaJA/9GCc9Y84y5GfTPsmAUUYV/jFKr5Z9lDIATQca+Ou323CQ76CF00IQiyBB
ECtPlFJSPxaaKac4k5QmwT4BA4EgzcTKKVhjCVlorHyJQYk5yYBD5xOk2G0fPthS
VA3Ckmi
...
💬 pinheadmz commented on pull request "docs: add ramdisk guide for running tests on OSX":
(https://github.com/bitcoin/bitcoin/pull/27124#discussion_r1114482913)
sure, updated
(https://github.com/bitcoin/bitcoin/pull/27124#discussion_r1114482913)
sure, updated
💬 martinus commented on pull request "clang-tidy: Add more `performance-*` checks and related fixes":
(https://github.com/bitcoin/bitcoin/pull/26642#issuecomment-1440230390)
I don't know much about how the CI works, maybe it's possible to install a newer clang version? I'm using 15.0.7
(https://github.com/bitcoin/bitcoin/pull/26642#issuecomment-1440230390)
I don't know much about how the CI works, maybe it's possible to install a newer clang version? I'm using 15.0.7
👍 stickies-v approved a pull request: "docs: add ramdisk guide for running tests on OSX"
(https://github.com/bitcoin/bitcoin/pull/27124)
re-ACK cb7be3a
(https://github.com/bitcoin/bitcoin/pull/27124)
re-ACK cb7be3a
✅ MarcoFalke closed an issue: "Bech32 address generation in regtest does not have the prefix `tb`"
(https://github.com/bitcoin/bitcoin/issues/12314)
(https://github.com/bitcoin/bitcoin/issues/12314)
💬 MarcoFalke commented on issue "Bech32 address generation in regtest does not have the prefix `tb`":
(https://github.com/bitcoin/bitcoin/issues/12314#issuecomment-1440252506)
Closing due to lack of direction and progress. Pull requests with improvements are always welcome.
(https://github.com/bitcoin/bitcoin/issues/12314#issuecomment-1440252506)
Closing due to lack of direction and progress. Pull requests with improvements are always welcome.