Bitcoin Core Github
44 subscribers
121K links
Download Telegram
๐Ÿ“ 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)
๐Ÿ’ฌ 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.