📝 1amhesus opened a pull request: "Add new kernel"
(https://github.com/bitcoin/bitcoin/pull/29322)
<!--
*** 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 improv
...

✅ 1amhesus closed a pull request: "Add new kernel"
(https://github.com/bitcoin/bitcoin/pull/29322)

📝 fanquake locked a pull request: "[temp] Add new kernel"
(https://github.com/bitcoin/bitcoin/pull/29322)
<!--
*** 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 improv
...

💬 Sjors commented on pull request "Support self-hosted Cirrus workers on forks (and multi-user)":
(https://github.com/bitcoin/bitcoin/pull/29274#issuecomment-1910846023)
I switched my own machine from Docker to Podman. This removes the need for 7cdb75d572e825b4c0cf74a516f912f94807853f so that's dropped.

Added a commit that explains the process a bit more clearly.

💬 Sjors commented on pull request "Support self-hosted Cirrus workers on forks (and multi-user)":
(https://github.com/bitcoin/bitcoin/pull/29274#discussion_r1466829063)
These lines are dropped now.

💬 Sjors commented on pull request "Support self-hosted Cirrus workers on forks (and multi-user)":
(https://github.com/bitcoin/bitcoin/pull/29274#discussion_r1466829719)
I nuked Docker in favor of Podman. So far it seems to work. I dropped 7cdb75d572e825b4c0cf74a516f912f94807853f.

💬 instagibbs commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1466864661)
Was thinking about why this is ok. AcceptMultipleTransactions doesn't allow RBF, and future package RBF should work with this?

```suggestion
// The mempool or in-package parent cannot have any other in-mempool children.
// We don't need to handle the "single RBF" case as that is
// handled in ApplyV3Rules for single transaction packages.
// Future package RBF would conflict with the parent as well,
// making this check sufficien
...

🤔 mzumsande reviewed a pull request: "test/BIP324: functional tests for v2 P2P encryption"
(https://github.com/bitcoin/bitcoin/pull/24748#pullrequestreview-1844567036)
Code Review ACK bc9283c4415a932ec1eeb70ca2aa4399c80437b3

💬 ryanofsky commented on pull request "Improve new LogDebug/Trace/Info/Warning/Error Macros":
(https://github.com/bitcoin/bitcoin/pull/29256#issuecomment-1910905907)
@ajtowns, to try to steer this discussion in a more constructive direction, would it be possible for you to make a list of the biggest harms you believe this PR would cause if it were merged? And if you have ideas on how the harms you see could be avoided, while the problems I am trying to solve (see "Problems this PR attempts to solve" in the PR description) could be addressed, it would be really helpful to know about. Maybe the harms you see this PR causing and the problems this PR is trying t
...

💬 achow101 commented on pull request "During IBD, prune as much as possible until we get close to where we will eventually keep blocks":
(https://github.com/bitcoin/bitcoin/pull/20827#issuecomment-1910911055)
ACK d298ff8b62b2624ed390c8a2f905c888ffc956ff

💬 achow101 commented on issue "getdescriptorinfo returns unusable descriptor":
(https://github.com/bitcoin/bitcoin/issues/29320#issuecomment-1910919438)
The purpose of the `descriptor` result of `getdescriptorinfo` is to reflect a descriptor that basically matches the user's input, just without including any private keys. `listdescriptors` specifically returns normalized descriptors so that they can be imported elsewhere.

`getdescriptorinfo` could also return the normalized descriptor, but it intentionally does not do that for `descriptor`. I suppose a separate field containing the normalized form could be added.

🚀 achow101 merged a pull request: "During IBD, prune as much as possible until we get close to where we will eventually keep blocks"
(https://github.com/bitcoin/bitcoin/pull/20827)

👍 kristapsk approved a pull request: "debugwindow: update session ID tooltip"
(https://github.com/bitcoin-core/gui/pull/788#pullrequestreview-1844619925)
ACK 3bf00e13609eefa6ddb11353519bb1aec2342513

✅ achow101 closed a pull request: "test: Generate coverage report without running tests"
(https://github.com/bitcoin/bitcoin/pull/28772)

🤔 ryanofsky reviewed a pull request: "wallet: guard against dangling to-be-reverted db transactions"
(https://github.com/bitcoin/bitcoin/pull/29253#pullrequestreview-1838949232)
Code review d4ebac8381801d024b69d5f1f5a13246c61edbe4. This seems like a nice change that could make the wallet more reliable, and adds good cleanup and test coverage.

Did not ack just because I noticed a bug in SQLiteBatch::TxnBegin if `m_db` is null, but none of my other comments are too important, so feel free to ignore them.

re: https://github.com/bitcoin/bitcoin/pull/29253#issuecomment-1910567093

> In general, we don't have very robust error handling for when the database fails to r
...

💬 ryanofsky commented on pull request "wallet: guard against dangling to-be-reverted db transactions":
(https://github.com/bitcoin/bitcoin/pull/29253#discussion_r1463377113)
In commit "sqlite: add ability to interrupt statements" (ca39d812e4766575e25721296fa2d43dfe7eba7e)

It would be good to define -999 as a constant so callers could check for this error. Or if there probably won't be a need for callers to do that, it would be clearer to return SQLITE_ERROR.

💬 ryanofsky commented on pull request "wallet: guard against dangling to-be-reverted db transactions":
(https://github.com/bitcoin/bitcoin/pull/29253#discussion_r1466931843)
In commit "sqlite: add ability to interrupt statements" (ca39d812e4766575e25721296fa2d43dfe7eba7e)

Would suggest calling this `SQliteExecBlocker`, or maybe `SQliteExecHook` or `SQliteTestHook` if you anticipate it being used more broadly for other things in the future. "ExecHandler" seems like the wrong name because the class itself isn't executing anything and doesn't call sqlite3_exec.

Would also suggest adding a comment saying this is used for testing, because otherwise it's not clear w
...

💬 ryanofsky commented on pull request "wallet: guard against dangling to-be-reverted db transactions":
(https://github.com/bitcoin/bitcoin/pull/29253#discussion_r1466947379)
In commit "sqlite: introduce HasActiveTxn method" (5670a21f5ef28748b6b7242c03b82f39dcbc983f)

It would be nice to have a comment explaining the use of sqlite3_get_autocommit, since the name doesn't really explain what it is doing. Maybe "// sqlite3_get_autocommit returns true by default, and false if a transaction has begun and not been committed or rolled back."

Also, I noticed https://www.sqlite.org/c3ref/get_autocommit.html says "If another thread changes the autocommit status of the dat
...

💬 ryanofsky commented on pull request "wallet: guard against dangling to-be-reverted db transactions":
(https://github.com/bitcoin/bitcoin/pull/29253#discussion_r1466966956)
In commit "sqlite: guard against dangling to-be-reverted db transactions" (508fda2fddfef0cefedde9777c914aec72adf7fb)

I found this comment confusing because I couldn't figure out when failing to abort a transaction would ever be expected to happen, and what the error handling strategy was supposed to be when it did happen. Would maybe suggest: "// If transaction cannot be aborted, it means there is a bug or there has been data corruption. Try to recover in this case by closing and reopening th
...

💬 ryanofsky commented on pull request "wallet: guard against dangling to-be-reverted db transactions":
(https://github.com/bitcoin/bitcoin/pull/29253#discussion_r1466968974)
In commit "test: sqlite, add coverage for dangling to-be-reverted db txns" (d4ebac8381801d024b69d5f1f5a13246c61edbe4)

I think it would be a little more consistent with other code to call this variable "batch" instead of "handler." Also calling it handler gets a little confusing when code below sets a handler on the handler.