💬 hebasto commented on pull request "ci: A few fixes of `ccache` issues":
(https://github.com/bitcoin/bitcoin/pull/27084#discussion_r1103617434)
I think the difference is not using absolute path `/tmp/...`.

💬 furszy commented on pull request "wallet: reuse change dest when re-creating TX with avoidpartialspends":
(https://github.com/bitcoin/bitcoin/pull/27053#discussion_r1103618796)
ha, spider sense. Nice. Great catch for you!.

Orthogonal topic; moving forward, one of the follow-ups that have post-#25806 is the removal of this secondary "grouped" tx creation. We will be able to run Coin Selection directly over the aps vs non-aps grouped outputs.

✅ 1440000bytes closed a pull request: "mempool: Increase mempool default size"
(https://github.com/bitcoin/bitcoin/pull/27079)

👍 stickies-v approved a pull request: "http: Track active requests and wait for last to finish - 2nd attempt"
(https://github.com/bitcoin/bitcoin/pull/26742)

💬 furszy commented on pull request "wallet: coin selection, don't return results that exceed the max allowed weight":
(https://github.com/bitcoin/bitcoin/pull/26720#discussion_r1103622002)
ah yeah, good catch. All the numbers are in weight units. And yes, inputs are all P2WPKH (all of them in every `coinselector_tests.cpp` test case).

💬 Sjors commented on pull request "mempool: Increase mempool default size":
(https://github.com/bitcoin/bitcoin/pull/27079#issuecomment-1426760411)
I believe there was no mempool limit until late 2015, a 300 MB limit was added in 794a8cec5db84fde1cce82ada51740070ec188ac.

With the recent influx of relatively large blocks it still fit 70 blocks worth of transactions. Assuming 50% overhead in the way we store transactions in the mempool, I think the worst case is 40 blocks, or about 6 hours. This is significantly less than the default expiration of mempool transactions, which is two weeks. Before SegWit, our mempool would fit 1 day of wors
...

💬 Sjors commented on pull request "verify-commits: Bump trusted git root to after most recent laanwj merge":
(https://github.com/bitcoin/bitcoin/pull/27076#issuecomment-1426761886)
I would prefer a solution that allows checking commits by retired maintainers, at least up to a few years back (e.g. the branch-off point for the last backport supported version).

💬 MarcoFalke commented on pull request "verify-commits: Bump trusted git root to after most recent laanwj merge":
(https://github.com/bitcoin/bitcoin/pull/27076#issuecomment-1426763832)
You can do `git checkout "$(cat contrib/verify-commits/trusted-git-root)~1"` and then verify the previous commits, as often as you want, to go as far back as you want.

💬 furszy commented on pull request "wallet: coin selection, don't return results that exceed the max allowed weight":
(https://github.com/bitcoin/bitcoin/pull/26720#discussion_r1103623941)
fixed

💬 furszy commented on pull request "wallet: coin selection, don't return results that exceed the max allowed weight":
(https://github.com/bitcoin/bitcoin/pull/26720#discussion_r1103623954)
done

💬 furszy commented on pull request "wallet: coin selection, don't return results that exceed the max allowed weight":
(https://github.com/bitcoin/bitcoin/pull/26720#discussion_r1103623962)
done

💬 furszy commented on pull request "wallet: coin selection, don't return results that exceed the max allowed weight":
(https://github.com/bitcoin/bitcoin/pull/26720#discussion_r1103623989)
done

💬 furszy commented on pull request "wallet: coin selection, don't return results that exceed the max allowed weight":
(https://github.com/bitcoin/bitcoin/pull/26720#issuecomment-1426764558)
Feedback tackled, thanks for the review Xekyo!
Pushed [diff](https://github.com/bitcoin/bitcoin/compare/f0add1d0a7507e5601d7da54f62fe9ab272b3281..7d5a02ecb2af934ea0fa76c9013b2a2fc968b726).

💬 Sjors commented on pull request "verify-commits: Bump trusted git root to after most recent laanwj merge":
(https://github.com/bitcoin/bitcoin/pull/27076#issuecomment-1426765029)
That's a good hint to add to the `README`. But having the root key not too recent still seems better, all things equal. I think we should only bump it if a former maintainer revokes their key or it expires, plus maybe every couple of years to preempt such issues. So far #27058 seems compatible with that approach.

💬 darosior commented on pull request "Signing support for Miniscript Descriptors":
(https://github.com/bitcoin/bitcoin/pull/24149#issuecomment-1426765831)
@instagibbs thanks for the suggestions and the examples. This functional test was clearly lacking imagination. :) However what you suggest is already thoroughly tested by the unit tests (and to a lesser extent by the fuzz tests). But it's inexpensive to test also in the functional test so i included your examples and added a couple more things to test for each descriptor: the number of signatures expected (always equal to the number of private keys we have in the descriptor), whether this "compl
...

💬 darosior commented on pull request "Signing support for Miniscript Descriptors":
(https://github.com/bitcoin/bitcoin/pull/24149#discussion_r1103625462)
Initially i wanted to tackle all the PSBT integration in a followup (see https://github.com/bitcoin/bitcoin/pull/24149#issuecomment-1046944178). This comment made me (finally) have a look at what should be done. I think `analyzepsbt`, `utxoupdatepsbt` would need to be updated to support hash preimages. Maybe `walletprocesspsbt` too, but i'm not sure yet (for instance hash preimages could be imported separately and kept in a signing provider?). However i think we should first make those commands
...

💬 darosior commented on pull request "Signing support for Miniscript Descriptors":
(https://github.com/bitcoin/bitcoin/pull/24149#discussion_r1103625470)
Done. I added a refactoring commit to update the watchonly descriptors, and then use the keys list in the signing test too.

💬 MarcoFalke commented on pull request "verify-commits: Bump trusted git root to after most recent laanwj merge":
(https://github.com/bitcoin/bitcoin/pull/27076#issuecomment-1426768304)
> I think we should only bump

Is there any reason for this? See also https://github.com/bitcoin/bitcoin/pull/27058#discussion_r1099900493
What would the alternative be? Listing hundreds or thousands of "revsig" commits in a file, to ensure it is impossible to review manually, only with special git commands, potentially making it trivial to sneak in malicious commits that are not actually revsig commits?

💬 Sjors commented on pull request "contrib: Improve verify-commits.py to work with maintainers leaving":
(https://github.com/bitcoin/bitcoin/pull/27058#issuecomment-1426770715)
I like the CSV approach in c090a7ff86257411761e22d3642d0e8e05859b73 better. See [inline discussion](https://github.com/bitcoin/bitcoin/pull/27058#discussion_r1099900493) and [here](https://github.com/bitcoin/bitcoin/pull/27076#issuecomment-1426761886). I don't like having to update the root trusted commit every time the maintainer list changes.

Imo we should only bump it if a former maintainer revokes their key or it expires, plus maybe every couple of years to preempt such issues. As @MarcoF
...

💬 Sjors commented on pull request "verify-commits: Bump trusted git root to after most recent laanwj merge":
(https://github.com/bitcoin/bitcoin/pull/27076#issuecomment-1426770956)
@MarcoFalke answered in https://github.com/bitcoin/bitcoin/pull/27058#issuecomment-1426770715. Depending on what we do there this PR can either be closed or merged.