💬 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
...
💬 fanquake commented on pull request "build: prune Boost headers in depends":
(https://github.com/bitcoin/bitcoin/pull/24742#issuecomment-1429926358)
Dropped one additional unused header.
(https://github.com/bitcoin/bitcoin/pull/24742#issuecomment-1429926358)
Dropped one additional unused header.
📝 fanquake opened a pull request: "refactor: don't avoid sys/types.h on when building for Windows"
(https://github.com/bitcoin/bitcoin/pull/27098)
We've already used it unguarded in `httpserver.cpp` for years, with no build issues.
Doesn't touch the usage in wallet/bdb.cpp. See #26832.
(https://github.com/bitcoin/bitcoin/pull/27098)
We've already used it unguarded in `httpserver.cpp` for years, with no build issues.
Doesn't touch the usage in wallet/bdb.cpp. See #26832.
💬 fanquake commented on pull request "compat: move (win) S_* defines into bdb":
(https://github.com/bitcoin/bitcoin/pull/26832#issuecomment-1429935927)
Rebased on #27098.
(https://github.com/bitcoin/bitcoin/pull/26832#issuecomment-1429935927)
Rebased on #27098.
💬 willcl-ark commented on issue "Expose PSBT AddInput/AddOutput via RPC":
(https://github.com/bitcoin/bitcoin/issues/19608#issuecomment-1429955448)
ISTM that although these methods could _perhaps_ be added in some way (so long as they do some verification on whether there were any signatures which had signed with `SIGHASH_ALL` already in the PSBT before proceeding?) there is not a lot of appetite to add them as they are very likely to cause issues.
@achow101 are we leaving this issue open as there's still a possibility of these RPCs being added?
(https://github.com/bitcoin/bitcoin/issues/19608#issuecomment-1429955448)
ISTM that although these methods could _perhaps_ be added in some way (so long as they do some verification on whether there were any signatures which had signed with `SIGHASH_ALL` already in the PSBT before proceeding?) there is not a lot of appetite to add them as they are very likely to cause issues.
@achow101 are we leaving this issue open as there's still a possibility of these RPCs being added?
💬 hebasto commented on pull request "refactor: don't avoid sys/types.h on when building for Windows":
(https://github.com/bitcoin/bitcoin/pull/27098#issuecomment-1429957486)
> don't avoid sys/types.h on when building for Windows
Does Windows builds require any symbols from `<sys/types.h>`? If not, why churn the code?
(https://github.com/bitcoin/bitcoin/pull/27098#issuecomment-1429957486)
> don't avoid sys/types.h on when building for Windows
Does Windows builds require any symbols from `<sys/types.h>`? If not, why churn the code?
💬 fanquake commented on pull request "refactor: don't avoid sys/types.h on when building for Windows":
(https://github.com/bitcoin/bitcoin/pull/27098#issuecomment-1429961005)
> Does Windows builds require any symbols from <sys/types.h>?
Aside from ultimately removing pointless per-platform conditionals,
> See https://github.com/bitcoin/bitcoin/pull/26832.
(https://github.com/bitcoin/bitcoin/pull/27098#issuecomment-1429961005)
> Does Windows builds require any symbols from <sys/types.h>?
Aside from ultimately removing pointless per-platform conditionals,
> See https://github.com/bitcoin/bitcoin/pull/26832.
💬 darosior commented on issue "Expose PSBT AddInput/AddOutput via RPC":
(https://github.com/bitcoin/bitcoin/issues/19608#issuecomment-1429962051)
For the record, i don't remember exactly why i advocated for this back when it was opened. I don't see why an application would need to use the `bitcoind` RPC to add inputs / outputs to a PSBT.
(https://github.com/bitcoin/bitcoin/issues/19608#issuecomment-1429962051)
For the record, i don't remember exactly why i advocated for this back when it was opened. I don't see why an application would need to use the `bitcoind` RPC to add inputs / outputs to a PSBT.