💬 hebasto commented on pull request "guix: consolidate to glibc 2.27 for Linux builds":
(https://github.com/bitcoin/bitcoin/pull/27029#issuecomment-1429668279)
Guix builds:
```
851f4116c395a6b89475871a9763a111abad3951424276135931d80b1c09615e guix-build-d5d4b75840b4/output/aarch64-linux-gnu/SHA256SUMS.part
bb9d3867b8f7dae5f9f8c326aa1b0e9fa28964659b3c7187e743152044c9dbe8 guix-build-d5d4b75840b4/output/aarch64-linux-gnu/bitcoin-d5d4b75840b4-aarch64-linux-gnu-debug.tar.gz
e4c0ab4883993362aea104ea776102a5d5beaa39a22e9aa22fd27a81d36415dc guix-build-d5d4b75840b4/output/aarch64-linux-gnu/bitcoin-d5d4b75840b4-aarch64-linux-gnu.tar.gz
01b239015610aea0c47
...
(https://github.com/bitcoin/bitcoin/pull/27029#issuecomment-1429668279)
Guix builds:
```
851f4116c395a6b89475871a9763a111abad3951424276135931d80b1c09615e guix-build-d5d4b75840b4/output/aarch64-linux-gnu/SHA256SUMS.part
bb9d3867b8f7dae5f9f8c326aa1b0e9fa28964659b3c7187e743152044c9dbe8 guix-build-d5d4b75840b4/output/aarch64-linux-gnu/bitcoin-d5d4b75840b4-aarch64-linux-gnu-debug.tar.gz
e4c0ab4883993362aea104ea776102a5d5beaa39a22e9aa22fd27a81d36415dc guix-build-d5d4b75840b4/output/aarch64-linux-gnu/bitcoin-d5d4b75840b4-aarch64-linux-gnu.tar.gz
01b239015610aea0c47
...
💬 Ayms commented on issue "Allow several OP_RETURN in one tx and no limited size":
(https://github.com/bitcoin/bitcoin/issues/27043#issuecomment-1429678640)
@MarcoFalke super simple...
@Semisol as mentionned in the thread there are so many ways to store things that there is no rationale to impose a limit in OP_RETURN, this is cleary a handicap for bitcoin and future uses/evolutions, bitcoin can clearly do far better what ethereum and all the messy stuff around pretend to do but we need this change
(https://github.com/bitcoin/bitcoin/issues/27043#issuecomment-1429678640)
@MarcoFalke super simple...
@Semisol as mentionned in the thread there are so many ways to store things that there is no rationale to impose a limit in OP_RETURN, this is cleary a handicap for bitcoin and future uses/evolutions, bitcoin can clearly do far better what ethereum and all the messy stuff around pretend to do but we need this change
💬 furszy commented on pull request "refactor: wallet, remove global 'ArgsManager' dependency":
(https://github.com/bitcoin/bitcoin/pull/26889#discussion_r1105772605)
np, small miscommunication. Done now, all of them tackled now. Thanks!
(https://github.com/bitcoin/bitcoin/pull/26889#discussion_r1105772605)
np, small miscommunication. Done now, all of them tackled now. Thanks!
💬 Sjors commented on pull request "p2p, validation: Don't download witnesses for assumed-valid blocks when running in prune mode":
(https://github.com/bitcoin/bitcoin/pull/27050#issuecomment-1429695634)
Making witness skipping opt-in makes sense. In addition our GUI initial setup wizard could help the user with this (as it does with suggesting pruning).
(https://github.com/bitcoin/bitcoin/pull/27050#issuecomment-1429695634)
Making witness skipping opt-in makes sense. In addition our GUI initial setup wizard could help the user with this (as it does with suggesting pruning).
💬 furszy commented on pull request "refactor: wallet, remove global 'ArgsManager' dependency":
(https://github.com/bitcoin/bitcoin/pull/26889#issuecomment-1429700293)
Feedback tackled.
Moved remaining variables to signed type. Small [diff](https://github.com/bitcoin/bitcoin/compare/fafaa7a7e15a49da0214ce9546a42117c63e611f..d90b54b4aa18d60faa0075348fbb29a513558098).
(https://github.com/bitcoin/bitcoin/pull/26889#issuecomment-1429700293)
Feedback tackled.
Moved remaining variables to signed type. Small [diff](https://github.com/bitcoin/bitcoin/compare/fafaa7a7e15a49da0214ce9546a42117c63e611f..d90b54b4aa18d60faa0075348fbb29a513558098).
💬 vasild commented on pull request "build: Add CMake-based build system (1 of N)":
(https://github.com/bitcoin/bitcoin/pull/27060#discussion_r1105847171)
Do autotools really pick `/foo/bin/clang` (assuming `/foo/bin` is in `$PATH`)? I tried that but for me it picked `/usr/bin/c++` nevertheless, even though I had a `clang` executable in a directory earlier than `/usr/bin` in `$PATH`.
(https://github.com/bitcoin/bitcoin/pull/27060#discussion_r1105847171)
Do autotools really pick `/foo/bin/clang` (assuming `/foo/bin` is in `$PATH`)? I tried that but for me it picked `/usr/bin/c++` nevertheless, even though I had a `clang` executable in a directory earlier than `/usr/bin` in `$PATH`.
💬 vasild commented on pull request "build: Add CMake-based build system (1 of N)":
(https://github.com/bitcoin/bitcoin/pull/27060#discussion_r1105862197)
nit:
```suggestion
message(WARNING "${warning}")
```
https://cmake.org/cmake/help/latest/command/message.html
(https://github.com/bitcoin/bitcoin/pull/27060#discussion_r1105862197)
nit:
```suggestion
message(WARNING "${warning}")
```
https://cmake.org/cmake/help/latest/command/message.html
👍 vasild approved a pull request: "build: Add CMake-based build system (1 of N)"
(https://github.com/bitcoin/bitcoin/pull/27060)
(https://github.com/bitcoin/bitcoin/pull/27060)
💬 vasild commented on pull request "Handle CJDNS from LookupSubNet()":
(https://github.com/bitcoin/bitcoin/pull/27071#discussion_r1105886420)
Done.
(https://github.com/bitcoin/bitcoin/pull/27071#discussion_r1105886420)
Done.
💬 willcl-ark commented on issue "libevent: Error from accept() call: Invalid argument on Android termux (arm)":
(https://github.com/bitcoin/bitcoin/issues/19432#issuecomment-1429822788)
Since this was reported:
* There have been no additional reports of occurance
* Lineage OS has been updated from v16.0 to v20.0
* Bitcoin Core has been updated from v0.20 to v24.0.1
* libevent in Bitcoin Core depends was [updated](https://github.com/bitcoin/bitcoin/pull/21991) to 2.1.12-stable (which their script [appears to use](https://github.com/termux/termux-packages/blob/master/packages/bitcoin/build.sh#L25))
(and is anyone still running a full node on Android phone)
@MarcoFalke
...
(https://github.com/bitcoin/bitcoin/issues/19432#issuecomment-1429822788)
Since this was reported:
* There have been no additional reports of occurance
* Lineage OS has been updated from v16.0 to v20.0
* Bitcoin Core has been updated from v0.20 to v24.0.1
* libevent in Bitcoin Core depends was [updated](https://github.com/bitcoin/bitcoin/pull/21991) to 2.1.12-stable (which their script [appears to use](https://github.com/termux/termux-packages/blob/master/packages/bitcoin/build.sh#L25))
(and is anyone still running a full node on Android phone)
@MarcoFalke
...
💬 vasild commented on pull request "Handle CJDNS from LookupSubNet()":
(https://github.com/bitcoin/bitcoin/pull/27071#discussion_r1105887566)
There is no reason for 0x55 specifically. I added a comment to clarify:
`// Just an arbitrary number, anything != 0xFC would do.`
(https://github.com/bitcoin/bitcoin/pull/27071#discussion_r1105887566)
There is no reason for 0x55 specifically. I added a comment to clarify:
`// Just an arbitrary number, anything != 0xFC would do.`
💬 john-moffett commented on pull request "validation: Improve error handling when VerifyDB fails due to insufficient dbcache":
(https://github.com/bitcoin/bitcoin/pull/25574#issuecomment-1429827332)
ACK https://github.com/bitcoin/bitcoin/commit/080f83e43ddc1fd2ddc57f6590b9166a373d38c2 apart from the incidental test failure, which you've probably already caught:
```diff
diff --git a/test/functional/feature_pruning.py b/test/functional/feature_pruning.py
index 664ed779db2..03f5f2c8037 100755
--- a/test/functional/feature_pruning.py
+++ b/test/functional/feature_pruning.py
@@ -226 +226 @@ class PruneTest(BitcoinTestFramework):
- with self.nodes[2].assert_debug_log(expected_msgs
...
(https://github.com/bitcoin/bitcoin/pull/25574#issuecomment-1429827332)
ACK https://github.com/bitcoin/bitcoin/commit/080f83e43ddc1fd2ddc57f6590b9166a373d38c2 apart from the incidental test failure, which you've probably already caught:
```diff
diff --git a/test/functional/feature_pruning.py b/test/functional/feature_pruning.py
index 664ed779db2..03f5f2c8037 100755
--- a/test/functional/feature_pruning.py
+++ b/test/functional/feature_pruning.py
@@ -226 +226 @@ class PruneTest(BitcoinTestFramework):
- with self.nodes[2].assert_debug_log(expected_msgs
...
💬 MarcoFalke commented on issue "libevent: Error from accept() call: Invalid argument on Android termux (arm)":
(https://github.com/bitcoin/bitcoin/issues/19432#issuecomment-1429830980)
Is this still an issue with a recent version of Bitcoin Core? If yes, what are the steps to reproduce?
(https://github.com/bitcoin/bitcoin/issues/19432#issuecomment-1429830980)
Is this still an issue with a recent version of Bitcoin Core? If yes, what are the steps to reproduce?
✅ MarcoFalke closed an issue: "libevent: Error from accept() call: Invalid argument on Android termux (arm)"
(https://github.com/bitcoin/bitcoin/issues/19432)
(https://github.com/bitcoin/bitcoin/issues/19432)
📝 SomberNight opened a pull request: "descriptors: fix docstring (param [in] vs [out])"
(https://github.com/bitcoin/bitcoin/pull/27097)
As in title, these docstrings look incorrect.
(https://github.com/bitcoin/bitcoin/pull/27097)
As in title, these docstrings look incorrect.
💬 hebasto commented on pull request "build: Add CMake-based build system (1 of N)":
(https://github.com/bitcoin/bitcoin/pull/27060#issuecomment-1429846434)
Updated 75c4333e484ffea433f32b48ca7d4a1dc819bb36 -> 12758d3f69c4900f0d23df06e1ad746417428f3a ([pr27060.05](https://github.com/hebasto/bitcoin/commits/pr27060.05) -> [pr27060.06](https://github.com/hebasto/bitcoin/commits/pr27060.06)):
- rebased on top of the recent changes in the build system (#27016)
- `cmake_policy()` commands replaced with version range in `project()` one
- documented choice of versions
- add a note [suggested](https://github.com/bitcoin/bitcoin/pull/27060#discussion_r1
...
(https://github.com/bitcoin/bitcoin/pull/27060#issuecomment-1429846434)
Updated 75c4333e484ffea433f32b48ca7d4a1dc819bb36 -> 12758d3f69c4900f0d23df06e1ad746417428f3a ([pr27060.05](https://github.com/hebasto/bitcoin/commits/pr27060.05) -> [pr27060.06](https://github.com/hebasto/bitcoin/commits/pr27060.06)):
- rebased on top of the recent changes in the build system (#27016)
- `cmake_policy()` commands replaced with version range in `project()` one
- documented choice of versions
- add a note [suggested](https://github.com/bitcoin/bitcoin/pull/27060#discussion_r1
...
💬 hebasto commented on pull request "build: Add CMake-based build system (1 of N)":
(https://github.com/bitcoin/bitcoin/pull/27060#discussion_r1105914637)
Thanks! [Updated](https://github.com/bitcoin/bitcoin/pull/27060#issuecomment-1429846434).
(https://github.com/bitcoin/bitcoin/pull/27060#discussion_r1105914637)
Thanks! [Updated](https://github.com/bitcoin/bitcoin/pull/27060#issuecomment-1429846434).
💬 hebasto commented on pull request "build: Add CMake-based build system (1 of N)":
(https://github.com/bitcoin/bitcoin/pull/27060#discussion_r1105917773)
[UPD](https://github.com/bitcoin/bitcoin/pull/27060#issuecomment-1429846434): Now it is covered by the version range in the `cmake_minimum_required()` command.
(https://github.com/bitcoin/bitcoin/pull/27060#discussion_r1105917773)
[UPD](https://github.com/bitcoin/bitcoin/pull/27060#issuecomment-1429846434): Now it is covered by the version range in the `cmake_minimum_required()` command.
💬 hebasto commented on pull request "build: Add CMake-based build system (1 of N)":
(https://github.com/bitcoin/bitcoin/pull/27060#discussion_r1105918851)
> > But IMO we need a note to the effect of: "Changes which affect binary results may _not_ be quietly gated by CMake version".
>
> Ah, you mean a comment in CMake's code, right?
Thanks! [Updated](https://github.com/bitcoin/bitcoin/pull/27060#issuecomment-1429846434).
(https://github.com/bitcoin/bitcoin/pull/27060#discussion_r1105918851)
> > But IMO we need a note to the effect of: "Changes which affect binary results may _not_ be quietly gated by CMake version".
>
> Ah, you mean a comment in CMake's code, right?
Thanks! [Updated](https://github.com/bitcoin/bitcoin/pull/27060#issuecomment-1429846434).
👍 john-moffett approved a pull request: "descriptors: fix docstring (param [in] vs [out])"
(https://github.com/bitcoin/bitcoin/pull/27097)
(https://github.com/bitcoin/bitcoin/pull/27097)
💬 john-moffett commented on pull request "wallet: SecureString to allow null characters":
(https://github.com/bitcoin/bitcoin/pull/27068#issuecomment-1429905802)
Thanks for the feedback!
I agree that it likely will only affect a very small proportion of users, but given that Bitcoiners can be somewhat peculiar, it's impossible to be certain. Since it's a non-destructive change, I think it's acceptable to just return a detailed error message for now. If it turns out that it affects more people than expected, we can add a more user-friendly process.
If nobody has any objections, I'll work on a commit that adds a detailed error message suggesting tha
...
(https://github.com/bitcoin/bitcoin/pull/27068#issuecomment-1429905802)
Thanks for the feedback!
I agree that it likely will only affect a very small proportion of users, but given that Bitcoiners can be somewhat peculiar, it's impossible to be certain. Since it's a non-destructive change, I think it's acceptable to just return a detailed error message for now. If it turns out that it affects more people than expected, we can add a more user-friendly process.
If nobody has any objections, I'll work on a commit that adds a detailed error message suggesting tha
...