💬 glozow commented on pull request "p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#issuecomment-2056422724)
Update since my last comment https://github.com/bitcoin/bitcoin/pull/28970#issuecomment-2025812846
Running for 20 days:
- My node made 5218 package evaluation attempts (x2 = 10,436 transactions). That's ~260 attempts per day.
- Of those attempts, 2260 were successful (x2 = 4520 transactions). That's ~43% acceptance rate.
- Some were repeats. I have 4202 unique txids accepted through package evaluation. I don't know if they were repeated packages or not.
- Of the accepted transactions, 3232
...

🤔 glozow reviewed a pull request: "rpc, bugfix: Enforce maximum value for setmocktime"
(https://github.com/bitcoin/bitcoin/pull/29869#pullrequestreview-2000639795)
ACK c2e0489b7125cceaeef355fc274dd8988822fff4

💬 glozow commented on pull request "rpc, bugfix: Enforce maximum value for setmocktime":
(https://github.com/bitcoin/bitcoin/pull/29869#discussion_r1565542030)
Maybe add a
```
assert_raises_rpc_error(-8, "Mocktime must be in the range [0, 9223372036], not 9223372037.", self.nodes[0].setmocktime, 9223372037)
```
?

💬 fanquake commented on pull request "[WIP] build: remove need to test for endianness":
(https://github.com/bitcoin/bitcoin/pull/29852#issuecomment-2056534000)
> However, the changes in MSVC generated assembly code look quite significant.
> Before stacking another performance deterioration change on top of the pile

Isn't that because optimisations haven't been turned on? Otherwise, can you provide a concrete example of what you're talking about.

📝 maflcko opened a pull request: " test: Add missing Assert(mock_time_in >= 0s) to SetMockTime "
(https://github.com/bitcoin/bitcoin/pull/29872)
Seems odd to have the assert in the *deprecated* function, but not in the other.

Fix this by adding it to the other, and by inlining the deprecated one.

Also, use chrono type for the global mocktime variable.

📝 glozow opened a pull request: "policy: restrict all TRUC (v3) transactions to 25KvB"
(https://github.com/bitcoin/bitcoin/pull/29873)
Opening for discussion / conceptual review.

We like the idea of a smaller maximum transaction size because:
- It lowers potential replacement cost (i.e. harder to do Rule 3 pinning via gigantic transaction)
- They are easier to bin-pack in block template production
- They equate to a tighter memory limit in data structures that are bounded by a number of transactions (e.g. orphanage and vExtraTxnForCompact). For example, the current memory bounds for orphanage is 100KvB * 100 = 40MB, and g
...

💬 glozow commented on pull request "policy: restrict all TRUC (v3) transactions to 25KvB":
(https://github.com/bitcoin/bitcoin/pull/29873#issuecomment-2056586768)
cc @sdaftuar @instagibbs @t-bast

💬 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)