💬 TheCharlatan commented on pull request "rpc: Optimize serialization disk space of dumptxoutset":
(https://github.com/bitcoin/bitcoin/pull/26045#issuecomment-1528716678)
Re https://github.com/bitcoin/bitcoin/pull/26045#issuecomment-1472285629

> I was also wondering why the bitcoin-cli savetxoutset API (same goes for savemempool) requires to specify a filename instead of writing to stdout that would offer better composition while maintaining the possibility to write to file with >utxo.bin

Writing to stdout would mean that the data would have to be carried over the rpc connection, right?

💬 theStack commented on pull request "script: remove unused bitwise `CScriptNum` operators":
(https://github.com/bitcoin/bitcoin/pull/27096#issuecomment-1528724501)
> Idk, this is probably too hard to review to be worth it. You are removing an overload, so reviewers need to check how this interacts with overload resolution. Maybe just close and reopen/mention it the next time the code is touched an reviewers are primed on it?

Agree that reviewing this is more cumbersome than it should be (this is just another example of how overloading leads to obfuscation and makes maintaining code harder). Closing for now.

✅ theStack closed a pull request: "script: remove unused bitwise `CScriptNum` operators"
(https://github.com/bitcoin/bitcoin/pull/27096)

📝 theStack opened a pull request: "test: add ripemd160 to test framework modules list"
(https://github.com/bitcoin/bitcoin/pull/27542)
Currently test runner doesn't execute the unit tests of the ripemd160 module, so add it to the list. All other framework modules that contain unit tests are included, as can be easily checked via
`$ git grep unittest.TestCase ./test/functional/test_framework/`

This is a late follow-up to PR #23716 (commit ad3e9e1f214d739e098c6ebbd300da5df1026a44).

💬 pablomartin4btc commented on pull request "httpserver, rest: fix segmentation fault on evhttp_uri_get_query":
(https://github.com/bitcoin/bitcoin/pull/27253#issuecomment-1528760293)
- Updated 2nd commit message.

💬 kevkevinpal commented on pull request "rpc: Add test-only RPC getaddrmaninfo for new/tried table address count":
(https://github.com/bitcoin/bitcoin/pull/27511#discussion_r1181081666)
Do you think we can add total to this test aswell?

💬 brunoerg commented on pull request "test: add coverage for `-bantime`":
(https://github.com/bitcoin/bitcoin/pull/26604#discussion_r1181104225)
Where is/why the one second sleep?

💬 Riahiamirreza commented on issue "rpc: show P2(W)SH redeemScript in getrawtransaction (and friends)":
(https://github.com/bitcoin/bitcoin/issues/27391#issuecomment-1528866113)
For backward compatibility, Should I add another level of verbosity that includes `redeemScript` in the `scriptSig`?

💬 darosior commented on pull request "Wallet: estimate the size of signed inputs using descriptors":
(https://github.com/bitcoin/bitcoin/pull/26567#issuecomment-1528873153)
Rebased, and added Miniscript support since signing support was merged.

🤔 TheCharlatan requested changes to a pull request: "rpc: Optimize serialization disk space of dumptxoutset"
(https://github.com/bitcoin/bitcoin/pull/26045#pullrequestreview-1403015467)
Thank you for picking this up again.


These are just patches for fixing up my initial code, I also put the entire diff [here](https://github.com/TheCharlatan/bitcoin/commit/a36bde4b5bea3965aefbc208a39525d57ff312fb). The rest looks good to me, I tested `dumptxoutset` extensively on mainnet. The runtime performance of this branch is very similar to the base.

The following can be ignored in the context of this PR, but I still wan to take note:

During review I noticed that the execution t
...

💬 TheCharlatan commented on pull request "rpc: Optimize serialization disk space of dumptxoutset":
(https://github.com/bitcoin/bitcoin/pull/26045#discussion_r1180726604)
Using explicit `make_pair` can be avoided by directly calling `emplace_back` instead (same for further below).

💬 TheCharlatan commented on pull request "rpc: Optimize serialization disk space of dumptxoutset":
(https://github.com/bitcoin/bitcoin/pull/26045#discussion_r1181134612)
The file writing would be more DRY if it were declared as a lambda and then called here and below, e.g. like:
```
auto write_coins_to_file = [&](AutoFile& afile, const uint256& last_hash, const std::vector<std::pair<uint32_t, Coin>>& coins) {
afile << last_hash;
afile << static_cast<uint16_t>(coins.size());
for (auto [vout, coin] : coins) {
afile << vout;
afile << coin;
}
};
```

💬 TheCharlatan commented on pull request "rpc: Optimize serialization disk space of dumptxoutset":
(https://github.com/bitcoin/bitcoin/pull/26045#discussion_r1178515444)
This needs to be dropped.

💬 0xB10C commented on pull request "Relay own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/27509#issuecomment-1528893509)
Concept ACK!

⚠️ Princeprince559 opened an issue: "Pur Chain"
(https://github.com/bitcoin/bitcoin/issues/27545)
### Issues, reports or feature requests related to the GUI should be opened directly on the GUI repo

- [X] I still think this issue should be opened here

### Report

Print @gregorycrawford`active`

💬 Princeprince559 commented on issue "Pur Chain":
(https://github.com/bitcoin/bitcoin/issues/27545#issuecomment-1528916271)
Help desk

✅ fanquake closed an issue: "Pur Chain"
(https://github.com/bitcoin/bitcoin/issues/27545)

:lock: fanquake locked an issue: "Pur Chain"
(https://github.com/bitcoin/bitcoin/issues/27545)

📝 cefikhan opened a pull request: "ScriptToUniv can get address of PUBKEY"
(https://github.com/bitcoin/bitcoin/pull/27546)
rpc: ScriptToUniv Can get address of PUBKEY

💬 sipa commented on pull request "ScriptToUniv can get address of PUBKEY":
(https://github.com/bitcoin/bitcoin/pull/27546#issuecomment-1528961205)
Concept NACK

Sorry, no. P2PK outputs do not *have* an address. It's an outdated and confusing practice to refer to them as their corresponding P2PKH address.