📝 MarcoFalke converted_to_draft a pull request: "refactor: Open file in FileCommit"
(https://github.com/bitcoin/bitcoin/pull/28006)
This improves `FileCommit`: The patch removes the requirement from the call site to open a raw C-style FILE pointer. Now the call site can use any style of file IO.

There are many other smaller improvements, which are explained in each commit.

💬 vasild commented on pull request "test: add end-to-end tests for CConnman and PeerManager":
(https://github.com/bitcoin/bitcoin/pull/26812#discussion_r1269027565)
I will look into replacing this with `Assert()`.

"exception from destructor in c++" is a topic well covered all over the internet. The compiler produces a warning for that. Here is an example:

```cpp
class A
{
public:
~A() {
std::cout << "before throw a\n";
// warning: '~A' has a non-throwing exception specification but can still throw [-Wexceptions]
//171 | throw std::runtime_error("a");
// | ^
throw std::runtime_er
...

💬 MarcoFalke commented on pull request "fuzz: Generate process_message targets individually":
(https://github.com/bitcoin/bitcoin/pull/28066#issuecomment-1643403050)
Anything left to do for a test-only change with two ACKs or is this rfm?

💬 MarcoFalke commented on pull request "test: add end-to-end tests for CConnman and PeerManager":
(https://github.com/bitcoin/bitcoin/pull/26812#discussion_r1269042432)
Yeah, but this being an "Abort trap" is well defined, not "not allowed". And I think the tests aborting on failure is fine.

✅ hebasto closed a pull request: "Add `UNREACHABLE` macro"
(https://github.com/bitcoin/bitcoin/pull/26504)

⚠️ hebasto opened an issue: "build: Windows debug cross-build fails with `-O0`"
(https://github.com/bitcoin/bitcoin/issues/28109)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

When cross-compiling for Windows with the `-O0` optimization level the build fails.

The error can be fixed by the `-Wa,-mbig-obj` assembler flag. However, @theuni [suggested](https://github.com/hebasto/bitcoin/pull/18#discussion_r1267219453):
> We should definitely look into breaking up that object.

### Expected behaviour

The build completes with no error.

### Steps to reproduce

``
...

✅ fanquake closed an issue: "bumpfee behavior with custom change address"
(https://github.com/bitcoin/bitcoin/issues/11233)

✅ fanquake closed an issue: "Privacy Issue - Increase Fee with Custom Change Address grabs new UTXO "
(https://github.com/bitcoin/bitcoin/issues/20795)

🚀 fanquake merged a pull request: "bumpfee: Allow the user to choose which output is change"
(https://github.com/bitcoin/bitcoin/pull/26467)

💬 whitslack commented on pull request "bumpfee: Allow the user to choose which output is change":
(https://github.com/bitcoin/bitcoin/pull/26467#discussion_r1269177446)
@furszy makes an important point. Consider the case where I am paying someone on chain under the stipulation that *they* will eat the mining fee. (Thus, I specify *their* address in `subtractfeefrom` when I'm constructing my transaction.) Later, they tell me that they can't wait for confirmation any longer and need an urgent fee bump. If I naïvely specify their output as the `reduce_output`, I may end up paying them ***much more*** than I intended to if I only have large UTxOs in my wallet.

🚀 fanquake merged a pull request: "fuzz: Generate process_message targets individually"
(https://github.com/bitcoin/bitcoin/pull/28066)

📝 fanquake opened a pull request: "doc: correct Fedora systemtap dep"
(https://github.com/bitcoin/bitcoin/pull/28110)

⚠️ hebasto opened an issue: ""missing operand" assembler warnings on Windows"
(https://github.com/bitcoin/bitcoin/issues/28111)
On the master branch @673acab223c0f896767b1ae784659df9f95452ae on Ubuntu 23.04:

```
$ make -C depends HOST=x86_64-w64-mingw32 NO_QT=1 DEBUG=1 NO_HARDEN=1
$ ./configure CONFIG_SITE=$PWD/depends/x86_64-w64-mingw32/share/config.site
$ make
...
CXX crypto/libbitcoin_crypto_base_la-aes.lo
CXX crypto/libbitcoin_crypto_base_la-chacha_poly_aead.lo
CXX crypto/libbitcoin_crypto_base_la-chacha20.lo
CXX crypto/libbitcoin_crypto_base_la-hkdf_sha256_32.lo
CXX cry
...

🤔 stickies-v reviewed a pull request: "rfc: Type-safe transaction identifiers"
(https://github.com/bitcoin/bitcoin/pull/28107#pullrequestreview-1538994126)
Concept ACK.

We don't consistently have clear variable naming (e.g. the ambiguous `hash`; or `txid` being used to refer to a wtxid), sometimes requiring quite a bit of developer time to figure out the expected ~type. I have experienced this quite a bit myself.

Introducing types makes this explicit at relatively low overhead, and I think the approach of subclassing `uint256` makes sense.

We'll have to be careful for this not to lead to unnecessary refactoring code churn, but that shouldn
...

💬 fanquake commented on issue ""missing operand" assembler warnings on Windows":
(https://github.com/bitcoin/bitcoin/issues/28111#issuecomment-1643712997)
I don't understand what the actual problem is, or what is broken? Can you elaborate?

💬 hebasto commented on issue ""missing operand" assembler warnings on Windows":
(https://github.com/bitcoin/bitcoin/issues/28111#issuecomment-1643715980)
> I don't understand what the actual problem is, or what is broken? Can you elaborate?

I'm not an expert in assembly code, but "missing operand' sounds scary.

I don't know whether this warning is safe to ignore.

👍 hebasto approved a pull request: "doc: correct Fedora systemtap dep"
(https://github.com/bitcoin/bitcoin/pull/28110#pullrequestreview-1539020657)
ACK 12edf7b155685c0e1a401cfa71ef28784362689e, tested on Fedora 38:

- with`systemtap` installed:
```
$ ./configure
...
USDT tracing = no
...
```

- with`systemtap-sdt-devel` installed:
```
$ ./configure
...
USDT tracing = yes
...
```

💬 fanquake commented on issue ""missing operand" assembler warnings on Windows":
(https://github.com/bitcoin/bitcoin/issues/28111#issuecomment-1643816890)
Probably needs some actual investigation then:
i.e what code + combination of compiler flags is causing the issue, produce a minimal test case.
Is this a "new" issue, or have the warnings existed in our codebase "forever".
Is it related to the fact that you're using ubuntu mantic (devel) and/or compiler/binutils versions etc.

🚀 fanquake merged a pull request: "contrib: move user32.dll from bitcoind.exe libs"
(https://github.com/bitcoin/bitcoin/pull/28099)

💬 glozow commented on pull request "rfc: Type-safe transaction identifiers":
(https://github.com/bitcoin/bitcoin/pull/28107#issuecomment-1643837866)
I've definitely written/seen bugs that could be prevented if Txid and Wtxid were different types. When I review code it's something that I look for and it comes up surprisingly often. Interfaces taking `uint256` often implicitly expect them to be a txid, eg #23325.

Agree that checking at compile time would be nice and probably prevent bugs / make review easier. Concept ACK assuming we're ok with the amount of code change.