📝 maflcko reopened a pull request: "precalculate SipHash constant salt XORs"
(https://github.com/bitcoin/bitcoin/pull/30442)
This change is part of [[IBD] - Tracking PR for speeding up Initial Block Download](https://github.com/bitcoin/bitcoin/pull/32043)
### Summary
The in-memory representation of the UTXO set uses (salted) [SipHash](https://github.com/bitcoin/bitcoin/blob/master/src/coins.h#L226) for avoiding key collision attacks.
Hashing `uint256` keys is performed frequently throughout the codebase. Previously, specialized optimizations existed as standalone functions (`SipHashUint256` and `SipHashUint25
...
(https://github.com/bitcoin/bitcoin/pull/30442)
This change is part of [[IBD] - Tracking PR for speeding up Initial Block Download](https://github.com/bitcoin/bitcoin/pull/32043)
### Summary
The in-memory representation of the UTXO set uses (salted) [SipHash](https://github.com/bitcoin/bitcoin/blob/master/src/coins.h#L226) for avoiding key collision attacks.
Hashing `uint256` keys is performed frequently throughout the codebase. Previously, specialized optimizations existed as standalone functions (`SipHashUint256` and `SipHashUint25
...
📝 maflcko opened a pull request: "clang-format: Set InsertNewlineAtEOF: true"
(https://github.com/bitcoin/bitcoin/pull/33896)
Now that the minimum supported clang version is 17, the `InsertNewlineAtEOF` setting can be set to `true` in the clang-format file. (https://releases.llvm.org/16.0.0/tools/clang/docs/ClangFormatStyleOptions.html#insertnewlineateof)
This is in line with the already existing newline linter. Can be tested via:
```
truncate -s -1 src/init.cpp
git diff
# Should fail:
cargo run --manifest-path ./test/lint/test_runner/Cargo.toml -- --lint=trailing_newline
# Restore newline:
git diff -U0
...
(https://github.com/bitcoin/bitcoin/pull/33896)
Now that the minimum supported clang version is 17, the `InsertNewlineAtEOF` setting can be set to `true` in the clang-format file. (https://releases.llvm.org/16.0.0/tools/clang/docs/ClangFormatStyleOptions.html#insertnewlineateof)
This is in line with the already existing newline linter. Can be tested via:
```
truncate -s -1 src/init.cpp
git diff
# Should fail:
cargo run --manifest-path ./test/lint/test_runner/Cargo.toml -- --lint=trailing_newline
# Restore newline:
git diff -U0
...
💬 optout21 commented on pull request "refactor: remove incorrect lifetimebounds":
(https://github.com/bitcoin/bitcoin/pull/33870#discussion_r2536712096)
Yes, a similar check could be done in the beginning, when `tip` is retrieved. But prob. outside of the scope of this PR.
(https://github.com/bitcoin/bitcoin/pull/33870#discussion_r2536712096)
Yes, a similar check could be done in the beginning, when `tip` is retrieved. But prob. outside of the scope of this PR.
💬 maflcko commented on pull request "guix: update per-host disk-space estimates in build gate":
(https://github.com/bitcoin/bitcoin/pull/33889#discussion_r2536789722)
> I did notice discrepancies for darwin and mingw though, which I've attempted to correct for.
That is fine, but you'll have to properly explain the changes.
https://github.com/kevkevinpal/bitcoin/issues/200 could be an oversight where someone ran the build twice by accident?
(https://github.com/bitcoin/bitcoin/pull/33889#discussion_r2536789722)
> I did notice discrepancies for darwin and mingw though, which I've attempted to correct for.
That is fine, but you'll have to properly explain the changes.
https://github.com/kevkevinpal/bitcoin/issues/200 could be an oversight where someone ran the build twice by accident?
💬 maflcko commented on pull request "test, refactor: Embedded ASMap [1/3]: Selected minor preparatory work":
(https://github.com/bitcoin/bitcoin/pull/33026#discussion_r2536798319)
The call should be fully optimized out anyway, no?
(https://github.com/bitcoin/bitcoin/pull/33026#discussion_r2536798319)
The call should be fully optimized out anyway, no?
💬 Sjors commented on pull request "mining: add getCoinbase()":
(https://github.com/bitcoin/bitcoin/pull/33819#issuecomment-3546154209)
> OTOH, overhauling NewTemplate shouldn't be a blocker for supporting sv2 efficiently, so Approach ACK for this PR in the meantime.
Thanks, I opened https://github.com/stratum-mining/sv2-spec/issues/167 to continue that discussion.
>> replacing m_options.coinbase_output_script with a vector.
See https://github.com/bitcoin/bitcoin/pull/33890
(https://github.com/bitcoin/bitcoin/pull/33819#issuecomment-3546154209)
> OTOH, overhauling NewTemplate shouldn't be a blocker for supporting sv2 efficiently, so Approach ACK for this PR in the meantime.
Thanks, I opened https://github.com/stratum-mining/sv2-spec/issues/167 to continue that discussion.
>> replacing m_options.coinbase_output_script with a vector.
See https://github.com/bitcoin/bitcoin/pull/33890
💬 Sjors commented on pull request "mining: add getCoinbase()":
(https://github.com/bitcoin/bitcoin/pull/33819#discussion_r2536880741)
Done in the latest push.
(https://github.com/bitcoin/bitcoin/pull/33819#discussion_r2536880741)
Done in the latest push.
💬 Sjors commented on pull request "mining: add getCoinbase()":
(https://github.com/bitcoin/bitcoin/pull/33819#issuecomment-3546216186)
I slightly simplified `ExtractCoinbaseTemplate` by having it assume that the dummy is always the first output. Also TIL about `| std::views::drop(1)`.
(https://github.com/bitcoin/bitcoin/pull/33819#issuecomment-3546216186)
I slightly simplified `ExtractCoinbaseTemplate` by having it assume that the dummy is always the first output. Also TIL about `| std::views::drop(1)`.
💬 janb84 commented on pull request "guix: update per-host disk-space estimates in build gate":
(https://github.com/bitcoin/bitcoin/pull/33889#discussion_r2536950754)
It was my remark, and I was still researching it. My Guix build is using GiB than before, but it's not related to 33181. I noticed it on that PR but a master compile (sans 33181) was also consuming more space than before.
(https://github.com/bitcoin/bitcoin/pull/33889#discussion_r2536950754)
It was my remark, and I was still researching it. My Guix build is using GiB than before, but it's not related to 33181. I noticed it on that PR but a master compile (sans 33181) was also consuming more space than before.
💬 fanquake commented on pull request "depends: update xcb-util packages to latest versions":
(https://github.com/bitcoin/bitcoin/pull/33851#issuecomment-3546351254)
Guix Build (x86_64):
```bash
3a70d632167f9e57328d9b8373a5eb9936428148feddf5bc1f9834f412ea4248 guix-build-daafaef87fee/output/aarch64-linux-gnu/SHA256SUMS.part
ca1c7ad00632d0ee3fdfad498a7c6cb5246240a9e49c89bee508355ee7dbb832 guix-build-daafaef87fee/output/aarch64-linux-gnu/bitcoin-daafaef87fee-aarch64-linux-gnu-debug.tar.gz
a5a5c5a22159a912f573915727b5a4cdbb1b270fdf099f8b465c3ef940496e20 guix-build-daafaef87fee/output/aarch64-linux-gnu/bitcoin-daafaef87fee-aarch64-linux-gnu.tar.gz
a660c55
...
(https://github.com/bitcoin/bitcoin/pull/33851#issuecomment-3546351254)
Guix Build (x86_64):
```bash
3a70d632167f9e57328d9b8373a5eb9936428148feddf5bc1f9834f412ea4248 guix-build-daafaef87fee/output/aarch64-linux-gnu/SHA256SUMS.part
ca1c7ad00632d0ee3fdfad498a7c6cb5246240a9e49c89bee508355ee7dbb832 guix-build-daafaef87fee/output/aarch64-linux-gnu/bitcoin-daafaef87fee-aarch64-linux-gnu-debug.tar.gz
a5a5c5a22159a912f573915727b5a4cdbb1b270fdf099f8b465c3ef940496e20 guix-build-daafaef87fee/output/aarch64-linux-gnu/bitcoin-daafaef87fee-aarch64-linux-gnu.tar.gz
a660c55
...
💬 laanwj commented on issue "guix build fails on RISC-V due to python-py-cpuinfo test failure":
(https://github.com/bitcoin/bitcoin/issues/33873#issuecomment-3546355023)
> It builds fine for me on an instance from the same provider:
Okay, now i'm confused. What operating system? Also debian forky/sid?
It could be that they got me a faulty server? i can't imagine having done anything wrong here, just followed basic GUIX build steps.
(https://github.com/bitcoin/bitcoin/issues/33873#issuecomment-3546355023)
> It builds fine for me on an instance from the same provider:
Okay, now i'm confused. What operating system? Also debian forky/sid?
It could be that they got me a faulty server? i can't imagine having done anything wrong here, just followed basic GUIX build steps.
💬 laanwj commented on issue "guix build fails on RISC-V due to python-py-cpuinfo test failure":
(https://github.com/bitcoin/bitcoin/issues/33873#issuecomment-3546364000)
Looks like it stopped an error in the `-rust-1.65.0.drv.gz` build this night. The "faulty server" hypothesis definitely disproven 😄 Why do i always have this little luck with RISC-V devices lol.
```
...
patch-cargo-checksums: generate-checksums for vendor/winapi
patch-cargo-checksums: generate-checksums for vendor/windows-sys-0.28.0
patch-cargo-checksums: generate-checksums for vendor/windows-sys
patch-cargo-checksums: generate-checksums for vendor/windows_aarch64_msvc-0.28.0
Fatal glibc error
...
(https://github.com/bitcoin/bitcoin/issues/33873#issuecomment-3546364000)
Looks like it stopped an error in the `-rust-1.65.0.drv.gz` build this night. The "faulty server" hypothesis definitely disproven 😄 Why do i always have this little luck with RISC-V devices lol.
```
...
patch-cargo-checksums: generate-checksums for vendor/winapi
patch-cargo-checksums: generate-checksums for vendor/windows-sys-0.28.0
patch-cargo-checksums: generate-checksums for vendor/windows-sys
patch-cargo-checksums: generate-checksums for vendor/windows_aarch64_msvc-0.28.0
Fatal glibc error
...
🚀 fanquake merged a pull request: "net: Remove unused `local_socket_bytes` variable in `CConnman::GetAddresses()`"
(https://github.com/bitcoin/bitcoin/pull/33894)
(https://github.com/bitcoin/bitcoin/pull/33894)
💬 hodlinator commented on pull request "build: Embedded ASMap [3/3]: Build binary dump header file":
(https://github.com/bitcoin/bitcoin/pull/28792#discussion_r2537150160)
Related: #33896
(https://github.com/bitcoin/bitcoin/pull/28792#discussion_r2537150160)
Related: #33896
💬 hebasto commented on issue "guix build fails on RISC-V due to python-py-cpuinfo test failure":
(https://github.com/bitcoin/bitcoin/issues/33873#issuecomment-3546510385)
> > It builds fine for me on an instance from the same provider:
>
> Okay, now i'm confused. What operating system? Also debian forky/sid?
Yes, debian forky/sid.
(https://github.com/bitcoin/bitcoin/issues/33873#issuecomment-3546510385)
> > It builds fine for me on an instance from the same provider:
>
> Okay, now i'm confused. What operating system? Also debian forky/sid?
Yes, debian forky/sid.
👍 hodlinator approved a pull request: "clang-format: Set InsertNewlineAtEOF: true"
(https://github.com/bitcoin/bitcoin/pull/33896#pullrequestreview-3476584168)
ACK fa1bf6818f0910f997e5235a197ff51f2e18780d
Makes sense for this project to conform to the UNIXy style of ending all lines with `\n`.
Previous setting of having it `false` comes from 13f36c020f0329b5e975282b45292fdf2a495e31 which just added a bunch of newer settings to src/.clang-format with default clang-format values, rather than being a conscious decision by this project.
Have recently been discussing how GitHub and Git also seem to agree on this: https://github.com/bitcoin/bitcoin/pull/2
...
(https://github.com/bitcoin/bitcoin/pull/33896#pullrequestreview-3476584168)
ACK fa1bf6818f0910f997e5235a197ff51f2e18780d
Makes sense for this project to conform to the UNIXy style of ending all lines with `\n`.
Previous setting of having it `false` comes from 13f36c020f0329b5e975282b45292fdf2a495e31 which just added a bunch of newer settings to src/.clang-format with default clang-format values, rather than being a conscious decision by this project.
Have recently been discussing how GitHub and Git also seem to agree on this: https://github.com/bitcoin/bitcoin/pull/2
...
💬 Sjors commented on issue "IPC via TCP Sockets":
(https://github.com/bitcoin/bitcoin/issues/32802#issuecomment-3546616865)
RPC connections are not encrypted either and I'm not aware of any plans for changing that.
Maybe virtio socket support would be the easiest approach for getting Docker setups to work? That might be useful for RPC too.
(https://github.com/bitcoin/bitcoin/issues/32802#issuecomment-3546616865)
RPC connections are not encrypted either and I'm not aware of any plans for changing that.
Maybe virtio socket support would be the easiest approach for getting Docker setups to work? That might be useful for RPC too.
💬 l0rinc commented on pull request "clang-format: Set InsertNewlineAtEOF: true":
(https://github.com/bitcoin/bitcoin/pull/33896#discussion_r2537251411)
llvm@17 has a `KeepEmptyLinesAtEOF: false` as well - added exactly to avoid the problem we're trying to avoid here: https://github.com/llvm/llvm-project/issues/56054
Should we make it explicit here?
```patch
diff --git a/src/.clang-format b/src/.clang-format
--- a/src/.clang-format (revision 9d5bad11b6cbf50c776f9e9a583ac1fb79271fe8)
+++ b/src/.clang-format (date 1763460129408)
@@ -123,7 +123,7 @@
IndentWidth: 4
IndentWrappedFunctionNames: false
InsertBraces: false
-InsertNewlineAtEOF:
...
(https://github.com/bitcoin/bitcoin/pull/33896#discussion_r2537251411)
llvm@17 has a `KeepEmptyLinesAtEOF: false` as well - added exactly to avoid the problem we're trying to avoid here: https://github.com/llvm/llvm-project/issues/56054
Should we make it explicit here?
```patch
diff --git a/src/.clang-format b/src/.clang-format
--- a/src/.clang-format (revision 9d5bad11b6cbf50c776f9e9a583ac1fb79271fe8)
+++ b/src/.clang-format (date 1763460129408)
@@ -123,7 +123,7 @@
IndentWidth: 4
IndentWrappedFunctionNames: false
InsertBraces: false
-InsertNewlineAtEOF:
...
💬 fanquake commented on pull request "build: Embedded ASMap [3/3]: Build binary dump header file":
(https://github.com/bitcoin/bitcoin/pull/28792#discussion_r2537289781)
> a new creation process
What is the "new creation process":
* Who is involved (and has the permissions needed to merge into the required repos).
* How many total people are required?
* Can it happen at anytime, or does it require coordination days ahead of time?
* What should happen if the process creates data that isn't usable?
(https://github.com/bitcoin/bitcoin/pull/28792#discussion_r2537289781)
> a new creation process
What is the "new creation process":
* Who is involved (and has the permissions needed to merge into the required repos).
* How many total people are required?
* Can it happen at anytime, or does it require coordination days ahead of time?
* What should happen if the process creates data that isn't usable?
💬 rustaceanrob commented on pull request "mining: add getCoinbase()":
(https://github.com/bitcoin/bitcoin/pull/33819#issuecomment-3546665825)
> moreover, I'm not sure what to expect in case getCoinbase doesn't exist? I guess capnp-rpc would bubble up some kind of error? does @rustaceanrob know?
I would assume a new client requesting `getCoinbase` of an old server would result in the [`ErrorKind::Unimplemented`](https://docs.rs/capnp/0.23.0/capnp/enum.ErrorKind.html). In the case of this response then a `getCoinbaseTx` could be used as a fallback.
As far as updating the `capnp` files in the Rust types, I can do so on a developmen
...
(https://github.com/bitcoin/bitcoin/pull/33819#issuecomment-3546665825)
> moreover, I'm not sure what to expect in case getCoinbase doesn't exist? I guess capnp-rpc would bubble up some kind of error? does @rustaceanrob know?
I would assume a new client requesting `getCoinbase` of an old server would result in the [`ErrorKind::Unimplemented`](https://docs.rs/capnp/0.23.0/capnp/enum.ErrorKind.html). In the case of this response then a `getCoinbaseTx` could be used as a fallback.
As far as updating the `capnp` files in the Rust types, I can do so on a developmen
...