📝 hebasto opened a pull request: "Update libsecp256k1 subtree to current master"
(https://github.com/bitcoin/bitcoin/pull/30333)
This includes recent changes to the CMake-based build system required for Bitcoin Core [migration](https://github.com/bitcoin/bitcoin/issues/28607) to CMake.

💬 hebasto commented on pull request "Update libsecp256k1 subtree to current master":
(https://github.com/bitcoin/bitcoin/pull/30333#issuecomment-2188660629)
cc @real-or-random

📝 fanquake opened a pull request: "Update libsecp256k1 subtree to latest master"
(https://github.com/bitcoin/bitcoin/pull/30334)
Updates the libsecp256k1 subtree to https://github.com/bitcoin-core/secp256k1/commit/f473c959f08edcb73669142f872d2189950bc54a. This includes a number of CMake related changes, including one that prevents CMake from segfaulting when we were configuring the subtree. A number of these changes have come from the review/discussion in https://github.com/hebasto/bitcoin/pull/192:

* https://github.com/bitcoin-core/secp256k1/pull/1529
* https://github.com/bitcoin-core/secp256k1/pull/1532
* https://g
...

💬 fanquake commented on pull request "Update libsecp256k1 subtree to latest master":
(https://github.com/bitcoin/bitcoin/pull/30334#issuecomment-2188664111)
cc @real-or-random. Depending on whether https://github.com/bitcoin-core/secp256k1/pull/1535 goes in, we could also pull that, but just a cleanup.

✅ hebasto closed a pull request: "Update libsecp256k1 subtree to current master"
(https://github.com/bitcoin/bitcoin/pull/30333)

💬 hebasto commented on pull request "Update libsecp256k1 subtree to current master":
(https://github.com/bitcoin/bitcoin/pull/30333#issuecomment-2188669426)
Closing in favour of #30334.

💬 ismaelsadeeq commented on pull request "Wallet: Add `maxfeerate` wallet startup option":
(https://github.com/bitcoin/bitcoin/pull/29278#issuecomment-2188672354)
Rebased to fix merge conflict.

👍 hebasto approved a pull request: "Update libsecp256k1 subtree to latest master"
(https://github.com/bitcoin/bitcoin/pull/30334#pullrequestreview-2138344325)
ACK 4c67e14cacef1903800b4cf4e32a12922d36025f, I've got a zero diff with my local branch, which reproduces the subtree update.

📝 Sjors opened a pull request: "Mining interface followups"
(https://github.com/bitcoin/bitcoin/pull/30335)
Followups from #30200

- `std::unique_ptr` needs `#include <memory>` (noticed while working on #30332, which has fewer includes than its parent PR that I originally tested with)
- move out argument `state` to the end of `testBlockValidity`, see https://github.com/bitcoin/bitcoin/pull/30200#discussion_r1647987176

💬 Sjors commented on pull request "Introduce Mining interface":
(https://github.com/bitcoin/bitcoin/pull/30200#discussion_r1652627253)
Actually, doing this now since I ran into another (minor) issue: #30335

💬 Sjors commented on pull request "Introduce Mining interface":
(https://github.com/bitcoin/bitcoin/pull/30200#discussion_r1652645443)
Fixed in #30335

💬 Sjors commented on pull request "Mining interface followups":
(https://github.com/bitcoin/bitcoin/pull/30335#discussion_r1652647962)
Added blank line here for consistency.

💬 BenWestgate commented on pull request "contrib: Fixup verify-binaries OS platform parsing":
(https://github.com/bitcoin/bitcoin/pull/30147#discussion_r1652648077)
That does look nicer, implementing your suggestion. Thanks for teaching me `.partition()` and `.startswith()`.

💬 ismaelsadeeq commented on pull request "[test]: prevent `create_self_transfer` failure when target weight is below tx weight":
(https://github.com/bitcoin/bitcoin/pull/30322#discussion_r1652649739)
updated.

💬 stickies-v commented on pull request "contrib: Fixup verify-binaries OS platform parsing":
(https://github.com/bitcoin/bitcoin/pull/30147#discussion_r1652654157)
Great. I didn't mean to take out your example docstrings btw so feel free to add those/similar ones in, it's helpful.

💬 paplorinc commented on pull request "optimization: Moved repeated `-printpriority` fetching out of AddToBlock":
(https://github.com/bitcoin/bitcoin/pull/30324#discussion_r1652654540)
The [commit](https://github.com/bitcoin/bitcoin/commit/400b15147cf1c4757927935222611e8d3481e739) by @morcos indicates there's still use for the print statement - even though it wasn't used correctly in the tests, I've removed that reference.

💬 BenWestgate commented on pull request "contrib: Fixup verify-binaries OS platform parsing":
(https://github.com/bitcoin/bitcoin/pull/30147#discussion_r1652656319)
```python3
def parse_version_string(version_str):
parts = version_str.split('-')
# "<version>" or "<version>-platform"
version_base, _, version_os = version_str.partition('-')
version_rc = ""
if version_os.startswith("rc"): # "<version>-rcN[-platform]"
version_rc, _, version_os = version_os.partition('-')

return version_base, version_rc, version_os
```

Look good to you?

💬 stickies-v commented on pull request "contrib: Fixup verify-binaries OS platform parsing":
(https://github.com/bitcoin/bitcoin/pull/30147#discussion_r1652658769)
Actually: would `version`, `rc`, `platform` be better variable names?

💬 stickies-v commented on pull request "contrib: Fixup verify-binaries OS platform parsing":
(https://github.com/bitcoin/bitcoin/pull/30147#discussion_r1652661626)
> Look good to you?

No need for the `parts = ` statement I think.

> `# "<version>" or "<version>-platform"`

That seems wrong, given that we may also have an `rc` part still?

💬 BenWestgate commented on pull request "contrib: Fixup verify-binaries OS platform parsing":
(https://github.com/bitcoin/bitcoin/pull/30147#discussion_r1652677181)
All of your comments are right! Glad I asked.

Looks better with the new local variable names:

```python3

def parse_version_string(version_str):
# "<version>[-rcN][-platform]"
version_base, _, platform = version_str.partition('-')
rc = ""
if platform.startswith("rc"): # "<version>-rcN[-platform]"
rc, _, platform = platform.partition('-')
# else "<version>" or "<version>-platform"

return version_base, rc, platform
```

I don't love my first comm
...