🤔 murchandamus reviewed a pull request: "policy: bump TX_MAX_STANDARD_VERSION to 3"
(https://github.com/bitcoin/bitcoin/pull/29496#pullrequestreview-2074297269)
Still looks good to me
crACK e41dae322d435cd8b32daf73883b466f30349584
(https://github.com/bitcoin/bitcoin/pull/29496#pullrequestreview-2074297269)
Still looks good to me
crACK e41dae322d435cd8b32daf73883b466f30349584
💬 Sjors commented on pull request "net: Replace libnatpmp with built-in PCP+NATPMP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1611896435)
`oldlenp=` does because it's key to the trick of how we get the length first. I find these variable name hints useful to e.g. search for them.
(https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1611896435)
`oldlenp=` does because it's key to the trick of how we get the length first. I find these variable name hints useful to e.g. search for them.
👍 GeorgeTsagk approved a pull request: "policy: restrict all TRUC (v3) transactions to 10kvB"
(https://github.com/bitcoin/bitcoin/pull/29873#pullrequestreview-2074316856)
ack https://github.com/bitcoin/bitcoin/pull/29873/commits/154b2b2296edccb5ed24e829798dacb6195edc11
(https://github.com/bitcoin/bitcoin/pull/29873#pullrequestreview-2074316856)
ack https://github.com/bitcoin/bitcoin/pull/29873/commits/154b2b2296edccb5ed24e829798dacb6195edc11
💬 luke-jr commented on pull request "Showing Local Addresses in Node Window":
(https://github.com/bitcoin-core/gui/pull/626#issuecomment-2127412227)
>did that. cleaner and atomic. thanks.
Hmm, it's not that way in your new push tho
(https://github.com/bitcoin-core/gui/pull/626#issuecomment-2127412227)
>did that. cleaner and atomic. thanks.
Hmm, it's not that way in your new push tho
⚠️ apulsifer opened an issue: "LevelDB read failure: Corruption: block checksum mismatch"
(https://github.com/bitcoin/bitcoin/issues/30159)
### Is there an existing issue for this?
- [X] I have searched the existing issues
### Current behaviour
When running in prune=550 mode, I consistently get the following error about once every 10 days per machine:
LevelDB read failure: Corruption: block checksum mismatch
There is no recovery from this error (reindex doesn't work in prune mode), so the only solution is to nuke the datadir and do a full resync or restore the datadir from a backup.
Searching the webs, the conventional w
...
(https://github.com/bitcoin/bitcoin/issues/30159)
### Is there an existing issue for this?
- [X] I have searched the existing issues
### Current behaviour
When running in prune=550 mode, I consistently get the following error about once every 10 days per machine:
LevelDB read failure: Corruption: block checksum mismatch
There is no recovery from this error (reindex doesn't work in prune mode), so the only solution is to nuke the datadir and do a full resync or restore the datadir from a backup.
Searching the webs, the conventional w
...
💬 kcalvinalvin commented on pull request "Testnet4 including PoW difficulty adjustment fix":
(https://github.com/bitcoin/bitcoin/pull/29775#issuecomment-2127414985)
Maybe I'm getting a wrong vibe for the whole PR but what happened to actually having a written out specifications. The best thing out there now is just the mailing list and this PR, which I don't even know which parts of the mailing list this PR is supporting without reading into the code.
Testnet is where devs actually test things and this is ridiculous that it's being pushed out like this. Yes for mainnet people mostly run Core but for testnet people are more open to alt-implementations and s
...
(https://github.com/bitcoin/bitcoin/pull/29775#issuecomment-2127414985)
Maybe I'm getting a wrong vibe for the whole PR but what happened to actually having a written out specifications. The best thing out there now is just the mailing list and this PR, which I don't even know which parts of the mailing list this PR is supporting without reading into the code.
Testnet is where devs actually test things and this is ridiculous that it's being pushed out like this. Yes for mainnet people mostly run Core but for testnet people are more open to alt-implementations and s
...
💬 Sjors commented on pull request "net: Replace libnatpmp with built-in PCP+NATPMP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1611906833)
Can you add "to gateway x.x.x.x" here? I'm getting this error on your latest commit, three times, but I can't tell if it's IPv4, IPv6 or both.
(https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1611906833)
Can you add "to gateway x.x.x.x" here? I'm getting this error on your latest commit, three times, but I can't tell if it's IPv4, IPv6 or both.
💬 Sjors commented on pull request "net: Replace libnatpmp with built-in PCP+NATPMP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1611917789)
Update: this happens when I'm connected with a physical LAN cable _and_ wifi. So the warning was safe to ignore, but I'm still curious about it.
(https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1611917789)
Update: this happens when I'm connected with a physical LAN cable _and_ wifi. So the warning was safe to ignore, but I'm still curious about it.
💬 maflcko commented on issue "LevelDB read failure: Corruption: block checksum mismatch":
(https://github.com/bitcoin/bitcoin/issues/30159#issuecomment-2127439462)
> I suspect a bug in the code is causing some thread to write to an incorrect memory location, possibly a memory use-after-free/reallocation/reorganization bug.
Would it be possible for you to compile and run with asan, or a similar sanitizer?
Also, what filesystem are you using on the drives? Something like `df --print-type --human-readable /bdata` should print it.
(https://github.com/bitcoin/bitcoin/issues/30159#issuecomment-2127439462)
> I suspect a bug in the code is causing some thread to write to an incorrect memory location, possibly a memory use-after-free/reallocation/reorganization bug.
Would it be possible for you to compile and run with asan, or a similar sanitizer?
Also, what filesystem are you using on the drives? Something like `df --print-type --human-readable /bdata` should print it.
💬 apulsifer commented on issue "LevelDB read failure: Corruption: block checksum mismatch":
(https://github.com/bitcoin/bitcoin/issues/30159#issuecomment-2127451533)
xfs on the root and data drive
sudo mkswap /dev/nvme[4 GB disk]
sudo swapon /dev/nvme[4 GB disk]
sudo mkdir /bdata
sudo mkfs -t xfs /dev/nvme[40 GB disk]
lsblk -o name,size,type,uuid
sudo nano /etc/fstab
add to fstab:
UUID=[4 GB disk uuid] swap swap defaults 0 0
UUID=[40 GB disk uuid] /bdata xfs defaults,nofail 0 2
sudo mount -a
Filesystem Type Size Used Avail Use% Mounted on
devtmpfs devtmpfs 4.0M 0 4.0M 0% /dev
tmpfs tmpfs 210M
...
(https://github.com/bitcoin/bitcoin/issues/30159#issuecomment-2127451533)
xfs on the root and data drive
sudo mkswap /dev/nvme[4 GB disk]
sudo swapon /dev/nvme[4 GB disk]
sudo mkdir /bdata
sudo mkfs -t xfs /dev/nvme[40 GB disk]
lsblk -o name,size,type,uuid
sudo nano /etc/fstab
add to fstab:
UUID=[4 GB disk uuid] swap swap defaults 0 0
UUID=[40 GB disk uuid] /bdata xfs defaults,nofail 0 2
sudo mount -a
Filesystem Type Size Used Avail Use% Mounted on
devtmpfs devtmpfs 4.0M 0 4.0M 0% /dev
tmpfs tmpfs 210M
...
📝 sipa opened a pull request: "util: add BitSet"
(https://github.com/bitcoin/bitcoin/pull/30160)
Extracted from #30126.
This introduces the `BitSet` data structure, inspired by `std::bitset`, but with a few features that cannot be implemented on top without efficiency loss:
* Finding the first set bit.
* Finding the last set bit.
* Iterating over all set bits.
And a few other operators that help readability for #30126:
* `operator/` for set subtraction
* `operator&&` for testing whether intersection is non-empty
* `Fill` to construct a set with all numbers from 0 to n-1, inclusi
...
(https://github.com/bitcoin/bitcoin/pull/30160)
Extracted from #30126.
This introduces the `BitSet` data structure, inspired by `std::bitset`, but with a few features that cannot be implemented on top without efficiency loss:
* Finding the first set bit.
* Finding the last set bit.
* Iterating over all set bits.
And a few other operators that help readability for #30126:
* `operator/` for set subtraction
* `operator&&` for testing whether intersection is non-empty
* `Fill` to construct a set with all numbers from 0 to n-1, inclusi
...
💬 luke-jr commented on pull request "rpc: introduce getversion RPC":
(https://github.com/bitcoin/bitcoin/pull/30112#issuecomment-2127454770)
Concept NACK, consumers should just check if features are available
(https://github.com/bitcoin/bitcoin/pull/30112#issuecomment-2127454770)
Concept NACK, consumers should just check if features are available
💬 apulsifer commented on issue "LevelDB read failure: Corruption: block checksum mismatch":
(https://github.com/bitcoin/bitcoin/issues/30159#issuecomment-2127461229)
> Would it be possible for you to compile and run with asan, or a similar sanitizer?
I don't see where I would get a chunk of time to do that right now.... But as I mentioned, I copied the corrupted files, and that might give some clues to someone familiar with their format (especially if the corruption is ascii in the middle of binary, or vice versa)
(https://github.com/bitcoin/bitcoin/issues/30159#issuecomment-2127461229)
> Would it be possible for you to compile and run with asan, or a similar sanitizer?
I don't see where I would get a chunk of time to do that right now.... But as I mentioned, I copied the corrupted files, and that might give some clues to someone familiar with their format (especially if the corruption is ascii in the middle of binary, or vice versa)
💬 achow101 commented on pull request "policy: restrict all TRUC (v3) transactions to 10kvB":
(https://github.com/bitcoin/bitcoin/pull/29873#issuecomment-2127467518)
ACK 154b2b2296edccb5ed24e829798dacb6195edc11
(https://github.com/bitcoin/bitcoin/pull/29873#issuecomment-2127467518)
ACK 154b2b2296edccb5ed24e829798dacb6195edc11
💬 ryanofsky commented on pull request "Encapsulate warnings in generalized node::Warnings and remove globals":
(https://github.com/bitcoin/bitcoin/pull/30058#discussion_r1611939057)
I think in the longer run the string/enum distinction should not matter too much here.
In general, requiring kernel applications to get warning information from `warning`, or `warningSet`/`warningUnset` callbacks is not a nice API. It would make more sense for the kernel to treat errors and warnings similarly, and return this information directly in function return values or output arguments, instead of indirect callbacks. We have PR #29642 and #29700 which change kernel code to bubble up err
...
(https://github.com/bitcoin/bitcoin/pull/30058#discussion_r1611939057)
I think in the longer run the string/enum distinction should not matter too much here.
In general, requiring kernel applications to get warning information from `warning`, or `warningSet`/`warningUnset` callbacks is not a nice API. It would make more sense for the kernel to treat errors and warnings similarly, and return this information directly in function return values or output arguments, instead of indirect callbacks. We have PR #29642 and #29700 which change kernel code to bubble up err
...
💬 TheCharlatan commented on pull request "indexes: Don't wipe indexes again when continuing a prior reindex":
(https://github.com/bitcoin/bitcoin/pull/30132#issuecomment-2127473363)
Updated 9de8b263dabd6dd2f86f1f0253c6ee3fac7a4407 -> dd290b3b58a1f9f9e182803866b8e2b9f72ecb3b ([preserveIndexOnRestart_1](https://github.com/TheCharlatan/bitcoin/tree/preserveIndexOnRestart_1) -> [preserveIndexOnRestart_2](https://github.com/TheCharlatan/bitcoin/tree/preserveIndexOnRestart_2), [compare](https://github.com/TheCharlatan/bitcoin/compare/preserveIndexOnRestart_1..preserveIndexOnRestart_2))
* Added a commit for testing that the indexes are still there when continuing a reindex.
(https://github.com/bitcoin/bitcoin/pull/30132#issuecomment-2127473363)
Updated 9de8b263dabd6dd2f86f1f0253c6ee3fac7a4407 -> dd290b3b58a1f9f9e182803866b8e2b9f72ecb3b ([preserveIndexOnRestart_1](https://github.com/TheCharlatan/bitcoin/tree/preserveIndexOnRestart_1) -> [preserveIndexOnRestart_2](https://github.com/TheCharlatan/bitcoin/tree/preserveIndexOnRestart_2), [compare](https://github.com/TheCharlatan/bitcoin/compare/preserveIndexOnRestart_1..preserveIndexOnRestart_2))
* Added a commit for testing that the indexes are still there when continuing a reindex.
💬 BenWestgate commented on pull request "Drop -dbcache limit":
(https://github.com/bitcoin/bitcoin/pull/28358#issuecomment-2127487328)
> If we are worried about someone using an inappropriate value we can try to solve that separately (maybe by testing that we can allocate that much at startup or something?).
The best would be never setting it greater than total RAM. `-dbcache=<total RAM>` is still higher than optimal as there will be applications in memory that won't be swapped to disk including Bitcoin. But measuring Available RAM or swap is precarious as it changes.
Since other reviewers mentioned it's only wildly exces
...
(https://github.com/bitcoin/bitcoin/pull/28358#issuecomment-2127487328)
> If we are worried about someone using an inappropriate value we can try to solve that separately (maybe by testing that we can allocate that much at startup or something?).
The best would be never setting it greater than total RAM. `-dbcache=<total RAM>` is still higher than optimal as there will be applications in memory that won't be swapped to disk including Bitcoin. But measuring Available RAM or swap is precarious as it changes.
Since other reviewers mentioned it's only wildly exces
...
🚀 achow101 merged a pull request: "policy: restrict all TRUC (v3) transactions to 10kvB"
(https://github.com/bitcoin/bitcoin/pull/29873)
(https://github.com/bitcoin/bitcoin/pull/29873)
💬 Sjors commented on pull request "net: Replace libnatpmp with built-in PCP+NATPMP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1611953200)
Possible alternative, makes it a bit more clear that we rely on `rt->rtm_msglen` to be correct. A `for` loop gives a false sense of safety. No strong feelings though.
```cpp
std::byte* p = buf.data();
while (true) {
rt = (const struct rt_msghdr*)p;
// ...
p += rt->rtm_msglen;
if (p == buf.data() + buf.size()) break;
assert(p < buf.data() + buf.size());
}
```
(https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1611953200)
Possible alternative, makes it a bit more clear that we rely on `rt->rtm_msglen` to be correct. A `for` loop gives a false sense of safety. No strong feelings though.
```cpp
std::byte* p = buf.data();
while (true) {
rt = (const struct rt_msghdr*)p;
// ...
p += rt->rtm_msglen;
if (p == buf.data() + buf.size()) break;
assert(p < buf.data() + buf.size());
}
```
💬 apoelstra commented on pull request "rpc: introduce getversion RPC":
(https://github.com/bitcoin/bitcoin/pull/30112#issuecomment-2127508227)
@luke-jr are you suggesting that all consumers should call the entire RPC surface to confirm which calls are present, what the types of all inputs are, what the types of all outputs are, etc., in order to determine whether their software is compatible with the interface?
(https://github.com/bitcoin/bitcoin/pull/30112#issuecomment-2127508227)
@luke-jr are you suggesting that all consumers should call the entire RPC surface to confirm which calls are present, what the types of all inputs are, what the types of all outputs are, etc., in order to determine whether their software is compatible with the interface?