💬 maflcko commented on pull request "wallet: Replace fee magic numbers with named constants":
(https://github.com/bitcoin/bitcoin/pull/33254#issuecomment-3223533891)
Thanks, but the value of purely LLM generated "vibe" pull requests is limited, because:

* The "author" does not understand the changes and can not reply to code review feedback.
* There are more than 300 pull requests (most written by real humans) waiting for review.
* Incentive to review purely LLM generated changes is limited, because the feedback likely does not help to train a real human planning to contribute in the future, but is likely only passed on to an LLM api.

My recommendati
...

✅ maflcko closed a pull request: "wallet: Replace fee magic numbers with named constants"
(https://github.com/bitcoin/bitcoin/pull/33254)

💬 maflcko commented on issue "Linux download needs installation instructions":
(https://github.com/bitcoin/bitcoin/issues/32097#issuecomment-3223540909)
I think this was fixed and can be closed?

✅ maflcko closed a pull request: "doc: add Linux GUI runtime instructions to doc/README.md"
(https://github.com/bitcoin/bitcoin/pull/33197)

💬 maflcko commented on pull request "doc: add Linux GUI runtime instructions to doc/README.md":
(https://github.com/bitcoin/bitcoin/pull/33197#issuecomment-3223543034)
Closing for now, as this was fixed already.

💬 ryanofsky commented on issue "build: indefinite mpgen hang on Ubuntu 22.04":
(https://github.com/bitcoin/bitcoin/issues/33176#issuecomment-3223640756)
From https://ubuntu.com/security/CVE-2022-46149 it seems to say ubuntu has looked at the issue and ignoring it in LTS releases ("Ignored" value in Status table), because the fix changes [changes the ABI](https://github.com/capnproto/capnproto/compare/v0.8.0..v0.8.1#diff-22b74c4b9a5b4d48aece4e5b3b4e73a0a680be156c8921c1d135c1c1cac3d8d1R1235) and could maybe break applications compiled against the 0.8.0 version of the headers but using the 0.8.1 runtime library (IIUC, I'm not exactly sure what they
...

⚠️ fanquake opened an issue: "ci: tidy job is producing output"
(https://github.com/bitcoin/bitcoin/issues/33256)
master (6ca6f3b37b992591726bd13b494369bee3bd6468) https://cirrus-ci.com/task/5658586602274816:
```bash
[17:17:19.943] [108/715][22.0s] clang-tidy-20 -p=/ci_container_base/ci/scratch/build-x86_64-pc-linux-gnu -quiet -load=/tidy-build/libbitcoin-tidy.so /ci_container_base/ci/scratch/build-x86_64-pc-linux-gnu/src/ipc/capnp/echo.capnp.proxy-server.c++
[17:17:34.032] /usr/include/kj/async-inl.h:609:37: warning: The left operand of '+' is a garbage value [clang-analyzer-core.UndefinedBinaryOperatorRes
...

💬 fanquake commented on issue "ci: tidy job is producing output":
(https://github.com/bitcoin/bitcoin/issues/33256#issuecomment-3223652850)
cc @Sjors @ryanofsky

💬 maflcko commented on pull request "test: Fix CLI_MAX_ARG_SIZE issues":
(https://github.com/bitcoin/bitcoin/pull/33243#discussion_r2300620446)
Yeah, encoding seems like another possible source for inconsistencies. I don't think we want to deal with encoding here, because the value is small enough for it to not matter.

💬 maflcko commented on pull request "test: Fix CLI_MAX_ARG_SIZE issues":
(https://github.com/bitcoin/bitcoin/pull/33243#discussion_r2300620527)
thx, reworded the comment a bit

💬 maflcko commented on pull request "test: Fix CLI_MAX_ARG_SIZE issues":
(https://github.com/bitcoin/bitcoin/pull/33243#discussion_r2300620620)
It is mostly a regression test, checking that on master a single arg with size of `CLI_MAX_ARG_SIZE = 131071` fails the test (with the reproducer shared in the pull description).

Force pushed to reduce to that (and added a comment).

💬 maflcko commented on pull request "test: Fix CLI_MAX_ARG_SIZE issues":
(https://github.com/bitcoin/bitcoin/pull/33243#discussion_r2300620722)
> This test also passes on master (MacOS environment with ARG_MAX = 1048576).

Thanks for adding a macos data point for reference. This further supports that the hard-coded value may be different on different platforms.

> However, the following test only passes on this PR, not on my master:

Nice, thanks for adapting the test and checking the bugfix on macos.

💬 maflcko commented on pull request "wallet, rpc: remove settxfee and paytxfee":
(https://github.com/bitcoin/bitcoin/pull/32138#issuecomment-3223742148)
Seems fine to pick this up now, if you want.

💬 kevkevinpal commented on pull request "fees: enable `CBlockPolicyEstimator` return sub 1 sat/vb fee rate estimates":
(https://github.com/bitcoin/bitcoin/pull/33199#discussion_r2300759162)
Just an fyi, this test passes even without the changes in `src/policy/fees.h` I also agree with @polespinasa that if another test already covers this, then I'd rather not have redundant tests added

💬 kevkevinpal commented on pull request "fees: enable `CBlockPolicyEstimator` return sub 1 sat/vb fee rate estimates":
(https://github.com/bitcoin/bitcoin/pull/33199#discussion_r2300768342)
I am not sure if we need a release note, since according to this note, it will invalidate old estimates files? Maybe someone else can chime in on this

💬 kevkevinpal commented on pull request "fees: enable `CBlockPolicyEstimator` return sub 1 sat/vb fee rate estimates":
(https://github.com/bitcoin/bitcoin/pull/33199#issuecomment-3223886285)
Concept ACK [413c81b](https://github.com/bitcoin/bitcoin/pull/33199/commits/413c81b134a16403cc0f64e21c3c0967c211b318)

It makes sense to update `MIN_BUCKET_FEERATE` with the recent changes to be sub 1 sat/vbyte

I do think the additional test is a bit redundant but I am okay in adding if others think it makes sense

💬 151henry151 commented on pull request "wallet: Replace fee magic numbers with named constants":
(https://github.com/bitcoin/bitcoin/pull/33254#discussion_r2300772336)
Thanks for the feedback, I must have misunderstood the TODO. I thought this would be an improvement for the reasons outlined in the pull request, and thought it would address the TODO item.

💬 151henry151 commented on pull request "wallet: Replace fee magic numbers with named constants":
(https://github.com/bitcoin/bitcoin/pull/33254#issuecomment-3223930711)
> Thanks, but the value of purely LLM generated "vibe" pull requests is limited, because:
>
> * The "author" does not understand the changes and can not reply to code review feedback.
> * There are more than 300 pull requests (most written by real humans) waiting for review.
> * Incentive to review purely LLM generated changes is limited, because the feedback likely does not help to train a real human planning to contribute in the future, but is likely only passed on to an LLM api.
>
> M
...

💬 cedwies commented on pull request "test: Fix CLI_MAX_ARG_SIZE issues":
(https://github.com/bitcoin/bitcoin/pull/33243#issuecomment-3224114903)
ACK fa96a4a

👍 stickies-v approved a pull request: "[29.x] backport #33212"
(https://github.com/bitcoin/bitcoin/pull/33251#pullrequestreview-3155597914)
ACK fcac8022d8

There have been quite substantial changes to `BaseIndex::Rewind()` in https://github.com/bitcoin/bitcoin/commit/6f1392cc42cde638773f2b697d7d2c58abcdc860 that might warrant extra careful review, even though I couldn't find any issues.

The rationale and mechanics from the original PR all still hold here, and the changes look correct to me. The backport commits are clean, and the added test in fcac8022d839572f5d8781096eec14ca7ea2e0dd still fails without the fix in 16b1710d97464
...