💬 stickies-v commented on pull request "log: Mitigate disk filling attacks by rate limiting LogPrintf, LogInfo, LogWarning, LogError":
(https://github.com/bitcoin/bitcoin/pull/32604#discussion_r2175213743)
nit: the `-logsourcelocation` behaviour change is unrelated to the rate limiting, so I think this phrasing is a bit confusing. Would put it in a separate paragraph:
```
Unconditional logging to disk via LogPrintf, LogInfo, LogWarning, LogError, and
LogPrintLevel is now rate limited by giving each source location a logging quota of
1MiB per hour. (#32604)
When `-logsourcelocation` is enabled, the log output now contains the entire function signature
instead of just the function name. (#
...
(https://github.com/bitcoin/bitcoin/pull/32604#discussion_r2175213743)
nit: the `-logsourcelocation` behaviour change is unrelated to the rate limiting, so I think this phrasing is a bit confusing. Would put it in a separate paragraph:
```
Unconditional logging to disk via LogPrintf, LogInfo, LogWarning, LogError, and
LogPrintLevel is now rate limited by giving each source location a logging quota of
1MiB per hour. (#32604)
When `-logsourcelocation` is enabled, the log output now contains the entire function signature
instead of just the function name. (#
...
💬 darosior commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2175235373)
That this does not touch consensus can be ensured through code review and is also validated through a functional test. I don't think anything else is necessary to do here, so i'm going to resolve this thread.
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2175235373)
That this does not touch consensus can be ensured through code review and is also validated through a functional test. I don't think anything else is necessary to do here, so i'm going to resolve this thread.
💬 TheCharlatan commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2175236097)
Could we just skip signing them?
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2175236097)
Could we just skip signing them?
📝 furszy opened a pull request: "test: fix feature_init.py intermittencies "
(https://github.com/bitcoin/bitcoin/pull/32835)
Aims to solve #32600. Found it while working on #26966 (this was really annoying there).
This ensures the node is index-synced before perturbing files.
If the index sync gets interrupted before it starts, the database could be empty,
making any following perturbation ineffective (which explains why the node
does not abort during startup in the #32600 logs).
Also, the first commit avoids initializing components not under test.
This reduces log flooding, which helped in understanding the
...
(https://github.com/bitcoin/bitcoin/pull/32835)
Aims to solve #32600. Found it while working on #26966 (this was really annoying there).
This ensures the node is index-synced before perturbing files.
If the index sync gets interrupted before it starts, the database could be empty,
making any following perturbation ineffective (which explains why the node
does not abort during startup in the #32600 logs).
Also, the first commit avoids initializing components not under test.
This reduces log flooding, which helped in understanding the
...
💬 furszy commented on issue "intermittent issue in feature_init.py (bitcoind should have exited with expected error LevelDB error: Corruption)":
(https://github.com/bitcoin/bitcoin/issues/32600#issuecomment-3019459837)
#32835.
(https://github.com/bitcoin/bitcoin/issues/32600#issuecomment-3019459837)
#32835.
💬 TheCharlatan commented on pull request "test: fix feature_init.py intermittencies":
(https://github.com/bitcoin/bitcoin/pull/32835#issuecomment-3019500162)
Approach ACK
(https://github.com/bitcoin/bitcoin/pull/32835#issuecomment-3019500162)
Approach ACK
💬 maflcko commented on pull request "test: fix feature_init.py intermittencies":
(https://github.com/bitcoin/bitcoin/pull/32835#discussion_r2175309533)
any reason to start the node again? Can't this wait_until be done in check_clean_start?
(https://github.com/bitcoin/bitcoin/pull/32835#discussion_r2175309533)
any reason to start the node again? Can't this wait_until be done in check_clean_start?
⚠️ Ciornenichii opened an issue: "Ciornenichii stepan"
(https://github.com/bitcoin/bitcoin/issues/32836)
(https://github.com/bitcoin/bitcoin/issues/32836)
✅ fanquake closed an issue: "Ciornenichii stepan"
(https://github.com/bitcoin/bitcoin/issues/32836)
(https://github.com/bitcoin/bitcoin/issues/32836)
💬 Crypt-iQ commented on pull request "log: Mitigate disk filling attacks by rate limiting LogPrintf, LogInfo, LogWarning, LogError":
(https://github.com/bitcoin/bitcoin/pull/32604#discussion_r2175338142)
Leaving a note that I am going to implement these changes and plan to push up either later today or tomorrow.
(https://github.com/bitcoin/bitcoin/pull/32604#discussion_r2175338142)
Leaving a note that I am going to implement these changes and plan to push up either later today or tomorrow.
💬 instagibbs commented on pull request "[WIP] Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#issuecomment-3019599266)
@dergoegge awesome coverage and functional test!
Seems this is the issue, we are `visit`ing children erroneously disallowing their removal later when parent is `visit`ed for immaturity:
```
diff --git a/src/txmempool.cpp b/src/txmempool.cpp
index 5c90bc43dd..3c278f6aad 100644
--- a/src/txmempool.cpp
+++ b/src/txmempool.cpp
@@ -362,5 +362,5 @@ void CTxMemPool::removeForReorg(CChain& chain, std::function<bool(txiter)> check
WITH_FRESH_EPOCH(m_epoch);
for (indexed_trans
...
(https://github.com/bitcoin/bitcoin/pull/28676#issuecomment-3019599266)
@dergoegge awesome coverage and functional test!
Seems this is the issue, we are `visit`ing children erroneously disallowing their removal later when parent is `visit`ed for immaturity:
```
diff --git a/src/txmempool.cpp b/src/txmempool.cpp
index 5c90bc43dd..3c278f6aad 100644
--- a/src/txmempool.cpp
+++ b/src/txmempool.cpp
@@ -362,5 +362,5 @@ void CTxMemPool::removeForReorg(CChain& chain, std::function<bool(txiter)> check
WITH_FRESH_EPOCH(m_epoch);
for (indexed_trans
...
💬 instagibbs commented on pull request "cluster mempool: add TxGraph reorg functionality":
(https://github.com/bitcoin/bitcoin/pull/31553#issuecomment-3019618093)
@sipa linked here, added to follow-up potentially?
(https://github.com/bitcoin/bitcoin/pull/31553#issuecomment-3019618093)
@sipa linked here, added to follow-up potentially?
💬 furszy commented on pull request "test: fix feature_init.py intermittencies":
(https://github.com/bitcoin/bitcoin/pull/32835#discussion_r2175362595)
> any reason to start the node again? Can't this wait_until be done in check_clean_start?
Just because `check_clean_start()` is also called inside the deletion rounds and didn't want to start all indexes there on each cycle round. But that seems easily solvable.. one minute..
(https://github.com/bitcoin/bitcoin/pull/32835#discussion_r2175362595)
> any reason to start the node again? Can't this wait_until be done in check_clean_start?
Just because `check_clean_start()` is also called inside the deletion rounds and didn't want to start all indexes there on each cycle round. But that seems easily solvable.. one minute..
💬 furszy commented on pull request "test: fix feature_init.py intermittencies":
(https://github.com/bitcoin/bitcoin/pull/32835#discussion_r2175376795)
Done.
(https://github.com/bitcoin/bitcoin/pull/32835#discussion_r2175376795)
Done.
💬 maflcko commented on pull request "test: fix feature_init.py intermittencies":
(https://github.com/bitcoin/bitcoin/pull/32835#discussion_r2175386203)
this fails lint and `getindexinfo` is already a no-op when no args are provided, so no need to conditionally run it?
(https://github.com/bitcoin/bitcoin/pull/32835#discussion_r2175386203)
this fails lint and `getindexinfo` is already a no-op when no args are provided, so no need to conditionally run it?
💬 furszy commented on pull request "test: fix feature_init.py intermittencies":
(https://github.com/bitcoin/bitcoin/pull/32835#discussion_r2175411102)
> this fails lint and `getindexinfo` is already a no-op when no args are provided, so no need to conditionally run it? could just inline it?
Saw it late, pushed.
(https://github.com/bitcoin/bitcoin/pull/32835#discussion_r2175411102)
> this fails lint and `getindexinfo` is already a no-op when no args are provided, so no need to conditionally run it? could just inline it?
Saw it late, pushed.
💬 romanz commented on pull request "rest: rename `strURIPart` to `uri_part`":
(https://github.com/bitcoin/bitcoin/pull/32825#issuecomment-3019771653)
Thanks!
(https://github.com/bitcoin/bitcoin/pull/32825#issuecomment-3019771653)
Thanks!
💬 Sjors commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#issuecomment-3019784087)
From inline discussion: https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2174917931
> I think it's good to keep the [`CCoinsCaching`] benchmark but its results should not be over interpreted. It is focused on a tiny portion of the total work performed when processing a transaction. I locally have benchmarks of `ProcessTransaction` which i may share in the future. To give you an idea, they suggest (as can be expected from just reading the code) that total work in processing a transact
...
(https://github.com/bitcoin/bitcoin/pull/32521#issuecomment-3019784087)
From inline discussion: https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2174917931
> I think it's good to keep the [`CCoinsCaching`] benchmark but its results should not be over interpreted. It is focused on a tiny portion of the total work performed when processing a transaction. I locally have benchmarks of `ProcessTransaction` which i may share in the future. To give you an idea, they suggest (as can be expected from just reading the code) that total work in processing a transact
...
💬 l0rinc commented on pull request "log: Mitigate disk filling attacks by rate limiting LogPrintf, LogInfo, LogWarning, LogError":
(https://github.com/bitcoin/bitcoin/pull/32604#discussion_r2175440506)
Sweet, I'm planning on reviewing it this week!
(https://github.com/bitcoin/bitcoin/pull/32604#discussion_r2175440506)
Sweet, I'm planning on reviewing it this week!
💬 luke-jr commented on pull request "fs: use `ftruncate` in `AllocateFileRange` on OpenBSD":
(https://github.com/bitcoin/bitcoin/pull/32645#discussion_r2175472130)
OTOH, `ftruncate` doesn't actually guarantee allocation... does OpenBSD provide such a guarantee?
(https://github.com/bitcoin/bitcoin/pull/32645#discussion_r2175472130)
OTOH, `ftruncate` doesn't actually guarantee allocation... does OpenBSD provide such a guarantee?