Bitcoin Core Github
42 subscribers
125K links
Download Telegram
maflcko closed a pull request: "wallet: clarify replace fields in help output"
(https://github.com/bitcoin/bitcoin/pull/27782)
💬 maflcko commented on pull request "wallet: clarify replace fields in help output":
(https://github.com/bitcoin/bitcoin/pull/27782#issuecomment-1808169132)
Closing as up for grabs for now, due to lack of reply
💬 maflcko commented on pull request "contrib: add tool to convert compact-serialized UTXO set to SQLite database":
(https://github.com/bitcoin/bitcoin/pull/27432#issuecomment-1808172554)
Could mark as draft while CI is red?
💬 fanquake commented on pull request "guix: update time-machine":
(https://github.com/bitcoin/bitcoin/pull/28580#issuecomment-1808174414)
Guix Build (aarch64):
```bash
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
d3224eb0eb66bf4433ed8757667ed438e419db4240b06d76122e8754de241742 guix-build-92d12f1c8903/output/aarch64-linux-gnu/bitcoin-92d12f1c8903-aarch64-linux-gnu.tar.gz
9f5ecd
...
💬 hebasto commented on pull request "[WIP] guix: update signapple (drop macho & altgraph)":
(https://github.com/bitcoin/bitcoin/pull/28859#issuecomment-1808174701)
Concept ACK.
💬 MatthewLM commented on pull request "Use LE hex-encoded representations in script ASM for pushed values <= 4 bytes":
(https://github.com/bitcoin/bitcoin/pull/28824#issuecomment-1808180595)
`0x` is much more standard and recognisable than `[]`.
💬 Riahiamirreza commented on pull request "rpc: show P2(W)SH redeemScript in getrawtransaction #27637":
(https://github.com/bitcoin/bitcoin/pull/27638#issuecomment-1808188088)
@sipa Yes and I'm not sure how should I fix the problem. Actually the `redeemScript` is a new field which is not part of the actual data, it's only a new representation of data (decompiled version of `redeemScript`). Would you suggest any approach to fix it?
💬 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)