💬 MarcoFalke commented on issue "ci: Future of macOS and Windows MSVC CI tasks":
(https://github.com/bitcoin/bitcoin/issues/28098#issuecomment-1647312889)
> CircleCI looks better than others
Can you elaborate on this? The free plan would give less than 10 hours of macOS and Windows per month. And the paid plan doesn't look cheaper than Cirrus CI, does it?
(https://github.com/bitcoin/bitcoin/issues/28098#issuecomment-1647312889)
> CircleCI looks better than others
Can you elaborate on this? The free plan would give less than 10 hours of macOS and Windows per month. And the paid plan doesn't look cheaper than Cirrus CI, does it?
💬 hebasto commented on issue "ci: Future of macOS and Windows MSVC CI tasks":
(https://github.com/bitcoin/bitcoin/issues/28098#issuecomment-1647315920)
> > CircleCI looks better than others
>
> Can you elaborate on this? The free plan would give less than 10 hours of macOS and Windows per month. And the paid plan doesn't look cheaper than Cirrus CI, does it?
I've made comparison among available free plans. I did not consider paid plans in my comment.
(https://github.com/bitcoin/bitcoin/issues/28098#issuecomment-1647315920)
> > CircleCI looks better than others
>
> Can you elaborate on this? The free plan would give less than 10 hours of macOS and Windows per month. And the paid plan doesn't look cheaper than Cirrus CI, does it?
I've made comparison among available free plans. I did not consider paid plans in my comment.
💬 MarcoFalke commented on issue "Bitcoin Core v25.0 Crashes":
(https://github.com/bitcoin/bitcoin/issues/28119#issuecomment-1647323100)
> Does this still work if the actual disk sector is corrupt or would it still throw the fread failed: iostream error at FlatFilePos(nFile=9, nPos=53762556)
Good point. This may actually still fail. I wonder if it makes sense to change that to make `-reindex` skip on exceptions instead of shutting down.
(https://github.com/bitcoin/bitcoin/issues/28119#issuecomment-1647323100)
> Does this still work if the actual disk sector is corrupt or would it still throw the fread failed: iostream error at FlatFilePos(nFile=9, nPos=53762556)
Good point. This may actually still fail. I wonder if it makes sense to change that to make `-reindex` skip on exceptions instead of shutting down.
💬 MarcoFalke commented on pull request "Bugfix: RPC: Remove quotes from non-string oneline descriptions":
(https://github.com/bitcoin/bitcoin/pull/28123#discussion_r1271848468)
I've modified my suggestion. (Sorry, I don't compile them)
(https://github.com/bitcoin/bitcoin/pull/28123#discussion_r1271848468)
I've modified my suggestion. (Sorry, I don't compile them)
💬 MarcoFalke commented on issue "ci: Future of macOS and Windows MSVC CI tasks":
(https://github.com/bitcoin/bitcoin/issues/28098#issuecomment-1647360046)
If the goal is to stay on a free plan, I think the only option is GitHub Actions CI.
If the goal is to pay a similar amount as with the previous appveyor task, I think the only options are appveyor or travis.
(https://github.com/bitcoin/bitcoin/issues/28098#issuecomment-1647360046)
If the goal is to stay on a free plan, I think the only option is GitHub Actions CI.
If the goal is to pay a similar amount as with the previous appveyor task, I think the only options are appveyor or travis.
💬 russeree commented on pull request "Bugfix: RPC: Remove quotes from non-string oneline descriptions":
(https://github.com/bitcoin/bitcoin/pull/28123#discussion_r1271859176)
No problem just was testing things. Compiles good but looks like a double quote on the end of the ```m_opts.oneline_description```.
The current PR displays correctly ```"options```
(https://github.com/bitcoin/bitcoin/pull/28123#discussion_r1271859176)
No problem just was testing things. Compiles good but looks like a double quote on the end of the ```m_opts.oneline_description```.
The current PR displays correctly ```"options```
💬 MarcoFalke commented on pull request "Bugfix: RPC: Remove quotes from non-string oneline descriptions":
(https://github.com/bitcoin/bitcoin/pull/28123#discussion_r1271871716)
It should be fine to just drop the `"`, as they serve no purpose. A newline already follows the message to delimit it from the next lines.
```diff
diff --git a/src/util/check.cpp b/src/util/check.cpp
index 795dce7124..c4d4b0cc28 100644
--- a/src/util/check.cpp
+++ b/src/util/check.cpp
@@ -18,7 +18,7 @@
std::string StrFormatInternalBug(std::string_view msg, std::string_view file, int line, std::string_view func)
{
- return strprintf("Internal bug detected: \"%s\"\n%s:%d (%s)\n"
...
(https://github.com/bitcoin/bitcoin/pull/28123#discussion_r1271871716)
It should be fine to just drop the `"`, as they serve no purpose. A newline already follows the message to delimit it from the next lines.
```diff
diff --git a/src/util/check.cpp b/src/util/check.cpp
index 795dce7124..c4d4b0cc28 100644
--- a/src/util/check.cpp
+++ b/src/util/check.cpp
@@ -18,7 +18,7 @@
std::string StrFormatInternalBug(std::string_view msg, std::string_view file, int line, std::string_view func)
{
- return strprintf("Internal bug detected: \"%s\"\n%s:%d (%s)\n"
...
👋 MarcoFalke's pull request is ready for review: "ci: Use qemu-user through container engine"
(https://github.com/bitcoin/bitcoin/pull/28087)
(https://github.com/bitcoin/bitcoin/pull/28087)
💬 MarcoFalke commented on pull request "ci: Use qemu-user through container engine":
(https://github.com/bitcoin/bitcoin/pull/28087#issuecomment-1647420555)
The arm task is now run by @DrahtBot, which should make it green
(https://github.com/bitcoin/bitcoin/pull/28087#issuecomment-1647420555)
The arm task is now run by @DrahtBot, which should make it green
💬 hebasto commented on pull request "kernel: Remove UniValue from kernel library":
(https://github.com/bitcoin/bitcoin/pull/28113#issuecomment-1647437043)
I've restarted the "Win64 native [vs2022]" CI task due to a remote Chocolatey issue.
(https://github.com/bitcoin/bitcoin/pull/28113#issuecomment-1647437043)
I've restarted the "Win64 native [vs2022]" CI task due to a remote Chocolatey issue.
📝 MarcoFalke reopened a pull request: "ci: Switch more tasks to self-hosted"
(https://github.com/bitcoin/bitcoin/pull/21652)
This is recommended by Cirrus CI to reduce the costs. See also https://github.com/bitcoin/bitcoin/issues/28098
(https://github.com/bitcoin/bitcoin/pull/21652)
This is recommended by Cirrus CI to reduce the costs. See also https://github.com/bitcoin/bitcoin/issues/28098
💬 glozow commented on pull request "validate package transactions with their in-package ancestor sets":
(https://github.com/bitcoin/bitcoin/pull/26711#issuecomment-1647523520)
> I'm starting to think that something closer to your idea here is right: trying ancestor sets of every transaction in the linearization in order, if the ancestor set feerate is suffiicently high. This indeed won't deal with multiple-children-pay-for-parent cases perfectly, but including everything connected may be too much as well.
> I don't think handling multiple-children-pay-for-parent cases perfectly should be a goal here
I agree that the non-ancestor-set-shaped submission is not some
...
(https://github.com/bitcoin/bitcoin/pull/26711#issuecomment-1647523520)
> I'm starting to think that something closer to your idea here is right: trying ancestor sets of every transaction in the linearization in order, if the ancestor set feerate is suffiicently high. This indeed won't deal with multiple-children-pay-for-parent cases perfectly, but including everything connected may be too much as well.
> I don't think handling multiple-children-pay-for-parent cases perfectly should be a goal here
I agree that the non-ancestor-set-shaped submission is not some
...
💬 hebasto commented on pull request "kernel: Remove UniValue from kernel library":
(https://github.com/bitcoin/bitcoin/pull/28113#issuecomment-1647528259)
ACK a3774d1b2a5ce9aa6d6d3cedc2c9b9a5d2f68240.
Should throwing `RPC_INVALID_PARAMETER` be mentioned in the release notes?
(https://github.com/bitcoin/bitcoin/pull/28113#issuecomment-1647528259)
ACK a3774d1b2a5ce9aa6d6d3cedc2c9b9a5d2f68240.
Should throwing `RPC_INVALID_PARAMETER` be mentioned in the release notes?
💬 recursive-rat4 commented on pull request "Enable full-rbf by default":
(https://github.com/bitcoin/bitcoin/pull/28132#issuecomment-1647569661)
I queried the explorer like this
```
~ $ curl -s https://mempool.space/api/v1/block/0000000000000000000153159d7b95debfb0dadcd1040aaf9dbeb0025a1ddeac/audit-summary | jq .fullrbfTxs
[
"53cec64b52989c531550ac4606bedf1ff83d5bfd90efdc4006f122ac6b1b7643",
"5bc64344c56f847e2d992fab241567075473eec9423776afadc187299352bce1",
"64ec51dedd1404a775d590f530a01bbdc4239c8eb6d33ce4b9217ec2f0b8ddae"
]
```
Only 4 of 6 mentioned transactions seem to be full-rbf, or was it a wrong request?
(https://github.com/bitcoin/bitcoin/pull/28132#issuecomment-1647569661)
I queried the explorer like this
```
~ $ curl -s https://mempool.space/api/v1/block/0000000000000000000153159d7b95debfb0dadcd1040aaf9dbeb0025a1ddeac/audit-summary | jq .fullrbfTxs
[
"53cec64b52989c531550ac4606bedf1ff83d5bfd90efdc4006f122ac6b1b7643",
"5bc64344c56f847e2d992fab241567075473eec9423776afadc187299352bce1",
"64ec51dedd1404a775d590f530a01bbdc4239c8eb6d33ce4b9217ec2f0b8ddae"
]
```
Only 4 of 6 mentioned transactions seem to be full-rbf, or was it a wrong request?
💬 russeree commented on pull request "Bugfix: RPC: Remove quotes from non-string oneline descriptions":
(https://github.com/bitcoin/bitcoin/pull/28123#discussion_r1272019533)
This compiles and looks great even for the existing errors! Modifying this does cause rpc_misc.py to fail test runner. This is quite nice.
(https://github.com/bitcoin/bitcoin/pull/28123#discussion_r1272019533)
This compiles and looks great even for the existing errors! Modifying this does cause rpc_misc.py to fail test runner. This is quite nice.
💬 darosior commented on pull request "Remove C-style const-violating cast, Use reinterpret_cast":
(https://github.com/bitcoin/bitcoin/pull/28127#discussion_r1272021098)
Why casting in the first place since [`memcpy` would reinterpret it as `unsigned char *`](https://en.cppreference.com/w/cpp/string/byte/memcpy) anyways?
(https://github.com/bitcoin/bitcoin/pull/28127#discussion_r1272021098)
Why casting in the first place since [`memcpy` would reinterpret it as `unsigned char *`](https://en.cppreference.com/w/cpp/string/byte/memcpy) anyways?
💬 russeree commented on issue "Bitcoin Core v25.0 Crashes":
(https://github.com/bitcoin/bitcoin/issues/28119#issuecomment-1647627017)
What the file copy method solves in a way is the fact that when you copy and move that data you will **likely** get new sectors and as such if you have a bad secor you have a chance or not hitting it or at least hitting it with the same file.
> Good point. This may actually still fail. I wonder if it makes sense to change that to make `-reindex` skip on exceptions instead of shutting down.
I would like to think about this for a bit before commenting further. With that said my first thought
...
(https://github.com/bitcoin/bitcoin/issues/28119#issuecomment-1647627017)
What the file copy method solves in a way is the fact that when you copy and move that data you will **likely** get new sectors and as such if you have a bad secor you have a chance or not hitting it or at least hitting it with the same file.
> Good point. This may actually still fail. I wonder if it makes sense to change that to make `-reindex` skip on exceptions instead of shutting down.
I would like to think about this for a bit before commenting further. With that said my first thought
...
💬 hebasto commented on pull request "Remove C-style const-violating cast, Use reinterpret_cast":
(https://github.com/bitcoin/bitcoin/pull/28127#issuecomment-1647646239)
Concept ACK.
(https://github.com/bitcoin/bitcoin/pull/28127#issuecomment-1647646239)
Concept ACK.
💬 MarcoFalke commented on issue "Bitcoin Core v25.0 Crashes":
(https://github.com/bitcoin/bitcoin/issues/28119#issuecomment-1647647245)
> I would like to think about this for a bit before commenting further. With that said my first thought is that having a good solution to this even if it is as simple as very clear log level notifications would be a massive benefit to node runners while debugging failed disks or disk access requests from the OS.
Right. It may be better to adjust the error message to say that this is either a critical bug in the software, or a hardware issue. `-reindex` in both cases won't solve the underlying
...
(https://github.com/bitcoin/bitcoin/issues/28119#issuecomment-1647647245)
> I would like to think about this for a bit before commenting further. With that said my first thought is that having a good solution to this even if it is as simple as very clear log level notifications would be a massive benefit to node runners while debugging failed disks or disk access requests from the OS.
Right. It may be better to adjust the error message to say that this is either a critical bug in the software, or a hardware issue. `-reindex` in both cases won't solve the underlying
...
💬 stratospher commented on pull request "rpc: Add test-only RPC getaddrmaninfo for new/tried table address count":
(https://github.com/bitcoin/bitcoin/pull/27511#issuecomment-1647650203)
updated the PR to include better docs inspired from https://github.com/bitcoin/bitcoin/pull/27511#discussion_r1269601639. left "RPC is for testing only" as it is since all hidden RPCs have that line in the help. `getaddrmaninfo`'s help looks like this now:
```
Provides high-level information about the node's address manager by returning the number of addresses in the `new` and `tried` tables and their sum for all networks.
This RPC is for testing only.
Result:
{ (json
...
(https://github.com/bitcoin/bitcoin/pull/27511#issuecomment-1647650203)
updated the PR to include better docs inspired from https://github.com/bitcoin/bitcoin/pull/27511#discussion_r1269601639. left "RPC is for testing only" as it is since all hidden RPCs have that line in the help. `getaddrmaninfo`'s help looks like this now:
```
Provides high-level information about the node's address manager by returning the number of addresses in the `new` and `tried` tables and their sum for all networks.
This RPC is for testing only.
Result:
{ (json
...