✅ cbradberry closed a pull request: "Convert Bitcoin source code to C#"
(https://github.com/bitcoin/bitcoin/pull/31143)
(https://github.com/bitcoin/bitcoin/pull/31143)
💬 davidgumberg commented on pull request "Don't zero-after-free `DataStream`: Faster IBD on some configurations":
(https://github.com/bitcoin/bitcoin/pull/30987#issuecomment-2434146916)
Given that others have been unable to reproduce the results that led me to open this PR, I am moving to draft until I better understand either what has gone wrong in my measurements or what setups reproduce the result I got.
(https://github.com/bitcoin/bitcoin/pull/30987#issuecomment-2434146916)
Given that others have been unable to reproduce the results that led me to open this PR, I am moving to draft until I better understand either what has gone wrong in my measurements or what setups reproduce the result I got.
📝 davidgumberg converted_to_draft a pull request: "Don't zero-after-free `DataStream`: Faster IBD on some configurations"
(https://github.com/bitcoin/bitcoin/pull/30987)
This PR modifies `DataStream`'s byte-vector `vch` to use the default allocator `std::allocator` rather than the `zero_after_free_allocator` which degrades performance greatly. The `zero_after_free_allocator` is identical to the default `std::allocator` except that it zeroes memory using `memory_cleanse()` before deallocating.
This PR also drops the `zero_after_free_allocator`, since this was only used by `DataStream` and `SerializeData`.
In my testing (n=2) on a Raspberry Pi 5 with 4GB of
...
(https://github.com/bitcoin/bitcoin/pull/30987)
This PR modifies `DataStream`'s byte-vector `vch` to use the default allocator `std::allocator` rather than the `zero_after_free_allocator` which degrades performance greatly. The `zero_after_free_allocator` is identical to the default `std::allocator` except that it zeroes memory using `memory_cleanse()` before deallocating.
This PR also drops the `zero_after_free_allocator`, since this was only used by `DataStream` and `SerializeData`.
In my testing (n=2) on a Raspberry Pi 5 with 4GB of
...
👍 rkrux approved a pull request: "doc: Make list of targets in depends README consistent"
(https://github.com/bitcoin/bitcoin/pull/31141#pullrequestreview-2391237673)
ACK a0c9595810c7d8bb17d8b5bea8d916db194b5239
(https://github.com/bitcoin/bitcoin/pull/31141#pullrequestreview-2391237673)
ACK a0c9595810c7d8bb17d8b5bea8d916db194b5239
👍 rkrux approved a pull request: "test: added test to assert TX decode rpc error on submitpackage rpc"
(https://github.com/bitcoin/bitcoin/pull/31139#pullrequestreview-2391315435)
tACK 4bdb78cfe76414227b749674ed660f4972fb6eda
Make, functional tests successful. Left a small suggestion.
(https://github.com/bitcoin/bitcoin/pull/31139#pullrequestreview-2391315435)
tACK 4bdb78cfe76414227b749674ed660f4972fb6eda
Make, functional tests successful. Left a small suggestion.
💬 rkrux commented on pull request "test: added test to assert TX decode rpc error on submitpackage rpc":
(https://github.com/bitcoin/bitcoin/pull/31139#discussion_r1814272127)
While I was going through this test after I checked out the branch in local, I realised that this assertion seems a bit out of context because of the comment on line 378. How about adding this assertion before line 378?
(https://github.com/bitcoin/bitcoin/pull/31139#discussion_r1814272127)
While I was going through this test after I checked out the branch in local, I realised that this assertion seems a bit out of context because of the comment on line 378. How about adding this assertion before line 378?
📝 fanquake locked a pull request: "Convert Bitcoin source code to C#"
(https://github.com/bitcoin/bitcoin/pull/31143)
---
For more details, open the [Copilot Workspace session](https://copilot-workspace.githubnext.com/bitcoin/bitcoin?shareId=ace74a54-f532-4441-bbd2-52fd31ad6e4a).
(https://github.com/bitcoin/bitcoin/pull/31143)
---
For more details, open the [Copilot Workspace session](https://copilot-workspace.githubnext.com/bitcoin/bitcoin?shareId=ace74a54-f532-4441-bbd2-52fd31ad6e4a).
💬 laanwj commented on pull request "rpc, cli: return "verificationprogress" of 1 when up to date":
(https://github.com/bitcoin/bitcoin/pull/31135#discussion_r1814289134)
it should probably only do the "round last bit" thing when it's actually within some small delta of 1, not like from 0.5 to 1
(https://github.com/bitcoin/bitcoin/pull/31135#discussion_r1814289134)
it should probably only do the "round last bit" thing when it's actually within some small delta of 1, not like from 0.5 to 1
📝 l0rinc opened a pull request: "optimization: pack util::Xor into 64 bit chunks instead of doing it byte-by-byte"
(https://github.com/bitcoin/bitcoin/pull/31144)
The recently merged [XOR obfuscation](https://github.com/bitcoin/bitcoin/pull/28207) is a non-trivial part of serialization, encountered during IBD and `reindex`:
<img width="500" alt="image" src="https://github.com/user-attachments/assets/6dc99dee-bda8-40d0-bdb0-2030982e0645">
----
In this PR, I've added a new test (comparing randomized inputs against a trivial implementation), updated the benchmark to validate a [more realistic scenario](https://github.com/bitcoin/bitcoin/blob/master/sr
...
(https://github.com/bitcoin/bitcoin/pull/31144)
The recently merged [XOR obfuscation](https://github.com/bitcoin/bitcoin/pull/28207) is a non-trivial part of serialization, encountered during IBD and `reindex`:
<img width="500" alt="image" src="https://github.com/user-attachments/assets/6dc99dee-bda8-40d0-bdb0-2030982e0645">
----
In this PR, I've added a new test (comparing randomized inputs against a trivial implementation), updated the benchmark to validate a [more realistic scenario](https://github.com/bitcoin/bitcoin/blob/master/sr
...
⚠️ laanwj opened an issue: "Cmake build system breaks with symbolic links"
(https://github.com/bitcoin/bitcoin/issues/31145)
If the `build` directory is a symbolic link, or when the entire source directory is symlinked (and we build from there), the current cmake build runs into trouble finding its own generated files.
To reproduce:
```
git clone --depth=1 https://github.com/bitcoin/bitcoin.git
mkdir build
cd bitcoin/
ln -s ../build .
cmake -Bbuild -DBUILD_GUI=ON
cd build
make
```
```
[ 37%] Building CXX object src/CMakeFiles/bitcoin_node.dir/validation.cpp.o
In file included from /data/src/tmp/bitcoi
...
(https://github.com/bitcoin/bitcoin/issues/31145)
If the `build` directory is a symbolic link, or when the entire source directory is symlinked (and we build from there), the current cmake build runs into trouble finding its own generated files.
To reproduce:
```
git clone --depth=1 https://github.com/bitcoin/bitcoin.git
mkdir build
cd bitcoin/
ln -s ../build .
cmake -Bbuild -DBUILD_GUI=ON
cd build
make
```
```
[ 37%] Building CXX object src/CMakeFiles/bitcoin_node.dir/validation.cpp.o
In file included from /data/src/tmp/bitcoi
...
💬 laanwj commented on issue "Remove BIP94 from regtest":
(https://github.com/bitcoin/bitcoin/issues/31137#issuecomment-2434584188)
Concept ACK. i think it's a good point that regtest, at least by default, should be as closest as possible to mainnet, not testnet.
(https://github.com/bitcoin/bitcoin/issues/31137#issuecomment-2434584188)
Concept ACK. i think it's a good point that regtest, at least by default, should be as closest as possible to mainnet, not testnet.
🚀 fanquake merged a pull request: "doc: Make list of targets in depends README consistent"
(https://github.com/bitcoin/bitcoin/pull/31141)
(https://github.com/bitcoin/bitcoin/pull/31141)
💬 fanquake commented on pull request "depends: bump miniupnpc to 2.2.8":
(https://github.com/bitcoin/bitcoin/pull/30301#issuecomment-2434608834)
> Or probably https://github.com/bitcoin/bitcoin/pull/31130?
If #31130 is merged, then yes, this is no-longer relevant. Drafted for now, given this also needs rebase.
(https://github.com/bitcoin/bitcoin/pull/30301#issuecomment-2434608834)
> Or probably https://github.com/bitcoin/bitcoin/pull/31130?
If #31130 is merged, then yes, this is no-longer relevant. Drafted for now, given this also needs rebase.
📝 fanquake converted_to_draft a pull request: "depends: bump miniupnpc to 2.2.8"
(https://github.com/bitcoin/bitcoin/pull/30301)
Drops two of our patches that have been merged upstream and adjusts the other to deal with recent changes.
Follow-up from #30283. I can't vouch for the upstream changes here.
(https://github.com/bitcoin/bitcoin/pull/30301)
Drops two of our patches that have been merged upstream and adjusts the other to deal with recent changes.
Follow-up from #30283. I can't vouch for the upstream changes here.
💬 brunoerg commented on pull request "test: added test to assert TX decode rpc error on submitpackage rpc":
(https://github.com/bitcoin/bitcoin/pull/31139#discussion_r1814519229)
I don't think just passing two empty strings is the right way of testing it because:
- it only plays with non-hex values and do not exercise the hex parsing.
- if the first tx is invalid, the second one has no effect.
(https://github.com/bitcoin/bitcoin/pull/31139#discussion_r1814519229)
I don't think just passing two empty strings is the right way of testing it because:
- it only plays with non-hex values and do not exercise the hex parsing.
- if the first tx is invalid, the second one has no effect.
💬 maflcko commented on pull request "optimization: pack util::Xor into 64 bit chunks instead of doing it byte-by-byte":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r1814518633)
I fail to see how this is not UB. This is identical to https://github.com/bitcoin/bitcoin/pull/30349#discussion_r1804480124
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r1814518633)
I fail to see how this is not UB. This is identical to https://github.com/bitcoin/bitcoin/pull/30349#discussion_r1804480124
💬 maflcko commented on pull request "optimization: pack util::Xor into 64 bit chunks instead of doing it byte-by-byte":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r1814518826)
same
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r1814518826)
same
💬 maflcko commented on pull request "optimization: pack util::Xor into 64 bit chunks instead of doing it byte-by-byte":
(https://github.com/bitcoin/bitcoin/pull/31144#issuecomment-2434625837)
I think your example may be a bit skewed? It shows how much time is spent when deserializing a `CScript` from a block file. However, block files contain full blocks, where many (most?) of the writes are single bytes (or 4 bytes), see https://github.com/bitcoin/bitcoin/issues/30833#issuecomment-2338250464. Thus, it would be useful to know what the overall end-to-end performance difference is. Also taking into account the utxo db.
If you want the micro-benchmark to be representative, I'd presum
...
(https://github.com/bitcoin/bitcoin/pull/31144#issuecomment-2434625837)
I think your example may be a bit skewed? It shows how much time is spent when deserializing a `CScript` from a block file. However, block files contain full blocks, where many (most?) of the writes are single bytes (or 4 bytes), see https://github.com/bitcoin/bitcoin/issues/30833#issuecomment-2338250464. Thus, it would be useful to know what the overall end-to-end performance difference is. Also taking into account the utxo db.
If you want the micro-benchmark to be representative, I'd presum
...
💬 fanquake commented on pull request "doc: replace `-?` with `-h` and `-help`":
(https://github.com/bitcoin/bitcoin/pull/31118#discussion_r1814528609)
Maybe this will be the last time we rewrite this.
Introduced as `-?`: b21244e0be5727f8c4e8c5de0a9aa2c597ae8ed2
Changed to `--help`: 05fdb97df46d0a0675b93e9791bd5d498e5e5117
Changed back to `-?`: d8513fe41102dcbfc05235f3b95e33eb1878f880
Now we are going back to `-h`: f0130ab1a1e65583637b6a362b879ea3253e7bb7.
(https://github.com/bitcoin/bitcoin/pull/31118#discussion_r1814528609)
Maybe this will be the last time we rewrite this.
Introduced as `-?`: b21244e0be5727f8c4e8c5de0a9aa2c597ae8ed2
Changed to `--help`: 05fdb97df46d0a0675b93e9791bd5d498e5e5117
Changed back to `-?`: d8513fe41102dcbfc05235f3b95e33eb1878f880
Now we are going back to `-h`: f0130ab1a1e65583637b6a362b879ea3253e7bb7.
💬 maflcko commented on pull request "optimization: pack util::Xor into 64 bit chunks instead of doing it byte-by-byte":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r1814534953)
CI seems to agree:
```
/usr/bin/../lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/stl_iterator.h:1100:16: runtime error: reference binding to misaligned address 0x7f10961d9084 for type 'unsigned long', which requires 8 byte alignment
0x7f10961d9084: note: pointer points here
94 8e 20 eb 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
^
#0 0x55780d85ab85 in __gnu_cxx::__normal_iterator<unsigned long*, std::span<unsig
...
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r1814534953)
CI seems to agree:
```
/usr/bin/../lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/stl_iterator.h:1100:16: runtime error: reference binding to misaligned address 0x7f10961d9084 for type 'unsigned long', which requires 8 byte alignment
0x7f10961d9084: note: pointer points here
94 8e 20 eb 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
^
#0 0x55780d85ab85 in __gnu_cxx::__normal_iterator<unsigned long*, std::span<unsig
...