💬 apoelstra commented on pull request "wallet: add `seeds` argument to `importdescriptors`":
(https://github.com/bitcoin/bitcoin/pull/27351#issuecomment-1486057058)
cc @achow101 @sipa @roconnor-blockstream

💬 ryanofsky commented on pull request "init: Error if ignored bitcoin.conf file is found":
(https://github.com/bitcoin/bitcoin/pull/27302#issuecomment-1486082157)
@willcl-ark this PR is not trying to error when one datadir setting takes precedence over another one. As the PR description states it adds an error for cases where "a bitcoin datadir **that is being used** contains a bitcoin.conf file that is ignored" (emphasis added). It does not show an error in cases where a bitcoin datadir that **not** being used contains a bitcoin.conf file that is ignored, because the whole directory is ignored, so there is nothing special about one file in it.

But we
...

💬 pablomartin4btc commented on pull request "httpserver, rest: fix segmentation fault on evhttp_uri_get_query":
(https://github.com/bitcoin/bitcoin/pull/27253#discussion_r1149975805)
I see, changing a pointer to a reference does not inherently prevent null pointer dereferences at runtime. However, by changing a pointer to a reference and documenting that the reference is expected to be non-null, we are effectively enforcing a contract that the caller of the function must provide a valid (non-null) reference. Suggestion taken, thanks!

💬 pablomartin4btc commented on pull request "httpserver, rest: fix segmentation fault on evhttp_uri_get_query":
(https://github.com/bitcoin/bitcoin/pull/27253#discussion_r1149977087)
I agree with this suggestion, first the manipulation of null values, mainly its return, and also removing the extra call to `evhttp_request_get_uri()`.

💬 pablomartin4btc commented on pull request "httpserver, rest: fix segmentation fault on evhttp_uri_get_query":
(https://github.com/bitcoin/bitcoin/pull/27253#discussion_r1150023699)
The problem removing `if (!uri_parsed) return std::nullopt;` line from `GetQueryParameterFromUri()` is that passing nullptr as the uri_parsed (from an invalid uri), as in the last test case in `httpserver_test.cpp` will produce the `segfault`, so I could opt that in the test but I dont think it's the real intention of testing `GetQueryParameterFromUri()`.

💬 pablomartin4btc commented on pull request "httpserver, rest: fix segmentation fault on evhttp_uri_get_query":
(https://github.com/bitcoin/bitcoin/pull/27253#issuecomment-1486236260)
Working on functional failure `interface_rest` on the invalid uri case, something didn't like from latest changes.

💬 josibake commented on pull request "ci: cleanup of CI_EXEC & CI_EXEC_ROOT (refs #27321)":
(https://github.com/bitcoin/bitcoin/pull/27333#issuecomment-1486306405)
ACK https://github.com/bitcoin/bitcoin/pull/27333/commits/b5ef1419ece77db77cc196eb541f99a72360a607

💬 ajtowns commented on pull request "Update MANDATORY_SCRIPT_VERIFY_FLAGS":
(https://github.com/bitcoin/bitcoin/pull/26291#issuecomment-1486312984)
> > Completely unsure on increasing the scope of what we disconnect for. Are we seeing these messages in the wild?
>
> I've added logging for `TX_NOT_STANDARD` in `MaybePunishNodeForTx`. After 20 hours or so running that on a listening node, I've had a couple of instances of rejections for `dust` (via `IsStandardTx` for outputs matching `IsDust`), but so far that's it.

I've been running this on mainnet for a week now and have only caught non-standard tx failures for dust, so, no, I don't t
...

💬 josibake commented on issue "Should be able to import an xpub descriptor to a privkey-enabled wallet if the wallet has the privkeys":
(https://github.com/bitcoin/bitcoin/issues/27336#issuecomment-1486333558)
FWIW, I think this is a good idea. I recently ran into this when trying to create a fairly complicated miniscript fragment: I created the wallet with private keys enabled and copied out some of the xpubs to use in my `wsh(miniscript)` descriptor. I was then unable to import it into the wallet until I ran `listdescriptors true` and replaced the xpub with the xpriv before then re-importing.

I agree that it's dangerous to require dumping the xpriv to the command line. I suppose the counter-argum
...

💬 martinus commented on pull request "Add pool based memory resource":
(https://github.com/bitcoin/bitcoin/pull/25325#issuecomment-1486343844)
@sipa could you have another look after my update from d87cb99 -> 9f947fc3d4b779f017332135323b34e8f216f613?

💬 josibake commented on pull request "test: use address_to_scriptpubkey instead of RPC call":
(https://github.com/bitcoin/bitcoin/pull/27349#discussion_r1150105696)
I think this should be in the second commit, right?

💬 josibake commented on pull request "test: use address_to_scriptpubkey instead of RPC call":
(https://github.com/bitcoin/bitcoin/pull/27349#discussion_r1150131175)
I'm not sure adding `spk` as a variable really helps anything here and it does add an extra line to be reviewed. For a refactor, I'd suggest keeping the diff as small and clear as possible

```suggestion
ms_unlock_details = {"scriptPubKey": address_to_scriptpubkey(self.ms_address).hex(),
```

💬 josibake commented on pull request "test: use address_to_scriptpubkey instead of RPC call":
(https://github.com/bitcoin/bitcoin/pull/27349#issuecomment-1486347805)
@ismaelsadeeq seems ready for review to me

💬 MarcoFalke commented on pull request "ci: cleanup of CI_EXEC & CI_EXEC_ROOT (refs #27321)":
(https://github.com/bitcoin/bitcoin/pull/27333#discussion_r1150154401)
> whether I should do the same for pip3

You are already doing what I suggested for pip3, which is why I left the comment here. I think it makes sense to use the same approach for all three pkg managers (dnf, apt, pip3)

💬 MarcoFalke commented on pull request "test: fix intermittent failure in ChainStateManager tests":
(https://github.com/bitcoin/bitcoin/pull/27348#issuecomment-1486375282)
lgtm ACK f8abcb3e3b2e731c002ec88f7559c21e26a2c079

👋 ismaelsadeeq's pull request is ready for review: "test: use address_to_scriptpubkey instead of RPC call"
(https://github.com/bitcoin/bitcoin/pull/27349)

💬 josibake commented on pull request "refactor: Remove CAddressBookData::destdata":
(https://github.com/bitcoin/bitcoin/pull/27224#discussion_r1150232826)
Perhaps there's some nuance that I'm missing, but it seems strange to have this be `Span<std::byte>` when the rest of the methods take `DataStream&&`. I changed the `ErasePrefix` method to also take `DataStream&&` as an argument and was able to compile and pass the tests. Seems like it would be preferable to match the other methods unless you have a reason to prefer `Span<std::byte>`?

💬 josibake commented on pull request "refactor: Remove CAddressBookData::destdata":
(https://github.com/bitcoin/bitcoin/pull/27224#discussion_r1150238393)
as part of changing `ErasePrefix` to accept a `DataStream&&`, I also changed this line:

```suggestion
return m_batch->ErasePrefix(DataStream(prefix));
```

to make this an `rvalue`. might be a cleaner way to do this

👍 TheCharlatan approved a pull request: "guix: use GCC tool wrappers"
(https://github.com/bitcoin/bitcoin/pull/27345)
ACK [4133c81](https://github.com/bitcoin/bitcoin/pull/27345/commits/4133c8104f522c403c55d26bd03436a8149ff106)

> Split out, to try move things along, as this change is isolated, and should be straight-forward.

Is there a reason for the LTO PR being bogged down?

```
find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
df90584fe5c1c68ec6a547ba04b81b66fae61035d5d4b586bc2c70f19176bc62 guix-build-4133c8104f52/output/aarch64-
...

⚠️ fanquake opened an issue: "test: failure in feature_notifications.py"
(https://github.com/bitcoin/bitcoin/issues/27352)
https://cirrus-ci.com/task/6373158677118976:
```bash
node1 2023-03-27T18:58:04.542245Z [C:\Users\ContainerAdministrator\AppData\Local\Temp\cirrus-ci-build\src\rpc\request.cpp:179] [parse] [rpc] ThreadRPCServer method=syncwithvalidationinterfacequeue user=__cookie__
test 2023-03-27T18:58:04.597000Z TestFramework (ERROR): Unexpected exception caught during testing
Traceback (most recent call last):
File "C:\Users\Co
...