Bitcoin Core Github
44 subscribers
121K links
Download Telegram
💬 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
...
💬 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.
👍 theStack approved a pull request: "test: Raise PRNG seed log to INFO"
(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`.
💬 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?
💬 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.
👍 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
💬 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
💬 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.
💬 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
...
💬 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.
💬 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).
💬 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 :)
👍 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
...
💬 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
💬 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
👍 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
MarcoFalke closed an issue: "Bech32 address generation in regtest does not have the prefix `tb`"
(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.