💬 MarcoFalke commented on pull request "ci: Remove deprecated container.greedy":
(https://github.com/bitcoin/bitcoin/pull/28024#issuecomment-1618569388)
> It is still mentioned

I asked by email.

💬 MarcoFalke commented on pull request "ci: Remove deprecated container.greedy":
(https://github.com/bitcoin/bitcoin/pull/28024#issuecomment-1618572545)
Added an unrelated commit to adjust the CCACHE_SIZE. For some reason the jammy macOS taks is uncacheable? https://cirrus-ci.com/task/4744875699077120?logs=ci#L2486

💬 hebasto commented on pull request "ci: Remove deprecated container.greedy":
(https://github.com/bitcoin/bitcoin/pull/28024#issuecomment-1618585164)
> For some reason the jammy macOS taks is uncacheable? [cirrus-ci.com/task/4744875699077120?logs=ci#L2486](https://cirrus-ci.com/task/4744875699077120?logs=ci#L2486)

See:
- https://github.com/bitcoin/bitcoin/issues/21552
- https://github.com/bitcoin/bitcoin/pull/24620

💬 MarcoFalke commented on issue "Intermittent failures in interface_usdt_mempool.py":
(https://github.com/bitcoin/bitcoin/issues/27380#issuecomment-1618628124)
Maybe remove the sub-test case?

📝 theStack opened a pull request: "test: refactor: deduplicate legacy ECDSA signing for tx inputs"
(https://github.com/bitcoin/bitcoin/pull/28025)
There are several instances in functional tests and the framework (MiniWallet, feature_block.py, p2p_segwit.py) where we create a legacy ECDSA signature for a certain transaction's input by doing the following steps:

1. calculate the `LegacySignatureHash` with the desired sighash type
2. create the actual digital signature by calling `ECKey.sign_ecdsa` on the signature message hash calculated above
3. put the DER-encoded result as CScript data push into tx input's scriptSig

Create a new
...

💬 john-moffett commented on pull request "Switch RPCConsole wallet selection to the one most recently opened/restored/created":
(https://github.com/bitcoin-core/gui/pull/696#issuecomment-1618803949)
Thanks for pings, @hebasto and @jarolrod. Apologies for the delay. I've renamed the method and moved the declaration to the `Q_SLOTS` section.

💬 theStack commented on pull request "Remove confusing "Dust" label from coincontrol / sendcoins dialog":
(https://github.com/bitcoin-core/gui/pull/719#issuecomment-1618813452)
> The commit message and the PR description should mention changes in the `SendCoinsDialog` as well.

Sorry for the reply there, missed that. Force-pushed with an adapted commit message (and changed PR title / description as well):
```
$ git range-diff 394300c18...ef4185d2b
1: 944263a93 ! 1: 210ef1e98 qt: remove confusing "Dust" label from coincontrol dialog
@@ Metadata
Author: Sebastian Falbesoner <sebastian.falbesoner@gmail.com>

## Commit message ##
- qt: rem
...

💬 theStack commented on pull request "Remove confusing "Dust" label from coincontrol / sendcoins dialog":
(https://github.com/bitcoin-core/gui/pull/719#discussion_r1251087089)
I'm not too experienced with Qt, can anyone elaborate on that question (ping @furszy as author of the second commit)? It seems at least that we already have a couple of instances in master where both Qt::AlignLeading and Qt::AlignLeft are used together:
```
$ git grep AlignLeading
src/qt/forms/debugwindow.ui: <set>Qt::AlignBottom|Qt::AlignLeading|Qt::AlignLeft</set>
src/qt/forms/helpmessagedialog.ui: <set>Qt::AlignLeading|Qt::AlignLeft|Qt::AlignTop</set>
src/qt/forms/hel
...

💬 pablomartin4btc commented on pull request "httpserver, rest: improving URI validation":
(https://github.com/bitcoin/bitcoin/pull/27253#issuecomment-1618849607)
Updates:
- Rebased
- Implemented refactoring [suggestion](https://github.com/bitcoin/bitcoin/pull/27253#discussion_r1245038873) from @stickies-v, moving other http reject validations into the constructor and regrouping them under a `Validate()` function to make the entire validation more consistent.

💬 furszy commented on pull request "Remove confusing "Dust" label from coincontrol / sendcoins dialog":
(https://github.com/bitcoin-core/gui/pull/719#discussion_r1251098387)
> It seems at least that we already have a couple of instances in master where both Qt::AlignLeading and Qt::AlignLeft are used together.

Quite sure that we have them because of QT creator setting the value automatically. Probably because there is some ancient qt version that doesn't have them as synonyms.

But for us, it should be fine to drop them, even qt 4.8 has them as synonyms ([link](https://het.as.utexas.edu/HET/Software/html/qt.html#AlignmentFlag-enum)).

💬 theStack commented on pull request "Remove confusing "Dust" label from coincontrol / sendcoins dialog":
(https://github.com/bitcoin-core/gui/pull/719#discussion_r1251103633)
@furszy: Thanks a lot for checking. I removed the redundant `Qt::AlignLeading` properties from the second commit, i.e. it's only `Qt::AlignLeft` remaining.

💬 instagibbs commented on pull request "validate package transactions with their in-package ancestor sets":
(https://github.com/bitcoin/bitcoin/pull/26711#issuecomment-1618871642)
Without going full cluster mempool, I think whatever linearization is given out, we should probably ensure that each remaining prefix of the ancestor package is an ancestor package itself, before attempting submission to the mempool. Skip the entry if it's not. Maybe with ancestor set scoring linearizer this is redundant? I can't tell. It's probably going to get a lot closer to the ordering you want regardless, as it catches things like https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1
...

💬 john-moffett commented on pull request "Exit and show error if unrecognized command line args are present":
(https://github.com/bitcoin-core/gui/pull/742#issuecomment-1618901090)
Modified to follow @hebasto 's idea of only allowing `bitcoin:` URIs if no other options follow.

💬 pinheadmz commented on pull request "test: Add unit & functional test coverage for blockstore":
(https://github.com/bitcoin/bitcoin/pull/27850#issuecomment-1618905687)
@MarcoFalke @ryanofsky this is ready for review if you have time 🕺

💬 pinheadmz commented on pull request "Support JSON-RPC 2.0 when requested by client":
(https://github.com/bitcoin/bitcoin/pull/27101#issuecomment-1618910236)
> 🐙 This pull request conflicts with the target branch and [needs rebase](https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#rebasing-changes).

👌

💬 instagibbs commented on pull request "[25.x] Parallel compact block downloads":
(https://github.com/bitcoin/bitcoin/pull/27752#issuecomment-1618917886)
utACK https://github.com/bitcoin/bitcoin/pull/27752/commits/b8ad3220a9068f10c2b3b14b40f211372aeece31

💬 instagibbs commented on pull request "Update MANDATORY_SCRIPT_VERIFY_FLAGS":
(https://github.com/bitcoin/bitcoin/pull/26291#issuecomment-1618920692)
I've been running this for the last week, no transaction-based misbehavior reports in my logs. still unsure the juice is worth the squeeze, but appears to not be obviously harmful.

💬 pablomartin4btc commented on pull request "httpserver, rest: improving URI validation":
(https://github.com/bitcoin/bitcoin/pull/27253#issuecomment-1618940865)
Updates:
- Checking fuzz test failure

💬 Xekyo commented on pull request "Bump unconfirmed ancestor transactions to target feerate":
(https://github.com/bitcoin/bitcoin/pull/26152#discussion_r1251179626)
Ah drats, I did not realize that `CoinsResult.All()` produces a copy. Reverting this.

💬 Xekyo commented on pull request "Bump unconfirmed ancestor transactions to target feerate":
(https://github.com/bitcoin/bitcoin/pull/26152#discussion_r1251181072)
I would prefer that the method `GetTotalBumpFee()` does not either return the sum of individual bump fees or the combined bump fee for the input set depending on when it is called. I’d be worried that it would be hard to understand later and might be a source of future bugs.