💬 brunoerg commented on pull request "fuzz: set the output argument of FuzzedSock::Accept()":
(https://github.com/bitcoin/bitcoin/pull/31316#issuecomment-2491407513)
> Perhaps this makes more sense in combination with a test that uses it? Currently FuzzedSock::Accept seems entirely unused: https://maflcko.github.io/b-c-cov/fuzz.coverage/src/test/fuzz/util/net.cpp.gcov.html

I agree. I was going to ask steps to reproduce then I noticed this is unused.

💬 Sjors commented on pull request "rpc: Remove submitblock pre-checks":
(https://github.com/bitcoin/bitcoin/pull/31175#discussion_r1852276089)
@mzumsande I don't recall such discussions. The only thing that matters for `getblockfrompeer` is that it _can_ store previously pruned blocks.

That said, I do think it's useful to allow `submitblock` to store a previously pruned block. It lets a user dump the block on one node and submit it on this node, without having go through the effort of establishing a p2p connection, figuring out the peer id and then calling `getblockfrompeer`.

💬 Sjors commented on pull request "rpc: Remove submitblock pre-checks":
(https://github.com/bitcoin/bitcoin/pull/31175#discussion_r1852312507)
@maflcko it's probably useful to briefly reopen #10146 and link to this thread for the likely real reason for that PR.

I'm speculating it was done in a stealthy way because there might be someone running a public service to submit blocks, and it would use that RPC under the hood. Or they were worried that since mining pools use this RPC internally (see e.g. #3658) they might have it exposed to the internet and therefore could be attacked.

cc @dergoegge @darosior in case you want to add it
...

📝 theStack opened a pull request: "test: add missing segwitv1 test cases to `script_standard_tests`"
(https://github.com/bitcoin/bitcoin/pull/31340)
Currently we have two segwitv1 output script types that are considered standard:
- `TxoutType::WITNESS_V1_TAPROOT` (P2TR): witness program has size 32 (introduced with taproot soft-fork)
- `TxoutType::ANCHOR` (P2A): witness program is {0x4e, 0x7e} (introduced with #30352)

This PR adds them to the script standardness unit tests where missing, i.e. for using them with the `ExtractDestination` and `GetScriptForDestination` functions.

💬 Sjors commented on pull request "rpc: Remove submitblock pre-checks":
(https://github.com/bitcoin/bitcoin/pull/31175#issuecomment-2491534171)
Note that #31196 drops `processNewBlock()` from the Mining IPC interface, which straight-forwardly uses `ProcessNewBlock`. The interface does still use it via `submitSolution()`.

💬 Sjors commented on pull request "Prune mining interface":
(https://github.com/bitcoin/bitcoin/pull/31196#issuecomment-2491565760)
I tested cherry-picking this PR on top of #31175 and the tests still pass.

📝 Parkeragan opened a pull request: "Add files via upload"
(https://github.com/bitcoin/bitcoin/pull/31341)

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that improve coverage are always welcome.
* All other changes should have accompanying unit tests (see `src/test/`) or
functional tests (see `test/`). Contributors sh
...

💬 Sjors commented on pull request "rpc: Remove submitblock pre-checks":
(https://github.com/bitcoin/bitcoin/pull/31175#issuecomment-2491616239)
Something I noticed while looking at df7ad9223025b5d75e27813ce3b89ef6fc431805

The release notes of 0.18 "promise" that the RPC never returns `duplicate-invalid`:

```md
- The `submitblock` RPC previously returned the reason a rejected block
was invalid the first time it processed that block, but returned a
generic "duplicate" rejection message on subsequent occasions it
processed the same block. It now always returns the fundamental
reason for rejecting an invalid block and on
...

✅ fanquake closed a pull request: "Add files via upload"
(https://github.com/bitcoin/bitcoin/pull/31341)

📝 fanquake locked a pull request: "Add files via upload"
(https://github.com/bitcoin/bitcoin/pull/31341)

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that improve coverage are always welcome.
* All other changes should have accompanying unit tests (see `src/test/`) or
functional tests (see `test/`). Contributors sh
...

💬 Sjors commented on pull request "rpc: Remove submitblock pre-checks":
(https://github.com/bitcoin/bitcoin/pull/31175#discussion_r1852357156)
6993836cb6488afa48922f19778ecef6d122bb69 nit: adding these log messages could be its own commit to de-obfuscate what it's actually testing.

💬 Parkeragan commented on issue "When opening or autoloading wallets there should be clear messages about rescanning in progress and wallets' names.":
(https://github.com/bitcoin-core/gui/issues/259#issuecomment-2491683772)
Get wallet type

💬 jobeirne-nydig commented on pull request "rpc: add getdescriptoractivity":
(https://github.com/bitcoin/bitcoin/pull/30708#issuecomment-2491695788)
Thanks for the review, all. 3 ACKs so far - any notes from maintainers?

👍 Sjors approved a pull request: "rpc: Remove submitblock pre-checks"
(https://github.com/bitcoin/bitcoin/pull/31175#pullrequestreview-2452017538)
utACK 563278b6a125f1a3d2111d4ebd74b9cc16d83ec5

It would be good to test the new pruned node behavior, in a way similar to `rpc_getblockfrompeer.py`.

💬 Sjors commented on pull request "rpc: Remove submitblock pre-checks":
(https://github.com/bitcoin/bitcoin/pull/31175#discussion_r1852467087)
3c6eeaf39e01a599829648ecd34990540835962f: even if it was previously pruned.

🤔 instagibbs reviewed a pull request: "test: add missing segwitv1 test cases to `script_standard_tests`"
(https://github.com/bitcoin/bitcoin/pull/31340#pullrequestreview-2451932627)
how about

```
diff --git a/src/addresstype.h b/src/addresstype.h
index 93cdf66c5b..09f3063c61 100644
--- a/src/addresstype.h
+++ b/src/addresstype.h
@@ -115,14 +115,17 @@ public:
if (w1.GetWitnessVersion() > w2.GetWitnessVersion()) return false;
return w1.GetWitnessProgram() < w2.GetWitnessProgram();
}
};

+/** Witness program for Pay-to-Anchor output script type */
+static const std::vector<unsigned char> anchor_bytes{0x4e, 0x73};
+
struct PayToAnchor
...

💬 instagibbs commented on pull request "test: add missing segwitv1 test cases to `script_standard_tests`":
(https://github.com/bitcoin/bitcoin/pull/31340#discussion_r1852412865)
nit: this would be "undefined" rather than "incorrect" like the segwitv0 case

💬 theStack commented on pull request "test: add missing segwitv1 test cases to `script_standard_tests`":
(https://github.com/bitcoin/bitcoin/pull/31340#discussion_r1852555732)
Reworked the comments to match the pattern used in previous cases, which is
`// TxoutType::INTENDED_TYPE with <some minor change that leads to different solver outcome>`
(the comment _"WITNESS_UNKNOWN with incorrect program size"_ didn't make any sense imho)

I kept the "incorrect" term, but wrote the outcome explanation in parantheses. Open for suggestions.

💬 theStack commented on pull request "test: add missing segwitv1 test cases to `script_standard_tests`":
(https://github.com/bitcoin/bitcoin/pull/31340#issuecomment-2491828622)

> how about
> ```
> ...
> ```
> which should get rid of all instances of 0x4e, 0x73?

@instagibbs: Thanks, taken!

> Could also move it into script/script.h and make it a static member of CScript?

Kept as-is for minimal diff, but happy to also do that change if others feel strongly about it.

💬 instagibbs commented on pull request "test: add missing segwitv1 test cases to `script_standard_tests`":
(https://github.com/bitcoin/bitcoin/pull/31340#issuecomment-2491852015)
ACK https://github.com/bitcoin/bitcoin/pull/31340/commits/a3a4d199e298a76725d0c0424195ae54c7e95fe0