💬 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
...

👋 MarcoFalke's pull request is ready for review: "ci: Use TSan new runtime (llvm-16, take 3)"
(https://github.com/bitcoin/bitcoin/pull/27298)

💬 jnewbery commented on pull request "net: #27257 follow-ups":
(https://github.com/bitcoin/bitcoin/pull/27324#discussion_r1150334044)
This is absolutely fine. I would have gone for something like "For performance reasons, once we've deserialized the bytes into a CNetMessage, we avoid re-allocating and copying the message as we pass it up the stack for processing. Delete the copy ctor/assignment to avoid accidentally introducing a copy operation.", which is more-or-less the same as what you came up with.

💬 jnewbery commented on pull request "net: #27257 follow-ups":
(https://github.com/bitcoin/bitcoin/pull/27324#issuecomment-1486557249)
utACK cd0c8eeb0940790b6ba83786d1c9e362d4dc4829

👍 fanquake approved a pull request: "fuzz: Remove legacy int parse fuzz tests"
(https://github.com/bitcoin/bitcoin/pull/27344)
ACK faf8dc496e761a15956f8226d727f4bbab8dff82

👍 dergoegge approved a pull request: "fuzz: Remove legacy int parse fuzz tests"
(https://github.com/bitcoin/bitcoin/pull/27344)
ACK faf8dc496e761a15956f8226d727f4bbab8dff82

📝 TheCharlatan opened a pull request: "refactor (tidy): Fixes after enable-debug configure"
(https://github.com/bitcoin/bitcoin/pull/27353)
This diff can be reproduced by running (tested with clang-14 and clang-15):

```
./autogen.sh && CC=clang CXX=clang++ ./configure --enable-suppress-external-warnings --enable-debug
bear --config src/.bear-tidy-config -- make -j $(nproc)
cd ./src/ && run-clang-tidy -quiet -fix -j $(nproc)
```

This pull request is set to draft, since I still don't know why `--enable-debug` allows tidy to catch this additional lint (performance-for-range-copy). Any pointers would be appreciated. The same l
...