Bitcoin Core Github
44 subscribers
122K links
Download Telegram
🚀 fanquake merged a pull request: "doc, rpc: Release notes and follow-ups for #29612"
(https://github.com/bitcoin/bitcoin/pull/30167)
💬 fanquake commented on issue "make cov fails with lcov-2":
(https://github.com/bitcoin/bitcoin/issues/28468#issuecomment-2144736523)
@maflcko was #30192 enough to close this?
💬 dergoegge commented on pull request "fuzz: Make FuzzedSock fuzz friendlier":
(https://github.com/bitcoin/bitcoin/pull/30211#discussion_r1624125082)
> did you encounter a case where the current/previous implementation that uses ConsumeBytes() is "unstable" or had problems in some other way?

No I just found this while investigating the peek issue (which was the main blocker).
💬 dergoegge commented on pull request "fuzz: Make FuzzedSock fuzz friendlier":
(https://github.com/bitcoin/bitcoin/pull/30211#discussion_r1624126459)
Fixed.
💬 dergoegge commented on pull request "fuzz: Make FuzzedSock fuzz friendlier":
(https://github.com/bitcoin/bitcoin/pull/30211#discussion_r1624128417)
Added some comments
💬 dergoegge commented on pull request "fuzz: Make FuzzedSock fuzz friendlier":
(https://github.com/bitcoin/bitcoin/pull/30211#discussion_r1624133280)
I could only find the following in the docs and based the changes here on that.

```
This flag causes the receive operation to return data from
the beginning of the receive queue without removing that
data from the queue. Thus, a subsequent receive call will
return the same data.
```

We could make it so that a peek read only returns one byte and a subsequent normal read return N bytes where the first byte corresponds to the peek byte. But I'm not sure if that would change much for our
...
💬 hebasto commented on pull request "feefrac: 128-bit multiply support in MSVC":
(https://github.com/bitcoin/bitcoin/pull/29758#discussion_r1624155073)
Why do this `Mul`'s return type and `MulFallback`'s one differ?
💬 maflcko commented on pull request "build: Fix building `fuzz` binary on on SunOS / illumos":
(https://github.com/bitcoin/bitcoin/pull/30216#issuecomment-2144798700)
ACK 3299abce948f205bb1354993614b669189f9b89f
💬 maflcko commented on pull request "depends: Update Boost download link":
(https://github.com/bitcoin/bitcoin/pull/30217#issuecomment-2144806703)
ACK ffbc173ca1ed6b93de8bf3f88b8aed0743f4916c
💬 hebasto commented on pull request "depends: Use `CC_FOR_BUILD` for `config.guess `":
(https://github.com/bitcoin/bitcoin/pull/29963#issuecomment-2144810134)
> Is this still an issue given recent CMake changes?

Yes. Tested with the master branch @ 80bdd4b6beb878c95478b5623c9f9ff0b948ad57.

And this PR still fixes it.
💬 maflcko commented on pull request "Lint: support running individual rust linters and improve subtree exclusion":
(https://github.com/bitcoin/bitcoin/pull/29965#issuecomment-2144818621)
Any thoughts on splitting the first commit out into a separate pull? Seems to be a requested feature by three people so far, but I am not sure about bundling it for review with the other changes here.
💬 hebasto commented on pull request "guix: show `*_FLAGS` variables in pre-build output":
(https://github.com/bitcoin/bitcoin/pull/30185#discussion_r1624187207)
Why `ADDITIONAL_GUIX_TIMEMACHINE_FLAGS` skipped? Despite that it was used before this point, it can be useful for users and consistency.
💬 sipa commented on pull request "feefrac: 128-bit multiply support in MSVC":
(https://github.com/bitcoin/bitcoin/pull/29758#discussion_r1624220296)
That's the point; they represent the 96-bit integer product in a different way.
💬 fanquake commented on pull request "guix: show `*_FLAGS` variables in pre-build output":
(https://github.com/bitcoin/bitcoin/pull/30185#discussion_r1624227288)
Sure. Added that as well
💬 laanwj commented on pull request "doc: JSON-RPC request Content-Type is application/json":
(https://github.com/bitcoin/bitcoin/pull/30215#issuecomment-2144912650)
ACK 3c08e11c3ea4499e8d20609e2417cac859b3e98e
This is an obvious documentation improvement.

> nit: Technically, I think the trailing semicolon application/json;

Good catch. Content type doesn't need to be suffixed with `;`. It's pretty uncommon, even. According to the `curl` doc the semicolon is a special syntax for providing an empty header, but not required otherwise, eg the example they give is:
```bash
curl -H "X-First-Name: Joe" http://example.com/
```
💬 fanquake commented on issue "clang-format returns error":
(https://github.com/bitcoin/bitcoin/issues/29214#issuecomment-2144925933)
@mzumsande I'm guessing you switched to using a newer Clang here? Note that our minimum required Clang is also now 15, which would "solve" the original issue, however not the `RequiresExpressionIndentation` issue, but my suggestion would be to use clang-16+ if possible.
👍 hebasto approved a pull request: "guix: show `*_FLAGS` variables in pre-build output"
(https://github.com/bitcoin/bitcoin/pull/30185#pullrequestreview-2093531787)
ACK 5f2c1d84e37697f4f8a20e3c12f37bba71b3c2a6.
💬 fanquake commented on issue "Erlay Project Tracking":
(https://github.com/bitcoin/bitcoin/issues/28646#issuecomment-2144932529)
I've updated the PR to review, at the top of the issue here. @sr-gi are you going to open your own tracking issue going forward? That'll likely be easier to keep up to date.
💬 sipa commented on pull request "feefrac: 128-bit multiply support in MSVC":
(https://github.com/bitcoin/bitcoin/pull/29758#discussion_r1624277101)
It would be possible to use `std::pair<int32_t, uint64_t>` here too (as in, that type would be big enough to store the result), but `_mul128` returns 64-bit results, which match the register size of x86_64, so no conversion is needed.
🚀 fanquake merged a pull request: "build: Fix building `fuzz` binary on on SunOS / illumos"
(https://github.com/bitcoin/bitcoin/pull/30216)