💬 glozow commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1725375345)
added tests for orphans with rejected parents

💬 glozow commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1724970656)
done

💬 glozow commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1725358054)
It makes them potentially larger transactions?

💬 glozow commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1725187075)
> womp womp

Can you be a bit more specific :joy:

💬 glozow commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1725359052)
done

💬 glozow commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1725360295)
I thought they were all positive?

💬 glozow commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1725361680)
added

💬 glozow commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1725374135)
should it? seems more complete if it handles all types, and we test them

💬 maflcko commented on issue "LevelDB read failure: Corruption: block checksum mismatch":
(https://github.com/bitcoin/bitcoin/issues/30159#issuecomment-2303933899)
> The very first machine I set up on AWS refused ssh access and had to be rebooted (fine so far after a reboot). Maybe all of this is just AWS hardware failures?

Another machine (or the same?) dropped dead, without SSH access anymore.



🤔 hodlinator reviewed a pull request: "test: [refactor] Use m_rng directly"
(https://github.com/bitcoin/bitcoin/pull/30571#pullrequestreview-2253654936)
Reviewed 38b03bc3af9fbf483b5a29a54a5ffb36c67b6b41 (still aiming to do some more testing).

> Thanks for checking the includes in the tests @hodlinator! I think I'll leave this as-is for now, because having leftover unused includes in tests should be harmless, apart from possibly being confusing, or needing more compile time. I have a follow-up to use iwyu (and enforce it), so that reviewers can use the re-review cycles to clean up includes for reviewing the code itself. However, this will be a
...

💬 hodlinator commented on pull request "test: [refactor] Use m_rng directly":
(https://github.com/bitcoin/bitcoin/pull/30571#discussion_r1726462944)
Could make `rand_seed` `const` to clarify usage (approved by clang-format-diff.py):
```diff
--- a/src/test/prevector_tests.cpp
+++ b/src/test/prevector_tests.cpp
@@ -27,7 +27,7 @@ class prevector_tester {

typedef typename pretype::size_type Size;
bool passed = true;
- uint256 rand_seed;
+ const uint256 rand_seed;


template <typename A, typename B>
@@ -207,8 +207,8 @@ public:
BOOST_CHECK_MESSAGE(passed, "insecure_rand: " + rand_seed.ToString());

...

💬 maflcko commented on pull request "test: [refactor] Use m_rng directly":
(https://github.com/bitcoin/bitcoin/pull/30571#discussion_r1726485189)
> Could make `rand_seed` `const` to clarify usage (approved by clang-format-diff.py):

Makes sense. However, I am not modifying this line of code, so I'll leave the style-nit as-is for now, to keep the already large pull request focussed on only the minimum diff required to fix the found bugs and apply the refactoring to achieve the stated goals in the pull motivation.

💬 maflcko commented on pull request "Fix maybe-uninitialized warning in IsSpentKey":
(https://github.com/bitcoin/bitcoin/pull/30691#issuecomment-2303970906)
lgtm ACK 17707db939cb09a16c002d226152e71fce9289f2

(I haven't tested this, as I don't use GCC)

Seems fine to refactor the code to work around the GCC bug, so that confusion is reduced when using GCC.

An alternative would be to disable the warning completely in GCC, because it is known to be buggy in all versions of GCC and I think has so far not found any real issue, did it?

💬 Sjors commented on pull request "Have miner account for timewarp mitigation, activate on regtest, lower nPowTargetTimespan to 144 and add test":
(https://github.com/bitcoin/bitcoin/pull/30681#discussion_r1726615759)
I'll move it to the other commit, or find a way to use it here, if I need to retouch.

💬 Sjors commented on pull request "Have miner account for timewarp mitigation, activate on regtest, lower nPowTargetTimespan to 144 and add test":
(https://github.com/bitcoin/bitcoin/pull/30681#discussion_r1726617405)
If we want to simulate a full timewarp attack, we'd need multiple such periods too.

💬 maflcko commented on issue "bitcoind shouldn't be shutdown automatically despite wallet synchronisation error":
(https://github.com/bitcoin/bitcoin/issues/30671#issuecomment-2304153431)
Updating the help texts in the GUI and the daemon around `-prune` to mention that wallets (and indexes) should ideally be kept loaded or active to avoid sync catch-up errors from happening at all and avoid this error in the first place could make sense as well.

💬 maflcko commented on issue "LevelDB read failure: Corruption: block checksum mismatch":
(https://github.com/bitcoin/bitcoin/issues/30159#issuecomment-2304162956)
A reboot fixed the network issue again and I still can not reproduce or otherwise see any corruption on any machine.

💬 hebasto commented on pull request "CMake replaces Autotools":
(https://github.com/bitcoin/bitcoin/pull/30664#issuecomment-2304196456)
> > It aims reveal conflicts with other PRs.
> > For more details, please refer to today's IRC meeting discussion.
>
> It would be good to mention any important key points inline, otherwise it will be harder to understand whether this is the pull request that will be merged as a follow-up, and whether it should be reviewed, or if there is another place where the review should happen.

The PR description has been updated with mentioning that this PR is not intended to be reviewed/merged in
...

💬 vasild commented on pull request "util: explicitly close all AutoFiles that have been written":
(https://github.com/bitcoin/bitcoin/pull/29307#issuecomment-2304200104)
`4533445fd7...1dcd626fe1`: rebase due to conflicts

💬 sipa commented on issue "Fatal LevelDB error: Corruption: block checksum mismatch on Linux ext4 SATA SSDs":
(https://github.com/bitcoin/bitcoin/issues/30692#issuecomment-2304213364)
@maflcko Ah yes, it is the version here that had a corruption in the txindex: https://bitcoin.stackexchange.com/q/123999/208