🤔 furszy reviewed a pull request: "doc, test: test and explain service flag handling"
(https://github.com/bitcoin/bitcoin/pull/29213#pullrequestreview-1822266935)
ACK 74ebd4d1359edce82a134dfcd3da9840f8d206e2

💬 TheCharlatan commented on pull request "depends: add NM output to gen_id":
(https://github.com/bitcoin/bitcoin/pull/29249#discussion_r1452755036)
Should this be `host_NM`? This does not rebuild for me if I change the `NM` var other otherwise.

📝 furszy opened a pull request: "wallet: guard against dangling to-be-reverted db transactions"
(https://github.com/bitcoin/bitcoin/pull/29253)
Discovered while was reviewing #29112, specifically https://github.com/bitcoin/bitcoin/pull/29112#pullrequestreview-1821862931.

If the db handler that initiated the database transaction is destroyed,
the ongoing transaction cannot be left dangling when the db txn fails
to abort. It must be forcefully reversed; otherwise, any subsequent
db handler executing a write operation will dump the dangling,
to-be-reverted transaction data to disk.

This not only breaks the database isolation prop
...

🤔 furszy reviewed a pull request: "sqlite: Disallow writing from multiple `SQLiteBatch`s"
(https://github.com/bitcoin/bitcoin/pull/29112#pullrequestreview-1822384265)
Done. Created #29253 to fix the root issue. I took some of my commits there.

With it, the fix for the problem https://github.com/bitcoin/bitcoin/pull/29112#pullrequestreview-1821862931 is straightforward. After resetting the database connection, we can release the lock, as closing the connection automatically rolls back the active transaction (see the PR). This ensures isolation across db handlers and will prevent deadlocks.

💬 ajtowns commented on pull request "logging: Update to new logging API":
(https://github.com/bitcoin/bitcoin/pull/29231#issuecomment-1892927743)
> Suggested fixes in [master...stickies-v:bitcoin:2024-01/fix-log-level-29231](https://github.com/bitcoin/bitcoin/compare/master...stickies-v:bitcoin:2024-01/fix-log-level-29231) - happy to open a separate pull for it too?

Added these commits

🤔 Sadiq272 requested changes to a pull request: "[WIP] wallet: standardize change output detection process"
(https://github.com/bitcoin/bitcoin/pull/25979#pullrequestreview-1822406678)
Oba

💬 Sadiq272 commented on pull request "[WIP] wallet: standardize change output detection process":
(https://github.com/bitcoin/bitcoin/pull/25979#issuecomment-1892948544)
Pul

💬 Sadiq272 commented on pull request "[WIP] wallet: standardize change output detection process":
(https://github.com/bitcoin/bitcoin/pull/25979#issuecomment-1892948690)
Heti canavarlar

🤔 furszy reviewed a pull request: "[WIP] wallet: standardize change output detection process"
(https://github.com/bitcoin/bitcoin/pull/25979#pullrequestreview-1822455771)
Hmm, this is on a long-working path. Will try to rebase it in the coming days or close it until we proceed with the preceding PRs if it is too behind.

💬 jonatack commented on pull request "[25.x] Backports":
(https://github.com/bitcoin/bitcoin/pull/28768#issuecomment-1893019031)
#29200 would be relevant.

📝 reardencode converted_to_draft a pull request: "OP_CHECKTEMPLATEVERIFY, OP_CHECKSIGFROMSTACK(VERIFY), OP_INTERNALKEY validation (LNHANCE)"
(https://github.com/bitcoin/bitcoin/pull/29198)
This pull request contains a the same implementation of OP_CHECKTEMPLATEVERIFY (BIP119) as @jamesob's [Covenant Tools](https://github.com/bitcoin/bitcoin/pull/28550), an implementation of [OP_CHECKSIGFROMSTACK(VERIFY)](https://github.com/bitcoin/bips/pull/1535) and of [OP_INTERNALKEY](https://github.com/bitcoin/bips/pull/1534).

There are no testnet or mainnet activation parameters proposed in this pull request. I am deeply uninterested in the details of activation semantics.

This combinati
...

💬 reardencode commented on pull request "OP_CHECKTEMPLATEVERIFY, OP_CHECKSIGFROMSTACK(VERIFY), OP_INTERNALKEY validation (LNHANCE)":
(https://github.com/bitcoin/bitcoin/pull/29198#issuecomment-1893147139)
Converted this to a draft while I work through AJ's review on my similar PR to inquisition. Will port those changes over here and undraft when the time comes.

💬 maflcko commented on pull request "kernel: Remove key module from kernel library":
(https://github.com/bitcoin/bitcoin/pull/29252#issuecomment-1893335214)
> The gui tests currently keep multiple `NodeContext` objects in memory, so relax the `secp256k1_context_sign` check in `ECC_Start` to instead return early if it has already been initialized.

An alternative would be to call `ECC_Stop` in the gui tests instead of modifying key code?

💬 maflcko commented on pull request "contrib: Add -binary option to clang-format-diff":
(https://github.com/bitcoin/bitcoin/pull/29251#issuecomment-1893338875)
I think you can just pull the latest version?

💬 BossMannKali commented on pull request "OP_CHECKTEMPLATEVERIFY, OP_CHECKSIGFROMSTACK(VERIFY), OP_INTERNALKEY validation (LNHANCE)":
(https://github.com/bitcoin/bitcoin/pull/29198#issuecomment-1893343037)
BO$$ KAL!

✅ glozow closed a pull request: "Fix typos"
(https://github.com/bitcoin/bitcoin/pull/29246)

💬 glozow commented on pull request "Fix typos":
(https://github.com/bitcoin/bitcoin/pull/29246#issuecomment-1893347901)
Thank you for your interest in contributing. Please see https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#pull-request-philosophy

💬 TheCharlatan commented on pull request "contrib: Add -binary option to clang-format-diff":
(https://github.com/bitcoin/bitcoin/pull/29251#issuecomment-1893355836)
There were a bunch of changes made to this file to make it "Bitcoin Core" style. Re-applying all of them seems a bit burdensome. Would it be ok to just overwrite these?

💬 maflcko commented on pull request "contrib: Add -binary option to clang-format-diff":
(https://github.com/bitcoin/bitcoin/pull/29251#issuecomment-1893357715)
Sure. Seems fine to drop them. (If there are any important, you can re-cherry-pick them again on top)?

💬 fanquake commented on pull request "[25.x] Backports":
(https://github.com/bitcoin/bitcoin/pull/28768#discussion_r1453154604)
Dropped.