💬 fjahr commented on pull request "Silent payment index (for light wallets and consistency check)":
(https://github.com/bitcoin/bitcoin/pull/28241#issuecomment-1950323631)
> Being able to specify a start_height sounds like something that would be generally useful for all indexes, not just this one. I've been toying with the idea of what it would take to have a "pruned" txindex and I would need to be able to start from a specific height for that. @fjahr I took the branch you started and got it working with a little tinkering here: https://github.com/josibake/bitcoin/tree/add-start-height-to-base-index . With a little more work this could probably be its own PR. Fee
...

🤔 sdaftuar reviewed a pull request: "Mempool util: Add RBF diagram checks for single chunks against clusters of size 2"
(https://github.com/bitcoin/bitcoin/pull/29242#pullrequestreview-1886598977)
Still reviewing all the tests -- leaving some nits that I've noticed so far.

In addition:
* I commented [here](https://github.com/bitcoin/bitcoin/pull/29242#discussion_r1493358857) that I think we should drop the `Assume(size != 0 || fee == 0)` calls in feefrac.h, so that we don't have to police all the places we invoke operator- to make sure we can never accidentally violate that.
* Another nit: in the commit message for commit 860823fb93212fa78ab928589bd403da462be222, it should say "Fee
...

💬 sdaftuar commented on pull request "Mempool util: Add RBF diagram checks for single chunks against clusters of size 2":
(https://github.com/bitcoin/bitcoin/pull/29242#discussion_r1493359169)
nit: As the functions here don't relate to the `FeeFrac` implementation, perhaps this should be in a different file? `feerate_diagram.cpp` maybe?

💬 sdaftuar commented on pull request "Mempool util: Add RBF diagram checks for single chunks against clusters of size 2":
(https://github.com/bitcoin/bitcoin/pull/29242#discussion_r1493390467)
nit: Could add a comment here more precisely defining the preconditions for this function and what definition we are using for feerate diagram: specifically that a feerate diagram consists of a list of (fee, size) points with the property that size is strictly increasing and that the first entry is (0, 0). (If these conditions are violated then the function can fail with an `Assume()` failure.)

💬 sdaftuar commented on pull request "Mempool util: Add RBF diagram checks for single chunks against clusters of size 2":
(https://github.com/bitcoin/bitcoin/pull/29242#discussion_r1493359700)
nit: "direction coefficient" is not a term I'm familiar with (and from googling it doesn't seem like a super common term); perhaps it would be clearer to just use the word "slope" here?

💬 sdaftuar commented on pull request "Mempool util: Add RBF diagram checks for single chunks against clusters of size 2":
(https://github.com/bitcoin/bitcoin/pull/29242#discussion_r1493401259)
nit: Maybe update the comment to be more explicit that this function only works for the situation where (replacement_fees, replacement_size) corresponds to a transaction package that has no in-mempool dependences.

💬 sdaftuar commented on pull request "Mempool util: Add RBF diagram checks for single chunks against clusters of size 2":
(https://github.com/bitcoin/bitcoin/pull/29242#discussion_r1493391184)
nit: "fee vs. size diagram" -- maybe we should standardize the terminology to "feerate diagram"? I was also thinking about adding some documentation to doc/policy/mempool-replacements.md explaining what we're doing now.

💬 fjahr commented on pull request "Silent payment index (for light wallets and consistency check)":
(https://github.com/bitcoin/bitcoin/pull/28241#discussion_r1493405713)
Why hardcode this value here when we can get it from chain params?

💬 sipa commented on pull request "Mempool util: Add RBF diagram checks for single chunks against clusters of size 2":
(https://github.com/bitcoin/bitcoin/pull/29242#discussion_r1493417950)
Oh, I seem to have translated literally from Dutch. "Slope" is indeed the proper English translation.

💬 fjahr commented on pull request "Silent payment index (for light wallets and consistency check)":
(https://github.com/bitcoin/bitcoin/pull/28241#discussion_r1493429857)
Removing this is causing an error on all non-mainnet chains now because m_start_height is 0 by default, which is the genesis block and it needs to be skipped because it's unspendable and has no undo data.

🤔 tdb3 reviewed a pull request: "Double -dbache maximum to 32GB"
(https://github.com/bitcoin/bitcoin/pull/28358#pullrequestreview-1887042452)
crACK

💬 ismaelsadeeq commented on pull request "Mempool util: Add RBF diagram checks for single chunks against clusters of size 2":
(https://github.com/bitcoin/bitcoin/pull/29242#discussion_r1493469026)
Curious to know the full meaning of `FeeFrac` is it Fee Fraction?
Maybe Chunk is will be a better name?

💬 fjahr commented on pull request "Silent payment index (for light wallets and consistency check)":
(https://github.com/bitcoin/bitcoin/pull/28241#discussion_r1493494203)
This isn't the right place to pass the `start_height`, this is actually setting the obfuscate option of the DB that is constructed here.

💬 fjahr commented on pull request "Silent payment index (for light wallets and consistency check)":
(https://github.com/bitcoin/bitcoin/pull/28241#discussion_r1493495599)
The `start height` goes into the `SilentPaymentIndex` constructor on line 45 below:

```
SilentPaymentIndex::SilentPaymentIndex(std::unique_ptr<interfaces::Chain> chain, size_t n_cache_size, bool f_memory, bool f_wipe)
: BaseIndex(std::move(chain), "silentpaymentindex", /*start_height=*/TAPROOT_MAINNET_ACTIVATION_HEIGHT), m_db(std::make_unique<SilentPaymentIndex::DB>(n_cache_size, f_memory, f_wipe))
{}
```

💬 fjahr commented on pull request "Silent payment index (for light wallets and consistency check)":
(https://github.com/bitcoin/bitcoin/pull/28241#issuecomment-1950636630)
For me the index now synced fine from the Taproot start height when I applied the suggestion fixes and removed the blockstatsindex commit.

📝 theStack opened a pull request: "depends: fix BDB compilation on OpenBSD"
(https://github.com/bitcoin/bitcoin/pull/29443)
Compiling C++ code with `-D_XOPEN_SOURCE=600` causes problems on OpenBSD. If that define is set, the C++ standard header detection routine in BDB's configure script fails due to a missing type name for `locale_t` (see https://gist.github.com/theStack/b41884e31ebc5cdca3220bcaa674cb70 for the relevant config.log part).

This results in `HAVE_CXX_STDHEADERS` not being defined, which then it turn leads to the inclusion of `<iostream.h>` (rather than `<iostream>`), which doesn't exist, as described
...

💬 paplorinc commented on issue "test: Write assumeutxo tests":
(https://github.com/bitcoin/bitcoin/issues/28648#issuecomment-1951014461)
https://github.com/bitcoin/bitcoin/pull/29394 is merged, anything else to do here?

📝 paplorinc opened a pull request: "Improve readability of numeric literals in consensus parameters"
(https://github.com/bitcoin/bitcoin/pull/29444)
This commit enhances the readability of large numeric literals across various consensus-related constants by introducing thousands separators ('), as per the C++14 digit separator feature. Changes are made in chainparams.cpp, amount.h, and consensus.h to include separators in the definition of constants such as nSubsidyHalvingInterval, COIN, MAX_MONEY, MAX_BLOCK_SERIALIZED_SIZE, MAX_BLOCK_WEIGHT, and MAX_BLOCK_SIGOPS_COST. This update aims to improve code clarity without affecting functionality.
...

💬 GregTonoski commented on issue "Witness scripts being abused to bypass datacarriersize limit (CVE-2023-50428)":
(https://github.com/bitcoin/bitcoin/issues/29187#issuecomment-1951100909)
[Common Weakness Enumerations](https://cwe.mitre.org/) (non-exhaustive list) in the Bitcoin Script implementation in Bitcoin Core:
[CWE-561: Dead Code](https://cwe.mitre.org/data/definitions/561.html)
[CWE-570: Expression is Always False](https://cwe.mitre.org/data/definitions/570.html)
[CWE-1164: Irrelevant Code](https://cwe.mitre.org/data/definitions/1164.html)

⚠️ paplorinc opened an issue: "Proposal: Adopt simple SPDX-License-Identifier Across Bitcoin Codebase"
(https://github.com/bitcoin/bitcoin/issues/29445)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

I propose replacing the diverse copyright headers in Bitcoin source files with a uniform, concise [SPDX identifier](https://www.kernel.org/doc/html/v4.18/process/license-rules.html#license-identifier-syntax) to ensure clarity and consistency, thereby streamlining licensing while reducing noise and focusing on modern, efficient license declaration.

This aligns with practices in projects
...