🤔 stickies-v reviewed a pull request: "[25.x] Backports"
(https://github.com/bitcoin/bitcoin/pull/28768#pullrequestreview-1822192937)
LGTM 115e31b8e902c02ad4b342843c4811b5e74415d0 modulo small release notes issue. Also, PR description needs to be updated to remove the reference to https://github.com/bitcoin/bitcoin/pull/28919.

💬 stickies-v commented on pull request "[25.x] Backports":
(https://github.com/bitcoin/bitcoin/pull/28768#discussion_r1452702149)
I don't think this is backported?

💬 furszy commented on pull request "init: handle empty settings file gracefully":
(https://github.com/bitcoin/bitcoin/pull/29144#discussion_r1452708666)
Since the confusion over the empty settings file error is resolved by the first commit. I agree with ryanofsky; while extending the file warning message sounds plausible, I'm also ok with the current warning.

👍 TheCharlatan approved a pull request: "contrib: add macho branch protection check"
(https://github.com/bitcoin/bitcoin/pull/29170#pullrequestreview-1822235032)
ACK 5335e454c0889c8a1bb05aa09435883322133974

💬 mzumsande commented on pull request "doc, test: test and explain service flag handling":
(https://github.com/bitcoin/bitcoin/pull/29213#discussion_r1452746812)
I think that followed from the `@return` doc, but it wasn't so obvious that adding an existing addr to another new bucket also affects the return value; Changed the `@return` description to `true if at least one address is successfully added, or added to an additional bucket. Unaffected by updates.`

💬 mzumsande commented on pull request "doc, test: test and explain service flag handling":
(https://github.com/bitcoin/bitcoin/pull/29213#discussion_r1452746873)
done

💬 mzumsande commented on pull request "doc, test: test and explain service flag handling":
(https://github.com/bitcoin/bitcoin/pull/29213#discussion_r1452747951)
I think it's better if the header doesn't refer to implementation details. If I remember correctly, this logic used to be in `Add()`, then `Add_()`, then `AddSingle()`, and I don't think it's nice if we have to update `addrman.h` for refactors like that.

💬 mzumsande commented on pull request "doc, test: test and explain service flag handling":
(https://github.com/bitcoin/bitcoin/pull/29213#discussion_r1452749219)
similar to above, I don't like referring to implementation details in comments unless really necessary.

💬 mzumsande commented on pull request "doc, test: test and explain service flag handling":
(https://github.com/bitcoin/bitcoin/pull/29213#issuecomment-1892777051)
[d536e5a ](https://github.com/bitcoin/bitcoin/commit/d536e5a6325d1885224f36debdcc5245b94efe8a)to [74ebd4d](https://github.com/bitcoin/bitcoin/commit/74ebd4d1359edce82a134dfcd3da9840f8d206e2): Addressed feedback

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