🤔 furszy reviewed a pull request: "bench: add support for custom data directory"
(https://github.com/bitcoin/bitcoin/pull/31000#pullrequestreview-2390711656)
Updated per feedback. Thanks achow.
(https://github.com/bitcoin/bitcoin/pull/31000#pullrequestreview-2390711656)
Updated per feedback. Thanks achow.
💬 achow101 commented on pull request "doc: Extend developer-notes with file-name-only debugging fix":
(https://github.com/bitcoin/bitcoin/pull/30670#issuecomment-2433523846)
ACK d1546b3c4f24d3ff6abe729a4288e827786cf6e9
(https://github.com/bitcoin/bitcoin/pull/30670#issuecomment-2433523846)
ACK d1546b3c4f24d3ff6abe729a4288e827786cf6e9
👍 theStack approved a pull request: "RPC: Return `permitbaremultisig` and `maxdatacarriersize` in `getmempoolinfo`"
(https://github.com/bitcoin/bitcoin/pull/29954#pullrequestreview-2390749484)
ACK modulo nits
As others have expressed before, I'd also slightly prefer to name the options exactly like their command line option (i.e. "datacarriersize" without the "max" prefix), but no blocker.
(https://github.com/bitcoin/bitcoin/pull/29954#pullrequestreview-2390749484)
ACK modulo nits
As others have expressed before, I'd also slightly prefer to name the options exactly like their command line option (i.e. "datacarriersize" without the "max" prefix), but no blocker.
💬 theStack commented on pull request "RPC: Return `permitbaremultisig` and `maxdatacarriersize` in `getmempoolinfo`":
(https://github.com/bitcoin/bitcoin/pull/29954#discussion_r1813770793)
nit, a little shorter:
```suggestion
ret.pushKV("maxdatacarriersize", pool.m_opts.max_datacarrier_bytes.value_or(0));
```
(https://github.com/bitcoin/bitcoin/pull/29954#discussion_r1813770793)
nit, a little shorter:
```suggestion
ret.pushKV("maxdatacarriersize", pool.m_opts.max_datacarrier_bytes.value_or(0));
```
💬 theStack commented on pull request "RPC: Return `permitbaremultisig` and `maxdatacarriersize` in `getmempoolinfo`":
(https://github.com/bitcoin/bitcoin/pull/29954#discussion_r1813775127)
```suggestion
from test_framework.util import (
assert_equal,
assert_raises_rpc_error,
)
```
(we prefer multi-line imports, see style guidelines in ./test/functional/README.md)
(https://github.com/bitcoin/bitcoin/pull/29954#discussion_r1813775127)
```suggestion
from test_framework.util import (
assert_equal,
assert_raises_rpc_error,
)
```
(we prefer multi-line imports, see style guidelines in ./test/functional/README.md)
💬 achow101 commented on pull request "test: improve BDB parser (handle internal/overflow pages, support all page sizes)":
(https://github.com/bitcoin/bitcoin/pull/30125#discussion_r1813825732)
In ad0a8ffceacfd5ce37826b2882b0e51a14c5a6bf "test: compare BDB dumps of test framework parser and wallet tool"
Since we generate the large labels, this could also check that all show up with the correct address.
(https://github.com/bitcoin/bitcoin/pull/30125#discussion_r1813825732)
In ad0a8ffceacfd5ce37826b2882b0e51a14c5a6bf "test: compare BDB dumps of test framework parser and wallet tool"
Since we generate the large labels, this could also check that all show up with the correct address.
💬 achow101 commented on pull request "test: improve BDB parser (handle internal/overflow pages, support all page sizes)":
(https://github.com/bitcoin/bitcoin/pull/30125#discussion_r1813809067)
In ad0a8ffceacfd5ce37826b2882b0e51a14c5a6bf "test: compare BDB dumps of test framework parser and wallet tool"
Could use `self.assert_tool_output`, see how the other calls to `dump` are done in this test.
(https://github.com/bitcoin/bitcoin/pull/30125#discussion_r1813809067)
In ad0a8ffceacfd5ce37826b2882b0e51a14c5a6bf "test: compare BDB dumps of test framework parser and wallet tool"
Could use `self.assert_tool_output`, see how the other calls to `dump` are done in this test.
💬 achow101 commented on pull request "test: improve BDB parser (handle internal/overflow pages, support all page sizes)":
(https://github.com/bitcoin/bitcoin/pull/30125#discussion_r1813830674)
In ad0a8ffceacfd5ce37826b2882b0e51a14c5a6bf "test: compare BDB dumps of test framework parser and wallet tool"
In addition to testing overflow pages, we can also test internal pages by generating a bunch of keys with `keypoolrefill`.
(https://github.com/bitcoin/bitcoin/pull/30125#discussion_r1813830674)
In ad0a8ffceacfd5ce37826b2882b0e51a14c5a6bf "test: compare BDB dumps of test framework parser and wallet tool"
In addition to testing overflow pages, we can also test internal pages by generating a bunch of keys with `keypoolrefill`.
🚀 achow101 merged a pull request: "init: Some small chainstate load improvements"
(https://github.com/bitcoin/bitcoin/pull/31046)
(https://github.com/bitcoin/bitcoin/pull/31046)
💬 achow101 commented on pull request "test: Fix intermittent issue in wallet_backwards_compatibility.py":
(https://github.com/bitcoin/bitcoin/pull/29982#issuecomment-2433618589)
ACK ec777917d6eba0b417dbc90b9b891240a44b7ec4
lgtm for a mitigation.
(https://github.com/bitcoin/bitcoin/pull/29982#issuecomment-2433618589)
ACK ec777917d6eba0b417dbc90b9b891240a44b7ec4
lgtm for a mitigation.
💬 kevkevinpal commented on pull request "test: added test to assert TX decode rpc error on submitpackage rpc":
(https://github.com/bitcoin/bitcoin/pull/31139#discussion_r1813849800)
thanks! fair point updated in [4bdb78c](https://github.com/bitcoin/bitcoin/pull/31139/commits/4bdb78cfe76414227b749674ed660f4972fb6eda)
(https://github.com/bitcoin/bitcoin/pull/31139#discussion_r1813849800)
thanks! fair point updated in [4bdb78c](https://github.com/bitcoin/bitcoin/pull/31139/commits/4bdb78cfe76414227b749674ed660f4972fb6eda)
💬 jarolrod commented on pull request "Drop miniupnp dependency":
(https://github.com/bitcoin/bitcoin/pull/31130#issuecomment-2433812861)
concept ack
(https://github.com/bitcoin/bitcoin/pull/31130#issuecomment-2433812861)
concept ack
👍 jarolrod approved a pull request: "doc: Make list of targets in depends README consistent"
(https://github.com/bitcoin/bitcoin/pull/31141#pullrequestreview-2390889264)
ACK a0c9595810c7d8bb17d8b5bea8d916db194b5239
(https://github.com/bitcoin/bitcoin/pull/31141#pullrequestreview-2390889264)
ACK a0c9595810c7d8bb17d8b5bea8d916db194b5239
📝 cbradberry opened 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).
✅ 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?