💬 pinheadmz commented on pull request "wallet: reuse change dest when re-creating TX with avoidpartialspends":
(https://github.com/bitcoin/bitcoin/pull/27053#discussion_r1107304905)
done, thanks

👍 furszy approved a pull request: "refactor: Disable unused special members functions in `UnlockContext`"
(https://github.com/bitcoin-core/gui/pull/711)

📝 MarcoFalke converted_to_draft a pull request: "ci: Use dedicated root path for ci"
(https://github.com/bitcoin/bitcoin/pull/27090)
Now that stuff (like depends) is no longer propagated back to the host
folder after commit 141115a0604001b8cce848804798dc174770a994, commit
85892f77c98c7a08834a06d52af3eb474275afd8 can be reverted and
`LOCAL_USER` removed.

So do that. Also for clarity, in the same commit, use a dedicated path
for the ci root folder `CI_ROOT_DIR`.

💬 TheCharlatan commented on pull request "Remove almost all blockstorage globals":
(https://github.com/bitcoin/bitcoin/pull/25781#discussion_r1107320759)
In this case why not decouple the two options and pass and construct them independently? I've been using this branch for further work on getting the gArgs out of blockstorage.cpp and got the impression like there is little benefit in nesting the options.

🚀 fanquake merged a pull request: "Net: Pass `MSG_MORE` flag when sending non-final network messages (round 2)"
(https://github.com/bitcoin/bitcoin/pull/26844)

👍 TheCharlatan approved a pull request: "refactor: wallet, remove global 'ArgsManager' dependency"
(https://github.com/bitcoin/bitcoin/pull/26889)

💬 fanquake commented on pull request "refactor: wallet, remove global 'ArgsManager' dependency":
(https://github.com/bitcoin/bitcoin/pull/26889#issuecomment-1431625966)
@achow101 can you take a look here

👍 ponury1990 approved a pull request: "refactor: wallet, remove global 'ArgsManager' dependency"
(https://github.com/bitcoin/bitcoin/pull/26889)

💬 achow101 commented on pull request "Remove laanwj from trusted-keys":
(https://github.com/bitcoin/bitcoin/pull/27054#issuecomment-1431661196)
ACK aafa5e945cef7a4f65ddadcf548932dd4e27ada1

💬 achow101 commented on pull request "refactor: wallet, remove global 'ArgsManager' dependency":
(https://github.com/bitcoin/bitcoin/pull/26889#discussion_r1107398449)
In e86214692d3fe7a8c835e07f7ebd47a9afb9fec9 "wallet: set keypool_size instead of access global args manager"

The `using ScriptPubKeyMan::ScriptPubKeyMan;` line above should be removed so that this new constructor is the only constructor to avoid possible programming errors where the keypool size is not passed in.

💬 vasild commented on pull request "Handle CJDNS from LookupSubNet()":
(https://github.com/bitcoin/bitcoin/pull/27071#issuecomment-1431696802)
`7a93d7b0f9...8991ed2c6e`: address suggestions

💬 vasild commented on pull request "Handle CJDNS from LookupSubNet()":
(https://github.com/bitcoin/bitcoin/pull/27071#discussion_r1107408761)
Ignoring to minimize the size of the diff.

💬 vasild commented on pull request "Handle CJDNS from LookupSubNet()":
(https://github.com/bitcoin/bitcoin/pull/27071#discussion_r1107409772)
Done.

💬 hebasto commented on pull request "guix: consolidate to glibc 2.27 for Linux builds":
(https://github.com/bitcoin/bitcoin/pull/27029#discussion_r1107418495)
Instead of removing this test entirely, maybe replace it with a symbol which was introduced glibc [2.28](https://abi-laboratory.pro/index.php?view=changelog&l=glibc&v=2.28)?

For example,
```python
with open(source, 'w', encoding="utf8") as f:
f.write('''
#define _GNU_SOURCE
#include <unistd.h>
#include <fcntl.h>

int main()
{
fcntl64(0, F_GETFD);

...

💬 vasild commented on pull request "Handle CJDNS from LookupSubNet()":
(https://github.com/bitcoin/bitcoin/pull/27071#discussion_r1107422252)
Added `[[nodiscard]]` and one `AssertLockNotHeld()` where it is not immediately followed by `LOCK()`. I do not see the point in:

```cpp
AssertLockNotHeld(m); // the point of this is to crash if m is held
LOCK(m); // but this will crash if m is held, so what is the point?
```

If it is really desirable to have `AssertLockNotHeld()` before every `LOCK()`, then `AssertLockNotHeld()` should be put inside `LOCK()`.

This is another story:

```cpp
AssertLockNotHeld(m);
if (x) {
LOCK
...

💬 fanquake commented on pull request "guix: consolidate to glibc 2.27 for Linux builds":
(https://github.com/bitcoin/bitcoin/pull/27029#discussion_r1107427700)
How will we compile that in a glibc 2.27 environment?

💬 hebasto commented on pull request "guix: consolidate to glibc 2.27 for Linux builds":
(https://github.com/bitcoin/bitcoin/pull/27029#discussion_r1107444710)
> How will we compile that in a glibc 2.27 environment?

Right... :man_facepalming:

👍 hebasto approved a pull request: "guix: consolidate to glibc 2.27 for Linux builds"
(https://github.com/bitcoin/bitcoin/pull/27029)

💬 achow101 commented on pull request "Fix minor typo":
(https://github.com/bitcoin/bitcoin/pull/27102#issuecomment-1431774221)
While contributions to documentation are appreciated, trivial single line fixes that provide no clear benefits are generally discouraged and tend to be noise that distracts from and competes with review of actual substantial code changes. Please refer to our [contributing guidelines](https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#pull-request-philosophy) for information on what kinds of PRs are acceptable.

✅ achow101 closed a pull request: "Fix minor typo"
(https://github.com/bitcoin/bitcoin/pull/27102)