💬 yancyribbens commented on pull request "Fee Estimation via Fee rate Forecasters":
(https://github.com/bitcoin/bitcoin/pull/30157#discussion_r1990153325)
should this be "estimatefeerate"?

💬 yancyribbens commented on pull request "Fee Estimation via Fee rate Forecasters":
(https://github.com/bitcoin/bitcoin/pull/30157#discussion_r1990157249)
shouldn't the first positional argument be `conf_target` ?

💬 yancyribbens commented on pull request "Fee Estimation via Fee rate Forecasters":
(https://github.com/bitcoin/bitcoin/pull/30157#discussion_r1990158173)
```suggestion
// Add transactions with high_fee until mempool transactions weight is more than 25th percent of DEFAULT_BLOCK_MAX_WEIGHT
```

💬 yancyribbens commented on pull request "Fee Estimation via Fee rate Forecasters":
(https://github.com/bitcoin/bitcoin/pull/30157#discussion_r1990159751)
> than 25th percent of DEFAULT_BLOCK_MAX_WEIGHT

Maybe others understand this, but I'm not sure what 25th percent of ... means. Is that the same as 25% of ..?

💬 hebasto commented on pull request "Require sqlite when building the wallet":
(https://github.com/bitcoin/bitcoin/pull/31961#issuecomment-2715787967)
> I could change depends here, but maybe it's easier to do that in the final PR that drops BDB itself?

Changes to depends shouldn't be big. Otherwise this statement from the PR description:
> ... it is no longer possible to not have sqlite available.

does not always hold (see a counter-example in https://github.com/bitcoin/bitcoin/pull/31961#issuecomment-2715187814).

💬 hebasto commented on pull request "qa: Enable feature_init.py on Windows":
(https://github.com/bitcoin/bitcoin/pull/32021#discussion_r1990194152)
Can this approach be used for all platforms?

💬 fjahr commented on issue "Fully validated AssumeUTXO starts revalidating after restart":
(https://github.com/bitcoin/bitcoin/issues/32029#issuecomment-2715987586)
> 23 minutes on my M4 Max - likely ~40 minutes on commodity hardware

Huh, that sounds wrong, were you using a dev/debug build? Or is this probably cause by low resource allocation?

This is me on a vanilla M4 and `gettoutsetinfo` shouldn't do much different than verifying the hash:

```
$ time build/src/bitcoin-cli gettxoutsetinfo
{
"height": 887381,
"bestblock": "000000000000000000023ce7c194520dd6ef38a1d2633f8a726e3a7346549c09",
"txouts": 174298133,
"bogosize": 13621053583,
"hash_ser
...

💬 fanquake commented on pull request "cmake: Check for `makensis` and `zip` tools before using them for optional `deploy` targets":
(https://github.com/bitcoin/bitcoin/pull/32019#discussion_r1990326043)
> find_program() command.

Details like this are completely unnecessary.

💬 aiswaryasankar commented on pull request "test: Check datadir cleanup after assumeutxo was successful":
(https://github.com/bitcoin/bitcoin/pull/32033#discussion_r1990348457)
Ensures proper cleanup of background chainstate after `assumeutxo` node restart to prevent disk space leaks

💬 aiswaryasankar commented on pull request "docs: fix grammatical typos in documentation files":
(https://github.com/bitcoin/bitcoin/pull/32036#discussion_r1990364567)
No new bugs were introduced by the user's changes.

💬 aiswaryasankar commented on pull request "docs: fix grammatical typos in documentation files":
(https://github.com/bitcoin/bitcoin/pull/32036#discussion_r1990364579)
The change from 'include directories' to 'included directories' is incorrect and introduces a grammatical error. The correct term is 'include directories'.

💬 yancyribbens commented on pull request "docs: fix grammatical typos in documentation files":
(https://github.com/bitcoin/bitcoin/pull/32036#discussion_r1990381314)
This change looks valid

🚀 fanquake merged a pull request: "qt: 29.0 translations update"
(https://github.com/bitcoin/bitcoin/pull/32004)

✅ fanquake closed a pull request: "docs: fix grammatical typos in documentation files"
(https://github.com/bitcoin/bitcoin/pull/32036)

📝 fanquake converted_to_draft a pull request: "tests: Improve stderr validation in test_runner.py"
(https://github.com/bitcoin/bitcoin/pull/31966)
This PR implements the improvement suggested in a TODO comment in test/util/test_runner.py.
It adds validation that stderr is empty when no errors are expected in test cases.

The change adds a check that verifies stderr is empty when no error_txt is specified in
the test object, with a special exception for bitcoin-tx running under Wine, which was
mentioned in the original TODO comment.

This improvement helps catch unexpected error messages that might otherwise go unnoticed
during
...

💬 fanquake commented on pull request "tests: Improve stderr validation in test_runner.py":
(https://github.com/bitcoin/bitcoin/pull/31966#issuecomment-2716138203)
> Sorry, my bad. I removed TODO

You partially removed it? You should also read https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#committing-patches in regards to fixing your commits messages & fixing your commits.

🚀 fanquake merged a pull request: "test: Fix authproxy named args debug logging"
(https://github.com/bitcoin/bitcoin/pull/31955)

💬 fanquake commented on pull request "Require sqlite when building the wallet":
(https://github.com/bitcoin/bitcoin/pull/31961#issuecomment-2716192596)
> but maybe it's easier to do that in the final PR that drops BDB itself?

I don't see why, and it just leaves things broken/confusing in the interim. If we are going to move forward with this PR, the `NO_SQLITE` depends option should be removed at the same time.

💬 luke-jr commented on issue "build: document `BITCOIN_GENBUILD_NO_GIT` environment variable?":
(https://github.com/bitcoin/bitcoin/issues/31999#issuecomment-2716226975)
> The last comment (7 years ago) suggests that there was no issue with 0.13.0, even though the fix from #7522 only went into 0.15.0?

Likely it was backported

> It'd be good if someone could demonstrate an issue today, with the current source code.

```
mkdir test
cd test
git init .
tar -xvpf /path/to/a/release/tarball.tar.gz
cd bitcoin-*
sandbox # set up to complain about reads to /tmp/test/.git
# do whatever to build
```

Note that `/tmp` is not a suitable place to run the test, as tools oft
...

💬 fanquake commented on issue "build: document `BITCOIN_GENBUILD_NO_GIT` environment variable?":
(https://github.com/bitcoin/bitcoin/issues/31999#issuecomment-2716235199)
> set up to complain about reads to /tmp/test/.git
> you need to have the access attempt trigger an alert or kill the process somehow.
> do whatever to build

Multiple of the steps here are "just do thing", where it sounds like there isn't actually a build issue in the default case. Please provide actual, full steps to reproduce (with a tarball produced via the release process, from current master).