💬 mjdietzx commented on pull request "tests: add functional test for miniscript decaying multisig":
(https://github.com/bitcoin/bitcoin/pull/29156#discussion_r1582265867)
After addressing comments from your review, I made some simplifications and the two tests are now different enough I think this is no longer the case. The tests are still similar in their high-level structure, but otherwise not much code is duplicated (and definitely no complex/deep logic)

💬 laanwj commented on pull request "depends: Fix build of Qt for 32-bit platforms with recent glibc":
(https://github.com/bitcoin/bitcoin/pull/29985#discussion_r1582279290)
Thanks, will update

💬 fjahr commented on pull request "prune, rpc: Check undo data when finding pruneheight":
(https://github.com/bitcoin/bitcoin/pull/29668#discussion_r1582311337)
done

💬 fjahr commented on pull request "prune, rpc: Check undo data when finding pruneheight":
(https://github.com/bitcoin/bitcoin/pull/29668#discussion_r1582311605)
done, I liked that `BLOCK_HAVE_DATA | BLOCK_HAVE_UNDO` is more expressive because we don't use `BLOCK_HAVE_MASK` much I feel like people would need to look it up more, but I have just added a comment instead now.

💬 fjahr commented on pull request "prune, rpc: Check undo data when finding pruneheight":
(https://github.com/bitcoin/bitcoin/pull/29668#discussion_r1582311745)
done

💬 fjahr commented on pull request "prune, rpc: Check undo data when finding pruneheight":
(https://github.com/bitcoin/bitcoin/pull/29668#issuecomment-2081618206)
@stickies-v Thank you, I mostly took your suggestions with some minor tweaks and made you co-author of the refactoring commit. I was a bit torn about letting the caller handle the Genesis block issue but since this was actually the previous behavior I guess it's better to not change it if you like it the way it is. I have adapted `GetPruneHeight` to return `std::optional` which I found a good fit and avoids the `-1` return value. And I think the change in `init.cpp` isn't needed here (yet). But
...

💬 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.