Bitcoin Core Github
42 subscribers
126K links
Download Telegram
🤔 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
...
⚠️ helpau opened an issue: "[CI/CD]Release channels?"
(https://github.com/bitcoin/bitcoin/issues/29446)
### Please describe the feature you'd like to see added.

Does it make any sense to use release channels(canary/beta/stable)? I don't have a full understanding of this feature for different users, but I think it might help developers of the core on Bitcoin Core software(Umbrel, Lightning nodes). Of course with a warning that this is a build for testing only.

### Is your feature related to a problem, if so please describe it.

_No response_

### Describe the solution you'd like

_No response_

#
...
💬 fjahr commented on pull request "contrib: add test for bucketing with asmap":
(https://github.com/bitcoin/bitcoin/pull/28869#issuecomment-1951342477)
> Just checked it, @fjahr. I think `assert_debug_log` is slowing it down. Can you please check with the following diff?

Yeah, with that change it doesn't slow down anymore!
🤔 sdaftuar reviewed a pull request: "policy: enable sibling eviction for v3 transactions"
(https://github.com/bitcoin/bitcoin/pull/29306#pullrequestreview-1887268593)
I was thinking this PR is ready to leave Draft status -- is there anything you're waiting on before doing that?

I'm not sure how important it is to return the v3 error messages if sibling RBF fails. It seems to me that if we just declare that attempting sibling eviction is part of the v3 validation rules, then we should failure to get in due to insufficient fees as an RBF failure and not as a v3 failure. (It would also simplify the implementation slightly, so it's possible I'm slightly blin
...
💬 sdaftuar commented on pull request "policy: enable sibling eviction for v3 transactions":
(https://github.com/bitcoin/bitcoin/pull/29306#discussion_r1493792426)
This should be impossible right? Maybe just an `Assume()` to check that there's no sibling eviction going on in this scenario?
💬 fjahr commented on pull request "Improve readability of numeric literals in consensus parameters and network settings":
(https://github.com/bitcoin/bitcoin/pull/29444#issuecomment-1951378674)
Hi @paplorinc, I'm not convinced we need to change the formatting in these particular places. We usually do such refactorings when the code in question is being touched in a PR anyway, otherwise we try to avoid the noise. But I would support it if you open a PR recommending the use of numeric literals in our coding style guidelines (`doc/developer-notes.md`).
🤔 fjahr reviewed a pull request: "rpc: Fixed signed integer overflow for large feerates"
(https://github.com/bitcoin/bitcoin/pull/29434#pullrequestreview-1887282990)
utACK dddd7be9bf038c25f3e53c5bd708fb9cf73d4493
💬 fjahr commented on pull request "rpc: Fixed signed integer overflow for large feerates":
(https://github.com/bitcoin/bitcoin/pull/29434#discussion_r1493803557)
nit: Would have named it `ParseReasonableFeeRate` or `ParseLimitedFeeRate` to make clear it's doing more than just parsing.
💬 fjahr commented on pull request "rpc: Fixed signed integer overflow for large feerates":
(https://github.com/bitcoin/bitcoin/pull/29434#discussion_r1493803772)
Alternatively could have made the limit a parameter, because that also makes it clearer.
💬 paplorinc commented on pull request "Improve readability of numeric literals in consensus parameters and network settings":
(https://github.com/bitcoin/bitcoin/pull/29444#issuecomment-1951389466)
Hey @fjahr, thanks for the quick feedback.

I have targeted these numbers, since they're quoted to normies all the time when asking "so how do you know there isn't an inflation bug in Bitcoin" and I can just show them:
<img alt="image" src="https://github.com/bitcoin/bitcoin/assets/1841944/27aaed6f-7223-49a2-b40b-a5a6e71be434">

It looks more reassuring if it's organized as such.
What do you think?
⚠️ theStack opened an issue: "depends: `touch` command for creating determinstic archive timestamps fails on OpenBSD"
(https://github.com/bitcoin/bitcoin/issues/29447)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

In the course of the "Caching..." step of a depends build, the `touch` command for creating determinstic package archive timestamps fails on OpenBSD. The reason is that the used option `-h` option (introduced in PR #21995, commit 6ebe57622cb70df529879b15f291166177f2827c) doesn't exist in OpenBSD's version of touch (see https://man.openbsd.org/touch.1). E.g. for zeromq:
```
[ ... ]
Postp
...