💬 Sjors commented on pull request "getblocktemplate improvements for segwit and sigops":
(https://github.com/bitcoin/bitcoin/pull/27433#issuecomment-1882738716)
Rebased due to (trivial) merge conflict in test.

I simplified this PR by dropping the debug argument total sigop and other debug info from the RPC result (52e24f662563dbd0adbcfda963052b3999ea689c , 4ef3261eb91c661a0483a730e156a475c2aad14e).

I'll use the Stratum v2 integration #28983 instead to improve debugging tools where needed.

I also dropped the additional check for the sigops limit in 52e24f662563dbd0adbcfda963052b3999ea689c. We've seen in practice that miners sometimes patch temp
...

💬 Sjors commented on pull request "getblocktemplate improvements for segwit and sigops":
(https://github.com/bitcoin/bitcoin/pull/27433#issuecomment-1882761201)
Also dropped b175798c67fbe50bc08007fe184d5407a8c4acf4 because the `default_witness_commitment` _is_ absent for empty-block templates.

💬 1440000bytes commented on pull request "OP_CHECKTEMPLATEVERIFY, OP_CHECKSIGFROMSTACK(VERIFY), OP_INTERNALKEY validation (LNHANCE)":
(https://github.com/bitcoin/bitcoin/pull/29198#issuecomment-1882769175)
- A few days back Ava Chow had mentioned on twitter that there is no pull request open for CHECKTEMPLATEVERIFY:

https://x.com/achow101/status/1742427508488729044
https://x.com/achow101/status/1742555598003048753

- There were some misunderstandings about unaddressed issues related to previous pull request:

https://x.com/JeremyRubin/status/1742582215354019956

- Since @reardencode was already working on CHECKSIGFROMSTACK and INTERNALKEY to be used in combination with CHE
...

💬 michaelfolkson commented on pull request "tests: add functional test for miniscript decaying multisig":
(https://github.com/bitcoin/bitcoin/pull/29156#issuecomment-1882786231)
Concept ACK

A couple of things on first skim read.

1) I think it is probably overkill to create a new functional test file for this. I'm not sure why it can't go in `wallet_miniscript.py`?

2) Miniscript has always been described by its authors/contributors as an extension of descriptors. I personally wouldn't be against adding some Miniscript explanation, examples in `descriptors.md` but I'd want a short explanation of what Miniscript is making it clear that this decaying multisig examp
...

👍 fanquake approved a pull request: "build: Fix `-Xclang -internal-isystem` option"
(https://github.com/bitcoin/bitcoin/pull/29195#pullrequestreview-1810865342)
ACK d742be3d3f5d5063d7160f72422bce2fec953f38. The same as what was done in #27328.

> For example, see: https://github.com/llvm/llvm-project/commit/cbbe1d44546db52c71c9a2b18f85b87ae82df9e7

I guess this is meant to be an example of someone else using `-Xclang`? However this commit is from > 10 years ago, and isn't related to, or an explanation for the recent change in Clang behaviour.

💬 michaelfolkson commented on pull request "OP_CHECKTEMPLATEVERIFY, OP_CHECKSIGFROMSTACK(VERIFY), OP_INTERNALKEY validation (LNHANCE)":
(https://github.com/bitcoin/bitcoin/pull/29198#issuecomment-1882816237)
> A few days back Ava Chow had mentioned on twitter that there is no pull request open for CHECKTEMPLATEVERIFY:

There is a [pull request](https://github.com/bitcoin/bitcoin/pull/28550) open (in draft) that contains CTV in addition to other opcodes and sighash flags (ie proposed consensus changes).

You don't have to be an expert in combinatorics to realize that continuing down this path results in multiple pull requests to this repo from different authors with different combinations of opco
...

💬 hebasto commented on pull request "build: Fix `-Xclang -internal-isystem` option":
(https://github.com/bitcoin/bitcoin/pull/29195#issuecomment-1882820244)
> I guess this is meant to be an example of someone else using `-Xclang`?

Right, just an example. I forgot about #27328.

💬 glozow commented on pull request "test: wallet rescan with reorged parent + IsFromMe child in mempool":
(https://github.com/bitcoin/bitcoin/pull/29179#issuecomment-1882825009)
I've added another commit which has pretty much the same test but using descriptor wallets i.e. a rescan through `importdescriptors`. Just like the other test, you can reproduce the bug by cherry-picking from #29019. You should get `JSONRPCEXCEPTION: Invalid or non-wallet transaction id (-5)` for the child tx.

✅ fanquake closed an issue: "Broken `--enable-suppress-external-warning` for Apple Clang 15 on `x86_64`"
(https://github.com/bitcoin/bitcoin/issues/29174)

🚀 fanquake merged a pull request: "build: Fix `-Xclang -internal-isystem` option"
(https://github.com/bitcoin/bitcoin/pull/29195)

💬 fanquake commented on pull request "build: Drop `ALLOW_HOST_PACKAGES` support in depends":
(https://github.com/bitcoin/bitcoin/pull/29203#issuecomment-1882846653)
ACK 080763a058f399583deb9dea6687e87fc1c097a9 - I can't imagine this option got any use outside our CI. It's also mostly just at odds with the idea of a self-contained dependency builder.

💬 theuni commented on pull request "build: Drop `ALLOW_HOST_PACKAGES` support in depends":
(https://github.com/bitcoin/bitcoin/pull/29203#issuecomment-1882847859)
Concept ACK. Happy to have this gone.

💬 hebasto commented on pull request "build: Bump clang minimum supported version to 15":
(https://github.com/bitcoin/bitcoin/pull/29165#discussion_r1445940541)
Might be dropped now.

👍 TheCharlatan approved a pull request: "build: Drop `ALLOW_HOST_PACKAGES` support in depends"
(https://github.com/bitcoin/bitcoin/pull/29203#pullrequestreview-1810909007)
ACK 080763a058f399583deb9dea6687e87fc1c097a9

💬 maflcko commented on pull request "build: Bump clang minimum supported version to 15":
(https://github.com/bitcoin/bitcoin/pull/29165#discussion_r1445941796)
thx, done

💬 theuni commented on pull request "build: always set `-g -O2` in `CORE_CXXFLAGS`":
(https://github.com/bitcoin/bitcoin/pull/29205#discussion_r1445938519)
Unrelated to this PR, but this line is busted. gcc/clang take the last `-O` option, so `-O0` is just clobbering the `-Og` here.

🤔 theuni reviewed a pull request: "build: always set `-g -O2` in `CORE_CXXFLAGS`"
(https://github.com/bitcoin/bitcoin/pull/29205#pullrequestreview-1810905286)
Concept ACK.

My only hesitation here was: can the user revert back to the default (no flag) behavior if desired. The gcc manpages answer this explicitly:

> Level 0 produces no debug information at all. Thus, -g0 negates -g

and

> -O0 Reduce compilation time and make debugging produce the expected results. This is the default.

So if the user uses: `./configure CXXFLAGS="-O0 -g0"`, the build should end up using:
`-g -O2 ... -O0 -g0`, which gets us back to the same behavior as if n
...

💬 fanquake commented on pull request "depends: remove `FORCE_USE_SYSTEM_CLANG` & native_llvm":
(https://github.com/bitcoin/bitcoin/pull/29188#issuecomment-1882868773)
Pulled back in the commit to fix Qt for Clang < 17.

🤔 glozow reviewed a pull request: "logging: Simplify API for level based logging"
(https://github.com/bitcoin/bitcoin/pull/28318#pullrequestreview-1810967229)
Concept ACK based on the developer-notes. I'm a fan of having 1 not-too-verbose one that we use most of the time, and then a few others based on severity.

💬 stickies-v commented on pull request "logging: Simplify API for level based logging":
(https://github.com/bitcoin/bitcoin/pull/28318#discussion_r1445981123)
> Ideally, LogInfo would optionally take a category.

I had that intuition at first too, but then changed my mind after reading [ajtowns's comment](https://github.com/bitcoin/bitcoin/pull/28318#issuecomment-1702318968) stating:

> My reasoning here is that the usefulness of the "category" isn't in telling us what bit of code triggered the log message (we have -logsourcelocations for that which is much more precise, and you can generally just grep for the message anyway), but in categorising
...