💬 1440000bytes commented on issue "Testnet4 consensus failure due to timewarp related "softfork"":
(https://github.com/bitcoin/bitcoin/issues/30786#issuecomment-2325291375)
> cc @mempool
I don't think this notified anyone
(https://github.com/bitcoin/bitcoin/issues/30786#issuecomment-2325291375)
> cc @mempool
I don't think this notified anyone
💬 tobtoht commented on pull request "depends: Qt 5.15.15":
(https://github.com/bitcoin/bitcoin/pull/30774#issuecomment-2325298740)
5.15.15 was released last year on [August 31, 2023](https://wiki.qt.io/Qt_5.15_Release). The git tag for this release is `v5.15.15-lts-lgpl`. It seems to follow the pattern of previous commercial 5.15.x point releases.
(https://github.com/bitcoin/bitcoin/pull/30774#issuecomment-2325298740)
5.15.15 was released last year on [August 31, 2023](https://wiki.qt.io/Qt_5.15_Release). The git tag for this release is `v5.15.15-lts-lgpl`. It seems to follow the pattern of previous commercial 5.15.x point releases.
💬 andrewtoth commented on pull request "validation: write chainstate to disk every hour":
(https://github.com/bitcoin/bitcoin/pull/30611#discussion_r1741250422)
This seems like just replacing `if (m_next_write == decltype(m_next_write){}) {` with `if (m_next_write == NodeClock::time_point::max()) {`, and since it's max we can just move the `if` block below `fPeriodicWrite` assignment. I don't see how this is an improvement? Doesn't having multiple checks for `fDoFullFlush` make it more complex?
(https://github.com/bitcoin/bitcoin/pull/30611#discussion_r1741250422)
This seems like just replacing `if (m_next_write == decltype(m_next_write){}) {` with `if (m_next_write == NodeClock::time_point::max()) {`, and since it's max we can just move the `if` block below `fPeriodicWrite` assignment. I don't see how this is an improvement? Doesn't having multiple checks for `fDoFullFlush` make it more complex?
📝 theStack opened a pull request: "qt, build: remove unneeded `Q_IMPORT_PLUGIN` macro calls"
(https://github.com/bitcoin-core/gui/pull/834)
After the recent full removal of Autotools (PR [#30664](https://github.com/bitcoin/bitcoin/pull/30664)), these macros are not needed anymore in the .cpp files according to the TODO in qt's CMakeLists.txt. Tested building on OpenBSD 7.5, where the XCB plugin was still imported according to the debug log:
```
2024-09-02T21:13:27Z Bitcoin Core version v28.99.0-7346b0109208 (release build)
2024-09-02T21:13:27Z Qt 5.15.12 (dynamic), plugin=xcb
2024-09-02T21:13:27Z No static plugins.
2024-09-02T2
...
(https://github.com/bitcoin-core/gui/pull/834)
After the recent full removal of Autotools (PR [#30664](https://github.com/bitcoin/bitcoin/pull/30664)), these macros are not needed anymore in the .cpp files according to the TODO in qt's CMakeLists.txt. Tested building on OpenBSD 7.5, where the XCB plugin was still imported according to the debug log:
```
2024-09-02T21:13:27Z Bitcoin Core version v28.99.0-7346b0109208 (release build)
2024-09-02T21:13:27Z Qt 5.15.12 (dynamic), plugin=xcb
2024-09-02T21:13:27Z No static plugins.
2024-09-02T2
...
💬 hebasto commented on pull request "qt, build: remove unneeded `Q_IMPORT_PLUGIN` macro calls":
(https://github.com/bitcoin-core/gui/pull/834#issuecomment-2325309737)
Concept ACK.
(https://github.com/bitcoin-core/gui/pull/834#issuecomment-2325309737)
Concept ACK.
💬 l0rinc commented on pull request "validation: write chainstate to disk every hour":
(https://github.com/bitcoin/bitcoin/pull/30611#discussion_r1741253014)
it's not just replacing the if, please apply this patch on top to see what I mean:
```patch
diff --git a/src/validation.cpp b/src/validation.cpp
index 87a144cb99..6fdc070db3 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -2778,13 +2778,6 @@ CoinsCacheSizeState Chainstate::GetCoinsCacheSizeState(
return CoinsCacheSizeState::OK;
}
-NodeClock::time_point CalculateNextWrite(NodeClock::time_point after)
-{
- const auto time{after + DATABASE_WRITE_INTERVAL_MIN};
-
...
(https://github.com/bitcoin/bitcoin/pull/30611#discussion_r1741253014)
it's not just replacing the if, please apply this patch on top to see what I mean:
```patch
diff --git a/src/validation.cpp b/src/validation.cpp
index 87a144cb99..6fdc070db3 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -2778,13 +2778,6 @@ CoinsCacheSizeState Chainstate::GetCoinsCacheSizeState(
return CoinsCacheSizeState::OK;
}
-NodeClock::time_point CalculateNextWrite(NodeClock::time_point after)
-{
- const auto time{after + DATABASE_WRITE_INTERVAL_MIN};
-
...
💬 hebasto commented on pull request "qt, build: remove unneeded `Q_IMPORT_PLUGIN` macro calls":
(https://github.com/bitcoin-core/gui/pull/834#issuecomment-2325316004)
> Tested building on OpenBSD 7.5, where the XCB plugin was still imported according to the debug log:
>
> ```
> 2024-09-02T21:13:27Z Bitcoin Core version v28.99.0-7346b0109208 (release build)
> 2024-09-02T21:13:27Z Qt 5.15.12 (dynamic), plugin=xcb
> 2024-09-02T21:13:27Z No static plugins.
> 2024-09-02T21:13:27Z Style: fusion / QFusionStyle
> 2024-09-02T21:13:27Z System: OpenBSD 7.5, x86_64-little_endian-lp64
> ```
The log shows "No static plugins."
To verify that static plugins ar
...
(https://github.com/bitcoin-core/gui/pull/834#issuecomment-2325316004)
> Tested building on OpenBSD 7.5, where the XCB plugin was still imported according to the debug log:
>
> ```
> 2024-09-02T21:13:27Z Bitcoin Core version v28.99.0-7346b0109208 (release build)
> 2024-09-02T21:13:27Z Qt 5.15.12 (dynamic), plugin=xcb
> 2024-09-02T21:13:27Z No static plugins.
> 2024-09-02T21:13:27Z Style: fusion / QFusionStyle
> 2024-09-02T21:13:27Z System: OpenBSD 7.5, x86_64-little_endian-lp64
> ```
The log shows "No static plugins."
To verify that static plugins ar
...
📝 tdb3 opened a pull request: "rpc: add getorphantxs"
(https://github.com/bitcoin/bitcoin/pull/30793)
PR https://github.com/bitcoin/bitcoin/pull/30784 adds a test that excessively large transactions aren't added to the orphanage. It checks the debug.log for confirmation that the transaction was not added.
As discussed in comment https://github.com/bitcoin/bitcoin/pull/30784#pullrequestreview-2274470190, knowing about the state of the orphanage is currently limited.
This PR adds a new rpc, `getorphantxs`, that provides the caller with a list of orphan transactions.
Currently looking for
...
(https://github.com/bitcoin/bitcoin/pull/30793)
PR https://github.com/bitcoin/bitcoin/pull/30784 adds a test that excessively large transactions aren't added to the orphanage. It checks the debug.log for confirmation that the transaction was not added.
As discussed in comment https://github.com/bitcoin/bitcoin/pull/30784#pullrequestreview-2274470190, knowing about the state of the orphanage is currently limited.
This PR adds a new rpc, `getorphantxs`, that provides the caller with a list of orphan transactions.
Currently looking for
...
💬 tdb3 commented on pull request "test: add check that too large txs aren't put into orphanage":
(https://github.com/bitcoin/bitcoin/pull/30784#issuecomment-2325361225)
> RPC already has a way to get mempool transactions (e.g. `getrawmempool` and `getmempoolentry`), but as far as I can tell, not orphans. Unless I'm overlooking something (or there's a history I'm not aware of), might be nice to have a `getorphantxs` (returning orphan txids, etc.) and `getorphan "txid"` (details for an orphan). Orphans do expire relatively quickly, so results from these RPC would be pretty ephemeral, but it might still be worth knowing. I'm taking a look at implementation options
...
(https://github.com/bitcoin/bitcoin/pull/30784#issuecomment-2325361225)
> RPC already has a way to get mempool transactions (e.g. `getrawmempool` and `getmempoolentry`), but as far as I can tell, not orphans. Unless I'm overlooking something (or there's a history I'm not aware of), might be nice to have a `getorphantxs` (returning orphan txids, etc.) and `getorphan "txid"` (details for an orphan). Orphans do expire relatively quickly, so results from these RPC would be pretty ephemeral, but it might still be worth knowing. I'm taking a look at implementation options
...
👋 hebasto's pull request is ready for review: "build: Fix linking for `fuzz` target when building with MSan"
(https://github.com/bitcoin/bitcoin/pull/30778)
(https://github.com/bitcoin/bitcoin/pull/30778)
💬 hebasto commented on pull request "build: Fix linking for `fuzz` target when building with MSan":
(https://github.com/bitcoin/bitcoin/pull/30778#issuecomment-2325363726)
> > review-only ACK [331e976](https://github.com/bitcoin/bitcoin/commit/331e9761b639f380c457582ae3c4b05e56ff02b0)
>
> I'm still working on it. MSan fuzzing CI job still fails when running locally.
The issue has been resolved. The PR description has been updated.
Ready for review and testing.
(https://github.com/bitcoin/bitcoin/pull/30778#issuecomment-2325363726)
> > review-only ACK [331e976](https://github.com/bitcoin/bitcoin/commit/331e9761b639f380c457582ae3c4b05e56ff02b0)
>
> I'm still working on it. MSan fuzzing CI job still fails when running locally.
The issue has been resolved. The PR description has been updated.
Ready for review and testing.
💬 andrewtoth commented on pull request "rpc: add getorphantxs":
(https://github.com/bitcoin/bitcoin/pull/30793#issuecomment-2325371764)
Can you expand on the motivation behind exposing this please? Why would someone want to use such an RPC?
(https://github.com/bitcoin/bitcoin/pull/30793#issuecomment-2325371764)
Can you expand on the motivation behind exposing this please? Why would someone want to use such an RPC?
💬 0xB10C commented on pull request "tracing: network connection tracepoints":
(https://github.com/bitcoin/bitcoin/pull/25832#discussion_r1741287188)
note to self: update the binary path to new cmake location
(https://github.com/bitcoin/bitcoin/pull/25832#discussion_r1741287188)
note to self: update the binary path to new cmake location
💬 0xB10C commented on pull request "doc: update documentation and scripts related to build directories":
(https://github.com/bitcoin/bitcoin/pull/30741#issuecomment-2325385681)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/30741#issuecomment-2325385681)
Concept ACK
💬 andrewtoth commented on pull request "validation: write chainstate to disk every hour":
(https://github.com/bitcoin/bitcoin/pull/30611#discussion_r1741358607)
Done.
(https://github.com/bitcoin/bitcoin/pull/30611#discussion_r1741358607)
Done.
💬 andrewtoth commented on pull request "validation: write chainstate to disk every hour":
(https://github.com/bitcoin/bitcoin/pull/30611#discussion_r1741358669)
Ok, done!
(https://github.com/bitcoin/bitcoin/pull/30611#discussion_r1741358669)
Ok, done!
💬 andrewtoth commented on pull request "rpc, rest: Improve block rpc error handling, check header before attempting to read block data.":
(https://github.com/bitcoin/bitcoin/pull/30410#issuecomment-2325490746)
I'm unsure why CI is failing now...
(https://github.com/bitcoin/bitcoin/pull/30410#issuecomment-2325490746)
I'm unsure why CI is failing now...
📝 ludete opened a pull request: "fix use int32_t instead of int type for risczero compile "
(https://github.com/bitcoin/bitcoin/pull/30794)
describe as above
--------------
### Issue
https://github.com/bitcoin/bitcoin/issues/30747
(https://github.com/bitcoin/bitcoin/pull/30794)
describe as above
--------------
### Issue
https://github.com/bitcoin/bitcoin/issues/30747
💬 maflcko commented on issue "Risczero Fit":
(https://github.com/bitcoin/bitcoin/issues/30747#issuecomment-2325665028)
> `assumptions.h` compiles successfully, but fails with `static_assert(std::is_same_v<int, int32_t>);`
Ok, that means `int` has the correct width and thus the correct behavior, apart from being a separate type that the serialization templates can't deal with.
(https://github.com/bitcoin/bitcoin/issues/30747#issuecomment-2325665028)
> `assumptions.h` compiles successfully, but fails with `static_assert(std::is_same_v<int, int32_t>);`
Ok, that means `int` has the correct width and thus the correct behavior, apart from being a separate type that the serialization templates can't deal with.
💬 maflcko commented on pull request "fix use int32_t instead of int type for risczero compile":
(https://github.com/bitcoin/bitcoin/pull/30794#issuecomment-2325676629)
Can you explain *why* this is correct and *why* it is needed in the pull request description? Otherwise, current and future reviewers of the change will have a hard time following it.
Maybe something along the lines:
> Some platforms may define `int` and `int32_t` to be different types of the same width. This means that `src/compat/assumptions.h` compiles fine, however the templated serialization code can not accept values of type `int`.
>
> Fix compilation on those platforms by seriali
...
(https://github.com/bitcoin/bitcoin/pull/30794#issuecomment-2325676629)
Can you explain *why* this is correct and *why* it is needed in the pull request description? Otherwise, current and future reviewers of the change will have a hard time following it.
Maybe something along the lines:
> Some platforms may define `int` and `int32_t` to be different types of the same width. This means that `src/compat/assumptions.h` compiles fine, however the templated serialization code can not accept values of type `int`.
>
> Fix compilation on those platforms by seriali
...