⚠️ sinetek opened an issue: "The logo icon doesn't show properly under Wayland"
(https://github.com/bitcoin-core/gui/issues/781)
### Is there an existing issue for this?
- [X] I have searched the existing issues
### Current behaviour
There should be a bitcoin logo icon when running the application under a wayland session.
Right now it looks like this on my machine.

### Expected behaviour
It should look like this (run on the same machine, same everything, but with platform xcb):

### Is there an existing issue for this?
- [X] I have searched the existing issues
### Current behaviour
There should be a bitcoin logo icon when running the application under a wayland session.
Right now it looks like this on my machine.

### Expected behaviour
It should look like this (run on the same machine, same everything, but with platform xcb):

ACK d08e820abf5da2be09b8a84b5bd3450d1a55a039
(https://github.com/bitcoin/bitcoin/pull/29048#pullrequestreview-1774091824)
ACK d08e820abf5da2be09b8a84b5bd3450d1a55a039
💬 douglaz commented on pull request "datacarriersize: Match more datacarrying":
(https://github.com/bitcoin/bitcoin/pull/28408#issuecomment-1849086282)
Concept NACK.
This is an attempt to increase transaction censorship under the guise of a bugfix.
(https://github.com/bitcoin/bitcoin/pull/28408#issuecomment-1849086282)
Concept NACK.
This is an attempt to increase transaction censorship under the guise of a bugfix.
💬 LarryRuane commented on pull request "test: test_bitcoin: allow -testdatadir=<datadir>":
(https://github.com/bitcoin/bitcoin/pull/26564#discussion_r1421821961)
> The default test behavior creates a random directory path within the OS temp directory; therefore, we don't need to remove anything on this execution path.
No, in current master, the test framework does delete the directory _after_ the test: https://github.com/bitcoin/bitcoin/blob/3e691258d8789a4a89cce42e7e71b130491594d7/src/test/util/setup_common.cpp#L162 and I think that's a good thing or else running many tests could fill up your temp directory. If the test stops unexpectedly, I think th
...
(https://github.com/bitcoin/bitcoin/pull/26564#discussion_r1421821961)
> The default test behavior creates a random directory path within the OS temp directory; therefore, we don't need to remove anything on this execution path.
No, in current master, the test framework does delete the directory _after_ the test: https://github.com/bitcoin/bitcoin/blob/3e691258d8789a4a89cce42e7e71b130491594d7/src/test/util/setup_common.cpp#L162 and I think that's a good thing or else running many tests could fill up your temp directory. If the test stops unexpectedly, I think th
...
💬 Retropex commented on pull request "datacarriersize: Match more datacarrying":
(https://github.com/bitcoin/bitcoin/pull/28408#issuecomment-1849100398)
> Concept NACK.
>
> This is an attempt to increase transaction censorship under the guise of a bugfix.
Bitcoin Core already standardizes the insertion of arbitrary data above 83 bytes and inscriptions are a diverted way to bypass this limit so it is a bug fix.
(https://github.com/bitcoin/bitcoin/pull/28408#issuecomment-1849100398)
> Concept NACK.
>
> This is an attempt to increase transaction censorship under the guise of a bugfix.
Bitcoin Core already standardizes the insertion of arbitrary data above 83 bytes and inscriptions are a diverted way to bypass this limit so it is a bug fix.
💬 aviv57 commented on pull request "datacarriersize: Match more datacarrying":
(https://github.com/bitcoin/bitcoin/pull/28408#issuecomment-1849105666)
Concept NACK.
I think ordinals and inscriptions will make a lot of people invest in shit jpegs.
But the way to stop people from encoding arbitrary data on the chain is not by adding censorship.
It's by waiting for it to be much more expensive to do it on bitcoin than doing it on other networks / centralized services
(https://github.com/bitcoin/bitcoin/pull/28408#issuecomment-1849105666)
Concept NACK.
I think ordinals and inscriptions will make a lot of people invest in shit jpegs.
But the way to stop people from encoding arbitrary data on the chain is not by adding censorship.
It's by waiting for it to be much more expensive to do it on bitcoin than doing it on other networks / centralized services
💬 TheCharlatan commented on pull request "ci: Use Ubuntu 24.04 Noble for asan,tsan,tidy,fuzz":
(https://github.com/bitcoin/bitcoin/pull/28992#issuecomment-1849106727)
When I apply this patch and run `FILE_ENV="./ci/test/00_setup_env_native_tidy.sh" ./ci/test_run_all.sh`, I get
```
131.2 CMake Error at /usr/lib/llvm-17/lib/cmake/clang/ClangTargets.cmake:833 (message):
131.2 The imported target "libclang" references the file
131.2
131.2 "/usr/lib/llvm-17/lib/libclang-17.so.1"
131.2
131.2 but this file does not exist. Possible reasons include:
131.2
131.2 * The file was deleted, renamed, or moved to another location.
131.2
131.2 * An
...
(https://github.com/bitcoin/bitcoin/pull/28992#issuecomment-1849106727)
When I apply this patch and run `FILE_ENV="./ci/test/00_setup_env_native_tidy.sh" ./ci/test_run_all.sh`, I get
```
131.2 CMake Error at /usr/lib/llvm-17/lib/cmake/clang/ClangTargets.cmake:833 (message):
131.2 The imported target "libclang" references the file
131.2
131.2 "/usr/lib/llvm-17/lib/libclang-17.so.1"
131.2
131.2 but this file does not exist. Possible reasons include:
131.2
131.2 * The file was deleted, renamed, or moved to another location.
131.2
131.2 * An
...
💬 ajtowns commented on pull request "kernel: Streamline util library":
(https://github.com/bitcoin/bitcoin/pull/29015#issuecomment-1849208455)
Would it make sense to just merge common into util; but have a "stripped" version of util available for the kernel, that excludes stuff that doesn't match the 5 points above? That way the difference is just at the build system level, it doesn't involve moving files around or moving code between namespaces...
Then the advice would be: (a) just put things in util, (b) but only add things to the "util-core" section of the build file when they're needed by the kernel; (c) keep things in the util:
...
(https://github.com/bitcoin/bitcoin/pull/29015#issuecomment-1849208455)
Would it make sense to just merge common into util; but have a "stripped" version of util available for the kernel, that excludes stuff that doesn't match the 5 points above? That way the difference is just at the build system level, it doesn't involve moving files around or moving code between namespaces...
Then the advice would be: (a) just put things in util, (b) but only add things to the "util-core" section of the build file when they're needed by the kernel; (c) keep things in the util:
...
💬 hmisty commented on pull request "datacarriersize: Match more datacarrying":
(https://github.com/bitcoin/bitcoin/pull/28408#issuecomment-1849244134)
Concept ACK.
Makes sense to consider the way inserting data as a trick and abuse of the taproot script.
(https://github.com/bitcoin/bitcoin/pull/28408#issuecomment-1849244134)
Concept ACK.
Makes sense to consider the way inserting data as a trick and abuse of the taproot script.
💬 hmisty commented on pull request "datacarriersize: Match more datacarrying":
(https://github.com/bitcoin/bitcoin/pull/28408#issuecomment-1849261876)
> Concept ACK.
>
> Makes sense to consider the way inserting data as a trick and abuse of the taproot script.
Since the obfuscated data in the script are neither verified nor enforced by the consensus, and these data are to any extent eventually rely on off-chain indexers to proceed, a more elegant way could be to move all data out of the chain for the off-chain indexers to store and deal with, while leaving only a hash on chain which fits well into the design intent with minimal on-chain
...
(https://github.com/bitcoin/bitcoin/pull/28408#issuecomment-1849261876)
> Concept ACK.
>
> Makes sense to consider the way inserting data as a trick and abuse of the taproot script.
Since the obfuscated data in the script are neither verified nor enforced by the consensus, and these data are to any extent eventually rely on off-chain indexers to proceed, a more elegant way could be to move all data out of the chain for the off-chain indexers to store and deal with, while leaving only a hash on chain which fits well into the design intent with minimal on-chain
...
💬 mmgen commented on pull request "datacarriersize: Match more datacarrying":
(https://github.com/bitcoin/bitcoin/pull/28408#issuecomment-1849289963)
Concept NACK. The ordinals transactions will end up in the blockchain anyway via private mempools, so this PR does nothing to solve/mitigate the problem of blockchain spam.
And as Peter Todd noted, blockchain propagation will be adversely impacted, as regular nodes will be seeing the ordinals TXs only when the block is announced.
(https://github.com/bitcoin/bitcoin/pull/28408#issuecomment-1849289963)
Concept NACK. The ordinals transactions will end up in the blockchain anyway via private mempools, so this PR does nothing to solve/mitigate the problem of blockchain spam.
And as Peter Todd noted, blockchain propagation will be adversely impacted, as regular nodes will be seeing the ordinals TXs only when the block is announced.
💬 osagie98 commented on pull request "test: Use test framework utils in functional tests":
(https://github.com/bitcoin/bitcoin/pull/28528#issuecomment-1849306970)
Hi @BrandonOdiwuor,
Sorry, I missed the notification of your messages. I missed a few asserts, so I went back to add them. I also initially chose not to change any `assert x is True/False` as I wasn't sure if it would be useful, but I decided now to change those as well.
Please take another look whenever you're free, thank you!
(https://github.com/bitcoin/bitcoin/pull/28528#issuecomment-1849306970)
Hi @BrandonOdiwuor,
Sorry, I missed the notification of your messages. I missed a few asserts, so I went back to add them. I also initially chose not to change any `assert x is True/False` as I wasn't sure if it would be useful, but I decided now to change those as well.
Please take another look whenever you're free, thank you!
💬 droptpackets commented on pull request "datacarriersize: Match more datacarrying":
(https://github.com/bitcoin/bitcoin/pull/28408#issuecomment-1849369141)
Attempting to report the underlying functionality that has lead to Ordinals as a 'vulnerability' via NIST (https://nvd.nist.gov/vuln/detail/CVE-2023-50428) is in bad faith.
This approach reeks of entitlement to try to attempt to bring in external validation through misinformation (and it **is** misinformation) around what the original underlying design that has given rise to inscriptions and ordinals on Bitcoin. Shame on the members of this community that has agreed and endorsed this path. Th
...
(https://github.com/bitcoin/bitcoin/pull/28408#issuecomment-1849369141)
Attempting to report the underlying functionality that has lead to Ordinals as a 'vulnerability' via NIST (https://nvd.nist.gov/vuln/detail/CVE-2023-50428) is in bad faith.
This approach reeks of entitlement to try to attempt to bring in external validation through misinformation (and it **is** misinformation) around what the original underlying design that has given rise to inscriptions and ordinals on Bitcoin. Shame on the members of this community that has agreed and endorsed this path. Th
...
📝 stevenroose opened a pull request: "Add OP_TXHASH and OP_CHECKTXHASHVERIFY opcodes"
(https://github.com/bitcoin/bitcoin/pull/29050)
Implementation of OP_TXHASH and OP_CHECKTXHASHVERIFY, as per the [draft BIP](https://github.com/bitcoin/bips/pull/1500).
This MR includes a test using the test vectors generated from the reference implementation in the BIP.
The implementation utilizes a caching strategy that hopefully alleviates concerns around resource usage like quadratic hashing.
(https://github.com/bitcoin/bitcoin/pull/29050)
Implementation of OP_TXHASH and OP_CHECKTXHASHVERIFY, as per the [draft BIP](https://github.com/bitcoin/bips/pull/1500).
This MR includes a test using the test vectors generated from the reference implementation in the BIP.
The implementation utilizes a caching strategy that hopefully alleviates concerns around resource usage like quadratic hashing.
💬 S3RK commented on pull request "wallet: Pass through transaction locktime and preset input sequences and scripts to CreateTransaction":
(https://github.com/bitcoin/bitcoin/pull/25273#issuecomment-1849599335)
Code review ACK 0295b44c257fe23f1ad344aff11372d67864f0db
Still not a huge fan of 596642c5a9 and I have same concerns as josibake. I prefer explicit defitino of external inputs rather than indirectly through `SetTxOut`. Though I don't want this to block the PR.
Also it seems two commits were squashed "Explicitly preserve scriptSig and scriptWitness in CreateTransaction" and "Preserve preset input order". Was that intentional?
(https://github.com/bitcoin/bitcoin/pull/25273#issuecomment-1849599335)
Code review ACK 0295b44c257fe23f1ad344aff11372d67864f0db
Still not a huge fan of 596642c5a9 and I have same concerns as josibake. I prefer explicit defitino of external inputs rather than indirectly through `SetTxOut`. Though I don't want this to block the PR.
Also it seems two commits were squashed "Explicitly preserve scriptSig and scriptWitness in CreateTransaction" and "Preserve preset input order". Was that intentional?
💬 S3RK commented on pull request "wallet: birth time update during tx scanning":
(https://github.com/bitcoin/bitcoin/pull/28920#issuecomment-1849606079)
Approach ACK
(https://github.com/bitcoin/bitcoin/pull/28920#issuecomment-1849606079)
Approach ACK
💬 josibake commented on pull request "wallet: Pass through transaction locktime and preset input sequences and scripts to CreateTransaction":
(https://github.com/bitcoin/bitcoin/pull/25273#issuecomment-1849613826)
> Still not a huge fan of https://github.com/bitcoin/bitcoin/commit/596642c5a9f52dda2599b0bde424366bb22b3c6e and I have same concerns as josibake
My concerns were addressed by https://github.com/bitcoin/bitcoin/pull/25273#discussion_r1420969629. I think the approach in https://github.com/bitcoin/bitcoin/commit/596642c5a9f52dda2599b0bde424366bb22b3c6e is much simpler and as the comment points out, the worst-case scenario if inputs are labeled incorrectly is overestimating the fee by 1 byte.
(https://github.com/bitcoin/bitcoin/pull/25273#issuecomment-1849613826)
> Still not a huge fan of https://github.com/bitcoin/bitcoin/commit/596642c5a9f52dda2599b0bde424366bb22b3c6e and I have same concerns as josibake
My concerns were addressed by https://github.com/bitcoin/bitcoin/pull/25273#discussion_r1420969629. I think the approach in https://github.com/bitcoin/bitcoin/commit/596642c5a9f52dda2599b0bde424366bb22b3c6e is much simpler and as the comment points out, the worst-case scenario if inputs are labeled incorrectly is overestimating the fee by 1 byte.
⚠️ nymkappa opened an issue: "[build] cannot build tests using v26.0"
(https://github.com/bitcoin/bitcoin/issues/29051)
### Is there an existing issue for this?
- [X] I have searched the existing issues
### Current behaviour
I'm trying to upgrade to core v26.0 on macOS (M1 processor) but I'm getting the following error when building from sources:
```
CXX test/test_bitcoin-base58_tests.o
test/base58_tests.cpp:26:22: error: no matching function for call to 'read_json'
UniValue tests = read_json(json_tests::base58_encode_decode);
^~~~~~~~~
./test/util/json.h:12:10: note: ca
...
(https://github.com/bitcoin/bitcoin/issues/29051)
### Is there an existing issue for this?
- [X] I have searched the existing issues
### Current behaviour
I'm trying to upgrade to core v26.0 on macOS (M1 processor) but I'm getting the following error when building from sources:
```
CXX test/test_bitcoin-base58_tests.o
test/base58_tests.cpp:26:22: error: no matching function for call to 'read_json'
UniValue tests = read_json(json_tests::base58_encode_decode);
^~~~~~~~~
./test/util/json.h:12:10: note: ca
...
💬 willcl-ark commented on issue "[build] cannot build tests using v26.0":
(https://github.com/bitcoin/bitcoin/issues/29051#issuecomment-1849617387)
Have you run `make clean` since updating your local repo to v26.0?
(https://github.com/bitcoin/bitcoin/issues/29051#issuecomment-1849617387)
Have you run `make clean` since updating your local repo to v26.0?
🚀 fanquake merged a pull request: "Add a note to msvc readme re building Qt for Bitcoin Core."
(https://github.com/bitcoin/bitcoin/pull/29048)
(https://github.com/bitcoin/bitcoin/pull/29048)