💬 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) {
+
...
📝 ishaanam opened a pull request: "test: fix race condition in encrypted wallet rescan tests"
(https://github.com/bitcoin/bitcoin/pull/27199)
This fixes https://github.com/bitcoin/bitcoin/pull/26347#discussion_r1123340738
(https://github.com/bitcoin/bitcoin/pull/27199)
This fixes https://github.com/bitcoin/bitcoin/pull/26347#discussion_r1123340738
💬 ishaanam commented on pull request "wallet: ensure the wallet is unlocked when needed for rescanning":
(https://github.com/bitcoin/bitcoin/pull/26347#discussion_r1125568137)
Ok, I think I understand what you mean now. I have implemented this using multi-threading in #27199. However, for me this no longer fails without the fix.
(https://github.com/bitcoin/bitcoin/pull/26347#discussion_r1125568137)
Ok, I think I understand what you mean now. I have implemented this using multi-threading in #27199. However, for me this no longer fails without the fix.
💬 kiminuo commented on pull request "Add feerate histogram to getmempoolinfo":
(https://github.com/bitcoin/bitcoin/pull/21422#discussion_r1125568212)
@0xB10C I modified
```cpp
for (const CTxMemPoolEntry& e : pool.mapTx) {
const CAmount fee{e.GetFee()};
const uint32_t size{uint32_t(e.GetTxSize())};
const CAmount fee_rate{CFeeRate{fee, size}.GetFee(1)};
// Distribute fee rates
for (size_t i = floors.size(); i-- > 0;) {
if (fee_rate >= floors[i]) {
sizes[i] += size;
++count[i];
fees[i] += fee;
break;
}
}
}
```
to
```cpp
std::vecto
...
(https://github.com/bitcoin/bitcoin/pull/21422#discussion_r1125568212)
@0xB10C I modified
```cpp
for (const CTxMemPoolEntry& e : pool.mapTx) {
const CAmount fee{e.GetFee()};
const uint32_t size{uint32_t(e.GetTxSize())};
const CAmount fee_rate{CFeeRate{fee, size}.GetFee(1)};
// Distribute fee rates
for (size_t i = floors.size(); i-- > 0;) {
if (fee_rate >= floors[i]) {
sizes[i] += size;
++count[i];
fees[i] += fee;
break;
}
}
}
```
to
```cpp
std::vecto
...
💬 LarryRuane commented on pull request "bench: update logging benchmarks":
(https://github.com/bitcoin/bitcoin/pull/26957#discussion_r1125575680)
Thanks, @MarcoFalke, good suggestion, [this branch](https://github.com/LarryRuane/bitcoin/commits/pr26957-1) changes that commit to not touch production code ([individual commit](https://github.com/LarryRuane/bitcoin/commit/a1cd3cc17b9fdce1d60d662c447f845a12063d81)) @jonatack, feel free to cherry-pick.
(https://github.com/bitcoin/bitcoin/pull/26957#discussion_r1125575680)
Thanks, @MarcoFalke, good suggestion, [this branch](https://github.com/LarryRuane/bitcoin/commits/pr26957-1) changes that commit to not touch production code ([individual commit](https://github.com/LarryRuane/bitcoin/commit/a1cd3cc17b9fdce1d60d662c447f845a12063d81)) @jonatack, feel free to cherry-pick.
✅ mraksoll4 closed an issue: "Release 0.24.0.1 File system error."
(https://github.com/bitcoin/bitcoin/issues/27198)
(https://github.com/bitcoin/bitcoin/issues/27198)
📝 theStack opened a pull request: "test: psbt: check non-witness UTXO removal for segwit v1 input"
(https://github.com/bitcoin/bitcoin/pull/27200)
This PR adds missing test coverage for dropping non-witness UTXOs from PSBTs for segwit v1+ inputs (see commit 103c6fd2791f7e73eeab7f3900fbedd5b550211d). The formerly [disabled](46004790588c24174a0bec49b540d158ce163ffd) method `test_utxo_conversion` is re-enabled and adapted to spend a Taproot (`bech32m`) instead of a wrapped SegWit (`p2sh-segwit`) output. Note that in contrast to the original test, we have to add the non-witness UTXO manually here using the test framework's PSBT module, since t
...
(https://github.com/bitcoin/bitcoin/pull/27200)
This PR adds missing test coverage for dropping non-witness UTXOs from PSBTs for segwit v1+ inputs (see commit 103c6fd2791f7e73eeab7f3900fbedd5b550211d). The formerly [disabled](46004790588c24174a0bec49b540d158ce163ffd) method `test_utxo_conversion` is re-enabled and adapted to spend a Taproot (`bech32m`) instead of a wrapped SegWit (`p2sh-segwit`) output. Note that in contrast to the original test, we have to add the non-witness UTXO manually here using the test framework's PSBT module, since t
...