💬 achow101 commented on pull request "refactor: Move coin_control variable to test setup section":
(https://github.com/bitcoin/bitcoin/pull/26154#issuecomment-1433353547)
> Tiny PRs like this in my mind should be reviewed and either closed or merged in a short period since it's trivial to review. I'm surprised it's taken 6 months to even be looked at.

The fact that a tiny PR like this one has taken 6 months for anyone to even look at it is why I think it is noise. There are over 300 other PRs to look at, with several large projects going on at the same time. People are busy looking at other things, and simply having this PR even exist means that time that coul
...

💬 MarcoFalke commented on pull request "refactor: Move coin_control variable to test setup section":
(https://github.com/bitcoin/bitcoin/pull/26154#issuecomment-1433359488)
I also have a template response:


Thank you for your contribution. While this stylistic change makes sense on its own, it comes at a cost and risk for the project as a whole. The weak motivation for the change does not justify the burden that it places on the project. A burden could be any of the following:

* Time spent on review
* Accidental introduction of bugs
* (Silent) merge conflicts, either in the branch or a backport branch. Those conflicts demand further developer and reviewer
...

⚠️ MarcoFalke opened an issue: "Intermittent issue in p2p_ibd_stalling.py self.wait_until(lambda: self.total_bytes_recv_for_blocks() == 172761)"
(https://github.com/bitcoin/bitcoin/issues/27112)
```
wget https://drahtbot.space/temp_scratch/p2p_ibd_stalling_96.tar.xz
tar -xvf p2p_ibd_stalling_96.tar.xz
test/functional/combine_logs.py -c ./p2p_ibd_stalling_96
```

```
test 2023-02-14T20:54:45.479000Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
File "/root/bitcoin-core/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/test/functional/test_framework/test_framework.py", line 134, in
...

📝 achow101 opened a 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)
`FillableSigningProvider` limits scripts to 520 bytes even though segwit allows scripts to be larger than that. We can avoid this limit by using a `FlatSigningProvider` so that such larger scripts can be decoded.

Fixes #27111

💬 achow101 commented on issue "decodescript miniscript functionality tops out at 520 bytes":
(https://github.com/bitcoin/bitcoin/issues/27111#issuecomment-1433385518)
Fixed in #27113

A related question is whether `decodescript` should check script sizes? Other than this particular issue, I don't think it does any size checking.

💬 sipa 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-1433385742)
Concept ACK

💬 LarryRuane commented on pull request "Debug Console implementation of generate method":
(https://github.com/bitcoin-core/gui/pull/692#discussion_r1108701947)
```suggestion
std::vector<std::string> vec{SplitString(strCommand, " (),\n")};
auto should_remove{[](const std::string& str) { return str.empty(); }};
```

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