💬 tnndbtc commented on pull request "fee estimate test: fix #31944 by handling a legitimate scenario that …":
(https://github.com/bitcoin/bitcoin/pull/32615#issuecomment-3032604420)
> Just ensure enough random transactions have been created to reliably return a fee estimate in any run?

@maflcko Random is random. How can we ensure randomly generated transactions can reliably return a fee estimate **in any run**?

🤔 rkrux reviewed a pull request: "rpc: add optional nodeid param to filter getpeerinfo"
(https://github.com/bitcoin/bitcoin/pull/32741#pullrequestreview-2983518394)
Concept ACK

Useful addition, I can see myself using the filtering here.

A release note would be helpful as per this: https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#release-notes

💬 rkrux commented on pull request "rpc: add optional nodeid param to filter getpeerinfo":
(https://github.com/bitcoin/bitcoin/pull/32741#discussion_r2183003505)
Can mention in the RPC help.

```diff
- "Returns data about each connected network peer as a json array of objects.",
+ "Returns data about each connected network peer as a json array of objects.\n"
+ "Optionally filter by peer index.\n",

✅ maflcko closed a pull request: "fee estimate test: fix #31944 by handling a legitimate scenario that …"
(https://github.com/bitcoin/bitcoin/pull/32615)

💬 maflcko commented on pull request "fee estimate test: fix #31944 by handling a legitimate scenario that …":
(https://github.com/bitcoin/bitcoin/pull/32615#issuecomment-3032629888)
I am not sure if you are using an LLM in the background, but the goal of a pull request is not to open it and then ask others to provide the solution for you, then ask others to review it.

The burden to understand the issue, to propose the fix, and to understand the fix is on the author.

Closing for now, because this is clearly the wrong approach here.

Discussion can continue, of course. Also, if a proper solution is found, a new pull request can be opened.



> Random is random.

...

💬 ismaelsadeeq commented on pull request "fee estimate test: fix #31944 by handling a legitimate scenario that …":
(https://github.com/bitcoin/bitcoin/pull/32615#issuecomment-3032663440)


> @ismaelsadeeq I'm not sure if your comment "Still NACK" is directed at this PR.

Yes, as it stands, I don't think this PR is reviewable, as other reviewers have also mentioned.

> This PR proposes using a hard-coded fee list generated from a properly seeded random source that reliably returns the fee rate. From my understanding, this matches your suggestion. If not, please clarify. Thanks.

@tnndbtc I think you may have misread the comment. Randomness should have defined bounds. What
...

💬 theStack commented on pull request "test: fix feature_init.py intermittencies":
(https://github.com/bitcoin/bitcoin/pull/32835#issuecomment-3032665076)
Concept ACK

🚀 fanquake merged a pull request: "doc: Add workaround for vcpkg issue with paths with embedded spaces"
(https://github.com/bitcoin/bitcoin/pull/32858)

💬 tnndbtc commented on pull request "fee estimate test: fix #31944 by handling a legitimate scenario that …":
(https://github.com/bitcoin/bitcoin/pull/32615#issuecomment-3032722076)
> I am not sure if you are using an LLM in the background, but the goal of a pull request is not to open it and then ask others to provide the solution for you, then ask others to review it.
>
> The burden to understand the issue, to propose the fix, and to understand the fix is on the author.
>
> Closing for now, because this is clearly the wrong approach here.
>
> Discussion can continue, of course. Also, if a proper solution is found, a new pull request can be opened.
>
> > Random
...

⚠️ maflcko opened an issue: "fuzz: Speed up mini_miner fuzz target?"
(https://github.com/bitcoin/bitcoin/issues/32870)
Looks like it is somewhat slow (c.f. https://cirrus-ci.com/task/6602192649453568?logs=ci#L5401), so it would be nice to make it faster.

Possibly with txgraph, see https://github.com/bitcoin-core/qa-assets/pull/227#issuecomment-3032434420

👍 hebasto approved a pull request: "[28.x] Backports"
(https://github.com/bitcoin/bitcoin/pull/32811#pullrequestreview-2983722824)
ACK 44b07b2d5a6937b71c984b4c2d3edb382346f96b.

💬 tnndbtc 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-3032777163)
@maflcko Is there a way to retrieve debug.log while the test fails so it may provide more clues on why RPC took longer?

💬 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.