Bitcoin Core Github
44 subscribers
121K links
Download Telegram
💬 maflcko commented on issue "`test/streams_tests.cpp` fails to compile on SunOS / illumos":
(https://github.com/bitcoin/bitcoin/issues/29884#issuecomment-2058434577)
Using `char` is not allowed in serialization code, because the sign of `char` is unknown/unclear. However, on some systems, `int8_t==char`, which means that `signed char` has no serialization helper.

This could be fixed in the serialization code by replacing `int8_t` by a concept of `!CharNotInt8 || std::same_as<T, signed char>`. However, my preference would be to completely avoid `signed char` and just use `int8_t` in the serialization code. That is, fix the test to use `int8_t`.
💬 laanwj commented on pull request "build: special instruction check script":
(https://github.com/bitcoin/bitcoin/pull/26693#issuecomment-2058437328)
BTW: along with some capstone magic like #29874, and a bare-bones DWARF (or linker map) parser to get function ranges, it would be possible to make a check like this that runs in the final binary, checking that special instructions only appear in allow-listed functions.
💬 maflcko commented on pull request "refactor/test: add a few more base32/64 calculation corner cases":
(https://github.com/bitcoin/bitcoin/pull/29847#discussion_r1566874051)
My comment applies to all changes in this file. I am just trying to explain why such style changes do not make sense.
💬 laanwj commented on pull request "refactor/test: add a few more base32/64 calculation corner cases":
(https://github.com/bitcoin/bitcoin/pull/29847#issuecomment-2058447165)
Take it how you wish, i was just trying to be helpful, not patronizing.
💬 maflcko commented on pull request "refactor/test: add a few more base32/64 calculation corner cases":
(https://github.com/bitcoin/bitcoin/pull/29847#issuecomment-2058449206)
Please understand that this software project requires review of all code changes. It is not a typical software project that auto-merges any change as soon as the CI is green.

When writing a patch as a pull request, please consider how reviewers are supposed to review it and how easy it will be for them to follow the changes and their motivation.
💬 glozow commented on pull request "p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1566889900)
Yes exactly, this is to bound computation (imagine if somebody sent us 100 fake orphans descended from 1 transaction and we processed them all here). My idea was to do something similar to regular orphan processing, where we have a work queue and limit to 1 item per `ProcessMessages`.

There is no work queue here, though, and we drop the parent as soon as we try 1 (pass or fail). At coredev, we discussed adding a work queue for 1p1c as well. However, since it involves finding a way to store th
...
💬 fanquake commented on issue "ci: feature_proxy failing in MSVC job":
(https://github.com/bitcoin/bitcoin/issues/29090#issuecomment-2058480638)
https://github.com/bitcoin/bitcoin/actions/runs/8690581373/job/23830847391:
```bash
test 2024-04-15T15:06:28.426000Z TestFramework.node3 (DEBUG): Stopping node
test 2024-04-15T15:06:28.426000Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
File "D:\a\bitcoin\bitcoin\test\functional\test_framework\test_framework.py", line 565, in start_nodes
node.
...
💬 glozow commented on pull request "p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1566907728)
Yes I think we could use either `m_recent_rejects_reconsiderable` and `m_recent_rejects` right now to get the same behavior.

I suppose one mild benefit of using `m_recent_rejects_reconsiderable` is that our `m_recent_rejects` bloom filter churns less frequently.

The other benefit is extensibility in the future. In more general ancestor package relay, we could reject a parent+child for being too low feerate, but later accept it as parent+child+grandchild (where the grandchild is very high f
...
💬 maflcko commented on issue "ci: feature_proxy failing in MSVC job":
(https://github.com/bitcoin/bitcoin/issues/29090#issuecomment-2058490446)
Maybe related to the races seen in https://github.com/bitcoin/bitcoin/issues/29871#issuecomment-2057480445 ?
📝 fanquake opened a pull request: "doc: archive 27.0 release notes"
(https://github.com/bitcoin/bitcoin/pull/29886)
💬 laanwj commented on pull request "guix: replace GCC unaligned VMOV patch with binutils patch":
(https://github.com/bitcoin/bitcoin/pull/29846#issuecomment-2058516921)
- I get the same output (for win32) as @hebasto
- I've checked the resulting binaries for instructions that enforce aligned memory accesses, and didn't find any dangerous ones

Code review ACK a0dc2ebcda9e33aa5320221cd4ea371f84d221fd
🤔 glozow reviewed a pull request: "fuzz: explicitly cap the vsize of RBFs for diagram checks"
(https://github.com/bitcoin/bitcoin/pull/29879#pullrequestreview-2002964571)
Approach of breaking when adding another tx would overflow seems fine to me. Did you forget to squash?
fanquake closed an issue: "Oksang"
(https://github.com/bitcoin/bitcoin/issues/29887)
:lock: fanquake locked an issue: "Oksang"
(https://github.com/bitcoin/bitcoin/issues/29887)
💬 maflcko commented on pull request "test: Add missing Assert(mock_time_in >= 0s) to SetMockTime":
(https://github.com/bitcoin/bitcoin/pull/29872#discussion_r1566955575)
Both can be used. In this context they are exactly the same.
💬 fanquake commented on issue "Release schedule for 27.0":
(https://github.com/bitcoin/bitcoin/issues/29028#issuecomment-2058539546)
v27.0 has now been tagged: https://github.com/bitcoin/bitcoin/releases/tag/v27.0.
fanquake closed an issue: "Release schedule for 27.0"
(https://github.com/bitcoin/bitcoin/issues/29028)
💬 laanwj commented on pull request "doc: archive 27.0 release notes":
(https://github.com/bitcoin/bitcoin/pull/29886#issuecomment-2058543218)
ACK c08754971d207bd2b60ba9c4faf34396a97bbc26
No output for
```
git diff c08754971d207bd2b60ba9c4faf34396a97bbc26:doc/release-notes/release-notes-27.0.md v27.0:doc/release-notes.md
```