💬 marcofleon commented on pull request "build: Make G_FUZZING constexpr, require -DBUILD_FOR_FUZZING=ON to fuzz":
(https://github.com/bitcoin/bitcoin/pull/31191#issuecomment-2450223371)
Tested ACK fafbf8acf419d5e2ca307e5804099361ca7471af
(https://github.com/bitcoin/bitcoin/pull/31191#issuecomment-2450223371)
Tested ACK fafbf8acf419d5e2ca307e5804099361ca7471af
💬 jb55 commented on issue "Fatal LevelDB error: Corruption: block checksum mismatch on Linux ext4 SATA SSDs":
(https://github.com/bitcoin/bitcoin/issues/30692#issuecomment-2450235448)
zfs-2.2.6-1
sata connected
zfs pool is not complicated, just a single drive (4tb ssd)
ryzen cpu `AMD Ryzen 7 1800X Eight-Core Processor`
I ran smartctl no issues, this is a brand new SSD because I thought it was my HDD.
```
=== START OF INFORMATION SECTION ===
Device Model: SPCC Solid State Disk
Serial Number: 230707575120019
LU WWN Device Id: 0 000000 000000000
Firmware Version: VE1R9007
User Capacity: 4,000,787,030,016 bytes [4.00 TB]
Sector Size: 512 bytes logic
...
(https://github.com/bitcoin/bitcoin/issues/30692#issuecomment-2450235448)
zfs-2.2.6-1
sata connected
zfs pool is not complicated, just a single drive (4tb ssd)
ryzen cpu `AMD Ryzen 7 1800X Eight-Core Processor`
I ran smartctl no issues, this is a brand new SSD because I thought it was my HDD.
```
=== START OF INFORMATION SECTION ===
Device Model: SPCC Solid State Disk
Serial Number: 230707575120019
LU WWN Device Id: 0 000000 000000000
Firmware Version: VE1R9007
User Capacity: 4,000,787,030,016 bytes [4.00 TB]
Sector Size: 512 bytes logic
...
💬 hodlinator commented on pull request "optimization: change XOR obfuscation key from `std::vector<std::byte>(8)` to `uint64_t`":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r1824712214)
My point about moving the negation and changing the name made more sense in the context of keeping it inside the `if`-block. If you are open to moving it out, I'd say it's better to keep the original `key_exists` name and original negation to avoid the churn and make it easier to review.
(Realized another reason for not having it inside the `if`-block is that we are mutating `obfuscate_key_vector`, which is used after the block).
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r1824712214)
My point about moving the negation and changing the name made more sense in the context of keeping it inside the `if`-block. If you are open to moving it out, I'd say it's better to keep the original `key_exists` name and original negation to avoid the churn and make it easier to review.
(Realized another reason for not having it inside the `if`-block is that we are mutating `obfuscate_key_vector`, which is used after the block).
🤔 hodlinator reviewed a pull request: "optimization: change XOR obfuscation key from `std::vector<std::byte>(8)` to `uint64_t`"
(https://github.com/bitcoin/bitcoin/pull/31144#pullrequestreview-2408330350)
(Reviewed a5cad729c76cafa047a2b1897595669ae9b2b0d5)
Since my previous Concept ACK, the PR was changed to switch the xor key more completely to `uint64_t`. Before the PR, we were already using fixed-size of 8 bytes for the obfuscation value in the file formats, so changing the type to `uint64_t` shouldn't be noticeable to users. :+1:
Even if we could move reading and XOR-ing out of the hot path as suggested by maflcko, we might as well make use the CPU architectures we have. I would expect
...
(https://github.com/bitcoin/bitcoin/pull/31144#pullrequestreview-2408330350)
(Reviewed a5cad729c76cafa047a2b1897595669ae9b2b0d5)
Since my previous Concept ACK, the PR was changed to switch the xor key more completely to `uint64_t`. Before the PR, we were already using fixed-size of 8 bytes for the obfuscation value in the file formats, so changing the type to `uint64_t` shouldn't be noticeable to users. :+1:
Even if we could move reading and XOR-ing out of the hot path as suggested by maflcko, we might as well make use the CPU architectures we have. I would expect
...
💬 hodlinator commented on pull request "optimization: change XOR obfuscation key from `std::vector<std::byte>(8)` to `uint64_t`":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r1824697175)
(Should be done in the initial commit which invalidates the comments IMO).
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r1824697175)
(Should be done in the initial commit which invalidates the comments IMO).
💬 hebasto commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#discussion_r1824750915)
> How was it addressed? I still see the same output using...
I assume you're still observing the warning for the `native_qt` package, not the `qt` package. In that case, I don’t think we need to worry about it.
(https://github.com/bitcoin/bitcoin/pull/30997#discussion_r1824750915)
> How was it addressed? I still see the same output using...
I assume you're still observing the warning for the `native_qt` package, not the `qt` package. In that case, I don’t think we need to worry about it.
💬 hebasto commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2450265488)
@Sjors
> I'm still confused why Ninja is needed and why nothing complains when it's missing.
>
> > Ninja is required to build the qt package in depends. It is mentioned in depends/README.md.
>
> It only mentions it in the context of an Ubuntu & Debian install.
>
> I did a depends build for [af05dd9](https://github.com/bitcoin/bitcoin/commit/af05dd9a12b89224dc7ad229698eeceb3e560ed4) again on Intel macOS 15.0.1 with Xcode 16 and no Ninja. It seems to work fine, at first glance.
>
>
...
(https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2450265488)
@Sjors
> I'm still confused why Ninja is needed and why nothing complains when it's missing.
>
> > Ninja is required to build the qt package in depends. It is mentioned in depends/README.md.
>
> It only mentions it in the context of an Ubuntu & Debian install.
>
> I did a depends build for [af05dd9](https://github.com/bitcoin/bitcoin/commit/af05dd9a12b89224dc7ad229698eeceb3e560ed4) again on Intel macOS 15.0.1 with Xcode 16 and no Ninja. It seems to work fine, at first glance.
>
>
...
💬 jb55 commented on issue "Fatal LevelDB error: Corruption: block checksum mismatch on Linux ext4 SATA SSDs":
(https://github.com/bitcoin/bitcoin/issues/30692#issuecomment-2450267609)
hmm I have a small dbcache size I just noticed... can't remember why I set it to that. maybe the increased fsyncs is increasing the chance of a bug somehow? can't think of anything else if its not a hardware issue.
(https://github.com/bitcoin/bitcoin/issues/30692#issuecomment-2450267609)
hmm I have a small dbcache size I just noticed... can't remember why I set it to that. maybe the increased fsyncs is increasing the chance of a bug somehow? can't think of anything else if its not a hardware issue.
💬 github12101 commented on issue "Fatal LevelDB error: Corruption: block checksum mismatch on Linux ext4 SATA SSDs":
(https://github.com/bitcoin/bitcoin/issues/30692#issuecomment-2450276326)
> hmm I have a small dbcache size I just noticed... can't remember why I set it to that. maybe the increased fsyncs is increasing the chance of a bug somehow? can't think of anything else if its not a hardware issue.
Can/did you run memtest86, for several hours? No errors?
(https://github.com/bitcoin/bitcoin/issues/30692#issuecomment-2450276326)
> hmm I have a small dbcache size I just noticed... can't remember why I set it to that. maybe the increased fsyncs is increasing the chance of a bug somehow? can't think of anything else if its not a hardware issue.
Can/did you run memtest86, for several hours? No errors?
💬 instagibbs commented on pull request "Remove mempoolfullrbf":
(https://github.com/bitcoin/bitcoin/pull/30592#discussion_r1824764954)
Guess I'd prefer to remove it, since it's been a slippery slope of noncompliance over time
(https://github.com/bitcoin/bitcoin/pull/30592#discussion_r1824764954)
Guess I'd prefer to remove it, since it's been a slippery slope of noncompliance over time
💬 jb55 commented on issue "Fatal LevelDB error: Corruption: block checksum mismatch on Linux ext4 SATA SSDs":
(https://github.com/bitcoin/bitcoin/issues/30692#issuecomment-2450280042)
> > hmm I have a small dbcache size I just noticed... can't remember why I set it to that. maybe the increased fsyncs is increasing the chance of a bug somehow? can't think of anything else if its not a hardware issue.
>
> Can/did you run memtest86, for several hours? No errors?
I haven't done that yet because I run my entire business (https://damus.io) off this node. my system has been stable for years.
(https://github.com/bitcoin/bitcoin/issues/30692#issuecomment-2450280042)
> > hmm I have a small dbcache size I just noticed... can't remember why I set it to that. maybe the increased fsyncs is increasing the chance of a bug somehow? can't think of anything else if its not a hardware issue.
>
> Can/did you run memtest86, for several hours? No errors?
I haven't done that yet because I run my entire business (https://damus.io) off this node. my system has been stable for years.
💬 hebasto commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#discussion_r1824765673)
Thanks! Updated.
(https://github.com/bitcoin/bitcoin/pull/30997#discussion_r1824765673)
Thanks! Updated.
💬 darosior commented on issue "(Past issue) On Windows, pruned nodes could crash while deleting a block file":
(https://github.com/bitcoin/bitcoin/issues/31193#issuecomment-2450283360)
Of course, better safe than sorry with this matter!
(https://github.com/bitcoin/bitcoin/issues/31193#issuecomment-2450283360)
Of course, better safe than sorry with this matter!
💬 Sjors commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2450313186)
> I believe your build log contains a warning like this:
Ah yes, I see those, though buried in the logs. It seems good to _recommend_ ninja in that case, for the QT depends build and all platforms.
(https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2450313186)
> I believe your build log contains a warning like this:
Ah yes, I see those, though buried in the logs. It seems good to _recommend_ ninja in that case, for the QT depends build and all platforms.
💬 hebasto commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2450317443)
> > I believe your build log contains a warning like this:
>
> Ah yes, I see those, though buried in the logs. It seems good to _recommend_ ninja in that case, for the QT depends build and all platforms.
GNU Make fails to build Qt on Linux though.
(https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2450317443)
> > I believe your build log contains a warning like this:
>
> Ah yes, I see those, though buried in the logs. It seems good to _recommend_ ninja in that case, for the QT depends build and all platforms.
GNU Make fails to build Qt on Linux though.
💬 Sjors commented on issue "Gracefully handle dropped UPnP support ":
(https://github.com/bitcoin-core/gui/issues/843#issuecomment-2450319051)
@darosior I could imagine a scenario in which we remove a Tor related setting and it would be unsafe to simply ignore it. But that's not the case here.
(https://github.com/bitcoin-core/gui/issues/843#issuecomment-2450319051)
@darosior I could imagine a scenario in which we remove a Tor related setting and it would be unsafe to simply ignore it. But that's not the case here.
💬 petertodd commented on pull request "Remove mempoolfullrbf":
(https://github.com/bitcoin/bitcoin/pull/30592#issuecomment-2450326549)
On Thu, Oct 31, 2024 at 09:14:40AM -0700, Gregory Sanders wrote:
> > @@ -34,7 +34,7 @@ BIPs that are implemented by Bitcoin Core:
> * [`BIP 111`](https://github.com/bitcoin/bips/blob/master/bip-0111.mediawiki): `NODE_BLOOM` service bit added, and enforced for all peer versions as of **v0.13.0** ([PR #6579](https://github.com/bitcoin/bitcoin/pull/6579) and [PR #6641](https://github.com/bitcoin/bitcoin/pull/6641)).
> * [`BIP 112`](https://github.com/bitcoin/bips/blob/master/bip-0112.mediawiki):
...
(https://github.com/bitcoin/bitcoin/pull/30592#issuecomment-2450326549)
On Thu, Oct 31, 2024 at 09:14:40AM -0700, Gregory Sanders wrote:
> > @@ -34,7 +34,7 @@ BIPs that are implemented by Bitcoin Core:
> * [`BIP 111`](https://github.com/bitcoin/bips/blob/master/bip-0111.mediawiki): `NODE_BLOOM` service bit added, and enforced for all peer versions as of **v0.13.0** ([PR #6579](https://github.com/bitcoin/bitcoin/pull/6579) and [PR #6641](https://github.com/bitcoin/bitcoin/pull/6641)).
> * [`BIP 112`](https://github.com/bitcoin/bips/blob/master/bip-0112.mediawiki):
...
💬 willcl-ark commented on issue "Fatal LevelDB error: Corruption: block checksum mismatch on Linux ext4 SATA SSDs":
(https://github.com/bitcoin/bitcoin/issues/30692#issuecomment-2450339413)
32MB does seem pretty small...
(https://github.com/bitcoin/bitcoin/issues/30692#issuecomment-2450339413)
32MB does seem pretty small...
💬 github12101 commented on issue "Fatal LevelDB error: Corruption: block checksum mismatch on Linux ext4 SATA SSDs":
(https://github.com/bitcoin/bitcoin/issues/30692#issuecomment-2450347899)
> > > hmm I have a small dbcache size I just noticed... can't remember why I set it to that. maybe the increased fsyncs is increasing the chance of a bug somehow? can't think of anything else if its not a hardware issue.
> >
> >
> > Can/did you run memtest86, for several hours? No errors?
>
> I haven't done that yet because I run my entire business (https://damus.io) off this node. my system has been stable for years.
I am afraid, until you do, you can't rule out memory problems. Unti
...
(https://github.com/bitcoin/bitcoin/issues/30692#issuecomment-2450347899)
> > > hmm I have a small dbcache size I just noticed... can't remember why I set it to that. maybe the increased fsyncs is increasing the chance of a bug somehow? can't think of anything else if its not a hardware issue.
> >
> >
> > Can/did you run memtest86, for several hours? No errors?
>
> I haven't done that yet because I run my entire business (https://damus.io) off this node. my system has been stable for years.
I am afraid, until you do, you can't rule out memory problems. Unti
...
💬 github12101 commented on issue "Fatal LevelDB error: Corruption: block checksum mismatch on Linux ext4 SATA SSDs":
(https://github.com/bitcoin/bitcoin/issues/30692#issuecomment-2450352153)
> 32MB does seem pretty small...
Good point, @jb55 any only 32MiB for dbcache?
I gave it the following:
`dbcache=16384`
Is it recommended to give it as much as you can spare. If you have free RAM, give it couple of gigabytes.
(https://github.com/bitcoin/bitcoin/issues/30692#issuecomment-2450352153)
> 32MB does seem pretty small...
Good point, @jb55 any only 32MiB for dbcache?
I gave it the following:
`dbcache=16384`
Is it recommended to give it as much as you can spare. If you have free RAM, give it couple of gigabytes.