💬 sipa commented on pull request "Add pool based memory resource":
(https://github.com/bitcoin/bitcoin/pull/25325#discussion_r1145435338)
Alternatively, just round up size 0 to be at least alignment?
(https://github.com/bitcoin/bitcoin/pull/25325#discussion_r1145435338)
Alternatively, just round up size 0 to be at least alignment?
💬 wbobeirne commented on issue "Bitcoind repeatedly crashing at `UpdateTip` with no error message":
(https://github.com/bitcoin/bitcoin/issues/24483#issuecomment-1480313309)
@pinheadmz I'm fairly certain that despite SMART not reporting any issues, there was hardware disk failure at play here. I started to see instability in my system elsewhere. I haven't yet swapped the offending hard drive to confirm, but this can probably be closed.
(https://github.com/bitcoin/bitcoin/issues/24483#issuecomment-1480313309)
@pinheadmz I'm fairly certain that despite SMART not reporting any issues, there was hardware disk failure at play here. I started to see instability in my system elsewhere. I haven't yet swapped the offending hard drive to confirm, but this can probably be closed.
💬 TheCharlatan commented on pull request "refactor: Move chain names to the kernel namespace":
(https://github.com/bitcoin/bitcoin/pull/27294#issuecomment-1480339025)
Thank you for the reviews and the helpful and patient comments!
Updated 571c7bd983035e4b684f289d97f00542ac9dc117 -> 5d166cae8cd16959bac7527779540daf81c328e4 ([kernelChainName_0](https://github.com/TheCharlatan/bitcoin/commits/kernelChainName_0) -> [kernelChainName_1](https://github.com/TheCharlatan/bitcoin/commits/kernelChainName_1), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernelChainName_0..kernelChainName_1)):
* Addressed @ajtowns [comment](https://github.com/bitcoin/bitc
...
(https://github.com/bitcoin/bitcoin/pull/27294#issuecomment-1480339025)
Thank you for the reviews and the helpful and patient comments!
Updated 571c7bd983035e4b684f289d97f00542ac9dc117 -> 5d166cae8cd16959bac7527779540daf81c328e4 ([kernelChainName_0](https://github.com/TheCharlatan/bitcoin/commits/kernelChainName_0) -> [kernelChainName_1](https://github.com/TheCharlatan/bitcoin/commits/kernelChainName_1), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernelChainName_0..kernelChainName_1)):
* Addressed @ajtowns [comment](https://github.com/bitcoin/bitc
...
💬 LarryRuane commented on pull request "Add pool based memory resource":
(https://github.com/bitcoin/bitcoin/pull/25325#discussion_r1145478812)
@martinus, good catch!
@sipa, good idea, this does seem to work (hope this is what you meant):
```
[[nodiscard]] static constexpr std::size_t NumElemAlignBytes(std::size_t bytes)
{
- return (bytes + ELEM_ALIGN_BYTES - 1) / ELEM_ALIGN_BYTES;
+ return bytes > 0 ? ((bytes + ELEM_ALIGN_BYTES - 1) / ELEM_ALIGN_BYTES) : 1;
}
```
Benchmark results are identical for me, but this should be confirmed.
(https://github.com/bitcoin/bitcoin/pull/25325#discussion_r1145478812)
@martinus, good catch!
@sipa, good idea, this does seem to work (hope this is what you meant):
```
[[nodiscard]] static constexpr std::size_t NumElemAlignBytes(std::size_t bytes)
{
- return (bytes + ELEM_ALIGN_BYTES - 1) / ELEM_ALIGN_BYTES;
+ return bytes > 0 ? ((bytes + ELEM_ALIGN_BYTES - 1) / ELEM_ALIGN_BYTES) : 1;
}
```
Benchmark results are identical for me, but this should be confirmed.
💬 sipa commented on pull request "Add pool based memory resource":
(https://github.com/bitcoin/bitcoin/pull/25325#discussion_r1145490940)
Yeah, or even:
```c++
return (bytes + ELEM_ALIGN_BYTES - 1) / ELEM_ALIGN_BYTES + (bytes == 0);
```
which might be a minuscule amount faster.
(https://github.com/bitcoin/bitcoin/pull/25325#discussion_r1145490940)
Yeah, or even:
```c++
return (bytes + ELEM_ALIGN_BYTES - 1) / ELEM_ALIGN_BYTES + (bytes == 0);
```
which might be a minuscule amount faster.
📝 achow101 opened a pull request: "bumpfee: avoid making bumped transactions with too low fee when replacing outputs"
(https://github.com/bitcoin/bitcoin/pull/27308)
When replacing the outputs of a transaction during `bumpfee`, it is possible to accidentally create a transaction that will not be accepted into the mempool as it does not meet the incremental relay fee requirements. This occurs because the size estimation used for checking the provided feerate does not account for the replaced outputs; it instead uses the original outputs. When the replaced outputs is significantly different from the original, there can be a large difference in estimated transa
...
(https://github.com/bitcoin/bitcoin/pull/27308)
When replacing the outputs of a transaction during `bumpfee`, it is possible to accidentally create a transaction that will not be accepted into the mempool as it does not meet the incremental relay fee requirements. This occurs because the size estimation used for checking the provided feerate does not account for the replaced outputs; it instead uses the original outputs. When the replaced outputs is significantly different from the original, there can be a large difference in estimated transa
...
💬 ProofOfKeags commented on issue "Manual-pruning cursor rewind":
(https://github.com/bitcoin/bitcoin/issues/19807#issuecomment-1480381691)
> would not be able to handle a deeper chain reorg than the original prune depth.
The smallest prune size is 550MB. If we have a 140 block reorg, we have a pretty apocalyptic scenario. afaiu there are numerous assumptions within bitcoin core that a 100 block reorg is impossible. Under that assumption, your described limitation isn't actually a limitation.
(https://github.com/bitcoin/bitcoin/issues/19807#issuecomment-1480381691)
> would not be able to handle a deeper chain reorg than the original prune depth.
The smallest prune size is 550MB. If we have a 140 block reorg, we have a pretty apocalyptic scenario. afaiu there are numerous assumptions within bitcoin core that a 100 block reorg is impossible. Under that assumption, your described limitation isn't actually a limitation.
💬 theStack commented on pull request "test: Support decoding segwit address in address_to_scriptpubkey()":
(https://github.com/bitcoin/bitcoin/pull/27269#discussion_r1145559813)
This unfortunately doesn't lead to the correct result for regtest segwit addresses, as those have the [four-character HRP 'bcrt'](https://github.com/bitcoin/bitcoin/blob/4c6b7d330a4e80460dcce19b1a0a47d77a0b99ea/src/kernel/chainparams.cpp#L510). To make it work for addresses on all networks, I'd suggest to:
1) finding the HRP by extracting the string before the bech32 separator character '1' (either the `find()` or `.split()` method on strings should be handy for this)
2) assert that the extr
...
(https://github.com/bitcoin/bitcoin/pull/27269#discussion_r1145559813)
This unfortunately doesn't lead to the correct result for regtest segwit addresses, as those have the [four-character HRP 'bcrt'](https://github.com/bitcoin/bitcoin/blob/4c6b7d330a4e80460dcce19b1a0a47d77a0b99ea/src/kernel/chainparams.cpp#L510). To make it work for addresses on all networks, I'd suggest to:
1) finding the HRP by extracting the string before the bech32 separator character '1' (either the `find()` or `.split()` method on strings should be handy for this)
2) assert that the extr
...
💬 Crypto2 commented on issue "Incorrect balance reported in getwalletinfo/getbalance":
(https://github.com/bitcoin/bitcoin/issues/21768#issuecomment-1480516783)
It happened again today, sometime between 12-1pm Toronto time. If you look at the unconfirmed balance in this hourly log it just drops and it is ignoring those pending funds. I did restart the node but that didn't fix it (and no they didn't just go in to the confirmed balance.) listtransactions for that hour shows less than 1 BTC of volume in either direction.
12a BTC: wallet 0 balances -> balance: X.30494649 / unconfirmed_balance: 22.45820421
BTC: wallet 0 balances -> balance: X.01931847 /
...
(https://github.com/bitcoin/bitcoin/issues/21768#issuecomment-1480516783)
It happened again today, sometime between 12-1pm Toronto time. If you look at the unconfirmed balance in this hourly log it just drops and it is ignoring those pending funds. I did restart the node but that didn't fix it (and no they didn't just go in to the confirmed balance.) listtransactions for that hour shows less than 1 BTC of volume in either direction.
12a BTC: wallet 0 balances -> balance: X.30494649 / unconfirmed_balance: 22.45820421
BTC: wallet 0 balances -> balance: X.01931847 /
...
💬 achow101 commented on pull request "fix: contrib: allow multi-sig binary verification":
(https://github.com/bitcoin/bitcoin/pull/23020#issuecomment-1480603300)
https://github.com/achow101/bitcoin/tree/pr23020-direct-bins-gpg-parse is a branch which implements a subcommand for verifying binaries directly and to use `--status-file` for the machine parseable output.
(https://github.com/bitcoin/bitcoin/pull/23020#issuecomment-1480603300)
https://github.com/achow101/bitcoin/tree/pr23020-direct-bins-gpg-parse is a branch which implements a subcommand for verifying binaries directly and to use `--status-file` for the machine parseable output.
💬 1440000bytes commented on issue "Incorrect balance reported in getwalletinfo/getbalance":
(https://github.com/bitcoin/bitcoin/issues/21768#issuecomment-1480666670)
> I tried increasing the maxmempool to 1000 instead of the default 300 and restarted again, and now the funds are showing again
Don't use default mempool size if you have enough RAM on machine running bitcoind. Default was set years ago and doesn't make sense anymore. There are other disadvantages as well for using default or lower size mempool.
(https://github.com/bitcoin/bitcoin/issues/21768#issuecomment-1480666670)
> I tried increasing the maxmempool to 1000 instead of the default 300 and restarted again, and now the funds are showing again
Don't use default mempool size if you have enough RAM on machine running bitcoind. Default was set years ago and doesn't make sense anymore. There are other disadvantages as well for using default or lower size mempool.
💬 S3RK commented on pull request "wallet: return error msg for "too-long-mempool-chain"":
(https://github.com/bitcoin/bitcoin/pull/24845#discussion_r1145824454)
q: do I understand correctly with current filters `total_discarded` is always equal to `total_unconf_long_chain`?
(https://github.com/bitcoin/bitcoin/pull/24845#discussion_r1145824454)
q: do I understand correctly with current filters `total_discarded` is always equal to `total_unconf_long_chain`?
💬 S3RK commented on pull request "wallet: return error msg for "too-long-mempool-chain"":
(https://github.com/bitcoin/bitcoin/pull/24845#issuecomment-1480768733)
Code review ACK f3221d373a8623fe4808e00c7a92fbb6e0d6419b
(https://github.com/bitcoin/bitcoin/pull/24845#issuecomment-1480768733)
Code review ACK f3221d373a8623fe4808e00c7a92fbb6e0d6419b
✅ MarcoFalke closed an issue: "Bitcoind repeatedly crashing at `UpdateTip` with no error message"
(https://github.com/bitcoin/bitcoin/issues/24483)
(https://github.com/bitcoin/bitcoin/issues/24483)
💬 MarcoFalke commented on issue "contrib/install_db4.sh script fails on OpenBSD 7.2 (RPi 3) (error: Unable to find a mutex implementation)":
(https://github.com/bitcoin/bitcoin/issues/26860#issuecomment-1480787053)
Closing for now. Let us know if this is still an issue and should be reopened.
(https://github.com/bitcoin/bitcoin/issues/26860#issuecomment-1480787053)
Closing for now. Let us know if this is still an issue and should be reopened.
✅ MarcoFalke closed an issue: "contrib/install_db4.sh script fails on OpenBSD 7.2 (RPi 3) (error: Unable to find a mutex implementation)"
(https://github.com/bitcoin/bitcoin/issues/26860)
(https://github.com/bitcoin/bitcoin/issues/26860)
💬 MarcoFalke commented on pull request "refactor: Extract util/fs from util/system":
(https://github.com/bitcoin/bitcoin/pull/27254#discussion_r1145848255)
You could do it in a separate commit, after the move
(https://github.com/bitcoin/bitcoin/pull/27254#discussion_r1145848255)
You could do it in a separate commit, after the move
💬 ismaelsadeeq commented on pull request "test: Support decoding segwit address in address_to_scriptpubkey()":
(https://github.com/bitcoin/bitcoin/pull/27269#discussion_r1145852472)
Thank you for your valuable feedback.
- I will use `.split('1')` on the address to extract the hrp, and also assert to verify if the hrp is valid as you noted.
- I will have to modify `address_to_scriptpubkey ` to check for base58 address first before calling `bech32_to_bytes` or it will assert false.
I hope this addresses the issue
(https://github.com/bitcoin/bitcoin/pull/27269#discussion_r1145852472)
Thank you for your valuable feedback.
- I will use `.split('1')` on the address to extract the hrp, and also assert to verify if the hrp is valid as you noted.
- I will have to modify `address_to_scriptpubkey ` to check for base58 address first before calling `bech32_to_bytes` or it will assert false.
I hope this addresses the issue
💬 MarcoFalke commented on pull request "refactor: Replace GetTimeMicros by SystemClock":
(https://github.com/bitcoin/bitcoin/pull/27233#discussion_r1145858622)
Yes, I am happy to review a pull doing this.
(https://github.com/bitcoin/bitcoin/pull/27233#discussion_r1145858622)
Yes, I am happy to review a pull doing this.
💬 josibake commented on pull request "test: Support decoding segwit address in address_to_scriptpubkey()":
(https://github.com/bitcoin/bitcoin/pull/27269#discussion_r1145873825)
I'm not sure I understand your second point, @ismaelsadeeq . The order shouldn't matter in `address_to_scriptpubkey`. It should be sufficient to properly extract the HRP and check that it is a bech32.
(https://github.com/bitcoin/bitcoin/pull/27269#discussion_r1145873825)
I'm not sure I understand your second point, @ismaelsadeeq . The order shouldn't matter in `address_to_scriptpubkey`. It should be sufficient to properly extract the HRP and check that it is a bech32.