💬 Sjors commented on pull request "Stratum v2 Template Provider (take 2)":
(https://github.com/bitcoin/bitcoin/pull/28983#issuecomment-1892642385)
Added a (ephemeral) client id to the log so it's easier to figure out what's going on when there's multiple connections.

💬 Kenshiro-28 commented on issue "Implement PayJoin / Pay-to-EndPoint":
(https://github.com/bitcoin/bitcoin/issues/19148#issuecomment-1892671561)
Great, I hope it's implemented :)

🤔 furszy reviewed a pull request: "init: handle empty settings file gracefully"
(https://github.com/bitcoin/bitcoin/pull/29144#pullrequestreview-1822184082)
> But It does seem unfortunate that both of the commits here make tests more verbose and repetitive. And the tests even before this PR were probably unnecessarily fragile. But I'm not sure I see a better way to write the tests, or a way to avoid the repetition of warning and error messages. If anybody has any ideas on how to improve the tests here, I'd be interested to know.

This is not a small change, but.. we could work on a `Resources` class that is auto-generated during the compile phase.
...

🤔 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