💬 instagibbs commented on pull request "refactor prep for package rbf":
(https://github.com/bitcoin/bitcoin/pull/30072#discussion_r1612164710)
In package RBF this changes: https://github.com/bitcoin/bitcoin/pull/28984/commits/f93b602e74e101f4fed51f6d43b4930fcd7b0530#diff-97c3a52bc5fad452d82670a7fd291800bae20c7bc35bb82686c2c0a4ea7b5b98R528
We don't want sibling eviction attempts to happen during package rbf logic
(https://github.com/bitcoin/bitcoin/pull/30072#discussion_r1612164710)
In package RBF this changes: https://github.com/bitcoin/bitcoin/pull/28984/commits/f93b602e74e101f4fed51f6d43b4930fcd7b0530#diff-97c3a52bc5fad452d82670a7fd291800bae20c7bc35bb82686c2c0a4ea7b5b98R528
We don't want sibling eviction attempts to happen during package rbf logic
💬 instagibbs commented on pull request "refactor prep for package rbf":
(https://github.com/bitcoin/bitcoin/pull/30072#discussion_r1612164921)
I reverted that change based on your feedback already :)
(https://github.com/bitcoin/bitcoin/pull/30072#discussion_r1612164921)
I reverted that change based on your feedback already :)
💬 instagibbs commented on pull request "refactor prep for package rbf":
(https://github.com/bitcoin/bitcoin/pull/30072#discussion_r1612164986)
My reading of it is if it fails *below* for a second time, we will want to have cached the first error string.
either way, this PR hopefully doesn't make this confusion worse than status quo?
(https://github.com/bitcoin/bitcoin/pull/30072#discussion_r1612164986)
My reading of it is if it fails *below* for a second time, we will want to have cached the first error string.
either way, this PR hopefully doesn't make this confusion worse than status quo?
💬 instagibbs commented on pull request "refactor prep for package rbf":
(https://github.com/bitcoin/bitcoin/pull/30072#discussion_r1612165294)
will touch if I have to retouch the PR
(https://github.com/bitcoin/bitcoin/pull/30072#discussion_r1612165294)
will touch if I have to retouch the PR
👍 theuni approved a pull request: "build: LLD based macOS toolchain"
(https://github.com/bitcoin/bitcoin/pull/21778#pullrequestreview-2074783866)
Tentative ACK e8c25e8a35e333e90514945c592557615641553f.
There's a lot going on here and I'm not 100% confident, but I'm out of things to complain about :)
(https://github.com/bitcoin/bitcoin/pull/21778#pullrequestreview-2074783866)
Tentative ACK e8c25e8a35e333e90514945c592557615641553f.
There's a lot going on here and I'm not 100% confident, but I'm out of things to complain about :)
💬 brunoerg commented on pull request "i2p: fix and improve logs":
(https://github.com/bitcoin/bitcoin/pull/29833#discussion_r1612179061)
done
(https://github.com/bitcoin/bitcoin/pull/29833#discussion_r1612179061)
done
💬 brunoerg commented on pull request "i2p: fix and improve logs":
(https://github.com/bitcoin/bitcoin/pull/29833#discussion_r1612179235)
done
(https://github.com/bitcoin/bitcoin/pull/29833#discussion_r1612179235)
done
💬 brunoerg commented on pull request "i2p: fix and improve logs":
(https://github.com/bitcoin/bitcoin/pull/29833#issuecomment-2127843639)
Force-pushed addressing https://github.com/bitcoin/bitcoin/pull/29833#discussion_r1599061492.
(https://github.com/bitcoin/bitcoin/pull/29833#issuecomment-2127843639)
Force-pushed addressing https://github.com/bitcoin/bitcoin/pull/29833#discussion_r1599061492.
💬 sr-gi commented on pull request "refactor prep for package rbf":
(https://github.com/bitcoin/bitcoin/pull/30072#discussion_r1612181309)
Yeah, I don't think it makes it worse
(https://github.com/bitcoin/bitcoin/pull/30072#discussion_r1612181309)
Yeah, I don't think it makes it worse
💬 sr-gi commented on pull request "refactor prep for package rbf":
(https://github.com/bitcoin/bitcoin/pull/30072#discussion_r1612181759)
Oh sorry, this was left on my first pass, which I never submitted 😆
(https://github.com/bitcoin/bitcoin/pull/30072#discussion_r1612181759)
Oh sorry, this was left on my first pass, which I never submitted 😆
💬 apulsifer commented on issue "LevelDB read failure: Corruption: block checksum mismatch":
(https://github.com/bitcoin/bitcoin/issues/30159#issuecomment-2127853991)
> Once per week is a lot and if this was a broader problem, I'd assume that more people were complaining.
It could just be that the memory pressure is uncovering the problem. Of course, any machine can experience memory pressure at times, but one thing that's unique is that those machines starting hard paging to SSD for about 20 seconds after each new block arrives.
(https://github.com/bitcoin/bitcoin/issues/30159#issuecomment-2127853991)
> Once per week is a lot and if this was a broader problem, I'd assume that more people were complaining.
It could just be that the memory pressure is uncovering the problem. Of course, any machine can experience memory pressure at times, but one thing that's unique is that those machines starting hard paging to SSD for about 20 seconds after each new block arrives.
💬 davidgumberg commented on pull request "ci: add markdown link check job":
(https://github.com/bitcoin/bitcoin/pull/30034#discussion_r1612189326)
Nit: It would be nice to have a message that briefly describes and motivates the lint check as elsewhere
e.g. from `lint_std_filesystem`
```rust
if found {
Err(r#"
^^^
Direct use of std::filesystem may be dangerous and buggy. Please include <util/fs.h> and use the
fs:: namespace, which has unsafe filesystem functions marked as deleted.
"#
.to_string())
```
(https://github.com/bitcoin/bitcoin/pull/30034#discussion_r1612189326)
Nit: It would be nice to have a message that briefly describes and motivates the lint check as elsewhere
e.g. from `lint_std_filesystem`
```rust
if found {
Err(r#"
^^^
Direct use of std::filesystem may be dangerous and buggy. Please include <util/fs.h> and use the
fs:: namespace, which has unsafe filesystem functions marked as deleted.
"#
.to_string())
```
💬 kcalvinalvin commented on pull request "Testnet4 including PoW difficulty adjustment fix":
(https://github.com/bitcoin/bitcoin/pull/29775#issuecomment-2127862043)
> I don't think we should see the lack of a testnet3 BIP as an argument against this.
Not even a BIP but some document that specifies testnet4 besides just a PR that still might get changed.
I think in 2024 we can agree that there's more than just Bitcoin Core and asking other implementations to "read the Bitcoin Core codebase" is a ridiculous ask.
(https://github.com/bitcoin/bitcoin/pull/29775#issuecomment-2127862043)
> I don't think we should see the lack of a testnet3 BIP as an argument against this.
Not even a BIP but some document that specifies testnet4 besides just a PR that still might get changed.
I think in 2024 we can agree that there's more than just Bitcoin Core and asking other implementations to "read the Bitcoin Core codebase" is a ridiculous ask.
💬 laanwj commented on pull request "net: Replace libnatpmp with built-in PCP+NATPMP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1612195743)
From what i understand, the error signalling and many of the flags and operations in the `rt_msghdr` are used with `PF_ROUTE` sockets, interactively. `sysctl` stores a read-only copy of the routing table data so that non-root users can access it.
Graned, it's kind of a weird API.
(https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1612195743)
From what i understand, the error signalling and many of the flags and operations in the `rt_msghdr` are used with `PF_ROUTE` sockets, interactively. `sysctl` stores a read-only copy of the routing table data so that non-root users can access it.
Graned, it's kind of a weird API.
💬 davidgumberg commented on pull request "ci: add markdown link check job":
(https://github.com/bitcoin/bitcoin/pull/30034#issuecomment-2127907853)
crACK https://github.com/bitcoin/bitcoin/pull/30034/commits/82c5f9531021c76180fa34fa9cbd243ad1c994f3
I tested by manually breaking relative and absolute offline links and verifying that the lint runner catches them.
Some notes:
- At present, mlc does not check for anchor link validity, but likely will in the future. (https://github.com/becheran/mlc/issues/31)
For example:
```diff
-Verification of [scripted diffs](/doc/developer-notes.md#scripted-diffs).
+Verification of [
...
(https://github.com/bitcoin/bitcoin/pull/30034#issuecomment-2127907853)
crACK https://github.com/bitcoin/bitcoin/pull/30034/commits/82c5f9531021c76180fa34fa9cbd243ad1c994f3
I tested by manually breaking relative and absolute offline links and verifying that the lint runner catches them.
Some notes:
- At present, mlc does not check for anchor link validity, but likely will in the future. (https://github.com/becheran/mlc/issues/31)
For example:
```diff
-Verification of [scripted diffs](/doc/developer-notes.md#scripted-diffs).
+Verification of [
...
💬 theuni commented on pull request "util: add BitSet":
(https://github.com/bitcoin/bitcoin/pull/30160#discussion_r1612232560)
`std::bitset` throws for these if pos is out of range.
Maybe add `Assume()`s? Or were you trying to keep the header self-contained?
(https://github.com/bitcoin/bitcoin/pull/30160#discussion_r1612232560)
`std::bitset` throws for these if pos is out of range.
Maybe add `Assume()`s? Or were you trying to keep the header self-contained?
💬 apulsifer commented on issue "LevelDB read failure: Corruption: block checksum mismatch":
(https://github.com/bitcoin/bitcoin/issues/30159#issuecomment-2127922118)
> Could you explain the paging in a bit more detail?
The machines page pretty hard to SSD for about 20 seconds after each new block arrives. That info comes from "sar -d 10", which I logged for a while when I was setting up the first machine.
> Is the node constantly being bombarded with lots of simultaneous RPC calls (which ones?) or is some other interface used?
None of these machines has at this point serviced a single RPC call (I'm still trying to get things set up). No other inter
...
(https://github.com/bitcoin/bitcoin/issues/30159#issuecomment-2127922118)
> Could you explain the paging in a bit more detail?
The machines page pretty hard to SSD for about 20 seconds after each new block arrives. That info comes from "sar -d 10", which I logged for a while when I was setting up the first machine.
> Is the node constantly being bombarded with lots of simultaneous RPC calls (which ones?) or is some other interface used?
None of these machines has at this point serviced a single RPC call (I'm still trying to get things set up). No other inter
...
💬 theuni commented on pull request "util: add BitSet":
(https://github.com/bitcoin/bitcoin/pull/30160#discussion_r1612240218)
I know you like dense code, but...
I suspect function names would make reviewing uses of this easier than operators. Especially for the non-obvious ones like `&&` that `std::bitset` doesn't support.
I know that personally I'd have an easier time reviewing `a.intersects(b)`.
(https://github.com/bitcoin/bitcoin/pull/30160#discussion_r1612240218)
I know you like dense code, but...
I suspect function names would make reviewing uses of this easier than operators. Especially for the non-obvious ones like `&&` that `std::bitset` doesn't support.
I know that personally I'd have an easier time reviewing `a.intersects(b)`.
💬 epiccurious commented on pull request "doc: Update mentions of generating bitcoin.conf":
(https://github.com/bitcoin/bitcoin/pull/30154#discussion_r1612244540)
Agreed. Resolved with aef43a3a996618d581e0a16f7fc5e842708e83c9.
(https://github.com/bitcoin/bitcoin/pull/30154#discussion_r1612244540)
Agreed. Resolved with aef43a3a996618d581e0a16f7fc5e842708e83c9.
💬 davidgumberg commented on pull request "doc: update mention of generating bitcoin.conf":
(https://github.com/bitcoin/bitcoin/pull/30154#issuecomment-2127946115)
You should squash this into a single commit and update the PR description
(https://github.com/bitcoin/bitcoin/pull/30154#issuecomment-2127946115)
You should squash this into a single commit and update the PR description