👍 stickies-v approved a pull request: "docs: add ramdisk guide for running tests on OSX"
(https://github.com/bitcoin/bitcoin/pull/27124)
ACK ab6f73a1f65bcfd59fea07f3067312a757dba2f8
I use this command on my M1 too.
(https://github.com/bitcoin/bitcoin/pull/27124)
ACK ab6f73a1f65bcfd59fea07f3067312a757dba2f8
I use this command on my M1 too.
💬 stickies-v commented on pull request "docs: add ramdisk guide for running tests on OSX":
(https://github.com/bitcoin/bitcoin/pull/27124#discussion_r1114256522)
nit: I think the formula can be expressed a bit more useable like this:
```suggestion
(`4096 MiB * 2048 blocks/MiB = 8388608 blocks` for 4GiB). To run the tests using the RAM disk:
```
(https://github.com/bitcoin/bitcoin/pull/27124#discussion_r1114256522)
nit: I think the formula can be expressed a bit more useable like this:
```suggestion
(`4096 MiB * 2048 blocks/MiB = 8388608 blocks` for 4GiB). To run the tests using the RAM disk:
```
💬 TheCharlatan commented on issue "`libbitcoinkernel`: Building `mingw-w64` dll's broken":
(https://github.com/bitcoin/bitcoin/issues/25008#issuecomment-1439953266)
> Mind testing https://github.com/theuni/bitcoin/commits/fix-dll-exports ? I'll PR if it works for you.
Mmh, I'm surprised that secp needs to be linked as well by the final binary and I'm not quite following why. What's different between the visibility of say linking leveldb and linking libsecp in libbitcoinkernel so these symbols end up undefined in the bitcoin-chainstate binary?
(https://github.com/bitcoin/bitcoin/issues/25008#issuecomment-1439953266)
> Mind testing https://github.com/theuni/bitcoin/commits/fix-dll-exports ? I'll PR if it works for you.
Mmh, I'm surprised that secp needs to be linked as well by the final binary and I'm not quite following why. What's different between the visibility of say linking leveldb and linking libsecp in libbitcoinkernel so these symbols end up undefined in the bitcoin-chainstate binary?
👍 TheCharlatan approved a pull request: "rpc, test: remove newline escape sequence from wallet warning fields"
(https://github.com/bitcoin/bitcoin/pull/27138)
ACK f0391cd3ea562d526db63996e7694ff4cd44b8b2
Was a bit hesitant about adding the messages to the functional tests, but I think testing the correct concatenation kind of makes sense.
(https://github.com/bitcoin/bitcoin/pull/27138)
ACK f0391cd3ea562d526db63996e7694ff4cd44b8b2
Was a bit hesitant about adding the messages to the functional tests, but I think testing the correct concatenation kind of makes sense.
💬 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