Bitcoin Core Github
43 subscribers
122K links
Download Telegram
💬 glozow commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1725185420)
woops, fixed now
💬 glozow commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1724970034)
done
💬 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.

![Screenshot from 2024-08-22 08-56-52](https://github.com/user-attachments/assets/a7992c6f-c82d-40e9-8ae3-9d5310f964ca)

![Screenshot from 2024-08-22 09-03-09](https://github.com/user-attachments/assets/c87809a8-9cfe-4e6c-9ef3-0c896a762fa7)
🤔 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.