💬 Sjors commented on pull request "Miner: never create a template which exploits the timewarp bug":
(https://github.com/bitcoin/bitcoin/pull/31376#issuecomment-2506010620)
I'm not sure. Maybe it's better to wait with this change until a timewarp fix soft fork is further along the proposal process. E.g. I'd rather not have to think about different Bitcoin Core versions having a different value for `MAX_TIMEWARP`.
(https://github.com/bitcoin/bitcoin/pull/31376#issuecomment-2506010620)
I'm not sure. Maybe it's better to wait with this change until a timewarp fix soft fork is further along the proposal process. E.g. I'd rather not have to think about different Bitcoin Core versions having a different value for `MAX_TIMEWARP`.
📝 maflcko opened a pull request: "util: Drop boost posix_time in ParseISO8601DateTime"
(https://github.com/bitcoin/bitcoin/pull/31391)
`boost::posix_time` in `ParseISO8601DateTime` has many issues:
* It parses random strings that are clearly invalid and returns a time value for them, see [1] below.
* None of the separators `-`, or `:`, or `T`, or `Z` are validated.
* It may crash when running under a hardened C++ library, see https://github.com/bitcoin/bitcoin/issues/28917.
* It has been unmaintained for years, so reporting or fixing any issues will most likely be useless.
* It pulls in a third-party dependency, when the
...
(https://github.com/bitcoin/bitcoin/pull/31391)
`boost::posix_time` in `ParseISO8601DateTime` has many issues:
* It parses random strings that are clearly invalid and returns a time value for them, see [1] below.
* None of the separators `-`, or `:`, or `T`, or `Z` are validated.
* It may crash when running under a hardened C++ library, see https://github.com/bitcoin/bitcoin/issues/28917.
* It has been unmaintained for years, so reporting or fixing any issues will most likely be useless.
* It pulls in a third-party dependency, when the
...
💬 maflcko commented on pull request "utils: replace boost::date_time usage with c++20 std::chrono":
(https://github.com/bitcoin/bitcoin/pull/30462#issuecomment-2506042037)
> should do everything in about 20 lines of code?
Ok, it is only 18 lines of code, see https://github.com/bitcoin/bitcoin/pull/31391
(https://github.com/bitcoin/bitcoin/pull/30462#issuecomment-2506042037)
> should do everything in about 20 lines of code?
Ok, it is only 18 lines of code, see https://github.com/bitcoin/bitcoin/pull/31391
💬 tdb3 commented on pull request "doc, test: more ephemeral dust follow-ups":
(https://github.com/bitcoin/bitcoin/pull/31371#issuecomment-2506047798)
> I think it could make sense to check the run-time of other functional tests as well and reorder them if necessary?
Yes, I think it's worth a deeper look. I did use `test_runner.py --resultsfile=runtimes.csv` and sorted by time. Did some rearranging, but was only able to shave off an additional 3% or so. That was just a quick/rough attempt though.
(https://github.com/bitcoin/bitcoin/pull/31371#issuecomment-2506047798)
> I think it could make sense to check the run-time of other functional tests as well and reorder them if necessary?
Yes, I think it's worth a deeper look. I did use `test_runner.py --resultsfile=runtimes.csv` and sorted by time. Did some rearranging, but was only able to shave off an additional 3% or so. That was just a quick/rough attempt though.
💬 instagibbs commented on pull request "doc, test: more ephemeral dust follow-ups":
(https://github.com/bitcoin/bitcoin/pull/31371#issuecomment-2506067535)
Part of the cost of the test is ensuring that 1P1C relay is working
properly. The merged ephemeral dust code is a bit more straightforward as
0-fee requirement is enforced only so maybe it's not as necessary?
On Thu, Nov 28, 2024, 7:50 AM tdb3 ***@***.***> wrote:
> I think it could make sense to check the run-time of other functional
> tests as well and reorder them if necessary?
>
> Yes, I think it's worth a deeper look. I did use test_runner.py
> --resultsfile=runtimes.csv and sort
...
(https://github.com/bitcoin/bitcoin/pull/31371#issuecomment-2506067535)
Part of the cost of the test is ensuring that 1P1C relay is working
properly. The merged ephemeral dust code is a bit more straightforward as
0-fee requirement is enforced only so maybe it's not as necessary?
On Thu, Nov 28, 2024, 7:50 AM tdb3 ***@***.***> wrote:
> I think it could make sense to check the run-time of other functional
> tests as well and reorder them if necessary?
>
> Yes, I think it's worth a deeper look. I did use test_runner.py
> --resultsfile=runtimes.csv and sort
...
💬 willcl-ark commented on issue "Bitcoin Core 28.0 (Flatpak Linux Mint) pointing or saving blockchain to external drives does not work":
(https://github.com/bitcoin/bitcoin/issues/31188#issuecomment-2506077912)
I can't reproduce this:
Running the following correctly locates the datadir on an external drive:
```bash
mkdir -p /mnt/bulk/flatpak
sudo flatpak override --system org.bitcoincore.bitcoin-qt --filesystem=/mnt/bulk
flatpak run org.bitcoincore.bitcoin-qt -datadir=/mnt/bulk/flatpak
```
Are you doing something different to me here?
In any case, I don't this there is any issue with Bitcoin Cor
...
(https://github.com/bitcoin/bitcoin/issues/31188#issuecomment-2506077912)
I can't reproduce this:
Running the following correctly locates the datadir on an external drive:
```bash
mkdir -p /mnt/bulk/flatpak
sudo flatpak override --system org.bitcoincore.bitcoin-qt --filesystem=/mnt/bulk
flatpak run org.bitcoincore.bitcoin-qt -datadir=/mnt/bulk/flatpak
```
Are you doing something different to me here?
In any case, I don't this there is any issue with Bitcoin Cor
...
💬 hebasto commented on pull request "util: Drop boost posix_time in ParseISO8601DateTime":
(https://github.com/bitcoin/bitcoin/pull/31391#issuecomment-2506083607)
Concept ACK.
(https://github.com/bitcoin/bitcoin/pull/31391#issuecomment-2506083607)
Concept ACK.
👍 brunoerg approved a pull request: "test: Add missing node.setmocktime(self.mocktime) to p2p_ibd_stalling.py"
(https://github.com/bitcoin/bitcoin/pull/31383#pullrequestreview-2468181663)
ACK faa16ed4b9edd126b5a2b0c13994f18273096efc
I verified the failure on master as expected:
```sh
2024-11-28T13:09:43.140000Z TestFramework (INFO): Check that a staller does not get disconnected if the 1024 block lookahead buffer is filled
2024-11-28T13:11:14.795000Z TestFramework.utils (ERROR): wait_until() failed. Predicate: ''''
self.wait_until(lambda: self.total_bytes_recv_for_blocks() == bytes_recv)
'''
2024-11-28T13:11:14.795000Z TestFramework (ERROR): Assertion failed
...
(https://github.com/bitcoin/bitcoin/pull/31383#pullrequestreview-2468181663)
ACK faa16ed4b9edd126b5a2b0c13994f18273096efc
I verified the failure on master as expected:
```sh
2024-11-28T13:09:43.140000Z TestFramework (INFO): Check that a staller does not get disconnected if the 1024 block lookahead buffer is filled
2024-11-28T13:11:14.795000Z TestFramework.utils (ERROR): wait_until() failed. Predicate: ''''
self.wait_until(lambda: self.total_bytes_recv_for_blocks() == bytes_recv)
'''
2024-11-28T13:11:14.795000Z TestFramework (ERROR): Assertion failed
...
✅ willcl-ark closed an issue: "Bitcoin Core 28.0 (Flatpak Linux Mint) pointing or saving blockchain to external drives does not work"
(https://github.com/bitcoin/bitcoin/issues/31188)
(https://github.com/bitcoin/bitcoin/issues/31188)
💬 willcl-ark commented on issue "Bitcoin Core 28.0 (Flatpak Linux Mint) pointing or saving blockchain to external drives does not work":
(https://github.com/bitcoin/bitcoin/issues/31188#issuecomment-2506150381)
In fact, I am going to close this now, as I think this is all expected behaviour. Command line options take precedence over config file arguments, and so in your case:
```
Config file arg: blocksdir="/media/USERNAME/DRIVE/BTC/data"
Config file arg: datadir="/media/USERNAME/DRIVE/BTC"
Command-line arg: datadir="/home/USERNAME/.var/app/org.bitcoincore.bitcoin-qt/data"
```
The command line `-datadir="/home/USERNAME/.var/app/org.bitcoincore.bitcoin-qt/data` arg is correctly overriding the
...
(https://github.com/bitcoin/bitcoin/issues/31188#issuecomment-2506150381)
In fact, I am going to close this now, as I think this is all expected behaviour. Command line options take precedence over config file arguments, and so in your case:
```
Config file arg: blocksdir="/media/USERNAME/DRIVE/BTC/data"
Config file arg: datadir="/media/USERNAME/DRIVE/BTC"
Command-line arg: datadir="/home/USERNAME/.var/app/org.bitcoincore.bitcoin-qt/data"
```
The command line `-datadir="/home/USERNAME/.var/app/org.bitcoincore.bitcoin-qt/data` arg is correctly overriding the
...
💬 hodlinator commented on pull request "util: Improve documentation and negation of args":
(https://github.com/bitcoin/bitcoin/pull/31212#issuecomment-2506175947)
Pushed 51e34ca022d6704dcf1fa955e3764a97d91bbba8
---
Unfortunately I wasn't able to use the simplification suggested by @ryanofsky:
https://github.com/bitcoin/bitcoin/blob/3a856c86685a5ab6f3cb78b17cb52cbb1b9ad95b/src/common/config.cpp#L138-L141
When opening a directory with an `ifstream`, Windows CI would not return `true` for `is_open()`:
https://github.com/hodlinator/bitcoin/actions/runs/12052081230/job/33604594555#step:12:3324
When opening a directory with an `ifstream` on MacOS
...
(https://github.com/bitcoin/bitcoin/pull/31212#issuecomment-2506175947)
Pushed 51e34ca022d6704dcf1fa955e3764a97d91bbba8
---
Unfortunately I wasn't able to use the simplification suggested by @ryanofsky:
https://github.com/bitcoin/bitcoin/blob/3a856c86685a5ab6f3cb78b17cb52cbb1b9ad95b/src/common/config.cpp#L138-L141
When opening a directory with an `ifstream`, Windows CI would not return `true` for `is_open()`:
https://github.com/hodlinator/bitcoin/actions/runs/12052081230/job/33604594555#step:12:3324
When opening a directory with an `ifstream` on MacOS
...
💬 willcl-ark commented on issue "Cmake build system breaks with symbolic links":
(https://github.com/bitcoin/bitcoin/issues/31145#issuecomment-2506181264)
I was able to reproduce this on Ubuntu 23.10 on master @ 7590e93 with the repro steps in OP:
<details>
<summary>Details</summary>
```
[ 66%] Automatic RCC for bitcoin_locale.qrc
[ 67%] Building CXX object src/qt/CMakeFiles/bitcoinqt.dir/bitcoinaddressvalidator.cpp.o
[ 67%] Building CXX object src/qt/CMakeFiles/bitcoinqt.dir/bantablemodel.cpp.o
[ 67%] Building CXX object src/qt/CMakeFiles/bitcoinqt.dir/bitcoinqt_autogen/mocs_compilation.cpp.o
[ 67%] Building CXX object src/qt/CMakeFil
...
(https://github.com/bitcoin/bitcoin/issues/31145#issuecomment-2506181264)
I was able to reproduce this on Ubuntu 23.10 on master @ 7590e93 with the repro steps in OP:
<details>
<summary>Details</summary>
```
[ 66%] Automatic RCC for bitcoin_locale.qrc
[ 67%] Building CXX object src/qt/CMakeFiles/bitcoinqt.dir/bitcoinaddressvalidator.cpp.o
[ 67%] Building CXX object src/qt/CMakeFiles/bitcoinqt.dir/bantablemodel.cpp.o
[ 67%] Building CXX object src/qt/CMakeFiles/bitcoinqt.dir/bitcoinqt_autogen/mocs_compilation.cpp.o
[ 67%] Building CXX object src/qt/CMakeFil
...
💬 TheCharlatan commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r1862247416)
Ok, this is what I have now: https://github.com/TheCharlatan/bitcoin/compare/kernelApi_7..kernelApi_8 , what do you think?
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r1862247416)
Ok, this is what I have now: https://github.com/TheCharlatan/bitcoin/compare/kernelApi_7..kernelApi_8 , what do you think?
💬 hebasto commented on issue "Cmake build system breaks with symbolic links":
(https://github.com/bitcoin/bitcoin/issues/31145#issuecomment-2506192717)
@willcl-ark
> So I agree, symlinked build dirs seem generally broken currently with cmake.
Does https://github.com/bitcoin/bitcoin/pull/31361 fix it?
(https://github.com/bitcoin/bitcoin/issues/31145#issuecomment-2506192717)
@willcl-ark
> So I agree, symlinked build dirs seem generally broken currently with cmake.
Does https://github.com/bitcoin/bitcoin/pull/31361 fix it?
👍 willcl-ark approved a pull request: "cmake, qt: Use absolute paths for includes in MOC-generated files"
(https://github.com/bitcoin/bitcoin/pull/31361#pullrequestreview-2468275553)
tACK 6f4128e3a838d03f46d397c15bc5333287e14863
Tested building using a symlinked build dir using Ninja. Failures I experienced in #31145 build successfully with this patch.
(https://github.com/bitcoin/bitcoin/pull/31361#pullrequestreview-2468275553)
tACK 6f4128e3a838d03f46d397c15bc5333287e14863
Tested building using a symlinked build dir using Ninja. Failures I experienced in #31145 build successfully with this patch.
💬 willcl-ark commented on issue "Cmake build system breaks with symbolic links":
(https://github.com/bitcoin/bitcoin/issues/31145#issuecomment-2506211739)
It does indeed.
(https://github.com/bitcoin/bitcoin/issues/31145#issuecomment-2506211739)
It does indeed.
💬 vasild commented on pull request "Fuzz: extend CConnman tests":
(https://github.com/bitcoin/bitcoin/pull/28584#issuecomment-2506252596)
`c97d49628a...687a9af2a8`: include #31316 as first commit here. It fixes a problem in `master` with `FuzzedSock::Accept()` which might be triggered by the tests added in this PR.
(https://github.com/bitcoin/bitcoin/pull/28584#issuecomment-2506252596)
`c97d49628a...687a9af2a8`: include #31316 as first commit here. It fixes a problem in `master` with `FuzzedSock::Accept()` which might be triggered by the tests added in this PR.
💬 vasild commented on pull request "fuzz: set the output argument of FuzzedSock::Accept()":
(https://github.com/bitcoin/bitcoin/pull/31316#issuecomment-2506260829)
Included this in #28584.
Leaving this open for an independent review/merge because it is smaller change and fixes a problem in `master`.
(https://github.com/bitcoin/bitcoin/pull/31316#issuecomment-2506260829)
Included this in #28584.
Leaving this open for an independent review/merge because it is smaller change and fixes a problem in `master`.
✅ willcl-ark closed an issue: "estimateSmartFee error: "Insufficient data or no feerate found""
(https://github.com/bitcoin/bitcoin/issues/31116)
(https://github.com/bitcoin/bitcoin/issues/31116)
💬 willcl-ark commented on issue "estimateSmartFee error: "Insufficient data or no feerate found"":
(https://github.com/bitcoin/bitcoin/issues/31116#issuecomment-2506318310)
In the absence of any followup from @ella-quicknode in a few weeks, and no reproduction available (as well as no other reports) I'm going to close this as not reproducible.
If you have an excerpt from a debug.log with the `estimatesmartfee` logging turned on that you could provide, or think this should be re-opened for any other reason, please drop a comment below and we can re-open this.
If this has been continuing it would be good also to know if you see the same behaviour with 28.0.
(https://github.com/bitcoin/bitcoin/issues/31116#issuecomment-2506318310)
In the absence of any followup from @ella-quicknode in a few weeks, and no reproduction available (as well as no other reports) I'm going to close this as not reproducible.
If you have an excerpt from a debug.log with the `estimatesmartfee` logging turned on that you could provide, or think this should be re-opened for any other reason, please drop a comment below and we can re-open this.
If this has been continuing it would be good also to know if you see the same behaviour with 28.0.