👍 yahiheb approved a pull request: "Fix typos in comments to make linter happy"
(https://github.com/bitcoin/bitcoin/pull/27197)
(https://github.com/bitcoin/bitcoin/pull/27197)
👍 kristapsk approved a pull request: "Fix typos in comments to make linter happy"
(https://github.com/bitcoin/bitcoin/pull/27197)
utACK 987f1bb41c0a8c54422066e10d1c63e19c4df66d
(https://github.com/bitcoin/bitcoin/pull/27197)
utACK 987f1bb41c0a8c54422066e10d1c63e19c4df66d
🚀 fanquake merged a pull request: "Fix typos in comments to make linter happy"
(https://github.com/bitcoin/bitcoin/pull/27197)
(https://github.com/bitcoin/bitcoin/pull/27197)
👍 fanquake approved a pull request: "util: add missing include and fix function signature"
(https://github.com/bitcoin/bitcoin/pull/27192)
ACK 8847ce44e0713350a6d3524f62eaeb10ba548bae
(https://github.com/bitcoin/bitcoin/pull/27192)
ACK 8847ce44e0713350a6d3524f62eaeb10ba548bae
🚀 fanquake merged a pull request: "util: add missing include and fix function signature"
(https://github.com/bitcoin/bitcoin/pull/27192)
(https://github.com/bitcoin/bitcoin/pull/27192)
💬 fanquake commented on issue "Adoption of BIP39/44/49/84, and classification of (extended) pub/priv keys, addresses, mnemonics, etc":
(https://github.com/bitcoin/bitcoin/issues/17748#issuecomment-1454608242)
cc @achow101
(https://github.com/bitcoin/bitcoin/issues/17748#issuecomment-1454608242)
cc @achow101
💬 pablomartin4btc commented on pull request "Mask values on Transactions View":
(https://github.com/bitcoin-core/gui/pull/708#issuecomment-1454642004)
Updates:
- Cleanup from previous approach.
- Closing all dialogs with transaction details that were opened from the transactions view when mask values is selected.
(https://github.com/bitcoin-core/gui/pull/708#issuecomment-1454642004)
Updates:
- Cleanup from previous approach.
- Closing all dialogs with transaction details that were opened from the transactions view when mask values is selected.
💬 mauri146K commented on issue "signrawtransactionwithkey command shouldn't output the "Witness program was passed an empty witness" error for a TapRoot transaction":
(https://github.com/bitcoin/bitcoin/issues/27017#issuecomment-1454644421)
> There is the irrelevant error message output by the signrawtransactionwithkey command.
>
> **Expected behavior**
>
> Hex string of the raw transaction with signature OR meaningful message about an alternative way to achieve one.
>
> **Actual behavior**
>
> ```
> {
> "hex": "0200000001a2c0d82460883696219dbca6f545f72963b2b3ee085d832eb5ef9a69a374af160000000000fdffffff01e011000000000000225120052e44f45a6e381be8e06d3f3362b58034a68ba98081e24de7bfc5795420a90b00000000",
> "complete": false,
>
...
(https://github.com/bitcoin/bitcoin/issues/27017#issuecomment-1454644421)
> There is the irrelevant error message output by the signrawtransactionwithkey command.
>
> **Expected behavior**
>
> Hex string of the raw transaction with signature OR meaningful message about an alternative way to achieve one.
>
> **Actual behavior**
>
> ```
> {
> "hex": "0200000001a2c0d82460883696219dbca6f545f72963b2b3ee085d832eb5ef9a69a374af160000000000fdffffff01e011000000000000225120052e44f45a6e381be8e06d3f3362b58034a68ba98081e24de7bfc5795420a90b00000000",
> "complete": false,
>
...
💬 fanquake commented on pull request "doc: Expand scantxoutset help text to cover tr() and miniscript":
(https://github.com/bitcoin/bitcoin/pull/27155#issuecomment-1454659148)
> Should we version the doc/descriptors.md file? By that I mean literally add details in the doc saying when specific formats were added.
cc @achow101
(https://github.com/bitcoin/bitcoin/pull/27155#issuecomment-1454659148)
> Should we version the doc/descriptors.md file? By that I mean literally add details in the doc saying when specific formats were added.
cc @achow101
💬 LarryRuane commented on pull request "Add pool based memory resource":
(https://github.com/bitcoin/bitcoin/pull/25325#discussion_r1124914676)
I think this condition is guaranteed by the other static assertions, but this expression seems incorrect. In general, `A & (B-1) == 0` doesn't mean that A is a multiple of B, does it? If A=8 and B=3, then `8 & (3-1)` is zero, but 8 isn't a multiple of 3. If A=8, B=4, then `8 & (4-1)` is also zero, so we get the correct result (8 is a multiple of 4), but it's kind of by accident. To state it differently, if `MAX_BLOCK_SIZE_BYTES` is some large power of 2 (which is guaranteed above), then in binar
...
(https://github.com/bitcoin/bitcoin/pull/25325#discussion_r1124914676)
I think this condition is guaranteed by the other static assertions, but this expression seems incorrect. In general, `A & (B-1) == 0` doesn't mean that A is a multiple of B, does it? If A=8 and B=3, then `8 & (3-1)` is zero, but 8 isn't a multiple of 3. If A=8, B=4, then `8 & (4-1)` is also zero, so we get the correct result (8 is a multiple of 4), but it's kind of by accident. To state it differently, if `MAX_BLOCK_SIZE_BYTES` is some large power of 2 (which is guaranteed above), then in binar
...
💬 LarryRuane commented on pull request "Add pool based memory resource":
(https://github.com/bitcoin/bitcoin/pull/25325#discussion_r1125417231)
I don't understand the purpose of adding `sizeof(void*) * 4`; could you leave a brief comment if you get the chance? (Unless I'm just being clueless!)
(https://github.com/bitcoin/bitcoin/pull/25325#discussion_r1125417231)
I don't understand the purpose of adding `sizeof(void*) * 4`; could you leave a brief comment if you get the chance? (Unless I'm just being clueless!)
💬 MarcoFalke commented on pull request "util: add missing include and fix function signature":
(https://github.com/bitcoin/bitcoin/pull/27192#discussion_r1125426076)
Yeah, I was mostly thinking that it would be good to check for other missing includes while touching the file, so that it doesn't have to be touched again later if there is a missing stdlib include or so.
(https://github.com/bitcoin/bitcoin/pull/27192#discussion_r1125426076)
Yeah, I was mostly thinking that it would be good to check for other missing includes while touching the file, so that it doesn't have to be touched again later if there is a missing stdlib include or so.
💬 martinus commented on pull request "Add pool based memory resource":
(https://github.com/bitcoin/bitcoin/pull/25325#discussion_r1125443783)
In line 89 there is an assert that `ELEM_ALIGN_BYTES` is multiple of 2:
```cpp
static_assert((ELEM_ALIGN_BYTES & (ELEM_ALIGN_BYTES - 1)) == 0, "ELEM_ALIGN_BYTES must be a power of two");
```
So given that line 91 should make sense, `ELEM_ALIGN_BYTES - 1` becomes a bitmask and actually asserts that `MAX_BLOCK_SIZE_BYTES` is multiple of `ELEM_ALIGN_BYTES`. But right, on its own that assert wouldn't be enough and using `%` is probably a bit more clear.
(https://github.com/bitcoin/bitcoin/pull/25325#discussion_r1125443783)
In line 89 there is an assert that `ELEM_ALIGN_BYTES` is multiple of 2:
```cpp
static_assert((ELEM_ALIGN_BYTES & (ELEM_ALIGN_BYTES - 1)) == 0, "ELEM_ALIGN_BYTES must be a power of two");
```
So given that line 91 should make sense, `ELEM_ALIGN_BYTES - 1` becomes a bitmask and actually asserts that `MAX_BLOCK_SIZE_BYTES` is multiple of `ELEM_ALIGN_BYTES`. But right, on its own that assert wouldn't be enough and using `%` is probably a bit more clear.
💬 martinus commented on pull request "Add pool based memory resource":
(https://github.com/bitcoin/bitcoin/pull/25325#discussion_r1125443786)
Posting here now, I can add this as a comment later:
The value here determines the maximum bytes that the `PoolAllocator` supports. When bigger blocks are allocated, this is just forwarded to `new`.
The thing with `sizeof(void*) * 4` is, it is not enough to just support up to sizes of the `std::pair`, because the different implementations of `std::unordered_map` use more memory for each node. Most implementations wrap the std::pair into a struct that contains a single pointer, so they can
...
(https://github.com/bitcoin/bitcoin/pull/25325#discussion_r1125443786)
Posting here now, I can add this as a comment later:
The value here determines the maximum bytes that the `PoolAllocator` supports. When bigger blocks are allocated, this is just forwarded to `new`.
The thing with `sizeof(void*) * 4` is, it is not enough to just support up to sizes of the `std::pair`, because the different implementations of `std::unordered_map` use more memory for each node. Most implementations wrap the std::pair into a struct that contains a single pointer, so they can
...
💬 martinus commented on pull request "Add pool based memory resource":
(https://github.com/bitcoin/bitcoin/pull/25325#issuecomment-1454703666)
@LarryRuane awesome that you'll hold a PR review club about this PR! I'll try to join, but can't yet guarantee that I'll have the time.
(https://github.com/bitcoin/bitcoin/pull/25325#issuecomment-1454703666)
@LarryRuane awesome that you'll hold a PR review club about this PR! I'll try to join, but can't yet guarantee that I'll have the time.
⚠️ mraksoll4 opened an issue: "Release 0.24.0.1 File system error."
(https://github.com/bitcoin/bitcoin/issues/27198)
Release 0.24.0.1
Executable Buildet at debian give error at windows file system.
`EXCEPTION: NSt10filesystem7__cxx1116filesystem_errorE
filesystem error: cannot remove [D:\Chains\Bitcoin22\anchors.dat]
H:\Testing\dssdds\bin\bitcoin-qt.exe in Runaway exception
2023-03-04T16:33:52Z torcontrol thread exit`
(https://github.com/bitcoin/bitcoin/issues/27198)
Release 0.24.0.1
Executable Buildet at debian give error at windows file system.
`EXCEPTION: NSt10filesystem7__cxx1116filesystem_errorE
filesystem error: cannot remove [D:\Chains\Bitcoin22\anchors.dat]
H:\Testing\dssdds\bin\bitcoin-qt.exe in Runaway exception
2023-03-04T16:33:52Z torcontrol thread exit`
💬 fjahr commented on pull request "http: Track active requests and wait for last to finish - 2nd attempt":
(https://github.com/bitcoin/bitcoin/pull/26742#discussion_r1125511981)
Thanks! I will fix this in the follow-up that will pick up #19434.
(https://github.com/bitcoin/bitcoin/pull/26742#discussion_r1125511981)
Thanks! I will fix this in the follow-up that will pick up #19434.
💬 saadbitcoin commented on pull request "http: Track active requests and wait for last to finish - 2nd attempt":
(https://github.com/bitcoin/bitcoin/pull/26742#issuecomment-1454847059)
> Running `test/functional/feature_abortnode.py` before and after change we get a significant performance gain
>
> running before changes [f3bc1a7](https://github.com/bitcoin/bitcoin/commit/f3bc1a72825fe2b51f4bc20e004cef464f05b965)
>
> ```
> test/functional/feature_abortnode.py
> 2022-12-23T04:47:19.826000Z TestFramework (INFO): Initializing test directory /var/folders/9g/cvx014yx4dq5lwl_x5zvn_j80000gn/T/bitcoin_func_test_k3_6xric
> 2022-12-23T04:47:21.229000Z TestFramework (INFO): Wait
...
(https://github.com/bitcoin/bitcoin/pull/26742#issuecomment-1454847059)
> Running `test/functional/feature_abortnode.py` before and after change we get a significant performance gain
>
> running before changes [f3bc1a7](https://github.com/bitcoin/bitcoin/commit/f3bc1a72825fe2b51f4bc20e004cef464f05b965)
>
> ```
> test/functional/feature_abortnode.py
> 2022-12-23T04:47:19.826000Z TestFramework (INFO): Initializing test directory /var/folders/9g/cvx014yx4dq5lwl_x5zvn_j80000gn/T/bitcoin_func_test_k3_6xric
> 2022-12-23T04:47:21.229000Z TestFramework (INFO): Wait
...
💬 stickies-v commented on pull request "doc: Fixup remove 'omitted...' doc for rpc getrawtransaction when verbose is 2":
(https://github.com/bitcoin/bitcoin/pull/26968#issuecomment-1454888362)
I've provided links [[1](https://github.com/bitcoin/bitcoin/pull/26968#discussion_r1087741997), [2](https://github.com/bitcoin/bitcoin/pull/26968#discussion_r1087746520)] in my previous comments to reference the code from which I've built my understanding about which fields are optional and which ones aren't. Are we disagreeing about behaviour? My understanding in short is: "vin" is always present for verbosity>=1, whereas "fee" and "prevout" are omitted if no block data.
So to me it seems l
...
(https://github.com/bitcoin/bitcoin/pull/26968#issuecomment-1454888362)
I've provided links [[1](https://github.com/bitcoin/bitcoin/pull/26968#discussion_r1087741997), [2](https://github.com/bitcoin/bitcoin/pull/26968#discussion_r1087746520)] in my previous comments to reference the code from which I've built my understanding about which fields are optional and which ones aren't. Are we disagreeing about behaviour? My understanding in short is: "vin" is always present for verbosity>=1, whereas "fee" and "prevout" are omitted if no block data.
So to me it seems l
...
💬 TheCharlatan commented on pull request "blockstorage: add an assert to avoid running oom with `-fastprune`":
(https://github.com/bitcoin/bitcoin/pull/27191#discussion_r1125565424)
Instead of asserting, how about setting the block file size dynamically in case the block to be written exceeds the limit? Something like:
```
- const unsigned int max_blockfile_size{gArgs.GetBoolArg("-fastprune", false) ? 0x10000 /* 64kb */ : MAX_BLOCKFILE_SIZE};
+ unsigned int max_blockfile_size = MAX_BLOCKFILE_SIZE;
+ if (gArgs.GetBoolArg("-fastprune", false)) {
+ max_blockfile_size = 0x10000; // 64kiB
+ if (nAddSize >= max_blockfile_size) {
+
...
(https://github.com/bitcoin/bitcoin/pull/27191#discussion_r1125565424)
Instead of asserting, how about setting the block file size dynamically in case the block to be written exceeds the limit? Something like:
```
- const unsigned int max_blockfile_size{gArgs.GetBoolArg("-fastprune", false) ? 0x10000 /* 64kb */ : MAX_BLOCKFILE_SIZE};
+ unsigned int max_blockfile_size = MAX_BLOCKFILE_SIZE;
+ if (gArgs.GetBoolArg("-fastprune", false)) {
+ max_blockfile_size = 0x10000; // 64kiB
+ if (nAddSize >= max_blockfile_size) {
+
...