💬 rkrux commented on pull request "test: fix MiniWallet script-path spend (missing parity bit in leaf version)":
(https://github.com/bitcoin/bitcoin/pull/30076#discussion_r1643398546)
Is the sole reason for creating 10 tagged wallets to be thorough? Or is there any other reason as well?

💬 rkrux commented on pull request "test: fix MiniWallet script-path spend (missing parity bit in leaf version)":
(https://github.com/bitcoin/bitcoin/pull/30076#discussion_r1643399436)
i.e. to target the following assumption
> we were so far just lucky that the used combinations of relevant data (internal pubkey, leaf script / version) didn't result in a tweaked pubkey with odd y-parity.

💬 achow101 commented on pull request "Encapsulate warnings in generalized node::Warnings and remove globals":
(https://github.com/bitcoin/bitcoin/pull/30058#issuecomment-2174371181)
ACK 260f8da71a35232d859d7705861fc1a88bfbbe81

💬 rkrux commented on pull request "test: Validate oversized transactions or without inputs":
(https://github.com/bitcoin/bitcoin/pull/29862#discussion_r1643416859)
Nice to have: To reduce duplication by using lambda expressions and captures.

👍 rkrux approved a pull request: "test: Validate oversized transactions or without inputs"
(https://github.com/bitcoin/bitcoin/pull/29862#pullrequestreview-2123875126)
reACK [fa9e680](https://github.com/bitcoin/bitcoin/pull/29862/commits/fa9e680b0ca3c401a33e9901e8732128113be85e)

Re-tested by running make and make check, both successful.
Mentioned couple suggestions but not blockers from my side, upto author.

💬 rkrux commented on pull request "test: Validate oversized transactions or without inputs":
(https://github.com/bitcoin/bitcoin/pull/29862#discussion_r1643413535)
Would be nice to add a comment explaining the presence of this constant 70 for future reference.

💬 prusnak commented on pull request "Revert "contrib: macdeploy: monkey-patch gen-sdk to be deterministic"":
(https://github.com/bitcoin/bitcoin/pull/30282#issuecomment-2174418333)
utACK

👍 prusnak approved a pull request: "Revert "contrib: macdeploy: monkey-patch gen-sdk to be deterministic""
(https://github.com/bitcoin/bitcoin/pull/30282#pullrequestreview-2123899637)
utACK

💬 achow101 commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#issuecomment-2174428338)
ACK 94ed4fbf8e1a396c650b5134d396d6c0be35ce10

🚀 achow101 merged a pull request: "Encapsulate warnings in generalized node::Warnings and remove globals"
(https://github.com/bitcoin/bitcoin/pull/30058)

💬 achow101 commented on pull request "Release `LockData::dd_mutex` before calling `*_detected` functions":
(https://github.com/bitcoin/bitcoin/pull/26781#issuecomment-2174448016)
@hebasto Are you planning on addressing review feedback here?

***

> I think it would be better if it had another commit getting rid of StdMutex/StdLockGuard. Otherwise nothing is really taking advantage of the change.

I agree. The PR description sounded like this was going to be the ultimate change, but it does not appear to have actually been done.

🚀 achow101 merged a pull request: "Cluster size 2 package rbf"
(https://github.com/bitcoin/bitcoin/pull/28984)

💬 ishaanam commented on pull request "wallet, rpc: add anti-fee-sniping to `send` and `sendall`":
(https://github.com/bitcoin/bitcoin/pull/28944#issuecomment-2174450450)
> > > ```diff
> > > --- a/test/functional/wallet_import_rescan.py
> > > +++ b/test/functional/wallet_import_rescan.py
> > > @@ -293,6 +293,7 @@ class ImportRescanTest(BitcoinTestFramework):
> > > add_to_wallet=False,
> > > inputs=[unspent_txid_map[variant.initial_txid]],
> > > outputs=[{ADDRESS_BCRT1_UNSPENDABLE : variant.initial_amount}],
> > > + locktime=0,
> > > subtract_fee_from_outputs=[0]
> > >
...

💬 cbergqvist commented on pull request "Support JSON-RPC 2.0 when requested by client":
(https://github.com/bitcoin/bitcoin/pull/27101#discussion_r1643503903)
(Came across this block again while working on something else. It struck me that the exception handling looked redundant as we catch the same exact exception types inside `JSONRPCExec(jreq, /*catch_errors=*/true`). *But* the `jreq.parse()` call will throw on missing methods etc.

One could move out `response = JSONRPCExec(jreq, /*catch_errors=*/true);` to after the block, but then one would need to guard against using a `jreq` that failed to parse. So the current version of the block is probab
...

💬 mzumsande commented on issue "RFC: Assumeutxo and large forks and reorgs":
(https://github.com/bitcoin/bitcoin/issues/30288#issuecomment-2174570099)
The problem I see with allowing the background sync to target the most-work chain is that it doesn't seem to fit well into the design philosophy that has been merged so far. The background sync is treated as a low-priority task that is mostly done as an extra precaution after having synced the snapshot to the tip. At some point in the future (I know this would be contentious) we might even want to give users the opt-in option to skip it completely to save bandwidth.
But as it is, the background
...

📝 CharlesCNorton opened a pull request: "fix: typo in benchmark documentation"
(https://github.com/bitcoin/bitcoin/pull/30296)
Corrected "coins selection" to "coin selection" for consistency with the other items listed.

Justification:

Consistency: Aligns with other singular nouns in the list.

<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

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

<!--
Please provide clear motivation f
...

💬 theStack commented on pull request "contrib: add tool to convert compact-serialized UTXO set to SQLite database":
(https://github.com/bitcoin/bitcoin/pull/27432#issuecomment-2174686278)
Rebased on master (resolving a trivial merging conflict in the test runner list after the merge of #28984).

💬 theStack commented on pull request "test: fix MiniWallet script-path spend (missing parity bit in leaf version)":
(https://github.com/bitcoin/bitcoin/pull/30076#discussion_r1643600316)
> Is the sole reason for creating 10 tagged wallets to be thorough? Or is there any other reason as well?

Yes it's thoroughness, no other reason. Since the fixed bugs only occur with a certain probability, creating several random tagged wallets in a row increase the likelihood of catching the bugs in a single test run (nothing special about the number 10 though).

⚠️ techy2 opened an issue: "unrecognized command line option '-std=c++20' "
(https://github.com/bitcoin/bitcoin/issues/30297)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

building depends qt
Project ERROR: Cannot run target compiler 'g++ -m64'. Output:
===================
Using built-in specs.
COLLECT_GCC=g++
OFFLOAD_TARGET_NAMES=nvptx-none:hsa
OFFLOAD_TARGET_DEFAULT=1
g++: error: unrecognized command line option '-std=c++20'; did you mean '-std=c++2a'?
Target: x86_64-linux-gnu
Configured with: ../src/configure -v --with-pkgversion='Ubuntu 9.4.0-1u
...

💬 kevkevinpal commented on pull request "upnp: fix build with miniupnpc 2.2.8":
(https://github.com/bitcoin/bitcoin/pull/30283#issuecomment-2174738368)
looks like this is the diff between verison 2.2.7 and 2.2.8 in case anyone is looking
https://github.com/miniupnp/miniupnp/compare/miniupnpc_2_2_7...miniupnpc_2_2_8