⚠️ 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
``
...
(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)
(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)
(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)
(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.
(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)
(https://github.com/bitcoin/bitcoin/pull/28066)
📝 fanquake opened a pull request: "doc: correct Fedora systemtap dep"
(https://github.com/bitcoin/bitcoin/pull/28110)
(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
...
(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
...
(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?
(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.
(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
...
```
(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.
(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)
(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.
(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.
💬 dergoegge commented on pull request "rfc: Type-safe transaction identifiers":
(https://github.com/bitcoin/bitcoin/pull/28107#issuecomment-1643838829)
> We'll have to be careful for this not to lead to unnecessary refactoring code churn, but that shouldn't be a reason not to make this change imo.
In terms of refactoring, this should be similar to the time type-safety PRs in that we can gradually convert our code.
(https://github.com/bitcoin/bitcoin/pull/28107#issuecomment-1643838829)
> We'll have to be careful for this not to lead to unnecessary refactoring code churn, but that shouldn't be a reason not to make this change imo.
In terms of refactoring, this should be similar to the time type-safety PRs in that we can gradually convert our code.
🚀 fanquake merged a pull request: "util: Show descriptive error messages when FileCommit fails"
(https://github.com/bitcoin/bitcoin/pull/26654)
(https://github.com/bitcoin/bitcoin/pull/26654)
💬 MarcoFalke commented on pull request "test: Ignore UTF-8 errors in assert_debug_log":
(https://github.com/bitcoin/bitcoin/pull/28035#issuecomment-1643863154)
Split into more commits
(https://github.com/bitcoin/bitcoin/pull/28035#issuecomment-1643863154)
Split into more commits
💬 glozow commented on pull request "Use `int32_t` type for most transaction size/weight values":
(https://github.com/bitcoin/bitcoin/pull/23962#issuecomment-1643870289)
> This causes warnings when compiling for 32 bit?
@hebasto did you end up looking into this?
(https://github.com/bitcoin/bitcoin/pull/23962#issuecomment-1643870289)
> This causes warnings when compiling for 32 bit?
@hebasto did you end up looking into this?
💬 darosior commented on pull request "Descriptors: rule out unspendable miniscript descriptors":
(https://github.com/bitcoin/bitcoin/pull/27997#issuecomment-1643890926)
I forgot also checking this when parsing from Script..
(https://github.com/bitcoin/bitcoin/pull/27997#issuecomment-1643890926)
I forgot also checking this when parsing from Script..