💬 ryanofsky commented on pull request "wallet: guard against dangling to-be-reverted db transactions":
(https://github.com/bitcoin/bitcoin/pull/29253#discussion_r1471853965)
re: https://github.com/bitcoin/bitcoin/pull/29253#discussion_r1471199191

Thanks for clarifying.

> I wanted to emphasize the importance of rolling back the transaction by explaining the consequences of leaving the transaction dangling.

That seems ok. Would maybe suggest adding to the previously suggested comment:

* // 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 the database. Closing t
...

👍 ryanofsky approved a pull request: "wallet: guard against dangling to-be-reverted db transactions"
(https://github.com/bitcoin/bitcoin/pull/29253#pullrequestreview-1852174380)
Code review ACK 756bed0529b46944162c02c783ecc804f802b637. New update consolidating the exec classes looks good.

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

It's technically not an abstract class anymore since it doesn't contain any `= 0` pure virtual methods. Would maybe suggest "Class responsible for executing SQL statements in SQLite databases. Methods are virtual so they can be overridden by unit tests testing unusual database conditions."

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

This doesn't seem right because the `&&` operator will convert the integer return value into a bool. Also if m_exec_handler is null the `res` value will be 0 which is SQLITE_OK so the call will appear to succed.

It probably would make more sense to do `int res = Assert(m_exec_handler->Exec`

💬 Wwwwwwuuuuu commented on issue "assumeutxo: nTx and nChainTx in RPC results are off":
(https://github.com/bitcoin/bitcoin/issues/29328#issuecomment-1917783750)
Please help me figure out why this transaction is not confirmed?8f62acf0f0ddc3e38db2e8ec2755f4e54aa5bf616d685e2f3361155fc9d44125

💬 Wwwwwwuuuuu commented on issue "Cluster mempool, CPFP carveout, and V3 transaction policy":
(https://github.com/bitcoin/bitcoin/issues/29319#issuecomment-1917814292)
Please help me figure out why this transaction is not confirmed?8f62acf0f0ddc3e38db2e8ec2755f4e54aa5bf616d685e2f3361155fc9d44125

💬 Wwwwwwuuuuu commented on issue "Proposal for a new mempool design":
(https://github.com/bitcoin/bitcoin/issues/27677#issuecomment-1917817576)
Please help me figure out why this transaction is not confirmed?8f62acf0f0ddc3e38db2e8ec2755f4e54aa5bf616d685e2f3361155fc9d44125

💬 instagibbs commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1471908574)
double-checking: are we sure we can't hit this? @sdaftuar

💬 furszy commented on pull request "wallet: guard against dangling to-be-reverted db transactions":
(https://github.com/bitcoin/bitcoin/pull/29253#discussion_r1471915202)
yes, fixed. Thanks. God knows why I did that.. it seems that I wasn't focused.

💬 furszy commented on pull request "wallet: guard against dangling to-be-reverted db transactions":
(https://github.com/bitcoin/bitcoin/pull/29253#discussion_r1471915666)
yeah, fixed. I pasted your doc suggestion first, then made the classes consolidation..

📝 sr-gi opened a pull request: "test: fixes flaky wallet_import_rescan functional test"
(https://github.com/bitcoin/bitcoin/pull/29350)
wallet_import_rescan is, potentially, not rounding values used for `sendtoaddress` in one of the test cases. Under normal conditions, the value is randomly picked and rounded to 8 decimal positions, however, for this specific case the value is limited to a maximum value that is not properly rounded. If that value happens to be picked, the test will fail.

I noticed this in a completely unrelated PR where the test failed: https://cirrus-ci.com/task/5467779164274688?logs=ci#L2684

<!--
*** Pl
...

💬 furszy commented on pull request "wallet: guard against dangling to-be-reverted db transactions":
(https://github.com/bitcoin/bitcoin/pull/29253#discussion_r1471941289)
All good, applied your suggestion. Thanks!

> This is exactly what I think the comment should not be saying because the error cannot be transient. And it's confusing to suggest that it could be transient, because it either makes the rest of the code seem broken, or it makes this code seem unnecessary.

I was also thinking about an I/O hardware malfunctioning error, which could be transient; sqlite has return codes for it. But it's probably better to abort the program if something like this e
...

💬 Wwwwwwuuuuu commented on pull request "test: fixes flaky wallet_import_rescan functional test":
(https://github.com/bitcoin/bitcoin/pull/29350#issuecomment-1917873911)
Please help me figure out why this transaction is not confirmed?8f62acf0f0ddc3e38db2e8ec2755f4e54aa5bf616d685e2f3361155fc9d44125

🤔 furszy reviewed a pull request: "wallet: guard against dangling to-be-reverted db transactions"
(https://github.com/bitcoin/bitcoin/pull/29253#pullrequestreview-1852350177)
Updated per feedback, thanks ryanofsky!
Docstrings were improved, and a bug related to a null `SQLiteExecHandler` has been fixed.
[Small diff](https://github.com/bitcoin/bitcoin/compare/756bed0529b46944162c02c783ecc804f802b637..b298242c8d495c36072415e1b95eaa7bf485a38a).

⚠️ Wwwwwwuuuuu opened an issue: "Please help me figure out why this transaction is not confirmed?"
(https://github.com/bitcoin/bitcoin/issues/29351)
### Issues, reports or feature requests related to the GUI should be opened directly on the GUI repo

- [X] I still think this issue should be opened here

### Report

Please help me figure out why this transaction is not confirmed?8f62acf0f0ddc3e38db2e8ec2755f4e54aa5bf616d685e2f3361155fc9d44125

🤔 furszy reviewed a pull request: "test: fixes flaky wallet_import_rescan functional test"
(https://github.com/bitcoin/bitcoin/pull/29350#pullrequestreview-1852354792)
See #29343.

💬 furszy commented on pull request "test: fixes flaky wallet_import_rescan functional test":
(https://github.com/bitcoin/bitcoin/pull/29350#discussion_r1471953382)
you are overwriting the random number.

💬 murchandamus commented on pull request "wallet: Add CoinGrinder coin selection algorithm":
(https://github.com/bitcoin/bitcoin/pull/27877#issuecomment-1917887536)
The `p2p_v2_earlykeyresponse.py` test is unrelated.

✅ willcl-ark closed an issue: "Please help me figure out why this transaction is not confirmed?"
(https://github.com/bitcoin/bitcoin/issues/29351)

💬 willcl-ark commented on issue "Please help me figure out why this transaction is not confirmed?":
(https://github.com/bitcoin/bitcoin/issues/29351#issuecomment-1917891085)
(It looks like the fee is a bit low for current mempool conditions.)

This issue tracker is used to track technical issues related to the Bitcoin Core code base.

General bitcoin questions and/or support requests are best directed to the [Bitcoin StackExchange](https://bitcoin.stackexchange.com/) or the #bitcoin IRC channel on the [Libera Chat](https://libera.chat/) network.

For proposed protocol changes you can post to the bitcoin-dev [mailing list](https://lists.linuxfoundation.org/mail
...