💬 glozow 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_r1565443465)
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
(https://github.com/bitcoin/bitcoin/pull/29827#discussion_r1565443465)
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
🤔 glozow reviewed a pull request: "test: p2p: add test for rejected tx request logic (`m_recent_rejects` filter)"
(https://github.com/bitcoin/bitcoin/pull/29827#pullrequestreview-2000325181)
CI failure looks related to the lines added here (I haven't debugged though)
(https://github.com/bitcoin/bitcoin/pull/29827#pullrequestreview-2000325181)
CI failure looks related to the lines added here (I haven't debugged though)
💬 glozow 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_r1565446681)
`peer.wait_for_getdata()` ?
(https://github.com/bitcoin/bitcoin/pull/29827#discussion_r1565446681)
`peer.wait_for_getdata()` ?
💬 maflcko commented on pull request "build: Fix false positive `CHECK_ATOMIC` test":
(https://github.com/bitcoin/bitcoin/pull/29859#issuecomment-2056362145)
For reference, on a fresh Ubuntu 24.04 Noble (which should be identical to the current CI config), it fails:
```
apt update && apt install clang-18 g++-multilib -y
clang++-18 -std=c++20 test.cpp -m32
/usr/bin/ld: /tmp/test-9da16c.o: in function `std::__atomic_float<double>::store(double, std::memory_order)':
test.cpp:(.text._ZNSt14__atomic_floatIdE5storeEdSt12memory_order[_ZNSt14__atomic_floatIdE5storeEdSt12memory_order]+0x5a): undefined reference to `__atomic_store'
/usr/bin/ld: /tmp
...
(https://github.com/bitcoin/bitcoin/pull/29859#issuecomment-2056362145)
For reference, on a fresh Ubuntu 24.04 Noble (which should be identical to the current CI config), it fails:
```
apt update && apt install clang-18 g++-multilib -y
clang++-18 -std=c++20 test.cpp -m32
/usr/bin/ld: /tmp/test-9da16c.o: in function `std::__atomic_float<double>::store(double, std::memory_order)':
test.cpp:(.text._ZNSt14__atomic_floatIdE5storeEdSt12memory_order[_ZNSt14__atomic_floatIdE5storeEdSt12memory_order]+0x5a): undefined reference to `__atomic_store'
/usr/bin/ld: /tmp
...
💬 laanwj commented on pull request "guix: replace GCC unaligned VMOV patch with binutils patch":
(https://github.com/bitcoin/bitcoin/pull/29846#issuecomment-2056372622)
> ```
> objdump -D bin/*.exe | rg "vmova|vmovdqa|vmovaps|vmovapd|vmovdqa64|vmovdqa32"
> 141b8be20: c5 f8 28 1a vmovaps (%rdx), %xmm3
> 1420564b3: c5 79 29 36 vmovapd %xmm14, (%rsi)
> 1403060f3: c5 79 29 36 vmovapd %xmm14, (%rsi)
> 140792b13: c5 79 29 36 vmovapd %xmm14, (%rsi)
> 140cb0693: c5 79 29 36 vmovapd %xmm14, (%rsi)
> 1415ea0f3: c5 79 29 36 vmovapd %xmm14, (%rsi)>
```
i've re-r
...
(https://github.com/bitcoin/bitcoin/pull/29846#issuecomment-2056372622)
> ```
> objdump -D bin/*.exe | rg "vmova|vmovdqa|vmovaps|vmovapd|vmovdqa64|vmovdqa32"
> 141b8be20: c5 f8 28 1a vmovaps (%rdx), %xmm3
> 1420564b3: c5 79 29 36 vmovapd %xmm14, (%rsi)
> 1403060f3: c5 79 29 36 vmovapd %xmm14, (%rsi)
> 140792b13: c5 79 29 36 vmovapd %xmm14, (%rsi)
> 140cb0693: c5 79 29 36 vmovapd %xmm14, (%rsi)
> 1415ea0f3: c5 79 29 36 vmovapd %xmm14, (%rsi)>
```
i've re-r
...
✅ maflcko closed an issue: "Errors in block header at FlatFilePos"
(https://github.com/bitcoin/bitcoin/issues/29861)
(https://github.com/bitcoin/bitcoin/issues/29861)
👍 brunoerg approved a pull request: "rpc, bugfix: Enforce maximum value for setmocktime"
(https://github.com/bitcoin/bitcoin/pull/29869#pullrequestreview-2000550197)
crACK c2e0489b7125cceaeef355fc274dd8988822fff4
(https://github.com/bitcoin/bitcoin/pull/29869#pullrequestreview-2000550197)
crACK c2e0489b7125cceaeef355fc274dd8988822fff4
💬 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
...
(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
(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)
```
?
(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.
(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.
(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
...
(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
(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.
(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.
(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
(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?
(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)
(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">
(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">