💬 purpleKarrot commented on pull request "cmake: Use `AUTHOR_WARNING` for warnings":
(https://github.com/bitcoin/bitcoin/pull/32865#issuecomment-3032802799)
ACK 04c4cc059e44a6d1b4f520af10c649648a002ebc

🤔 ryanofsky reviewed a pull request: "wallet: Fix relative path backup during migration."
(https://github.com/bitcoin/bitcoin/pull/32273#pullrequestreview-2983599593)
re: https://github.com/bitcoin/bitcoin/pull/32273#issuecomment-3029455047

Sorry I didn't post my complete suggestion before. Here is the change I would suggest based on the current PR (2e8a21a43a3c321c9f3a8c75024d036165658225):

<details><summary>diff</summary>
<p>

```diff
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -4253,30 +4253,16 @@ util::Result<MigrationResult> MigrateLegacyToDescriptor(std::shared_ptr<CWallet>

// Make a backup of the DB in the wallet's d
...

💬 ryanofsky commented on pull request "wallet: Fix relative path backup during migration.":
(https://github.com/bitcoin/bitcoin/pull/32273#discussion_r2183052507)
In commit "test: Migration of a wallet ending in `/`" (499099320b138c3d895d3ad8a622a08e404008d6)

This fails on my system because the test is racy. By the time backup is created time.time() is higher than the backup time.

Also the os.path.exists() return value is discarded here so this doesn't actually check anything.

Here are the changes I made to fix these problems. (Another approach might be to set a mock time instead)

<details><summary>diff</summary>
<p>

```diff
--- a/test/fu
...

💬 maflcko commented on issue "intermittent timeout in wallet_signer.py : 'createwallet' RPC took longer than 1200.000000 seconds":
(https://github.com/bitcoin/bitcoin/issues/32855#issuecomment-3032857821)
You can just follow the links to "Open full logs" to get to https://api.cirrus-ci.com/v1/task/5704849158832128/logs/ci.log, but I don't think there are any details in the log that give more hints, on a quick skim.

💬 glozow commented on pull request "test: fix an incorrect `feature_fee_estimation.py` subtest":
(https://github.com/bitcoin/bitcoin/pull/32463#discussion_r2183220585)
I'm suggesting linking both, or explaining it directly. Currently it requires clicking on the issue comment, then on the commit linked in that comment, then the PR linked in that commit to understand where the 4000 comes from

🚀 fanquake merged a pull request: "[28.x] Backports"
(https://github.com/bitcoin/bitcoin/pull/32811)

💬 sr-gi commented on pull request "Fix tiebreak when loading blocks from disk (and add tests for comparing chain ties)":
(https://github.com/bitcoin/bitcoin/pull/29640#discussion_r2183229775)
IIRC, the rationale here was that `nSequenceId` is supposed to be monotonically increasing, so having a chain of blocks where the previous blocks all have nSequenceId = `x-1` and the tip has `x` was weird. Since this is something that should only happen on restarts, the extra fraction of a second was not that relevant.

I'm happy to change it so only the tip has 0 if you think the time savings are more relevant than the inconsistency with the design. Comments will also need to be updated.

💬 fanquake commented on pull request "test: Fix list index out of range error in feature_bip68_sequence.py":
(https://github.com/bitcoin/bitcoin/pull/32765#issuecomment-3032896037)
Backported to 28.x in #32811.

💬 fanquake commented on pull request "doc: taproot became always active in v24.0 (doc/bips.md)":
(https://github.com/bitcoin/bitcoin/pull/32776#issuecomment-3032896451)
Backported to 28.x in #32811.

💬 fanquake commented on pull request "doc: fix Transifex 404s":
(https://github.com/bitcoin/bitcoin/pull/32777#issuecomment-3032896761)
Backported to 28.x in #32811.

💬 fanquake commented on pull request "doc: Add workaround for vcpkg issue with paths with embedded spaces":
(https://github.com/bitcoin/bitcoin/pull/32858#issuecomment-3032900614)
Backported to 29.x in #32863.

💬 fanquake commented on pull request "build: Make config warnings fatal if -DWCONFIGURE_ERROR=ON":
(https://github.com/bitcoin/bitcoin/pull/31665#issuecomment-3032903677)
@davidgumberg FYI I've implemented something using `AUTHOR_WARNING`, as discussed above, in #32865.

💬 fanquake commented on pull request "doc: clarify that the "-j N" goes after the "--build build" part":
(https://github.com/bitcoin/bitcoin/pull/32846#issuecomment-3032913817)
Backported to 29.x in #32863.

💬 ismaelsadeeq commented on pull request "test: fix an incorrect `feature_fee_estimation.py` subtest":
(https://github.com/bitcoin/bitcoin/pull/32463#discussion_r2183250651)
Yeah, I think that will be clearer. I will fix when retouching.

💬 sr-gi commented on pull request "Fix tiebreak when loading blocks from disk (and add tests for comparing chain ties)":
(https://github.com/bitcoin/bitcoin/pull/29640#discussion_r2183252361)
Fixed in [8d73371](https://github.com/bitcoin/bitcoin/pull/29640/commits/8d7337189a1b7024a40933bf082b7b574fd007e1)

💬 davidgumberg commented on pull request "build: Make config warnings fatal if -DWCONFIGURE_ERROR=ON":
(https://github.com/bitcoin/bitcoin/pull/31665#issuecomment-3033009863)
OK, closing this in favor of #32865.

✅ davidgumberg closed a pull request: "build: Make config warnings fatal if -DWCONFIGURE_ERROR=ON"
(https://github.com/bitcoin/bitcoin/pull/31665)

💬 davidgumberg commented on pull request "build: Make config warnings fatal if -DWCONFIGURE_ERROR=ON":
(https://github.com/bitcoin/bitcoin/pull/31665#discussion_r2183332678)
Thanks for catching this, I've fixed this for the sake of posterity, but I've closed this PR in favor of #32865.

💬 davidgumberg commented on pull request "build: Make config warnings fatal if -DWCONFIGURE_ERROR=ON":
(https://github.com/bitcoin/bitcoin/pull/31665#discussion_r2183339114)
OK, looks like the PR won't update with my branch now that it's closed, but it's here: https://github.com/davidgumberg/bitcoin/tree/1-15-24-configure_warnings

💬 mzumsande commented on pull request "Fix tiebreak when loading blocks from disk (and add tests for comparing chain ties)":
(https://github.com/bitcoin/bitcoin/pull/29640#discussion_r2183379788)
If the situation "previous blocks all have nSequenceId = x-1 and the tip has x" is weird, wouldn't that exact same situation occur anyway when the next block arrives via p2p (which would get `nSequenceId=2`, where all predecessors have `nSequenceId=1`)?

And if `nSequenceId` is supposed to be monotonically increasing (which I think would mean that no connectable block should have a lower `nSequenceId` than its predecessor), wouldn't that be an argument **for** only setting the tip rather than
...