💬 theuni commented on pull request "rpc: avoid copying into UniValue":
(https://github.com/bitcoin/bitcoin/pull/30115#issuecomment-2127371384)
@ryanofsky Thanks.
I'm going to continue working on the more complicated copies in follow-up PRs. I'll play around with your `Copy`/`CopyFrom` idea while I'm at it. If nothing else, that would be a good way to help me track down the remaining cases.
(https://github.com/bitcoin/bitcoin/pull/30115#issuecomment-2127371384)
@ryanofsky Thanks.
I'm going to continue working on the more complicated copies in follow-up PRs. I'll play around with your `Copy`/`CopyFrom` idea while I'm at it. If nothing else, that would be a good way to help me track down the remaining cases.
💬 Eunovo commented on pull request "wallet: fix unrelated parent conflict doesn't cause child tx to be marked as conflict":
(https://github.com/bitcoin/bitcoin/pull/29680#discussion_r1611867224)
> Would also be nice if we prevented these objects from be constructed with `nullptrs`. I'm not sure if we have a convention in our codebase around this or other examples to point to, but would be worth looking into.
It looks like I may be able to do this by deleting the constructor
```
// Deleted constructor
ReplacedReason(std::nullptr_t) = delete;
```
I'll test it
(https://github.com/bitcoin/bitcoin/pull/29680#discussion_r1611867224)
> Would also be nice if we prevented these objects from be constructed with `nullptrs`. I'm not sure if we have a convention in our codebase around this or other examples to point to, but would be worth looking into.
It looks like I may be able to do this by deleting the constructor
```
// Deleted constructor
ReplacedReason(std::nullptr_t) = delete;
```
I'll test it
💬 murchandamus commented on pull request "policy: restrict all TRUC (v3) transactions to 10kvB":
(https://github.com/bitcoin/bitcoin/pull/29873#discussion_r1611867777)
Oh right, I missed that. That mitigates my main concern, and given that this hack is going away eventually, I agree on the lack of urgency.
(https://github.com/bitcoin/bitcoin/pull/29873#discussion_r1611867777)
Oh right, I missed that. That mitigates my main concern, and given that this hack is going away eventually, I agree on the lack of urgency.
💬 achow101 commented on pull request "Make it possible to disable Tor binds and abort startup on bind failure":
(https://github.com/bitcoin/bitcoin/pull/22729#issuecomment-2127373603)
There seems to be an intermittent failure with `feature_discover.py`. 2 of 4 functional test suite runs that I just did failed there with:
```
175/307 - feature_discover.py failed, Duration: 1 s
stdout:
2024-05-23T15:01:57.868000Z TestFramework (INFO): PRNG seed is: 6285884694359884871
2024-05-23T15:01:57.869000Z TestFramework (INFO): Initializing test directory /tmp/test_runner_₿_🏃_20240523_110117/feature_discover_78
2024-05-23T15:01:58.127000Z TestFramework (INFO): Restart node with
...
(https://github.com/bitcoin/bitcoin/pull/22729#issuecomment-2127373603)
There seems to be an intermittent failure with `feature_discover.py`. 2 of 4 functional test suite runs that I just did failed there with:
```
175/307 - feature_discover.py failed, Duration: 1 s
stdout:
2024-05-23T15:01:57.868000Z TestFramework (INFO): PRNG seed is: 6285884694359884871
2024-05-23T15:01:57.869000Z TestFramework (INFO): Initializing test directory /tmp/test_runner_₿_🏃_20240523_110117/feature_discover_78
2024-05-23T15:01:58.127000Z TestFramework (INFO): Restart node with
...
💬 Sjors commented on pull request "net: Replace libnatpmp with built-in PCP+NATPMP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1611882820)
> Also they're variable-length
That makes sense. Whereas `rt_msghdr` is fixed length I guess, so you're able to use `rt + 1` above.
(https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1611882820)
> Also they're variable-length
That makes sense. Whereas `rt_msghdr` is fixed length I guess, so you're able to use `rt + 1` above.
💬 Sjors commented on pull request "net: Replace libnatpmp with built-in PCP+NATPMP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1611884887)
I'm not sure, but there reason I wrote this comment is because initially I thought: yikes, what if this is out of bound?
(https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1611884887)
I'm not sure, but there reason I wrote this comment is because initially I thought: yikes, what if this is out of bound?
💬 sipa commented on pull request "Low-level cluster linearization code":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1611885584)
Remnant of a long-lost pre-C++20 past. Gone.
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1611885584)
Remnant of a long-lost pre-C++20 past. Gone.
💬 Sjors commented on pull request "net: Replace libnatpmp with built-in PCP+NATPMP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1611888217)
Mmm, I thought perhaps it would return a single error entry to indicate failure.
(https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1611888217)
Mmm, I thought perhaps it would return a single error entry to indicate failure.
💬 sipa commented on pull request "Low-level cluster linearization code":
(https://github.com/bitcoin/bitcoin/pull/30126#issuecomment-2127404990)
I've dropped the dependency on #29625; there is a measurable slowdown from using the (ChaCha20-based) FastRandomContext over the (xoroshiro128++-based) InsecureRandomContext introduced there, but it's no more than 1-2%. I can switch back to that approach if 29625 were to make it in.
(https://github.com/bitcoin/bitcoin/pull/30126#issuecomment-2127404990)
I've dropped the dependency on #29625; there is a measurable slowdown from using the (ChaCha20-based) FastRandomContext over the (xoroshiro128++-based) InsecureRandomContext introduced there, but it's no more than 1-2%. I can switch back to that approach if 29625 were to make it in.
💬 murchandamus commented on pull request "policy: bump TX_MAX_STANDARD_VERSION to 3":
(https://github.com/bitcoin/bitcoin/pull/29496#discussion_r1611889254)
There is either too many or too few words here:
```suggestion
// This module enforces rules for BIP 431 TRUC transactions (with nVersion=3) which help make
```
(https://github.com/bitcoin/bitcoin/pull/29496#discussion_r1611889254)
There is either too many or too few words here:
```suggestion
// This module enforces rules for BIP 431 TRUC transactions (with nVersion=3) which help make
```
💬 murchandamus commented on pull request "policy: bump TX_MAX_STANDARD_VERSION to 3":
(https://github.com/bitcoin/bitcoin/pull/29496#discussion_r1611894367)
Good idea to add this!
(https://github.com/bitcoin/bitcoin/pull/29496#discussion_r1611894367)
Good idea to add this!
🤔 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.