💬 ryanofsky commented on pull request "sqlite: Disallow writing from multiple `SQLiteBatch`s":
(https://github.com/bitcoin/bitcoin/pull/29112#discussion_r1480783903)
In commit "tests: Test for concurrent writes with db tx" (548ecd11553bea28bc79e6f9840836283e9c4e99)

Could there be a comment to explain unsafesqlitesync? Is it just to make the test faster, or is it actually important to the functioning of the test?

💬 ryanofsky commented on pull request "sqlite: Disallow writing from multiple `SQLiteBatch`s":
(https://github.com/bitcoin/bitcoin/pull/29112#discussion_r1480796544)
In commit "test: wallet, coverage for concurrent db transactions" (cfcb9b1ecf9be5267487713fa1e112ca09a1ae55

Nothing about this test seems sqlite-specific. Seems like it would be nicer to call `TestDatabases` and be generic.

💬 ryanofsky commented on pull request "sqlite: Disallow writing from multiple `SQLiteBatch`s":
(https://github.com/bitcoin/bitcoin/pull/29112#discussion_r1480789358)
In commit "test: wallet, coverage for concurrent db transactions" (cfcb9b1ecf9be5267487713fa1e112ca09a1ae55)

Would be good to clarify why it is not possible. Maybe "Which must not be possible because batch2->TxnBegin() was never called."

💬 ryanofsky commented on pull request "sqlite: Disallow writing from multiple `SQLiteBatch`s":
(https://github.com/bitcoin/bitcoin/pull/29112#discussion_r1480799534)
In commit "tests: Test for concurrent writes with db tx" (548ecd11553bea28bc79e6f9840836283e9c4e99)

This seems fine, but you could probably increase the probability of this by calling keypoolrefill in a loop until gettxoutsetinfo returns instead of calling it once.

💬 sipa commented on issue "doc: List identifiers used from header in #if(def)":
(https://github.com/bitcoin/bitcoin/issues/26972#issuecomment-1931178143)
@theuni That sounds to me like the only sensible approach for *any* header file: include it if you need something from it directly. If A needs B and C, and B includes C, then A should still include both B and C, because its includes shouldn't depend on the knowledge of what B depends on (as that can change from under it).

💬 Isa232323 commented on issue "Release schedule for 27.0":
(https://github.com/bitcoin/bitcoin/issues/29028#issuecomment-1931216646)
> Here is a proposed release schedule for `v27.0`, the next major release of Bitcoin Core. Dates roughly chosen as discussed in the most recent IRC meeting.
>
> ## 2024-02-01 🚧
> - Open Transifex translations for `v27.0`
> - Soft translation string freeze (no large or non-critical string changes until release)
> - Finalize and close translations for `v25.0`
>
> ## 2024-02-12 🚧 :
> - Feature freeze (bug fixes only until release)
> - Translation string freeze (no more source language ch
...

📝 hernanmarino opened a pull request: "test, assumeutxo: Add test to ensure failure when mempool not empty"
(https://github.com/bitcoin/bitcoin/pull/29394)
Add a test to ensure that loadtxoutset fials when the node's mempool is not empty, as suggested by @maflcko here: https://github.com/bitcoin/bitcoin/pull/27596#discussion_r1344713537

💬 hernanmarino commented on pull request "assumeutxo (2)":
(https://github.com/bitcoin/bitcoin/pull/27596#discussion_r1480923337)
I> Could add a functional test for this?

I just did, see https://github.com/bitcoin/bitcoin/pull/29394

💬 hernanmarino commented on issue "test: Write assumeutxo tests":
(https://github.com/bitcoin/bitcoin/issues/28648#issuecomment-1931330307)
> There'd be [#27596 (comment)](https://github.com/bitcoin/bitcoin/pull/27596#discussion_r1344713537) if someone wants to tackle it.

Done, reviews welcome https://github.com/bitcoin/bitcoin/pull/29394

📝 araujo88 opened a pull request: "test: handle wallet_reorgrestore.py failed"
(https://github.com/bitcoin/bitcoin/pull/29395)
This PR fixes test issue mentioned in https://github.com/bitcoin/bitcoin/issues/29392.

📝 araujo88 opened a pull request: "rpc: getdescriptorinfo also returns normalized descriptor"
(https://github.com/bitcoin/bitcoin/pull/29396)
This pull request introduces enhancements to the getdescriptorinfo RPC command, specifically by incorporating the functionality to return the `normalized_descriptor` alongside the original `descriptor` information. This improvement addresses the need for a standardized format that facilitates the use of descriptors in public key operations, especially important for scenarios requiring the derivation of addresses without access to private keys.

Related issue: https://github.com/bitcoin/bitcoin
...

💬 maflcko commented on pull request "test: handle wallet_reorgrestore.py failed":
(https://github.com/bitcoin/bitcoin/pull/29395#issuecomment-1931492134)
Can you explain *why* this fixes the issue? What are the steps to reproduce the issue?

💬 maflcko commented on pull request "test, assumeutxo: Add test to ensure failure when mempool not empty":
(https://github.com/bitcoin/bitcoin/pull/29394#discussion_r1481054678)
Could use one of the other nodes and restart?

💬 maflcko commented on pull request "test, assumeutxo: Add test to ensure failure when mempool not empty":
(https://github.com/bitcoin/bitcoin/pull/29394#discussion_r1481055367)
```suggestion

tx = self.miniwallet.send_self_transfer(from_node=node)

```

It already exists

💬 maflcko commented on pull request "assumeutxo: Get rid of faked nTx and nChainTx values":
(https://github.com/bitcoin/bitcoin/pull/29370#issuecomment-1931513793)
Could include the test https://github.com/bitcoin/bitcoin/pull/29370#issuecomment-1927200215 , or did you want to leave that for later?

💬 TheCharlatan commented on pull request "build: LLD based macOS toolchain":
(https://github.com/bitcoin/bitcoin/pull/21778#discussion_r1481087970)
Why is this overriding `CC` and `CXX` here (and below)? Seems like this causes the current CI failure, or at least calling this `darwin_CC` and `darwin_CXX` allows me to properly override everything with `AR=llvm-ar-17 OBJDUMP=llvm-objdump-17 RANLIB=llvm-ranlib-17 STRIP=llvm-strip-17 CC=clang-17 CXX=clang++-17 make HOST=arm64-apple-darwin -j 30` and complete a depends build on my machine.

💬 fanquake commented on pull request "build: LLD based macOS toolchain":
(https://github.com/bitcoin/bitcoin/pull/21778#discussion_r1481099257)
We have to override below, otherwise a build where the user sets CC and CCX won't work, because there will be no target or SDK flags, and all configure checks / compilation will fail.

💬 TheCharlatan commented on pull request "build: LLD based macOS toolchain":
(https://github.com/bitcoin/bitcoin/pull/21778#discussion_r1481128674)
Right, what I'm suggesting is keeping the `override` lines and:
```diff
diff --git a/depends/hosts/darwin.mk b/depends/hosts/darwin.mk
index 9e201eb2bc..d31ac398a4 100644
--- a/depends/hosts/darwin.mk
+++ b/depends/hosts/darwin.mk
@@ -9,2 +9,2 @@ OSX_SDK=$(SDK_PATH)/Xcode-$(XCODE_VERSION)-$(XCODE_BUILD_ID)-extracted-SDK-with-
-CC = clang
-CXX = clang++
+darwin_CC = clang
+darwin_CXX = clang++
@@ -46,0 +47,2 @@ override CXX += $(host_and_SDK)
+override darwin_CC += $(host_and_SDK)
+o
...

💬 araujo88 commented on pull request "test: handle wallet_reorgrestore.py failed":
(https://github.com/bitcoin/bitcoin/pull/29395#issuecomment-1931594513)
> Can you explain _why_ this fixes the issue? What are the steps to reproduce the issue?

AFAIK, `SyncWithValidationInterfaceQueue()` is a method used to synchronize the main thread with the validation interface queue. This synchronization ensures that all pending callbacks, which might include transaction validation, block connections, and wallet notifications, are processed before the method returns.

To reproduce the issue:

1. Set up a testing environment with multiple nodes.
2. Creat
...

💬 maflcko commented on pull request "test: handle wallet_reorgrestore.py failed":
(https://github.com/bitcoin/bitcoin/pull/29395#issuecomment-1931648941)
The issue is a race, so it is not deterministically reproducible. Usually, to deterministically reproduce it on all hardware, a sleep will have to be added in the right place in the source code or test code.

A pull request will either have to explain where to put the sleep, or, if not possible, come up with a plausible explanation how the race could happen. Also, the pull request should explain if the bug is in the wallet code or in the test code, in which case the fix should be applied to th
...