💬 fjahr commented on pull request "RPC: access RPC arguments by name":
(https://github.com/bitcoin/bitcoin/pull/29277#issuecomment-2081631545)
Code review ACK 30a6c999351041d6a1e8712a9659be1296a1b46a

I still prefer my suggested approach but it's still an improvement and I don't want to block this.

📝 sdaftuar opened a pull request: "fuzz: don't allow adding duplicate transactions to the mempool"
(https://github.com/bitcoin/bitcoin/pull/29990)
Filter duplicate transaction ids from being added to the mempool in the `partially_downloaded_block` fuzz target.

I think a prerequisite for calling `CTxMemPool::addUnchecked` should be that the underlying txid doesn't already exist in the mempool (otherwise `addUnchecked` would need a way to return failure, which we don't currently have).

🤔 tdb3 reviewed a pull request: "RPC: Return `permitbaremultisig` and `maxdatacarriersize` in `getmempoolinfo`"
(https://github.com/bitcoin/bitcoin/pull/29954#pullrequestreview-2027366385)
Concept ACK.
Thank you. Appears to work well. Seems helpful to me for clients to have this information provided through RPC. I took a quick look at other RPC calls (in https://bitcoincore.org/en/doc/27.0.0/), but didn't see any that had `permitbaremultisig` and `maxdatacarriersize`.

Recently, Issue #29912 was opened, which discusses implementing a formal specification for the RPC API. RPC API changes have the potential to adjust the meaning, fields/types, and/or values used in or returne
...

💬 tdb3 commented on pull request "RPC: Return `permitbaremultisig` and `maxdatacarriersize` in `getmempoolinfo`":
(https://github.com/bitcoin/bitcoin/pull/29954#discussion_r1582400466)
I second this. It would add value to have tests that cover:
1) permitbaremultisig disabled
2) permitbaremultisig enabled

This might be accomplished through the use of a second node (`self.num_nodes`) or perhaps through stopping the node, modifying configuration with `replace_in_config()`, and restarting it. Typically num_nodes is to be minimized and start/stops avoided when possible, but I do not immediately see how different `permitbaremultisig` states could be tested without doing one o
...

👍 cbergqvist approved a pull request: "test: adds outbound eviction functional tests, updates comment in ConsiderEviction"
(https://github.com/bitcoin/bitcoin/pull/29122#pullrequestreview-2027367015)
ACK d53d84834747c37f4060a9ef379e0a6b50155246

Functional including `--extended` tests passed (skipped unrelated `feature_dbcrash`).

💬 tdb3 commented on issue "RFC: Formal description of the RPC API":
(https://github.com/bitcoin/bitcoin/issues/29912#issuecomment-2081678433)
Concept ACK. IMHO, a formal specification (especially machine ingestible) would add value for downstream clients.

There is some great info/guidance in
https://github.com/bitcoin/bitcoin/blob/master/doc/JSON-RPC-interface.md#versioning
https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#rpc-interface-guidelines

Maybe it's just a matter of adding content to the RPC interface guidelines section of the developer notes, but I think there is value in defining the approach t
...

👍 cbergqvist approved a pull request: "p2p: gives seednode priority over dnsseed if both are provided"
(https://github.com/bitcoin/bitcoin/pull/28016#pullrequestreview-2027389319)
ACK 82f41d76f1c6ad38290917dad5499ffbe6b3974d

After checking out ran: `git range-diff 2842e51~2..2842e51 HEAD~2..HEAD`
Functional including `--extended` tests passed (skipped unrelated `feature_dbcrash`).

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

📝 fanquake locked a pull request: "Update bitcoin_config.h.in"
(https://github.com/bitcoin/bitcoin/pull/29988)
<!--
*** 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 for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that improve cove
...

🚀 fanquake merged a pull request: "guix: remove bzip2 from deps"
(https://github.com/bitcoin/bitcoin/pull/29895)

💬 fanquake commented on pull request "build: Bump clang minimum supported version to 15":
(https://github.com/bitcoin/bitcoin/pull/29165#issuecomment-2081780084)
Yea. I think we can merge our independant oss-fuzz bump, then merge this, then revert the oss-fuzz change if/when the global oss-fuzz bump happens. That has dragged on a bit too long, and is just blocking things here.

📝 fanquake opened a pull request: "depends: sqlite 3.45.3"
(https://github.com/bitcoin/bitcoin/pull/29991)
Update sqlite in depends from [3.38.5](https://sqlite.org/releaselog/3_38_5.html) to [3.45.3](https://sqlite.org/releaselog/3_45_3.html).

💬 maflcko commented on pull request "guix: build with glibc 2.31":
(https://github.com/bitcoin/bitcoin/pull/29987#issuecomment-2082038386)
It would be good to mention that this drops support for Ubuntu Bionic 18.04 and RHEL-8 (and forks) completely, going forward.

💬 fanquake commented on pull request "guix: build with glibc 2.31":
(https://github.com/bitcoin/bitcoin/pull/29987#issuecomment-2082041899)
That is shown in the changes in symbol-check, but I'll add it to the op, and can add a rel note.

💬 laanwj commented on pull request "depends: Remove Qt build-time dependencies":
(https://github.com/bitcoin/bitcoin/pull/29923#issuecomment-2082089346)
Rebased for merge conflict and to apply fixup commits.

👍 maflcko approved a pull request: "fuzz: don't allow adding duplicate transactions to the mempool"
(https://github.com/bitcoin/bitcoin/pull/29990#pullrequestreview-2027780727)
lgtm ACK cc15c5bfd1eb3903de246c124702a7c66c687008

I guess this isn't due to a fuzz crash, because the only thing that's off is `cachedInnerUsage` (and friends), which are not checked in this fuzz target?

💬 laanwj commented on pull request "depends: Fix build of Qt for 32-bit platforms with recent glibc":
(https://github.com/bitcoin/bitcoin/pull/29985#issuecomment-2082091570)
Force-pushed to apply @hebasto's comment to make the patch apply cleanly.

💬 virtu commented on pull request "contrib: Add asmap-tool":
(https://github.com/bitcoin/bitcoin/pull/28793#issuecomment-2082138376)
ACK [6abe772](https://github.com/bitcoin/bitcoin/commit/6abe772a17e09fe96e68cd3311280d5a30f6378b)

Tested decoding and encoding some [demo ASMaps](https://github.com/fjahr/asmap-data).

Also made sure encoding yields a reproducible ordering by decoding, `shuf`'ling, and re-encoded an ASMap and making sure the resulting encoding was identical to the original one.

👋 Sjors's pull request is ready for review: "doc: add LLVM instruction for macOS < 13"
(https://github.com/bitcoin/bitcoin/pull/29934)

💬 Sjors commented on pull request "doc: add LLVM instruction for macOS < 13":
(https://github.com/bitcoin/bitcoin/pull/29934#issuecomment-2082144574)
I dropped the `@14` from the default instruction, but added a hint to try `@17` if that fails. That worked when I tested and I don't expect Homebrew formulas for older versions to change much moving forward, nor llvm 17 itself.