👍 willcl-ark approved a pull request: "doc, rpc : `#30275` followups"
(https://github.com/bitcoin/bitcoin/pull/30525#pullrequestreview-2221555799)
ACK fa2f26960ee

This changes the help text from

```
2. estimate_mode (string, optional, default="conservative") The fee estimate mode.
Whether to return a more conservative estimate which also satisfies
a longer history. A conservative estimate potentially returns a
higher feerate and is more likely to be sufficient for the desired
target, but is not as responsive to short term drops in the

...

👍 willcl-ark approved a pull request: "ci: Silent Homebrew's reinstall warnings"
(https://github.com/bitcoin/bitcoin/pull/30591#pullrequestreview-2221594981)
utACK 032ebe5be4dfc3eb3d3693f1284fac6458bacbf3

💬 ryanofsky commented on pull request "fix: increase consistency of rpcauth parsing":
(https://github.com/bitcoin/bitcoin/pull/30401#issuecomment-2271558508)
> Since `-rpcauth=somethingvalid` does reverse `-norpcauth` (including reinstating `-rpcauth` options prior to the `-norpcauth`), this seems like it should be supported?

I think this behavior is surprising and we should not make the code more complicated to support it. It is simpler to just reject empty always, or ignore them always like the previous PR, instead of sometimes accepting or rejecting them based on presence of other values.

For background, the reinstated options are also known
...

📝 hebasto opened a pull request: "doc: Drop no longer needed workaround for WSL"
(https://github.com/bitcoin/bitcoin/pull/30597)
This PR effectively reverts commit 4f890ba6bc8caba5394c7a5388d7f07959ced78b from https://github.com/bitcoin/bitcoin/pull/11437, which fixed some build issues on WSL seven years ago.

Testing the current master branch @ 31a3ff55154bf15fb35b157c3f67ec05408ecdf9 on Windows 11 + WSL using Ubuntu 24.04 or Debian images, I noticed that the workaround is no longer required. Moreover, it doesn't affect the build process at all, which means the hashes of the built packages in depends remain the same an
...

👍 ryanofsky approved a pull request: "fix: increase consistency of rpcauth parsing"
(https://github.com/bitcoin/bitcoin/pull/30401#pullrequestreview-2221580600)
Code review ACK 0534fa2c216dbd9e1ae43d30a8a406a0a39c409d but I would really encourage reordering commits in this PR so the tests are added first and the fix is the last commit, so the fix commit updates the tests and you can see what effects it has.

💬 ryanofsky commented on pull request "fix: increase consistency of rpcauth parsing":
(https://github.com/bitcoin/bitcoin/pull/30401#discussion_r1705704678)
In commit "fix: increase consistency of rpcauth parsing" (dc38788312e7cdbbc106103e671d548d5c236862)

I think it would be better for code clarity to drop these 4 lines. The only thing they are doing is triggering a special-case "Invalid (empty) -rpcauth argument" error instead of the more generic "Invalid -rpcauth argument." error that would already happen below. The generic error seems sufficient, and If it is not sufficient it would still seem better to handle the two errors together rather t
...

👍 willcl-ark approved a pull request: "build: Remove unused visibility checks"
(https://github.com/bitcoin/bitcoin/pull/30590#pullrequestreview-2221640629)
ACK bbcba09cd5ca5fdd9055aaf64781125c5e505576

Makes sense to remove this while it's unused.

I did find a usage of `HAVE_DEFAULT_VISIBILITY_ATTRIBUTE` in my source dir, but it's only present because `make distclean` does not remove the file `src/config/bitcoin-config.h.in` (which I don't think is worth tidying with a new build system on the way)

💬 hebasto commented on pull request "doc: Drop no longer needed workaround for WSL":
(https://github.com/bitcoin/bitcoin/pull/30597#issuecomment-2271574462)
cc @sipsorcery @fanquake @m3dwards

✅ ryanofsky closed an issue: "TransactionError::ALREADY_IN_CHAIN inconsistency "
(https://github.com/bitcoin/bitcoin/issues/19363)

🚀 ryanofsky merged a pull request: "rename TransactionError:ALREADY_IN_CHAIN"
(https://github.com/bitcoin/bitcoin/pull/30212)

💬 hebasto commented on pull request "build: Introduce CMake-based build system":
(https://github.com/bitcoin/bitcoin/pull/30454#discussion_r1705751853)
> I managed to make it work...

I consider this conversation as resolved then.

💬 willcl-ark commented on pull request "get miniupnpc from ssl, this reverts commit 21b8a14d37c19ce292d5529597e0d52338db48a9":
(https://github.com/bitcoin/bitcoin/pull/30564#issuecomment-2271593936)
@justinvforvendetta you've changed the PR title here, but not the commit message. Running the command that @maflcko gave on master will generate a specific revert commit with the expected title.

🤔 furszy reviewed a pull request: "descriptors: Be able to specify change and receiving in a single descriptor string"
(https://github.com/bitcoin/bitcoin/pull/22838#pullrequestreview-2221669789)
Dropping another out loud thinking:
The multipath specifier can be used in a way it degrades wallet performance for no reason if it is used in the last segment of single key constructions. For instance, a `pkh(xpub../0/<0;1;2;3;4;5;6;7;8;9;10,...>)` imports 10 different descriptors into the wallet while the same can be expressed using a single ranged descriptor `pkh(xpub../0/*)` with range 0-10.

Topic here is whether we should disable it for top-level single key functions, alert users about
...

💬 sipsorcery commented on pull request "doc: Drop no longer needed workaround for WSL":
(https://github.com/bitcoin/bitcoin/pull/30597#issuecomment-2271606451)
> Testing the current master branch @ [31a3ff5](https://github.com/bitcoin/bitcoin/commit/31a3ff55154bf15fb35b157c3f67ec05408ecdf9) on Windows 11 + WSL using Ubuntu 24.04 or Debian images, I noticed that the workaround is no longer required.

The command was originally added because some people had Windows environment variables that when carried across to WSL caused errors with the configure script.

I suspect that condition still exists. Maybe it's safer to leave the command there until the
...

💬 hebasto commented on pull request "doc: Drop no longer needed workaround for WSL":
(https://github.com/bitcoin/bitcoin/pull/30597#issuecomment-2271614876)
> I suspect that condition still exists. Maybe it's safer to leave the command there until the configure step is removed by cmake?

I would be great to document such a condition more explicitly.

However, I'm okay with either outcome.

💬 hebasto commented on pull request "doc: Drop no longer needed workaround for WSL":
(https://github.com/bitcoin/bitcoin/pull/30597#issuecomment-2271624870)
> Maybe it's safer to leave the command there until the configure step is removed by cmake?

I've read https://github.com/bitcoin/bitcoin/issues/10269 again, and it seems all complains were about building a native package in depends, which is not longer the case.

💬 sipsorcery commented on pull request "doc: Drop no longer needed workaround for WSL":
(https://github.com/bitcoin/bitcoin/pull/30597#issuecomment-2271636097)
> I've read #10269 again, and it seems all complaints were about building a native package in depends, which is no longer the case.

That does ring a bell.

utACK 16d82611812de4e91e7950fe6d31484cc7a9c937.

💬 fanquake commented on pull request "doc: Drop no longer needed workaround for WSL":
(https://github.com/bitcoin/bitcoin/pull/30597#issuecomment-2271641435)
> were about building a native package in depends, which is no longer the case.

Qt still builds a native package ?

💬 tdb3 commented on pull request "fix: increase consistency of rpcauth parsing":
(https://github.com/bitcoin/bitcoin/pull/30401#discussion_r1705793809)
Thanks. Will do (i.e. handle the errors together)

💬 hebasto commented on pull request "doc: Drop no longer needed workaround for WSL":
(https://github.com/bitcoin/bitcoin/pull/30597#issuecomment-2271661544)
> > were about building a native package in depends, which is no longer the case.
>
> Qt still builds a native package ?

Right. It's a `qmake` itself. I meant the `native_*` packages that are handled by the depends build subsystem in its own way.