🤔 enirox001 reviewed a pull request: "test: fix p2p_leak_tx.py"
(https://github.com/bitcoin/bitcoin/pull/33121#pullrequestreview-3148172896)
Concept ACK ab43e0b on fixing the test flakiness.
I took a shot at making `test_notfound_on_unannounced_tx()` more deterministic using mocktime. Feel free to use it if helpful:
```diff
diff --git a/test/functional/p2p_leak_tx.py b/test/functional/p2p_leak_tx.py
index 3fd324a029..f50993e932 100755
--- a/test/functional/p2p_leak_tx.py
+++ b/test/functional/p2p_leak_tx.py
@@ -11,6 +11,7 @@ from test_framework.util import (
assert_equal,
)
from test_framework.wallet import MiniW
...
(https://github.com/bitcoin/bitcoin/pull/33121#pullrequestreview-3148172896)
Concept ACK ab43e0b on fixing the test flakiness.
I took a shot at making `test_notfound_on_unannounced_tx()` more deterministic using mocktime. Feel free to use it if helpful:
```diff
diff --git a/test/functional/p2p_leak_tx.py b/test/functional/p2p_leak_tx.py
index 3fd324a029..f50993e932 100755
--- a/test/functional/p2p_leak_tx.py
+++ b/test/functional/p2p_leak_tx.py
@@ -11,6 +11,7 @@ from test_framework.util import (
assert_equal,
)
from test_framework.wallet import MiniW
...
🚀 fanquake merged a pull request: "threading: remove ancient CRITICAL_SECTION macros"
(https://github.com/bitcoin/bitcoin/pull/32592)
(https://github.com/bitcoin/bitcoin/pull/32592)
💬 hebasto commented on pull request "Update libmultiprocess subtree to fix build issues":
(https://github.com/bitcoin/bitcoin/pull/33241#issuecomment-3217208986)
> > failure looks real? The unit test should normally pass in a few milliseconds, so taking 40 minutes seems odd?
>
> I reproduced this by repeatedly calling `mptest`. Took about 50'000 tries until it hung indefinitely.
Is this a newly introduced issue, or does it also exist on the master branch?
(https://github.com/bitcoin/bitcoin/pull/33241#issuecomment-3217208986)
> > failure looks real? The unit test should normally pass in a few milliseconds, so taking 40 minutes seems odd?
>
> I reproduced this by repeatedly calling `mptest`. Took about 50'000 tries until it hung indefinitely.
Is this a newly introduced issue, or does it also exist on the master branch?
💬 ryanofsky commented on issue "intermittent timeout in mptest unit test":
(https://github.com/bitcoin/bitcoin/issues/33244#issuecomment-3217234389)
As noted https://github.com/bitcoin/bitcoin/pull/33241#issuecomment-3216999952, I'm pretty sure this is caused by https://github.com/bitcoin-core/libmultiprocess/issues/189. It's possible to reproduce the issue locally by just running mptest in a loop thousands of times until it locks up.
https://github.com/bitcoin-core/libmultiprocess/issues/189 happens because the new "disconnecting and blocking" test introduced in https://github.com/bitcoin-core/libmultiprocess/issues/160 tests for for probl
...
(https://github.com/bitcoin/bitcoin/issues/33244#issuecomment-3217234389)
As noted https://github.com/bitcoin/bitcoin/pull/33241#issuecomment-3216999952, I'm pretty sure this is caused by https://github.com/bitcoin-core/libmultiprocess/issues/189. It's possible to reproduce the issue locally by just running mptest in a loop thousands of times until it locks up.
https://github.com/bitcoin-core/libmultiprocess/issues/189 happens because the new "disconnecting and blocking" test introduced in https://github.com/bitcoin-core/libmultiprocess/issues/160 tests for for probl
...
💬 ryanofsky commented on pull request "Update libmultiprocess subtree to fix build issues":
(https://github.com/bitcoin/bitcoin/pull/33241#issuecomment-3217235762)
> Is this a newly introduced issue, or does it also exist on the master branch?
I'm not sure if it's been seen in master but I would expect it happens in master. Seems unlikely any change in this PR would cause it, see #33244
(https://github.com/bitcoin/bitcoin/pull/33241#issuecomment-3217235762)
> Is this a newly introduced issue, or does it also exist on the master branch?
I'm not sure if it's been seen in master but I would expect it happens in master. Seems unlikely any change in this PR would cause it, see #33244
🤔 naiyoma reviewed a pull request: "test: add option to skip large re-org test in feature_block"
(https://github.com/bitcoin/bitcoin/pull/33003#pullrequestreview-3148930871)
Tested, and `--skipreorg` successfully reduces test runtime from `60s to 35s`.
I’m not very sure about this approach — splitting was also suggested here https://github.com/bitcoin/bitcoin/issues/16613#issuecomment-548952103 I think it’s a better long-term solution and could be done in a follow-up (not sure)
(https://github.com/bitcoin/bitcoin/pull/33003#pullrequestreview-3148930871)
Tested, and `--skipreorg` successfully reduces test runtime from `60s to 35s`.
I’m not very sure about this approach — splitting was also suggested here https://github.com/bitcoin/bitcoin/issues/16613#issuecomment-548952103 I think it’s a better long-term solution and could be done in a follow-up (not sure)
📝 151henry151 opened a pull request: "build: Remove deprecated CMAKE_SKIP_BUILD_RPATH and SKIP_BUILD_RPATH settings"
(https://github.com/bitcoin/bitcoin/pull/33247)
(https://github.com/bitcoin/bitcoin/pull/33247)
📝 151henry151 opened a pull request: "doc: Replace FIXME placeholders with correct libblkmaker version numbers"
(https://github.com/bitcoin/bitcoin/pull/33248)
## Summary
This PR replaces FIXME placeholders in the release notes with the correct libblkmaker version numbers.
## Changes
- Replace FIXME placeholders in release notes with correct version numbers:
- `FIXME` → `0.4.2`
- `FIXME` → `0.5.1`
## Context
The release notes for version 0.10.4 contained FIXME placeholders for libblkmaker version numbers. These same version numbers are correctly specified in other release notes (e.g., 0.9.5, 0.10.0, 0.11.2) as 0.4.2 and 0.5.1.
## Testing
- [
...
(https://github.com/bitcoin/bitcoin/pull/33248)
## Summary
This PR replaces FIXME placeholders in the release notes with the correct libblkmaker version numbers.
## Changes
- Replace FIXME placeholders in release notes with correct version numbers:
- `FIXME` → `0.4.2`
- `FIXME` → `0.5.1`
## Context
The release notes for version 0.10.4 contained FIXME placeholders for libblkmaker version numbers. These same version numbers are correctly specified in other release notes (e.g., 0.9.5, 0.10.0, 0.11.2) as 0.4.2 and 0.5.1.
## Testing
- [
...
💬 151henry151 commented on pull request "doc: Replace FIXME placeholders with correct libblkmaker version numbers":
(https://github.com/bitcoin/bitcoin/pull/33248#issuecomment-3217461249)
I thought this might be a worthwhile first attempt at contributing to the bitcoin core project, but it looks like the pull request was closed automatically. Can someone help me understand why, or check this out to see if that is correct?
(https://github.com/bitcoin/bitcoin/pull/33248#issuecomment-3217461249)
I thought this might be a worthwhile first attempt at contributing to the bitcoin core project, but it looks like the pull request was closed automatically. Can someone help me understand why, or check this out to see if that is correct?
📝 andradepsa opened a pull request: "Bitcoin 2.0: The Continuity Protocol "
(https://github.com/bitcoin/bitcoin/pull/33249)
# 🪙 Bitcoin 2.0: The Continuity Protocol
## The Future of Bitcoin: Evolution or Extinction?
> The original Bitcoin protocol is a masterpiece of engineering.
> But it contains a silent flaw, programmed into its very DNA: the inevitability of generational wealth loss.
---
## ❗ The Inevitable Crisis: Bitcoin's Great Filter
The greatest long-term threat to Bitcoin's utility is not regulation, competition, or quantum computing. It is a simple, unavoidable demographic reality:
...
(https://github.com/bitcoin/bitcoin/pull/33249)
# 🪙 Bitcoin 2.0: The Continuity Protocol
## The Future of Bitcoin: Evolution or Extinction?
> The original Bitcoin protocol is a masterpiece of engineering.
> But it contains a silent flaw, programmed into its very DNA: the inevitability of generational wealth loss.
---
## ❗ The Inevitable Crisis: Bitcoin's Great Filter
The greatest long-term threat to Bitcoin's utility is not regulation, competition, or quantum computing. It is a simple, unavoidable demographic reality:
...
⚠️ w0xlt opened an issue: "`feature_bind_extra.py` test fails in `test_runner` if new nodes are added"
(https://github.com/bitcoin/bitcoin/issues/33250)
When new nodes are added to the `feature_bind_extra.py` test, it succeeds if run individually, but fails if run via `test_runner` with the error message:
```
TestFramework (ERROR): Unexpected exception
Traceback (most recent call last):
File "/home/node/test/functional/test_framework/test_framework.py", line 191, in main
self.setup()
File "/home/node/test/functional/test_framework/test_framework.py", line 349, in setup
self.setup_network()
File "/home/node/build/test/functional/fe
...
(https://github.com/bitcoin/bitcoin/issues/33250)
When new nodes are added to the `feature_bind_extra.py` test, it succeeds if run individually, but fails if run via `test_runner` with the error message:
```
TestFramework (ERROR): Unexpected exception
Traceback (most recent call last):
File "/home/node/test/functional/test_framework/test_framework.py", line 191, in main
self.setup()
File "/home/node/test/functional/test_framework/test_framework.py", line 349, in setup
self.setup_network()
File "/home/node/build/test/functional/fe
...
💬 w0xlt commented on pull request "net: Prevent node from binding to the same `CService`":
(https://github.com/bitcoin/bitcoin/pull/33231#issuecomment-3217858176)
Thank you all for the review.
I completely changed the approach.
While addressing the reviews, I realized that using `std::set` to avoid duplicates doesn't solve the problem. If the options `-bind=127.0.0.1:8333 -bind=127.0.0.1:8333=onion` are present, the node will crash anyway, as one option will be allocated in the `CConnman::Options::vBinds` vector and the other in `CConnman::Options::onion_binds`.
Therefore, even after deduplicating each vector, it is still necessary to check for con
...
(https://github.com/bitcoin/bitcoin/pull/33231#issuecomment-3217858176)
Thank you all for the review.
I completely changed the approach.
While addressing the reviews, I realized that using `std::set` to avoid duplicates doesn't solve the problem. If the options `-bind=127.0.0.1:8333 -bind=127.0.0.1:8333=onion` are present, the node will crash anyway, as one option will be allocated in the `CConnman::Options::vBinds` vector and the other in `CConnman::Options::onion_binds`.
Therefore, even after deduplicating each vector, it is still necessary to check for con
...
🤔 w0xlt reviewed a pull request: "doc: unify `datacarriersize` warning with release notes"
(https://github.com/bitcoin/bitcoin/pull/33224#pullrequestreview-3149210453)
ACK https://github.com/bitcoin/bitcoin/pull/33224/commits/2885bd0e1c4fc863a7f28ff0fd353f5cffb03442
Will the `src/qt/bitcoinstrings.cpp` file be changed in the GUI repository?
(https://github.com/bitcoin/bitcoin/pull/33224#pullrequestreview-3149210453)
ACK https://github.com/bitcoin/bitcoin/pull/33224/commits/2885bd0e1c4fc863a7f28ff0fd353f5cffb03442
Will the `src/qt/bitcoinstrings.cpp` file be changed in the GUI repository?
🤔 hebasto reviewed a pull request: "build: Remove deprecated CMAKE_SKIP_BUILD_RPATH and SKIP_BUILD_RPATH settings"
(https://github.com/bitcoin/bitcoin/pull/33247#pullrequestreview-3149213360)
> This is my first attempt at contributing to bitcoin core so please, provide me any feedback that you might have. Thanks!
Thank you for your interest in contributing to this project!
The changes you are referring to as completed, i.e. "reordering Guix script commands to perform binary checks after the installation step", have not actually been done yet. See [`contrib/guix/libexec/build.sh`](https://github.com/bitcoin/bitcoin/blob/master/contrib/guix/libexec/build.sh), from line 251 onward
...
(https://github.com/bitcoin/bitcoin/pull/33247#pullrequestreview-3149213360)
> This is my first attempt at contributing to bitcoin core so please, provide me any feedback that you might have. Thanks!
Thank you for your interest in contributing to this project!
The changes you are referring to as completed, i.e. "reordering Guix script commands to perform binary checks after the installation step", have not actually been done yet. See [`contrib/guix/libexec/build.sh`](https://github.com/bitcoin/bitcoin/blob/master/contrib/guix/libexec/build.sh), from line 251 onward
...
👍 hebasto approved a pull request: "Update libmultiprocess subtree to fix build issues"
(https://github.com/bitcoin/bitcoin/pull/33241#pullrequestreview-3149218774)
ACK 323b3fd27283282f2f8eb1096f56f23d8230f2d6, I've reproduced the subtree update locally. The two issues noted in this PR are unrelated to its changes and can be addressed separately.
(https://github.com/bitcoin/bitcoin/pull/33241#pullrequestreview-3149218774)
ACK 323b3fd27283282f2f8eb1096f56f23d8230f2d6, I've reproduced the subtree update locally. The two issues noted in this PR are unrelated to its changes and can be addressed separately.
💬 maflcko commented on issue "`feature_bind_extra.py` test fails in `test_runner` if new nodes are added":
(https://github.com/bitcoin/bitcoin/issues/33250#issuecomment-3217960830)
There is a limit on the number of nodes `MAX_NODES` and ports that can be used per test. You'll have to stay under it, or increase the limit.
(https://github.com/bitcoin/bitcoin/issues/33250#issuecomment-3217960830)
There is a limit on the number of nodes `MAX_NODES` and ports that can be used per test. You'll have to stay under it, or increase the limit.
💬 IdotMaster1 commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-3217994772)
Concept NACK.
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-3217994772)
Concept NACK.
💬 hebasto commented on pull request "doc: Replace FIXME placeholders with correct libblkmaker version numbers":
(https://github.com/bitcoin/bitcoin/pull/33248#issuecomment-3218035869)
> ... it looks like the pull request was closed automatically. Can someone help me understand why...
See https://github.com/bitcoin/bitcoin/pull/33248#issuecomment-3217454645.
(https://github.com/bitcoin/bitcoin/pull/33248#issuecomment-3218035869)
> ... it looks like the pull request was closed automatically. Can someone help me understand why...
See https://github.com/bitcoin/bitcoin/pull/33248#issuecomment-3217454645.
💬 hebasto commented on pull request "doc: unify `datacarriersize` warning with release notes":
(https://github.com/bitcoin/bitcoin/pull/33224#issuecomment-3218109881)
> cc @hebasto; given this would change translations after translation string freeze.
The translation workflow on Transifex includes marking translated strings as "Reviewed", which locks them from further changes. Not every translation team uses this feature, but those who do rely on it. Unfortunately, the "opensource" plan used by the [Bitcoin organization](https://explore.transifex.com/bitcoin/) on Transifex has very limited functionality, and it is not guaranteed that updating the translati
...
(https://github.com/bitcoin/bitcoin/pull/33224#issuecomment-3218109881)
> cc @hebasto; given this would change translations after translation string freeze.
The translation workflow on Transifex includes marking translated strings as "Reviewed", which locks them from further changes. Not every translation team uses this feature, but those who do rely on it. Unfortunately, the "opensource" plan used by the [Bitcoin organization](https://explore.transifex.com/bitcoin/) on Transifex has very limited functionality, and it is not guaranteed that updating the translati
...
💬 hebasto commented on pull request "doc: unify `datacarriersize` warning with release notes":
(https://github.com/bitcoin/bitcoin/pull/33224#issuecomment-3218111653)
@w0xlt
> Will the `src/qt/bitcoinstrings.cpp` file be changed in the GUI repository?
It makes no difference.
(https://github.com/bitcoin/bitcoin/pull/33224#issuecomment-3218111653)
@w0xlt
> Will the `src/qt/bitcoinstrings.cpp` file be changed in the GUI repository?
It makes no difference.