💬 maflcko commented on pull request "rpc: show P2(W)SH redeemScript in getrawtransaction #27637":
(https://github.com/bitcoin/bitcoin/pull/27638#issuecomment-1808190478)
> Would you suggest any approach to fix it?

Fix what? Without any information, there is nothing we can do here.

If you want to fix the tests, make sure to revert the test changes and run the test locally.

💬 vasild commented on pull request "fuzz: call lookup functions before calling `Ban`":
(https://github.com/bitcoin/bitcoin/pull/27935#issuecomment-1808191887)
ACK fca0a8938e34cb4f6c400e1d1d0be02f027d80c5

💬 sipa commented on pull request "rpc: show P2(W)SH redeemScript in getrawtransaction #27637":
(https://github.com/bitcoin/bitcoin/pull/27638#issuecomment-1808196658)
I don't see why you'd need to make changes to the test framework at all.

💬 Riahiamirreza commented on pull request "rpc: show P2(W)SH redeemScript in getrawtransaction #27637":
(https://github.com/bitcoin/bitcoin/pull/27638#issuecomment-1808199147)
@sipa @maflcko I added new field to the output of `TxToUniv` function, called `redeemScript`. I've manually tested the rpc and that works fine. After running tests I faced issue of decoding transactions. Because of adding a new field, transactions couldn't be decoded successfully. I attempt to change `CTxIn` message in my next effort and add the `redeemScript` which now seems trivially incorrect way.

💬 Riahiamirreza commented on pull request "rpc: show P2(W)SH redeemScript in getrawtransaction #27637":
(https://github.com/bitcoin/bitcoin/pull/27638#issuecomment-1808204753)
@maflcko Sure, I'll revert my changes on test framework and then try another way to fix tests.

💬 hebasto commented on pull request "depends: Build `capnp` with CMake":
(https://github.com/bitcoin/bitcoin/pull/28856#issuecomment-1808205539)
> Any reason to not change `native_capnp` at the same time?

The `libmultiprocess` package build fails when linking the `mpgen` executable. Going to look into this issue, but that shouldn't be a blocker here, right?

💬 Sjors commented on pull request "guix: update time-machine":
(https://github.com/bitcoin/bitcoin/pull/28580#issuecomment-1808209653)
Briefly tested the 92d12f1c890350f40d8e5d5c6a59d5c172ea7550 guix build on (Intel) macOS Ventura 13.6, and it seems to work fine.

<details>
<summary>Guix hashes...</summary>

```
13a7d1be447ecb614cf43034af4f7a3a7ce7dffbcdb6c1773bc939ba80587ef6 guix-build-92d12f1c8903/output/aarch64-linux-gnu/SHA256SUMS.part
5ab66c69b742a89b0aa52705e48563749cb72ebcc92745a4eb07df285a20c62a guix-build-92d12f1c8903/output/aarch64-linux-gnu/bitcoin-92d12f1c8903-aarch64-linux-gnu-debug.tar.gz
d3224eb0eb66b
...

🤔 ismaelsadeeq reviewed a pull request: "mempool: Persist with XOR"
(https://github.com/bitcoin/bitcoin/pull/28207#pullrequestreview-1727370338)
ACK fa6b053b5c964fb35935fa994cb782c0731a56f8

Used this PR to successfully read a `mempool.dat` saved in the legacy format.
Additionally, the `persistmempoolv1` option downgraded and allowed dumping using the legacy format.

Code looks good to me.

💬 ismaelsadeeq commented on pull request "mempool: Persist with XOR":
(https://github.com/bitcoin/bitcoin/pull/28207#discussion_r1391148323)

This is an improvement, this patch allows you to read `mempool.dat` dumped using the legacy format.
Whats the a rationale to temporary retention of `persistmempoolv1`, and when can we expect to remove this option?

💬 fanquake commented on pull request "depends: Build `capnp` with CMake":
(https://github.com/bitcoin/bitcoin/pull/28856#issuecomment-1808217135)
Not really, but it's a bit odd to be building the same code, in two different ways, in depends. Would be good to know why one build fails and the other doesn't.

💬 maflcko commented on pull request "mempool: Persist with XOR":
(https://github.com/bitcoin/bitcoin/pull/28207#discussion_r1391161373)
Should be fine to remove in the second next release after this is merged. I presume the read code (`// Leave XOR-key empty` ) can stay forever/longer, because it is just one line of code, so no strong opinion.

👍 fanquake approved a pull request: "util: Replace std::filesystem with util/fs.h"
(https://github.com/bitcoin/bitcoin/pull/28076#pullrequestreview-1727398963)
ACK bbbbdb0cd57d75a06357d2811363d30a498f4499 🦀

🚀 fanquake merged a pull request: "util: Replace std::filesystem with util/fs.h"
(https://github.com/bitcoin/bitcoin/pull/28076)

💬 ismaelsadeeq commented on pull request "mempool: Persist with XOR":
(https://github.com/bitcoin/bitcoin/pull/28207#discussion_r1391178207)
AFAIU we are persisting with `XOR` to "protect against programs that accidentally and unintentionally are trying to mess with the dat file" so I think it will be better if we completely prevent dumping without `XOR`.

💬 instagibbs commented on pull request "fuzz: Minor improvements to tx_package_eval target":
(https://github.com/bitcoin/bitcoin/pull/28825#discussion_r1391191028)
will do if I touch again

💬 hebasto commented on pull request "depends: Build the `native_capnp` and `capnp` packages with CMake":
(https://github.com/bitcoin/bitcoin/pull/28856#issuecomment-1808284816)
> Any reason to not change `native_capnp` at the same time?

Done.

👍 theStack approved a pull request: "doc: rewrite explanation for `-par=`"
(https://github.com/bitcoin/bitcoin/pull/28858#pullrequestreview-1727479651)
ACK d799ea26edfd63434b6d1cf55500de184dc67fac

💬 Riahiamirreza commented on pull request "rpc: show P2(W)SH redeemScript in getrawtransaction #27637":
(https://github.com/bitcoin/bitcoin/pull/27638#issuecomment-1808308529)
```feature_abortnode.py | ✖ Failed | 1 s
feature_bip68_sequence.py | ✖ Failed | 1 s
feature_cltv.py | ✖ Failed | 1 s
feature_coinstatsindex.py | ✖ Failed | 2 s
feature_config_args.py | ✖ Failed | 10 s
feature_csv_activation.py | ✖ Failed | 1 s
feature_dersig.py
...

💬 Sjors commented on pull request "build: LLD based macOS toolchain":
(https://github.com/bitcoin/bitcoin/pull/21778#issuecomment-1808312187)
Guix building df71f448cdaf429f54367ab251e21e628c1903d3 is having issues for me:

```
Extracting libevent...
/home/guix/depends-sources-cache/libevent-2.1.12-stable.tar.gz: OK
Preprocessing libevent...
Configuring libevent...
checking for a BSD-compatible install... /home/guix/.guix-profile/bin/install -c
checking whether build environment is sane... yes
checking for x86_64-apple-darwin-strip... no
checking for strip... strip
configure: WARNING: using cross tools not prefixed with host
...

💬 fanquake commented on pull request "build: LLD based macOS toolchain":
(https://github.com/bitcoin/bitcoin/pull/21778#issuecomment-1808313723)
> Can add more details...

I'd suggest reviewing the base PRs instead, for now.