β
fanquake closed a pull request: "BIP324: Enable v2 P2P encrypted transport"
(https://github.com/bitcoin/bitcoin/pull/24545)
(https://github.com/bitcoin/bitcoin/pull/24545)
π¬ 1ma commented on pull request "Allow accepting non-standard transactions on mainnet":
(https://github.com/bitcoin/bitcoin/pull/27578#issuecomment-1537136352)
Let it be known for the record that those of us who've been playing with https://github.com/supertestnet/breaker-of-jpegs are very much in favor of enacting a new standardness rule, not removing them. @benthecarman knows this, and citing that project as an example denotes bad faith.
If anything node runners need more control for filtering whatever they may want, such as this new kind of transactions who are abusing the "Lady Gaga Video Attack Vector" by exploiting a particular sequence of mea
...
(https://github.com/bitcoin/bitcoin/pull/27578#issuecomment-1537136352)
Let it be known for the record that those of us who've been playing with https://github.com/supertestnet/breaker-of-jpegs are very much in favor of enacting a new standardness rule, not removing them. @benthecarman knows this, and citing that project as an example denotes bad faith.
If anything node runners need more control for filtering whatever they may want, such as this new kind of transactions who are abusing the "Lady Gaga Video Attack Vector" by exploiting a particular sequence of mea
...
π¬ BullishNode commented on pull request "Allow accepting non-standard transactions on mainnet":
(https://github.com/bitcoin/bitcoin/pull/27578#issuecomment-1537140748)
Strong NACK. Standardness rules protect the network from ddos and spam. The fact that you have to email a miner is a good thing: it's rate limiting.
(https://github.com/bitcoin/bitcoin/pull/27578#issuecomment-1537140748)
Strong NACK. Standardness rules protect the network from ddos and spam. The fact that you have to email a miner is a good thing: it's rate limiting.
π¬ jamesob commented on pull request "assumeutxo: keep cache when flushing snapshot (#17487 followup)":
(https://github.com/bitcoin/bitcoin/pull/27008#issuecomment-1537140998)
Actually this _is_ broken at the moment, since although we don't flush after snapshot load, we do flush (and not sync) in the MaybeRebalanceCaches() -> FlushStateToDisk() call right after activating the chainstate snapshot. Will close this and think about how to rework it.
(https://github.com/bitcoin/bitcoin/pull/27008#issuecomment-1537140998)
Actually this _is_ broken at the moment, since although we don't flush after snapshot load, we do flush (and not sync) in the MaybeRebalanceCaches() -> FlushStateToDisk() call right after activating the chainstate snapshot. Will close this and think about how to rework it.
β
jamesob closed a pull request: "assumeutxo: keep cache when flushing snapshot (#17487 followup)"
(https://github.com/bitcoin/bitcoin/pull/27008)
(https://github.com/bitcoin/bitcoin/pull/27008)
π¬ jamesob commented on pull request "assumeutxo":
(https://github.com/bitcoin/bitcoin/pull/15606#issuecomment-1537141951)
Thanks for testing @fanquake and thanks for the torrent @Sjors. I've pushed some fixes, rebased, and CI is green.
> Runing through your steps above, everything seems to be working, except that my mempool was empty until I stopped and restarted bitcoind?
Fixed in https://github.com/bitcoin/bitcoin/pull/15606/commits/7cbbf2adad5a7760d6db389790062fee959943a6 - forgot to swap the `m_mempool` references on snapshot activation. I've verified that the mempool now starts to populate during backgro
...
(https://github.com/bitcoin/bitcoin/pull/15606#issuecomment-1537141951)
Thanks for testing @fanquake and thanks for the torrent @Sjors. I've pushed some fixes, rebased, and CI is green.
> Runing through your steps above, everything seems to be working, except that my mempool was empty until I stopped and restarted bitcoind?
Fixed in https://github.com/bitcoin/bitcoin/pull/15606/commits/7cbbf2adad5a7760d6db389790062fee959943a6 - forgot to swap the `m_mempool` references on snapshot activation. I've verified that the mempool now starts to populate during backgro
...
π¬ jamesob commented on pull request "assumeutxo":
(https://github.com/bitcoin/bitcoin/pull/15606#issuecomment-1537145078)
The history of the pull request is getitng unwieldy; the 400+ comments are now creating a situation where comments (like the testing instructions) posted a few days ago are buried under a minute of clicking "load more."
I've added the testing instructions to the PR description, but should I consider opening a fresh PR? Do we have any process for dealing with this Github limitation?
(https://github.com/bitcoin/bitcoin/pull/15606#issuecomment-1537145078)
The history of the pull request is getitng unwieldy; the 400+ comments are now creating a situation where comments (like the testing instructions) posted a few days ago are buried under a minute of clicking "load more."
I've added the testing instructions to the PR description, but should I consider opening a fresh PR? Do we have any process for dealing with this Github limitation?
π¬ fanquake commented on pull request "assumeutxo":
(https://github.com/bitcoin/bitcoin/pull/15606#issuecomment-1537145783)
@jamesob I would be in favour of you opening a new PR (carrying over relevant current context into the description, and pointing back to anything else relevant). I ran into the same annoyance today, when trying to leave my most recent comment. Having to expand 400+ comments check recent discussion/context, is not great.
(https://github.com/bitcoin/bitcoin/pull/15606#issuecomment-1537145783)
@jamesob I would be in favour of you opening a new PR (carrying over relevant current context into the description, and pointing back to anything else relevant). I ran into the same annoyance today, when trying to leave my most recent comment. Having to expand 400+ comments check recent discussion/context, is not great.
π¬ jamesob commented on pull request "assumeutxo":
(https://github.com/bitcoin/bitcoin/pull/15606#discussion_r1186700559)
Done!
(https://github.com/bitcoin/bitcoin/pull/15606#discussion_r1186700559)
Done!
π¬ ishaanam commented on pull request "rpc: add `descriptorprocesspsbt` rpc":
(https://github.com/bitcoin/bitcoin/pull/25796#discussion_r1186711555)
Thanks, I've fixed it.
(https://github.com/bitcoin/bitcoin/pull/25796#discussion_r1186711555)
Thanks, I've fixed it.
π¬ Sjors commented on pull request "assumeutxo":
(https://github.com/bitcoin/bitcoin/pull/15606#issuecomment-1537181954)
I also ended up with an empty mempool with 2dd8ea0f2cf07486d1a44263c6976b1ecac563b9 on Ubuntu 23.04, with pruning enabled. Will try again once you believe that's fixed. When I (cleanly) shut down the node and restarted it, I noticed this log message: `[snapshot] computing UTXO stats for background chainstate to validate snapshot - this could take a few minutes`, followed by `[snapshot] snapshot beginning at 00000000000000000001f3fa1b4c03c877740778f56b0d5456b18dd88f7f695e has been fully validated
...
(https://github.com/bitcoin/bitcoin/pull/15606#issuecomment-1537181954)
I also ended up with an empty mempool with 2dd8ea0f2cf07486d1a44263c6976b1ecac563b9 on Ubuntu 23.04, with pruning enabled. Will try again once you believe that's fixed. When I (cleanly) shut down the node and restarted it, I noticed this log message: `[snapshot] computing UTXO stats for background chainstate to validate snapshot - this could take a few minutes`, followed by `[snapshot] snapshot beginning at 00000000000000000001f3fa1b4c03c877740778f56b0d5456b18dd88f7f695e has been fully validated
...
π¬ Sjors commented on pull request "assumeutxo":
(https://github.com/bitcoin/bitcoin/pull/15606#issuecomment-1537182198)
It's gotten to the point where even refreshing the page doesn't always get you the latest comments. +1 for opening a new one.
Meanwhile I'll recompile and do an unpruned with assumevalid=0.
(https://github.com/bitcoin/bitcoin/pull/15606#issuecomment-1537182198)
It's gotten to the point where even refreshing the page doesn't always get you the latest comments. +1 for opening a new one.
Meanwhile I'll recompile and do an unpruned with assumevalid=0.
π¬ TheCharlatan commented on pull request "refactor: Move chain constants to the util library":
(https://github.com/bitcoin/bitcoin/pull/27491#issuecomment-1537182312)
Rebased dd95e2a3353b5ded87d1d5408a51bf461f1f90b4 -> 95744f9cf1f143b6449a5046a914557b3886e3a2 ([kernelChainType_3](https://github.com/TheCharlatan/bitcoin/tree/kernelChainType_3) -> [kernelChainType_4](https://github.com/TheCharlatan/bitcoin/tree/kernelChainType_4), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernelChainType_3..kernelChainType_4))
* Fixed conflict with #17860
(https://github.com/bitcoin/bitcoin/pull/27491#issuecomment-1537182312)
Rebased dd95e2a3353b5ded87d1d5408a51bf461f1f90b4 -> 95744f9cf1f143b6449a5046a914557b3886e3a2 ([kernelChainType_3](https://github.com/TheCharlatan/bitcoin/tree/kernelChainType_3) -> [kernelChainType_4](https://github.com/TheCharlatan/bitcoin/tree/kernelChainType_4), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernelChainType_3..kernelChainType_4))
* Fixed conflict with #17860
π¬ Sjors commented on pull request "assumeutxo":
(https://github.com/bitcoin/bitcoin/pull/15606#issuecomment-1537183168)
On Ubuntu 23.04 (gcc 12.2.0) I get a bunch of these warnings, that I don't get on master:
```
In file included from ./wallet/wallet.h:10,
from wallet/dump.cpp:10:
./interfaces/chain.h:274:22: warning: βvirtual void interfaces::Chain::Notifications::blockConnected(const interfaces::BlockInfo&)β was hidden [-Woverloaded-virtual]
274 | virtual void blockConnected(const BlockInfo& block) {}
```
(https://github.com/bitcoin/bitcoin/pull/15606#issuecomment-1537183168)
On Ubuntu 23.04 (gcc 12.2.0) I get a bunch of these warnings, that I don't get on master:
```
In file included from ./wallet/wallet.h:10,
from wallet/dump.cpp:10:
./interfaces/chain.h:274:22: warning: βvirtual void interfaces::Chain::Notifications::blockConnected(const interfaces::BlockInfo&)β was hidden [-Woverloaded-virtual]
274 | virtual void blockConnected(const BlockInfo& block) {}
```
π¬ TheBlueMatt commented on pull request "Allow accepting non-standard transactions on mainnet":
(https://github.com/bitcoin/bitcoin/pull/27578#issuecomment-1537191167)
> "Mempool divergence" is a non-issue, just a testament of the decentralized nature of the Bitcoin network. Any concerted efforts at homogenizing them across all nodes can only be understood as unwarranted overreach by bad actors.
This is inaccurate - there are a whole host of issues across the stack with large mempool divergence. From compact block relay losing their efficiency, to feerate estimates getting thrown off, to transaction validation caches becoming less efficient leading to block
...
(https://github.com/bitcoin/bitcoin/pull/27578#issuecomment-1537191167)
> "Mempool divergence" is a non-issue, just a testament of the decentralized nature of the Bitcoin network. Any concerted efforts at homogenizing them across all nodes can only be understood as unwarranted overreach by bad actors.
This is inaccurate - there are a whole host of issues across the stack with large mempool divergence. From compact block relay losing their efficiency, to feerate estimates getting thrown off, to transaction validation caches becoming less efficient leading to block
...
π¬ hebasto commented on pull request "Enable HW-accelerated implementations of SHA256 for MSVC builds":
(https://github.com/bitcoin/bitcoin/pull/24773#issuecomment-1537197977)
I had a chance to benchmark on a Windows laptop equipped with Intel Core i9 -12950HX with the `sha_ni` flag set.
No significant difference has been observed. Going to close this PR.
(https://github.com/bitcoin/bitcoin/pull/24773#issuecomment-1537197977)
I had a chance to benchmark on a Windows laptop equipped with Intel Core i9 -12950HX with the `sha_ni` flag set.
No significant difference has been observed. Going to close this PR.
π¬ hebasto commented on pull request "Enable HW-accelerated implementations of SHA256 for MSVC builds":
(https://github.com/bitcoin/bitcoin/pull/24773#discussion_r1186731838)
> Does MSVC not define `__x86_64__` or `__amd64__` or `__i386__` ?
No, it does not. Easy to verify with the following diff:
```diff
--- a/src/compat/cpuid.h
+++ b/src/compat/cpuid.h
@@ -6,6 +6,7 @@
#define BITCOIN_COMPAT_CPUID_H
#if defined(__x86_64__) || defined(__amd64__) || defined(__i386__)
+#error
#define HAVE_GETCPUID
#include <cpuid.h>
```
(https://github.com/bitcoin/bitcoin/pull/24773#discussion_r1186731838)
> Does MSVC not define `__x86_64__` or `__amd64__` or `__i386__` ?
No, it does not. Easy to verify with the following diff:
```diff
--- a/src/compat/cpuid.h
+++ b/src/compat/cpuid.h
@@ -6,6 +6,7 @@
#define BITCOIN_COMPAT_CPUID_H
#if defined(__x86_64__) || defined(__amd64__) || defined(__i386__)
+#error
#define HAVE_GETCPUID
#include <cpuid.h>
```
π¬ hebasto commented on pull request "Enable HW-accelerated implementations of SHA256 for MSVC builds":
(https://github.com/bitcoin/bitcoin/pull/24773#discussion_r1186733180)
> Why does the existing asm function not work?
From https://learn.microsoft.com/en-us/cpp/assembler/inline/inline-assembler:
> Inline assembly is not supported on the ARM and x64 processors.
---
> What is the specific error? Does it not like the inline asm? Or does it just have a problem with the opcode?
With the diff as follows:
```diff
--- a/src/crypto/sha256.cpp
+++ b/src/crypto/sha256.cpp
@@ -567,7 +567,6 @@ bool SelfTest() {
return true;
}
-#if defined(USE_ASM) &
...
(https://github.com/bitcoin/bitcoin/pull/24773#discussion_r1186733180)
> Why does the existing asm function not work?
From https://learn.microsoft.com/en-us/cpp/assembler/inline/inline-assembler:
> Inline assembly is not supported on the ARM and x64 processors.
---
> What is the specific error? Does it not like the inline asm? Or does it just have a problem with the opcode?
With the diff as follows:
```diff
--- a/src/crypto/sha256.cpp
+++ b/src/crypto/sha256.cpp
@@ -567,7 +567,6 @@ bool SelfTest() {
return true;
}
-#if defined(USE_ASM) &
...
π st3b1t opened a pull request: "rpc: append rpcauth.py hash in config and show pass"
(https://github.com/bitcoin/bitcoin/pull/27588)
Small change improves security during the rpcauth directive generation procedure. In this way you can hang the configuration line directly inside the bitcoin.conf file and at the same time show the new password to the user, but without writing it to disk usage:
append generated hash in config:
```bash
rpcauth.py >> bitcoin.conf
```
output writeless is only:
```bash
Your password:
xxxxx...
```
(https://github.com/bitcoin/bitcoin/pull/27588)
Small change improves security during the rpcauth directive generation procedure. In this way you can hang the configuration line directly inside the bitcoin.conf file and at the same time show the new password to the user, but without writing it to disk usage:
append generated hash in config:
```bash
rpcauth.py >> bitcoin.conf
```
output writeless is only:
```bash
Your password:
xxxxx...
```
π¬ 1ma commented on pull request "Allow accepting non-standard transactions on mainnet":
(https://github.com/bitcoin/bitcoin/pull/27578#issuecomment-1537216325)
I did not claim mempool diversity is "good", just a "non-issue", i.e. simply part of how Bitcoin works.
Trying to mitigate these quirks by imposing a uniform mempool on all node runners would rob them of an important part of their sovereignty, therefore it'd be much worse than the quirks.
(https://github.com/bitcoin/bitcoin/pull/27578#issuecomment-1537216325)
I did not claim mempool diversity is "good", just a "non-issue", i.e. simply part of how Bitcoin works.
Trying to mitigate these quirks by imposing a uniform mempool on all node runners would rob them of an important part of their sovereignty, therefore it'd be much worse than the quirks.