💬 benthecarman commented on pull request "Allow accepting non-standard transactions on mainnet":
(https://github.com/bitcoin/bitcoin/pull/27578#issuecomment-1536189204)
The problem is that standardness is an overloaded term, it contains some DoS protection while also just disallowing other things. If it is that dangerous, proper warnings can be added, however, DoS attacks are already happening on bitcoin that are within standardness rules (stamps).
(https://github.com/bitcoin/bitcoin/pull/27578#issuecomment-1536189204)
The problem is that standardness is an overloaded term, it contains some DoS protection while also just disallowing other things. If it is that dangerous, proper warnings can be added, however, DoS attacks are already happening on bitcoin that are within standardness rules (stamps).
💬 hebasto commented on pull request "test: Treat `bitcoin-wallet` binary in the same way as others":
(https://github.com/bitcoin/bitcoin/pull/27554#discussion_r1186040867)
Adjusted.
(https://github.com/bitcoin/bitcoin/pull/27554#discussion_r1186040867)
Adjusted.
⚠️ fanquake opened an issue: "TSAN: lock-order-inversion (potential deadlock) in ZapSelectTx test"
(https://github.com/bitcoin/bitcoin/issues/27582)
master (6c7ebcc14b7908a67a8f8764b398e76c8fb4fe8b) running the TSAN CI job, on Fedora 38, with podman:
```bash
==================
WARNING: ThreadSanitizer: lock-order-inversion (potential deadlock) (pid=94411)
Cycle in lock order graph: M0 (0xffff8c427208) => M1 (0xffff8c42d868) => M2 (0xffff8c42da18) => M0
Mutex M1 acquired here while holding mutex M0 in main thread:
#0 pthread_rwlock_wrlock <null> (test_bitcoin+0x150920) (BuildId: e6d773dbb6475135a1747fe19e2eb18593ee1a4b)
#
...
(https://github.com/bitcoin/bitcoin/issues/27582)
master (6c7ebcc14b7908a67a8f8764b398e76c8fb4fe8b) running the TSAN CI job, on Fedora 38, with podman:
```bash
==================
WARNING: ThreadSanitizer: lock-order-inversion (potential deadlock) (pid=94411)
Cycle in lock order graph: M0 (0xffff8c427208) => M1 (0xffff8c42d868) => M2 (0xffff8c42da18) => M0
Mutex M1 acquired here while holding mutex M0 in main thread:
#0 pthread_rwlock_wrlock <null> (test_bitcoin+0x150920) (BuildId: e6d773dbb6475135a1747fe19e2eb18593ee1a4b)
#
...
💬 glozow commented on pull request "Allow accepting non-standard transactions on mainnet":
(https://github.com/bitcoin/bitcoin/pull/27578#issuecomment-1536207253)
Concept NACK. These checks exist for very good reasons; being able to bypass them on mainnet is a huge footgun. If there is a specific standardness rule that you can show (1) provides little utility to the node or network and (2) is getting in the way of a use case, we can discuss changing that rule.
> There are many projects today that have part of their workflow "email a miner this transaction" (inscriptions, https://github.com/supertestnet/breaker-of-jpegs, etc). This is a problem and will
...
(https://github.com/bitcoin/bitcoin/pull/27578#issuecomment-1536207253)
Concept NACK. These checks exist for very good reasons; being able to bypass them on mainnet is a huge footgun. If there is a specific standardness rule that you can show (1) provides little utility to the node or network and (2) is getting in the way of a use case, we can discuss changing that rule.
> There are many projects today that have part of their workflow "email a miner this transaction" (inscriptions, https://github.com/supertestnet/breaker-of-jpegs, etc). This is a problem and will
...
💬 MarcoFalke commented on pull request "test, init: perturb file to ensure failure instead of only deleting them":
(https://github.com/bitcoin/bitcoin/pull/26653#issuecomment-1536207299)
lgtm ACK c371cae07a7ba045130568b6abc470eaa4f95ef4
(https://github.com/bitcoin/bitcoin/pull/26653#issuecomment-1536207299)
lgtm ACK c371cae07a7ba045130568b6abc470eaa4f95ef4
💬 MarcoFalke commented on issue "TSAN: lock-order-inversion (potential deadlock) in ZapSelectTx test":
(https://github.com/bitcoin/bitcoin/issues/27582#issuecomment-1536209476)
Is this reproducible?
(https://github.com/bitcoin/bitcoin/issues/27582#issuecomment-1536209476)
Is this reproducible?
💬 jamesob commented on pull request "assumeutxo":
(https://github.com/bitcoin/bitcoin/pull/15606#issuecomment-1536222186)
Thanks for testing!
> except that my mempool was empty until I stopped and restarted bitcoind?
Interesting, I'll look at this today; probably some part of net_processing that's looking at `chainman.IsAnyIBD()` when it should be using `ActiveChainstate().IsInitialBlockDownload()`.
(https://github.com/bitcoin/bitcoin/pull/15606#issuecomment-1536222186)
Thanks for testing!
> except that my mempool was empty until I stopped and restarted bitcoind?
Interesting, I'll look at this today; probably some part of net_processing that's looking at `chainman.IsAnyIBD()` when it should be using `ActiveChainstate().IsInitialBlockDownload()`.
💬 furszy commented on pull request "net: call getaddrinfo() in detachable thread to prevent stalling":
(https://github.com/bitcoin/bitcoin/pull/27557#discussion_r1186063927)
> Afaict this won't work here, because the thread might not finish and then the pool would just get clogged (see comment about abandoning the thread).
k, `getaddrinfo` doesn't have a timeout and `getaddrinfo_a` seems to have a segfault. Cool..
> Performance should also not really matter here since this doesn't happen that often and startup time for a thread vs. time of a DNS lookup is probably negligible.
I was thinking more about context switching. Isn't this used for `OpenNetworkConne
...
(https://github.com/bitcoin/bitcoin/pull/27557#discussion_r1186063927)
> Afaict this won't work here, because the thread might not finish and then the pool would just get clogged (see comment about abandoning the thread).
k, `getaddrinfo` doesn't have a timeout and `getaddrinfo_a` seems to have a segfault. Cool..
> Performance should also not really matter here since this doesn't happen that often and startup time for a thread vs. time of a DNS lookup is probably negligible.
I was thinking more about context switching. Isn't this used for `OpenNetworkConne
...
💬 sipa commented on issue "[Linux] Add wayland support":
(https://github.com/bitcoin/bitcoin/issues/19950#issuecomment-1536232647)
Is it the case that our `bitcion-qt` release binaries don't work on Wayland at all? If so, that should be fixed, I think. Many distributions are increasingly migrating away from X.
(https://github.com/bitcoin/bitcoin/issues/19950#issuecomment-1536232647)
Is it the case that our `bitcion-qt` release binaries don't work on Wayland at all? If so, that should be fixed, I think. Many distributions are increasingly migrating away from X.
👍 stickies-v approved a pull request: "test: Treat `bitcoin-wallet` binary in the same way as others"
(https://github.com/bitcoin/bitcoin/pull/27554#pullrequestreview-1414754734)
re-ACK f6d7636be4eb0b19878428906dd5e394df7d07a2
(https://github.com/bitcoin/bitcoin/pull/27554#pullrequestreview-1414754734)
re-ACK f6d7636be4eb0b19878428906dd5e394df7d07a2
💬 hebasto commented on issue "[Linux] Add wayland support":
(https://github.com/bitcoin/bitcoin/issues/19950#issuecomment-1536240176)
> Is it the case that our `bitcion-qt` release binaries don't work on Wayland at all? If so, that should be fixed, I think. Many distributions are increasingly migrating away from X.
Unfortunately, https://github.com/bitcoin/bitcoin/pull/22708 did not find enough support. The main concerns [were](https://github.com/bitcoin/bitcoin/pull/22708#issuecomment-1100861599):
> this change introduces a second Linux graphics backend to support, along with more packages in depends, new runtime dependen
...
(https://github.com/bitcoin/bitcoin/issues/19950#issuecomment-1536240176)
> Is it the case that our `bitcion-qt` release binaries don't work on Wayland at all? If so, that should be fixed, I think. Many distributions are increasingly migrating away from X.
Unfortunately, https://github.com/bitcoin/bitcoin/pull/22708 did not find enough support. The main concerns [were](https://github.com/bitcoin/bitcoin/pull/22708#issuecomment-1100861599):
> this change introduces a second Linux graphics backend to support, along with more packages in depends, new runtime dependen
...
💬 dergoegge commented on pull request "net: call getaddrinfo() in detachable thread to prevent stalling":
(https://github.com/bitcoin/bitcoin/pull/27557#discussion_r1186077225)
> k, getaddrinfo doesn't have a timeout and getaddrinfo_a seems to have a segfault. Cool..
Yea one would think that making DNS requests in c++ would be a solved problem.
Anyway, I'm also not sure about the approach here. The original issue was that we would block indefinitely while joining the DNS thread on shutdown (only happens if the user's system is broken I think?). I suggested to detach the dns thread if it doesn't join in a reasonable amount of time (would be fine, the OS will clean
...
(https://github.com/bitcoin/bitcoin/pull/27557#discussion_r1186077225)
> k, getaddrinfo doesn't have a timeout and getaddrinfo_a seems to have a segfault. Cool..
Yea one would think that making DNS requests in c++ would be a solved problem.
Anyway, I'm also not sure about the approach here. The original issue was that we would block indefinitely while joining the DNS thread on shutdown (only happens if the user's system is broken I think?). I suggested to detach the dns thread if it doesn't join in a reasonable amount of time (would be fine, the OS will clean
...
💬 RandyMcMillan commented on pull request "util: Use steady clock instead of system clock to measure durations":
(https://github.com/bitcoin/bitcoin/pull/27405#issuecomment-1536244850)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/27405#issuecomment-1536244850)
Concept ACK
💬 fanquake commented on issue "TSAN: lock-order-inversion (potential deadlock) in ZapSelectTx test":
(https://github.com/bitcoin/bitcoin/issues/27582#issuecomment-1536245439)
> Is this reproducible?
Has happened 2/2 runs so far.
(https://github.com/bitcoin/bitcoin/issues/27582#issuecomment-1536245439)
> Is this reproducible?
Has happened 2/2 runs so far.
🚀 fanquake merged a pull request: "test, init: perturb file to ensure failure instead of only deleting them"
(https://github.com/bitcoin/bitcoin/pull/26653)
(https://github.com/bitcoin/bitcoin/pull/26653)
💬 dergoegge commented on pull request "net: Continuous ASMap health check":
(https://github.com/bitcoin/bitcoin/pull/27581#discussion_r1186081315)
I think this belongs in `net.cpp` and shouldn't use the block height.
(https://github.com/bitcoin/bitcoin/pull/27581#discussion_r1186081315)
I think this belongs in `net.cpp` and shouldn't use the block height.
💬 dergoegge commented on pull request "net: Continuous ASMap health check":
(https://github.com/bitcoin/bitcoin/pull/27581#discussion_r1186082093)
(ignore if you were going to change this once out of draft anyway)
(https://github.com/bitcoin/bitcoin/pull/27581#discussion_r1186082093)
(ignore if you were going to change this once out of draft anyway)
👍 stickies-v approved a pull request: "util: improve FindByte() performance"
(https://github.com/bitcoin/bitcoin/pull/19690#pullrequestreview-1414773712)
re-ACK 72efc26439da9a1344a19569fb0cab01f82ae7d1
Verified that the only difference is to include `<util/fs.h>` instead of `<fs.h>` (from https://github.com/bitcoin/bitcoin/pull/27254/commits/00e9b97f37e0bdf4c647236838c10b68b7ad5be3)
```range-diff
% git range-diff HEAD~2 0fe832c4a4b2049fdf967bca375468d5ac285563 HEAD
1: 5842d92c8 ! 1: 604df63f6 [bench] add streams findbyte
@@ src/bench/streams_findbyte.cpp (new)
+
+#include <bench/bench.h>
+
-+#include <fs.h>
...
(https://github.com/bitcoin/bitcoin/pull/19690#pullrequestreview-1414773712)
re-ACK 72efc26439da9a1344a19569fb0cab01f82ae7d1
Verified that the only difference is to include `<util/fs.h>` instead of `<fs.h>` (from https://github.com/bitcoin/bitcoin/pull/27254/commits/00e9b97f37e0bdf4c647236838c10b68b7ad5be3)
```range-diff
% git range-diff HEAD~2 0fe832c4a4b2049fdf967bca375468d5ac285563 HEAD
1: 5842d92c8 ! 1: 604df63f6 [bench] add streams findbyte
@@ src/bench/streams_findbyte.cpp (new)
+
+#include <bench/bench.h>
+
-+#include <fs.h>
...
⚠️ Sjors opened an issue: "psbt: set global_xpubs (at least for multisig descriptors)"
(https://github.com/bitcoin/bitcoin/issues/27583)
### Please describe the feature you'd like to see added.
The `walletcreatefundedpsbt`, `walletprocesspsbt` and `send`* RPC, as well as the send dialog* in the GUI should populate the `PSBT_GLOBAL_XPUB` field (defined in [BIP 174](https://github.com/bitcoin/bips/blob/master/bip-0174.mediawiki)).
At least when used in a multisig context, e.g. when spending from a `multi()` descriptor.
The Ledger Bitcoin app enforces this as of version 2.1.1., see https://github.com/bitcoin-core/HWI/issues/6
...
(https://github.com/bitcoin/bitcoin/issues/27583)
### Please describe the feature you'd like to see added.
The `walletcreatefundedpsbt`, `walletprocesspsbt` and `send`* RPC, as well as the send dialog* in the GUI should populate the `PSBT_GLOBAL_XPUB` field (defined in [BIP 174](https://github.com/bitcoin/bips/blob/master/bip-0174.mediawiki)).
At least when used in a multisig context, e.g. when spending from a `multi()` descriptor.
The Ledger Bitcoin app enforces this as of version 2.1.1., see https://github.com/bitcoin-core/HWI/issues/6
...
💬 MarcoFalke commented on issue "TSAN: lock-order-inversion (potential deadlock) in ZapSelectTx test":
(https://github.com/bitcoin/bitcoin/issues/27582#issuecomment-1536249866)
Ok, then it would be nice to try outside the CI env, starting with the same configure flags
(https://github.com/bitcoin/bitcoin/issues/27582#issuecomment-1536249866)
Ok, then it would be nice to try outside the CI env, starting with the same configure flags