💬 jonatack commented on pull request "I2P network optimizations":
(https://github.com/bitcoin/bitcoin/pull/26837#issuecomment-1433431073)
FWIW, I2P adoption by Bitcoin nodes has been increasing significantly over the past 3 months. From 200-350 in November to over 1100 active peers now, possibly spurred in part by Umbrel and Raspiblitz adding I2P support in December.

<img width="210" alt="Screenshot 2023-02-15 at 08 03 47" src="https://user-images.githubusercontent.com/2415484/219438014-2ee0fe24-2955-4b0b-9f60-b96cc9fbd73c.png">

Anyone else like to have a look at testing/reviewing this PR?

💬 jonatack commented on pull request "script: add description for the functionality of each opcode":
(https://github.com/bitcoin/bitcoin/pull/27109#discussion_r1108823232)
Here and elsewhere in your changes.
```suggestion
OP_RESERVED = 0x50, // mark tx invalid unless occurring in an unexecuted OP_IF branch
```

(You can run a spelling check by running `test/lint/lint-spelling.py` from the repository root.)

💬 jonatack commented on pull request "script: add description for the functionality of each opcode":
(https://github.com/bitcoin/bitcoin/pull/27109#discussion_r1108827460)
```suggestion
OP_ENDIF = 0x68, // end if/else block (must include, otherwise tx becomes invalid)
```

💬 jonatack commented on pull request "script: add description for the functionality of each opcode":
(https://github.com/bitcoin/bitcoin/pull/27109#discussion_r1108831224)
Here and in the next 8 lines and also in additional lines after that.
```suggestion
OP_CAT = 0x7e, // disabled, fail the script unconditionally (apply in an unexecuted conditional branch)
```

💬 jonatack commented on pull request "script: add description for the functionality of each opcode":
(https://github.com/bitcoin/bitcoin/pull/27109#discussion_r1108829784)
```suggestion
OP_2DUP = 0x6e, // duplicate top and second from top stack items
```

💬 jonatack commented on pull request "script: add description for the functionality of each opcode":
(https://github.com/bitcoin/bitcoin/pull/27109#discussion_r1108829461)
```suggestion
OP_3DUP = 0x6f, // duplicate top, second from top and third from top stack items
```

💬 jonatack commented on pull request "script: add description for the functionality of each opcode":
(https://github.com/bitcoin/bitcoin/pull/27109#discussion_r1108833548)
```suggestion
OP_NEGATE = 0x8f, // multiply the top stack item by -1
```

💬 martinus commented on pull request "clang-tidy: Add more `performance-*` checks and related fixes":
(https://github.com/bitcoin/bitcoin/pull/26642#discussion_r1108845830)
Good call, I find the `noexcept` stuff so brittle it's really hard to use. E.g. my changes to nanobench.h too were not completely correct because there's no guarantee that `std::string` move assignment is noexcept, see here: https://github.com/martinus/nanobench/pull/87

💬 RandyMcMillan commented on pull request "script: add description for the functionality of each opcode":
(https://github.com/bitcoin/bitcoin/pull/27109#issuecomment-1433504037)
Concept ACK

💬 kristapsk commented on pull request "script: add description for the functionality of each opcode":
(https://github.com/bitcoin/bitcoin/pull/27109#issuecomment-1433509860)
Concept ACK

💬 sipa commented on pull request "script: add description for the functionality of each opcode":
(https://github.com/bitcoin/bitcoin/pull/27109#discussion_r1108865326)
It pushes `"\x81"` (the byte array consisting of a single byte with value 129).

💬 sipa commented on pull request "script: add description for the functionality of each opcode":
(https://github.com/bitcoin/bitcoin/pull/27109#discussion_r1108865017)
I'd just say `""` or `the empty array`. "\x" isn't any valid syntax I know.

👍 pinheadmz approved a pull request: "net: remove orphaned CSubNet::SanityCheck()"
(https://github.com/bitcoin/bitcoin/pull/27106)

👍 pinheadmz approved a pull request: "net: remove orphaned CSubNet::SanityCheck()"
(https://github.com/bitcoin/bitcoin/pull/27106)
ACK 30a3230e86dfd49c771432be6219841df5066eb4
<details><summary>Show Signature</summary>

```
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

ACK 30a3230e86dfd49c771432be6219841df5066eb4
-----BEGIN PGP SIGNATURE-----

iQIzBAEBCAAdFiEE5hdzzW4BBA4vG9eM5+KYS2KJyToFAmPudFYACgkQ5+KYS2KJ
yTrYnA/+LcTOT8/zvgcEeQ/nMhbO3LijgTmdio0OZS/dMc6SKdeAT/mDXv0ThjhB
+K7FuHY9fYO9sbXqPNkrbEfGR3iwqCNu5ElEAmvOMqVZIfkLHy//AX/uKNlUx2h7
1bICzt3PnnhaeGUWWxUeU+HFSpLikl6OHUYZLJfT+2VTz8rdMxhSY5iKSUEZNDKZ
J3D3B5nM
...

💬 jonatack commented on pull request "Add pool based memory resource":
(https://github.com/bitcoin/bitcoin/pull/25325#issuecomment-1433537441)
re-ACK b5eba9ad001f1035dd641bc5880cb6bb53a8b07f

💬 achow101 commented on pull request "wallet: be able to specify a wallet name and passphrase to migratewallet":
(https://github.com/bitcoin/bitcoin/pull/26595#discussion_r1108881969)
I've updated the passphrase handling to match what #27068 does

💬 achow101 commented on pull request "wallet: be able to specify a wallet name and passphrase to migratewallet":
(https://github.com/bitcoin/bitcoin/pull/26595#discussion_r1108883010)
> It would be useful to also test the behavior of the wallet encryption post-migration (eg. that migratewallet re-locks the wallet).

Added a check for that.

> A test for when the passphrase is provided but is incorrect could also be added here.

Done

> A test for when the wallet name is provided but is incorrect could also be added.

There's a test for that already in `test_unloaded`.

💬 achow101 commented on pull request "wallet: be able to specify a wallet name and passphrase to migratewallet":
(https://github.com/bitcoin/bitcoin/pull/26595#discussion_r1108884105)
Fixed

💬 achow101 commented on pull request "wallet: be able to specify a wallet name and passphrase to migratewallet":
(https://github.com/bitcoin/bitcoin/pull/26595#issuecomment-1433550228)
> > Wallet file verification failed. Failed to load database path '/tmp/bitcoin_func_test_w3iehdpt/node0/regtest/wallets/wrong_wallet_name'. Path does not exist. (-4)
>
> It would make more sense to throw a simpler error from `migratewallet` (as opposed to from `MakeDatabase`).

This is the same error that we currently give for `loadwallet`. I'm going to leave it as is so these are consistent with each other, but I agree it could be improved.

🚀 achow101 merged a pull request: "New `outputs` argument for `bumpfee`/`psbtbumpfee`"
(https://github.com/bitcoin/bitcoin/pull/25344)