💬 LarryRuane commented on pull request "Debug Console implementation of generate method":
(https://github.com/bitcoin-core/gui/pull/692#discussion_r1108713424)
```suggestion
auto executableCommand = "generatetoaddress " + nblocks + " " + address + " " + maxtries;
if (!RPCConsole::RPCExecuteCommandLine(m_node, result, executableCommand + "\n", nullptr, wallet_model)) {
```
Also remove the declaration of `executableCommand` near the top of the function; better to declare variables only where and when needed. You could do similarly with `result` and `address`.

💬 LarryRuane commented on pull request "Debug Console implementation of generate method":
(https://github.com/bitcoin-core/gui/pull/692#discussion_r1108706223)
```suggestion
if (!RPCConsole::RPCExecuteCommandLine(m_node, address, "getnewaddress\n", /*pstrFilteredOut=*/nullptr, wallet_model)) {
```

💬 LarryRuane commented on pull request "Debug Console implementation of generate method":
(https://github.com/bitcoin-core/gui/pull/692#discussion_r1108736164)
Formatting-only suggestion (personal preference to avoid long lines), and the argument to `QString()` had extra parentheses. Could make a similar change below.
```suggestion
if ((parsedCommand.size() > 1 && parsedCommand[0] == "help" && parsedCommand[1] == "generate") ||
(parsedCommand.size() > 3 && parsedCommand[0] == "generate")) {
auto message{
"\n"
"Generate blocks, equivalent to RPC getnewaddress followed by R
...

💬 LarryRuane commented on pull request "Debug Console implementation of generate method":
(https://github.com/bitcoin-core/gui/pull/692#discussion_r1108718036)
This line is indented one tab stop too much, and same for the two lines just below (after the `else`).

💬 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)