💬 maflcko commented on pull request "Revert "ci: Only run functional tests on windows in master"":
(https://github.com/bitcoin/bitcoin/pull/29059#issuecomment-1851817205)
lgtm ACK 7b22cd80e050b903b5765133b8116f4b17ce0296
(https://github.com/bitcoin/bitcoin/pull/29059#issuecomment-1851817205)
lgtm ACK 7b22cd80e050b903b5765133b8116f4b17ce0296
📝 fanquake converted_to_draft a pull request: "build: switch to using LLVM 17.x for macOS builds"
(https://github.com/bitcoin/bitcoin/pull/28880)
This is the last step before #21778. We need LLVM 17.x so that lld has `-fixup_chains`.
This is currently in draft, as there is a reproducibility issue with Qt: https://github.com/bitcoin/bitcoin/pull/28880#issuecomment-1830224605.
(https://github.com/bitcoin/bitcoin/pull/28880)
This is the last step before #21778. We need LLVM 17.x so that lld has `-fixup_chains`.
This is currently in draft, as there is a reproducibility issue with Qt: https://github.com/bitcoin/bitcoin/pull/28880#issuecomment-1830224605.
💬 fanquake commented on pull request "[POC] C++20 `std::endian`":
(https://github.com/bitcoin/bitcoin/pull/28674#discussion_r1423843923)
Will be possible after we pull the changes from minisketch: https://github.com/sipa/minisketch/pull/80.
(https://github.com/bitcoin/bitcoin/pull/28674#discussion_r1423843923)
Will be possible after we pull the changes from minisketch: https://github.com/sipa/minisketch/pull/80.
💬 fanquake commented on pull request "[WIP] C++20 std::views::reverse":
(https://github.com/bitcoin/bitcoin/pull/28687#issuecomment-1851843817)
https://github.com/bitcoin/bitcoin/pull/28687/checks?check_run_id=19528202548
```bash
/usr/bin/ccache clang++-13 -stdlib=libc++ -std=c++20 -DHAVE_CONFIG_H -I. -I../src/config -fmacro-prefix-map=/ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu=. -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=3 -DHAVE_BUILD_INFO -DPROVIDE_FUZZ_MAIN_FUNCTION -I. -I./minisketch/include -I./secp256k1/include -I./univalue/include -I./leveldb/include -isystem /ci_container_base/depends/x86_64-pc-linux-gnu/includ
...
(https://github.com/bitcoin/bitcoin/pull/28687#issuecomment-1851843817)
https://github.com/bitcoin/bitcoin/pull/28687/checks?check_run_id=19528202548
```bash
/usr/bin/ccache clang++-13 -stdlib=libc++ -std=c++20 -DHAVE_CONFIG_H -I. -I../src/config -fmacro-prefix-map=/ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu=. -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=3 -DHAVE_BUILD_INFO -DPROVIDE_FUZZ_MAIN_FUNCTION -I. -I./minisketch/include -I./secp256k1/include -I./univalue/include -I./leveldb/include -isystem /ci_container_base/depends/x86_64-pc-linux-gnu/includ
...
💬 Retropex commented on pull request "datacarriersize: Match more datacarrying":
(https://github.com/bitcoin/bitcoin/pull/28408#issuecomment-1851853229)
@conduition is very relevant as a question.
What fundamentally bothers me with inscriptions is that they do not play the same game as those who make financial transactions.
When you pay to send 10,000 times the same funds to the same wallet, you pay to move Bitcoin from one address to another and even if it's "useless" it doesn't bother me because there is no incentive to do that.
On the other hand, inscriptions have an incentive that is external to the Bitcoin protocol, it does not pa
...
(https://github.com/bitcoin/bitcoin/pull/28408#issuecomment-1851853229)
@conduition is very relevant as a question.
What fundamentally bothers me with inscriptions is that they do not play the same game as those who make financial transactions.
When you pay to send 10,000 times the same funds to the same wallet, you pay to move Bitcoin from one address to another and even if it's "useless" it doesn't bother me because there is no incentive to do that.
On the other hand, inscriptions have an incentive that is external to the Bitcoin protocol, it does not pa
...
🚀 fanquake merged a pull request: "doc/reduce-traffic: update/clarify max outbound connection count"
(https://github.com/bitcoin/bitcoin/pull/29052)
(https://github.com/bitcoin/bitcoin/pull/29052)
✅ fanquake closed an issue: "Nit: Inconsistency in the docs regarding block-relay-only connections"
(https://github.com/bitcoin/bitcoin/issues/29046)
(https://github.com/bitcoin/bitcoin/issues/29046)
💬 1ma commented on pull request "datacarriersize: Match more datacarrying":
(https://github.com/bitcoin/bitcoin/pull/28408#issuecomment-1851877095)
> If i publish a chain of 1000 transactions pointlessly bouncing coins around my own wallet addresses, is that spam? What about a single big (in terms of weight) transaction which only pays from myself to myself? The whole BTC network (including myself) would be better off if i didn't do that. I'm needlessly inflating the fee market for zero utility. In my humble opinion, inscriptions are similarly redundant because they could be done off-chain, so their on-chain utility to the network is near z
...
(https://github.com/bitcoin/bitcoin/pull/28408#issuecomment-1851877095)
> If i publish a chain of 1000 transactions pointlessly bouncing coins around my own wallet addresses, is that spam? What about a single big (in terms of weight) transaction which only pays from myself to myself? The whole BTC network (including myself) would be better off if i didn't do that. I'm needlessly inflating the fee market for zero utility. In my humble opinion, inscriptions are similarly redundant because they could be done off-chain, so their on-chain utility to the network is near z
...
🚀 fanquake merged a pull request: "tests, bench: Fix issue with CWallet::LoadWallet() being called in the wrong places"
(https://github.com/bitcoin/bitcoin/pull/29055)
(https://github.com/bitcoin/bitcoin/pull/29055)
💬 maflcko commented on pull request "refactor: Remove pre-C++20 code, fs::path cleanup":
(https://github.com/bitcoin/bitcoin/pull/29040#discussion_r1423882821)
> What would be an example where it is broken by calling `string()`
Any place, which correctly calls `u8string()`, and instead incorrectly calls `string()` is broken. For example, the code may be broken on systems whose native encoding is not UTF-8.
See also
```diff
diff --git a/src/test/fs_tests.cpp b/src/test/fs_tests.cpp
index 7cfecb2b22..34168a551b 100644
--- a/src/test/fs_tests.cpp
+++ b/src/test/fs_tests.cpp
@@ -19,7 +19,7 @@ BOOST_AUTO_TEST_CASE(fsbridge_pathtostring)
{
...
(https://github.com/bitcoin/bitcoin/pull/29040#discussion_r1423882821)
> What would be an example where it is broken by calling `string()`
Any place, which correctly calls `u8string()`, and instead incorrectly calls `string()` is broken. For example, the code may be broken on systems whose native encoding is not UTF-8.
See also
```diff
diff --git a/src/test/fs_tests.cpp b/src/test/fs_tests.cpp
index 7cfecb2b22..34168a551b 100644
--- a/src/test/fs_tests.cpp
+++ b/src/test/fs_tests.cpp
@@ -19,7 +19,7 @@ BOOST_AUTO_TEST_CASE(fsbridge_pathtostring)
{
...
✅ fanquake closed an issue: "Discussion: Upgrading to C++20"
(https://github.com/bitcoin/bitcoin/issues/23363)
(https://github.com/bitcoin/bitcoin/issues/23363)
💬 fanquake commented on issue "Discussion: Upgrading to C++20":
(https://github.com/bitcoin/bitcoin/issues/23363#issuecomment-1851893448)
Going to close this now that #28349 has been merged, and work is already underway to use C++20 code. I think further discussion about specific C++20 features, and their usage / compiler requirements, can continue in their own issues.
(https://github.com/bitcoin/bitcoin/issues/23363#issuecomment-1851893448)
Going to close this now that #28349 has been merged, and work is already underway to use C++20 code. I think further discussion about specific C++20 features, and their usage / compiler requirements, can continue in their own issues.
💬 maflcko commented on pull request "[WIP] C++20 std::views::reverse":
(https://github.com/bitcoin/bitcoin/pull/28687#issuecomment-1851904825)
Hmm, there's no clang-16 in the current LTS release of Ubuntu, so I guess this will have to wait for some time.
https://packages.ubuntu.com/jammy/clang-16
I wonder whether std::span will be usable, given that it internally assumes ranges?
(https://github.com/bitcoin/bitcoin/pull/28687#issuecomment-1851904825)
Hmm, there's no clang-16 in the current LTS release of Ubuntu, so I guess this will have to wait for some time.
https://packages.ubuntu.com/jammy/clang-16
I wonder whether std::span will be usable, given that it internally assumes ranges?
💬 petertodd commented on pull request "datacarriersize: Match more datacarrying":
(https://github.com/bitcoin/bitcoin/pull/28408#issuecomment-1851916112)
> The original Satoshi client didn't allow embedding data onchain, this has always been an exploit.
Bitcoin was released with no standardness rules at all. You absolutely could embed data onchain. You could even publish arbitrary data to the UTXO set easily.
(https://github.com/bitcoin/bitcoin/pull/28408#issuecomment-1851916112)
> The original Satoshi client didn't allow embedding data onchain, this has always been an exploit.
Bitcoin was released with no standardness rules at all. You absolutely could embed data onchain. You could even publish arbitrary data to the UTXO set easily.
💬 maflcko commented on pull request "build: Enable -Wunreachable-code":
(https://github.com/bitcoin/bitcoin/pull/28999#issuecomment-1851949647)
> I don't think this works, because it will replace `__FILE__, __LINE__, __func__` with a meaningless string literal constant.
C++20 fixed that, see https://en.cppreference.com/w/cpp/utility/source_location
(https://github.com/bitcoin/bitcoin/pull/28999#issuecomment-1851949647)
> I don't think this works, because it will replace `__FILE__, __LINE__, __func__` with a meaningless string literal constant.
C++20 fixed that, see https://en.cppreference.com/w/cpp/utility/source_location
💬 furszy commented on pull request "wallet: Pass through transaction locktime and preset input sequences and scripts to CreateTransaction":
(https://github.com/bitcoin/bitcoin/pull/25273#discussion_r1423940397)
nit:
it seems that the version field is not sanitized. The user provided `tx.nVersion` is a int32_t while `coinControl.m_version` is a uint32_t.
(https://github.com/bitcoin/bitcoin/pull/25273#discussion_r1423940397)
nit:
it seems that the version field is not sanitized. The user provided `tx.nVersion` is a int32_t while `coinControl.m_version` is a uint32_t.
👍 TheCharlatan approved a pull request: "Revert "ci: Only run functional tests on windows in master""
(https://github.com/bitcoin/bitcoin/pull/29059#pullrequestreview-1777427931)
ACK 7b22cd80e050b903b5765133b8116f4b17ce0296
(https://github.com/bitcoin/bitcoin/pull/29059#pullrequestreview-1777427931)
ACK 7b22cd80e050b903b5765133b8116f4b17ce0296
💬 sdaftuar commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#issuecomment-1852009874)
> size-2 package rbf can have 6 types of conflicts:
>
> 1. parent conflicts with solo
> 2. parent conflicts with child
> 3. parent conflicts with parent
> 4. child conflicts with parent
> 5. child conflicts with solo
> 6. child conflicts with child
I think if you state it this way, then there are more cases to write out (but they all simplify down to the same thing) -- the parent transaction in the incoming package can directly conflict with {solo tx, child tx, parent tx, both child t
...
(https://github.com/bitcoin/bitcoin/pull/28984#issuecomment-1852009874)
> size-2 package rbf can have 6 types of conflicts:
>
> 1. parent conflicts with solo
> 2. parent conflicts with child
> 3. parent conflicts with parent
> 4. child conflicts with parent
> 5. child conflicts with solo
> 6. child conflicts with child
I think if you state it this way, then there are more cases to write out (but they all simplify down to the same thing) -- the parent transaction in the incoming package can directly conflict with {solo tx, child tx, parent tx, both child t
...
📝 ismaelsadeeq opened a pull request: "Policy: Report reason inputs are non standard"
(https://github.com/bitcoin/bitcoin/pull/29060)
This PR is another attempt at #13525.
Transactions that fail `PreChecks` Validation due to non-standard inputs now returns invalid validation state`TxValidationResult::TX_INPUTS_NOT_STANDARD` along with a debug error message.
Previously, the debug error message for non-standard inputs do not specify why the inputs were considered non-standard.
Instead, the same error string, `bad-txns-nonstandard-inputs`, used for all types of non-standard input scriptSigs.
This PR updates the `AreI
...
(https://github.com/bitcoin/bitcoin/pull/29060)
This PR is another attempt at #13525.
Transactions that fail `PreChecks` Validation due to non-standard inputs now returns invalid validation state`TxValidationResult::TX_INPUTS_NOT_STANDARD` along with a debug error message.
Previously, the debug error message for non-standard inputs do not specify why the inputs were considered non-standard.
Instead, the same error string, `bad-txns-nonstandard-inputs`, used for all types of non-standard input scriptSigs.
This PR updates the `AreI
...
💬 glozow commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1423981108)
taken
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1423981108)
taken
💬 glozow commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1423981481)
Added mention of no topological restrictions to the comment
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1423981481)
Added mention of no topological restrictions to the comment