💬 maflcko commented on pull request "rpc, bugfix: Enforce maximum value for setmocktime":
(https://github.com/bitcoin/bitcoin/pull/29869#discussion_r1565619686)
If you are changing this, it could also make sense to test that a full round trip of the value `9223372036` works.

📝 laanwj opened a pull request: "test: Add large aligned vmov check for mingw"
(https://github.com/bitcoin/bitcoin/pull/29874)
Add a check for 32-byte (256 bit) and 64-byte (512 bit) aligned AVX memory accesses (vmova instructions), which cause issues combined with a GCC stack alignment bug on Windows. This check is added to the existing symbol-check.py.

Makes use of the capstone disassembler library.

Also add a test to test the behavior of the check on a series of assembly instructions against the expected output.

Closes #28413.

💬 m3dwards commented on issue "IBD performance regression in 27.0rc1 on Windows":
(https://github.com/bitcoin/bitcoin/issues/29785#issuecomment-2056614400)
> Did someone with Windows try to enable the benchmarks in guix and compile them, and run them?

I can do

💬 maflcko commented on pull request "lint: Refactor lint checks to reuse exclusion logic; Support running individual lint checks in lint runner":
(https://github.com/bitcoin/bitcoin/pull/29744#issuecomment-2056616694)
Are you still working on this?

🚀 fanquake merged a pull request: "doc: fixup help output for -upnp and -natpmp"
(https://github.com/bitcoin/bitcoin/pull/28874)

👍 byaye approved a pull request: "test: check disconnection when sending sendaddrv2 after verack"
(https://github.com/bitcoin/bitcoin/pull/29699#pullrequestreview-2000919218)
Tested ACK b4c9ace6ff36c54755e4b12f204212c1b938f509

<img width="854" alt="Screenshot 2024-04-15 at 09 03 30" src="https://github.com/bitcoin/bitcoin/assets/19962379/4bc020e1-ecfb-4ac2-88b0-b3cd03cf9a00">

💬 maflcko commented on pull request "test: check disconnection when sending sendaddrv2 after verack":
(https://github.com/bitcoin/bitcoin/pull/29699#issuecomment-2056697218)
lgtm ACK b4c9ace6ff36c54755e4b12f204212c1b938f509

📝 StevenMia opened a pull request: "chore: fix some typos in comments"
(https://github.com/bitcoin/bitcoin/pull/29875)
<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

fix some typos in comments
<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvem
...

💬 davidgumberg commented on pull request "lint: Refactor lint checks to reuse exclusion logic; Support running individual lint checks in lint runner":
(https://github.com/bitcoin/bitcoin/pull/29744#issuecomment-2056727957)
> Are you still working on this?

Yes, I was just waiting to see if there was any other feedback before putting all of the linter code back into a single file.

I think a lot of the reason for the individual lint checks being placed in separate files previously was so that they could be run individually, but I feel that a single file that contains both the lint checks and the lint runner is a bit clunky.

I would rather leave splitting up `main.rs` to the tastes of a future PR, unless som
...

📝 fanquake opened a pull request: "build: add `-Wundef`"
(https://github.com/bitcoin/bitcoin/pull/29876)
Turn on `-Wundef`.

[> Warn if an undefined identifier is evaluated in an #if directive. Such identifiers are replaced with zero.](https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wundef).

Note that this is still beneficial with CMake, and may even be nice to have enabled prior, to catch any change in behaviour.

If we end up with this enabled, it should probably be enough to fix #16419.

📝 0xB10C opened a pull request: "tracing: cast block_connected duration to µs"
(https://github.com/bitcoin/bitcoin/pull/29877)
When the `validation:block_connected` tracepoint was introduced in 8f37f5c2a562c38c83fc40234ade9c301fc4e685, the connect block duration was passed in microseconds `µs`. By starting to use steady clock in fabf1cdb206e368a9433abf99a5ea2762a5ed2c0 this changed to nanoseconds `ns`. As the test only checked if the duration value is `> 0` as a plausibility check, this went unnoticed. This was detected this when setting up monitoring for block validation time as part of the Great Consensus Cleanup Revi
...

💬 maflcko commented on pull request "tracing: cast block_connected duration to µs":
(https://github.com/bitcoin/bitcoin/pull/29877#issuecomment-2056849284)
How is is possible that this even compiled? I presume tracing just takes any type and value and passes it along as raw bytes, so `std::chrono` being type-safe does not help here?

📝 fanquake opened a pull request: "depends: build expat with CMake"
(https://github.com/bitcoin/bitcoin/pull/29878)
Bumps expat from `2.4.8` -> `2.6.2`
* Currently unreviewed, but done for now, given the amount of CMake related fixes listed in [the changelog](https://github.com/libexpat/libexpat/blob/R_2_6_2/expat/Changes).

Switch to building Expat with CMake, instead of Autotools.

🚀 fanquake merged a pull request: "ci: use Clang 16 for Valgrind"
(https://github.com/bitcoin/bitcoin/pull/29848)

💬 laanwj commented on pull request "build: add `-Wundef`":
(https://github.com/bitcoin/bitcoin/pull/29876#issuecomment-2056868984)
Concept ACK, issues with misspelled or otherwise missing defines can be really sneaky (and have caused some hard to find bugs in the past), this ia good check to have.

💬 instagibbs commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#issuecomment-2056882691)
ready for review

💬 theStack commented on pull request "test: p2p: add test for rejected tx request logic (`m_recent_rejects` filter)":
(https://github.com/bitcoin/bitcoin/pull/29827#discussion_r1565820341)
Good idea, done.

💬 theStack commented on pull request "test: p2p: add test for rejected tx request logic (`m_recent_rejects` filter)":
(https://github.com/bitcoin/bitcoin/pull/29827#discussion_r1565820757)
> Nice catch! I wonder if a more wholistic way to avoid unintentionally creating tx packages is to gather all the (confirmed only) UTXOs at the beginning? e.g. [this](https://github.com/glozow/bitcoin/commits/2024-04-fill-mempool-29827/), but feel free to ignore

That seems like a good approach to avoid similar problems popping up in the future 👌 I've cherry-picked your commit (with small conflict resolutions to have it introduced before the new test commit).

Btw, other approaches I've
...

💬 theStack commented on pull request "test: p2p: add test for rejected tx request logic (`m_recent_rejects` filter)":
(https://github.com/bitcoin/bitcoin/pull/29827#discussion_r1565822381)
Thanks, done.

💬 maflcko commented on pull request "build: Fix false positive `CHECK_ATOMIC` test":
(https://github.com/bitcoin/bitcoin/pull/29859#issuecomment-2056902062)
> On the master branch @ [0de63b8](https://github.com/bitcoin/bitcoin/commit/0de63b8b46eff5cda85b4950062703324ba65a80), a building `bitcoind` with clang-15 for `i686-pc-linux-gnu` fails to link:

Can you add exact steps to reproduce, ideally starting from a fresh install of the operating system?