💬 instagibbs commented on pull request "rpc: Use a FlatSigningProvider in decodescript to allow inferring descriptors for scripts larger than 520 bytes":
(https://github.com/bitcoin/bitcoin/pull/27113#issuecomment-1433398818)
ACK https://github.com/bitcoin/bitcoin/pull/27113/commits/73ec4b2a8347c796b9aadc1f2576b286c469f9e7

thanks for the quick turnaround

💬 Hyunhum commented on pull request "script: add explanation for the functionality of each opcode":
(https://github.com/bitcoin/bitcoin/pull/27109#issuecomment-1433401897)
Grateful for kind feedbacks, @sipa
I made commit to reflect feedbacks, and will squash this down to 1 commit if no more feedback to be approved!

💬 jonatack commented on pull request "refactor: Move coin_control variable to test setup section":
(https://github.com/bitcoin/bitcoin/pull/26154#issuecomment-1433421228)
> The time it's taken to get any feedback on a number of PR's I made 6 months ago has changed my focus away from core to other projects for now.

> meaningful feedback for how I can contribute meaningful changes would be appreciated.

@yancyribbens It's not easy, and feedback is not only a technical process but also a social one. Anyway, feel free to ignore but some writings that may help:

- https://jonatack.github.io/articles/how-to-review-pull-requests-in-bitcoin-core
- https://jonata
...

💬 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`.