💬 maflcko commented on pull request "test: Add Compact Block Encoding test `ReceiveWithExtraTransactions` covering non-empty `extra_txn`":
(https://github.com/bitcoin/bitcoin/pull/30237#issuecomment-2154833376)
Would it be difficult to make the test deterministic?

💬 pinheadmz commented on pull request "json-rpc 2.0 followups: docs, tests, cli":
(https://github.com/bitcoin/bitcoin/pull/30238#discussion_r1631204799)
Yeah good idea here thanks, updating

💬 pinheadmz commented on pull request "json-rpc 2.0 followups: docs, tests, cli":
(https://github.com/bitcoin/bitcoin/pull/30238#discussion_r1631206987)
hmmm I think I'm going to pass on this, there's a few other files with the `;` and if it doesn't break anything I'll just leave them all the same... but if @fanquake agrees I'll add a commit to this PR.

💬 pinheadmz commented on pull request "json-rpc 2.0 followups: docs, tests, cli":
(https://github.com/bitcoin/bitcoin/pull/30238#discussion_r1631209745)
Great work thanks! I monitored with Wireshark as well. The other local test I did is to build bitcoin with legacy support removed. Only one test should fail, the `interface_rpc` test from #27101 which explicitly makes a legacy request.

```diff
diff --git a/src/rpc/request.cpp b/src/rpc/request.cpp
index 87b9f18b33..e72e349bf6 100644
--- a/src/rpc/request.cpp
+++ b/src/rpc/request.cpp
@@ -200,9 +200,7 @@ void JSONRPCRequest::parse(const UniValue& valRequest)
// The "jsonrpc" key
...

🚀 glozow merged a pull request: "util: add VecDeque"
(https://github.com/bitcoin/bitcoin/pull/30161)

💬 pinheadmz commented on pull request "json-rpc 2.0 followups: docs, tests, cli":
(https://github.com/bitcoin/bitcoin/pull/30238#discussion_r1631214784)
There also would be no `error` or `result` fields in the 204 case! I think it's self explanatory that the authproxy call function expects a very specific type of response.

💬 tdb3 commented on pull request "test: Added test coverage to listsinceblock rpc":
(https://github.com/bitcoin/bitcoin/pull/30195#discussion_r1631217975)
The `exists()` asserts are a nice improvement/sanity check.

💬 tdb3 commented on pull request "json-rpc 2.0 followups: docs, tests, cli":
(https://github.com/bitcoin/bitcoin/pull/30238#discussion_r1631249895)
As a follow-up, checked that bitcoind does actually send the exact reason phrase "No Content", so the updates made are correct.

💬 Sjors commented on pull request "Introduce Mining interface":
(https://github.com/bitcoin/bitcoin/pull/30200#issuecomment-2154903353)
All the above should be fixed now, except for `g_best_block_cv.wait_until` (but that's not blocking).

The biggest change is in 914c26bbd4d7e87af4e0d0edf9af69ae76668b7e. It now makes the `options` argument mandatory for `BlockAssembler` constructor, dropping implicit handling of `ArgsManager`. The caller i.e. the `Mining` interface implementation now handles this.

In Stratum v2 the pool communicates how many extra bytes it needs for its own outputs (payouts, extra commitments, etc). This ne
...

💬 instagibbs commented on pull request "test: expand LimitOrphan and EraseForPeer coverage":
(https://github.com/bitcoin/bitcoin/pull/30082#discussion_r1631271509)
done in first commit now

💬 instagibbs commented on pull request "test: expand LimitOrphan and EraseForPeer coverage":
(https://github.com/bitcoin/bitcoin/pull/30082#discussion_r1631271559)
done

💬 sdaftuar commented on pull request "policy: bump TX_MAX_STANDARD_VERSION to 3":
(https://github.com/bitcoin/bitcoin/pull/29496#issuecomment-2154918042)
utACK 30a01134c

💬 instagibbs commented on pull request "test: expand LimitOrphan and EraseForPeer coverage":
(https://github.com/bitcoin/bitcoin/pull/30082#issuecomment-2154934386)
rebased due to silent merge conflict, with cleanups suggested by @glozow :+1:

💬 Sjors commented on pull request "Introduce Mining interface":
(https://github.com/bitcoin/bitcoin/pull/30200#issuecomment-2154948888)
You can see the interface in action in https://github.com/bitcoin/bitcoin/pull/29432/commits/1618c3dd86d65cf00f5f3c2c30c1db2b0e9dd9ca

👋 Sjors's pull request is ready for review: "Introduce Mining interface"
(https://github.com/bitcoin/bitcoin/pull/30200)

⚠️ maflcko opened an issue: "fuzz: wallet_bdb_parser: implicit-signed-integer-truncation wallet/migrate.cpp:554:35 "
(https://github.com/bitcoin/bitcoin/issues/30247)
Full error:

```
wallet/migrate.cpp:554:35: runtime error: implicit conversion from type 'int64_t' (aka 'long') of value -1 (64-bit, signed) to type 'uint32_t' (aka 'unsigned int') changed the value to 4294967295 (32-bit, unsigned)
```

The code:

```cpp
uint32_t expected_last_page = (size / page_size) - 1;
```

This may lead to a fuzz runtime error when `(size / page_size)` is `0`. I don't think this can lead to issues for real users, because it can only happen with corrupt files an
...

💬 Sjors commented on pull request "Stratum v2 Template Provider (take 3)":
(https://github.com/bitcoin/bitcoin/pull/29432#issuecomment-2154958567)
Updated to use the interface proposed in #30200. This also fixes a small bug: `-blockmintxfee` and `-blockmaxweight` are no longer ignored. When the latter argument is unset, by default blocks have a `4000` byte safety margin, just like with `getblocktemplate`.

💬 maflcko commented on issue "fuzz: wallet_bdb_parser: implicit-signed-integer-truncation wallet/migrate.cpp:554:35 ":
(https://github.com/bitcoin/bitcoin/issues/30247#issuecomment-2154959411)
Found by @murchandamus in https://github.com/bitcoin-core/qa-assets/pull/186#issuecomment-2154348655

```
Run wallet_bdb_parser with args ['/ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/test/fuzz/fuzz', '-runs=1', PosixPath('/ci_container_base/ci/scratch/qa-assets/fuzz_seed_corpus/wallet_bdb_parser')]INFO: Running with entropic power schedule (0xFF, 100).
INFO: Seed: 1090373666
INFO: Loaded 1 modules (584206 inline 8-bit counters): 584206 [0x55fc8b5c2ec8, 0x55fc8b651
...

💬 fanquake commented on pull request "json-rpc 2.0 followups: docs, tests, cli":
(https://github.com/bitcoin/bitcoin/pull/30238#discussion_r1631315946)
No objections from me.

🤔 glozow reviewed a pull request: "test: MiniWallet: respect passed feerate for padded txs (using `target_weight`)"
(https://github.com/bitcoin/bitcoin/pull/30162#pullrequestreview-2104768021)
light review ACK 39d135e79f3f0c40dfd8fad2c53723d533cd19b4

Would be nice to have this work for `create_self_transfer_multi` as well