💬 MarcoFalke commented on pull request "test: Fix intermittent issue in mining_getblocktemplate_longpoll.py":
(https://github.com/bitcoin/bitcoin/pull/27941#issuecomment-1619804135)
@luke-jr I presume if this was a bug on the RPC side, it could be fixed with something like:

```diff
diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp
index 074cecadd2..ca7d1acda8 100644
--- a/src/rpc/mining.cpp
+++ b/src/rpc/mining.cpp
@@ -742,12 +742,12 @@ static RPCHelpMan getblocktemplate()
WAIT_LOCK(g_best_block_mutex, lock);
while (g_best_block == hashWatchedChain && IsRPCRunning())
{
+ // Check transactions for update
+
...

👍 willcl-ark approved a pull request: "Support JSON-RPC 2.0 when requested by client"
(https://github.com/bitcoin/bitcoin/pull/27101#pullrequestreview-1511661787)
re-ACK 5f9179a

Verified that since my last review `git range-diff af86462e...5f9179a` comprised rebase-related changes. The CI failure is unrelated. Left a non-blocking comment.

💬 willcl-ark commented on pull request "Support JSON-RPC 2.0 when requested by client":
(https://github.com/bitcoin/bitcoin/pull/27101#discussion_r1251203825)
Just thinking out loud here; we use a code comment to indicate what the error code represents but in many of the other tests it appears we don't (admittedly here it is the point of the test, and in the other locations it's not). I wonder whether it might be clearer to have these enumerated in a single location, e.g. _util.py_, and then they could be used by name here and elsewhere? (this might be a change someone else will just make in the future otherwise, too).

I think it's probably better
...

💬 ajtowns commented on pull request "[25.x] Parallel compact block downloads":
(https://github.com/bitcoin/bitcoin/pull/27752#issuecomment-1619965422)
ACK b8ad3220a9068f10c2b3b14b40f211372aeece31 ; confirmed patches are clean cherry-picks from master, and already tested patches prior to 25.0 release

💬 stickies-v commented on issue "rpc: doc: RPCHelpMan missing request parameters for getblocktemplate":
(https://github.com/bitcoin/bitcoin/issues/27998#issuecomment-1619992173)
The description for "data" is wrong, copy-pasting from another (unrelated) data field is not a good approach here. I would suggest digging into the code to understand what "data" is used for, and/or reading the spec of [BIP22](https://github.com/bitcoin/bips/blob/master/bip-0022.mediawiki) and [BIP23](https://github.com/bitcoin/bips/blob/master/bip-0023.mediawiki)

🚀 fanquake merged a pull request: "[25.x] Parallel compact block downloads"
(https://github.com/bitcoin/bitcoin/pull/27752)

💬 glozow commented on pull request "[NO MERGE] BIP331 Ancestor Package Relay":
(https://github.com/bitcoin/bitcoin/pull/27742#issuecomment-1620036644)
Thanks everyone for the reviews!
Last push:
- Implemented changes that were discussed on the BIP PR https://github.com/bitcoin/bips/pull/1382
- Maximum of 100 transactions per `getpkgtxns` request
- Changed `versions` field in `sendpackages` to be 64b instead of 32b.
- Added a `PKG_RELAY_PKGTXNS` bit for negotiating `getpkgtxns`/`pkgtxns` support in `sendpackages`.
- Fixed some of the bigger problems I found / pointed out by reviewers, e.g.
- https://github.com/bitcoin/bit
...

📝 MarcoFalke opened a pull request: "test: Check expected_stderr after stop"
(https://github.com/bitcoin/bitcoin/pull/28028)
This fixes a bug where stderr wasn't checked for the shutdown sequence.

Fix that by waiting for the shutdown to finish and then check stderr.

💬 ItIsOHM commented on issue "rpc: doc: RPCHelpMan missing request parameters for getblocktemplate":
(https://github.com/bitcoin/bitcoin/issues/27998#issuecomment-1620202748)
> The description for "data" is wrong, copy-pasting from another (unrelated) data field is not a good approach here. I would suggest digging into the code to understand what "data" is used for, and/or reading the spec of [BIP22](https://github.com/bitcoin/bips/blob/master/bip-0022.mediawiki) and [BIP23](https://github.com/bitcoin/bips/blob/master/bip-0023.mediawiki)

Oh yes I agree, I'll read more about it and change the descriptions. I was asking in terms of syntax and usage, if it was correc
...

👍 furszy approved a pull request: "Remove confusing "Dust" label from coincontrol / sendcoins dialog"
(https://github.com/bitcoin-core/gui/pull/719#pullrequestreview-1512893564)
ACK a582b41

💬 stickies-v commented on issue "rpc: doc: RPCHelpMan missing request parameters for getblocktemplate":
(https://github.com/bitcoin/bitcoin/issues/27998#issuecomment-1620285061)
Yeah the approach is good. Great, have a look and then for further implementation feedback I'd suggest you open a PR with the code changes (definitely worth going through [CONTRIBUTING.md](https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md) first).

💬 Sjors commented on pull request "test: Fixes and updates to wallet_backwards_compatibility.py for 25.0 and descriptor wallets":
(https://github.com/bitcoin/bitcoin/pull/28027#issuecomment-1620356102)
Thanks for improving these tests. Added to review list.

💬 t-bast commented on pull request "Bump unconfirmed ancestor transactions to target feerate":
(https://github.com/bitcoin/bitcoin/pull/26152#issuecomment-1620368501)
> I’ve fixed a number of issues, if this latest push’s checks turn up green, it might be more fruitful to try now.

I confirm that, I've ran my set of tests against [eclair](https://github.com/ACINQ/eclair) and everything looks good using https://github.com/bitcoin/bitcoin/pull/26152/commits/0f6c13665c4ab7d4928ff0fa63c4e755667f7fd6 :tada:

The tests I ran can be found in the last two commits of [this branch](https://github.com/ACINQ/eclair/commits/wip-test-ancestor-aware-funding-bitcoind-25
...

💬 glozow commented on pull request "validate package transactions with their in-package ancestor sets":
(https://github.com/bitcoin/bitcoin/pull/26711#issuecomment-1620375694)
> I don't think you need to special case individual transactions even, actually. Instead, use this, instead of chunking:

I'm not sure about using a group's aggregate feerate without checking their spending relationships, as it may allow unrelated transactions to pay for each other. For example:
```
A(1) B(3)
^ ^
C(100)
```
Where minfeerate is 2sat/vB. Imagine C is invalid (e.g. a fake child created to connect A and B).
The ancestor score-based linearization I was imagining (i
...

💬 instagibbs commented on pull request "[NO MERGE] BIP331 Ancestor Package Relay":
(https://github.com/bitcoin/bitcoin/pull/27742#discussion_r1252120078)
> download should be dedup when we receive the ancpkginfos ?

If it's in your orphanage it won't be fetched again. See `ReceivedAncPkgInfo`

💬 instagibbs commented on pull request "validate package transactions with their in-package ancestor sets":
(https://github.com/bitcoin/bitcoin/pull/26711#issuecomment-1620395050)
Nothing is perfect, but a linearizer would ideally pick `B` before `A`, yes. So you might get:

`B, A, C`
or
`B, C, A`

depending on if the strategy is greedy. "topo" sort may miss this, which is why we should probably be smarter than that?

👍 hebasto approved a pull request: "Remove confusing "Dust" label from coincontrol / sendcoins dialog"
(https://github.com/bitcoin-core/gui/pull/719#pullrequestreview-1513056670)
Looks good. ACK a582b4141f0756faa3793fb1c772898a984c83e4.

💬 sipa commented on pull request "validate package transactions with their in-package ancestor sets":
(https://github.com/bitcoin/bitcoin/pull/26711#issuecomment-1620413203)
@glozow You're right; I didn't think this through.

@instagibbs To a limited extent better linearization can help here (though within-chunk optimization isn't something I've been looking at, as it doesn't matter for mining/eviction) but I think you can construct more complex examples where even a "perfect" linearization results in grouping of things that should not pay for each other.

I'm starting to think that something closer to your idea here is right: trying ancestor sets of every transacti
...

✅ hebasto closed an issue: "Confusing/misleading "Dust:" label in coin selection dialog"
(https://github.com/bitcoin-core/gui/issues/699)

🚀 hebasto merged a pull request: "Remove confusing "Dust" label from coincontrol / sendcoins dialog"
(https://github.com/bitcoin-core/gui/pull/719)