Bitcoin Core Github
43 subscribers
122K links
Download Telegram
🚀 fanquake merged a pull request: "doc: JSON-RPC request Content-Type is application/json"
(https://github.com/bitcoin/bitcoin/pull/30215)
💬 hebasto commented on issue "Guix builds are affected by external environment":
(https://github.com/bitcoin/bitcoin/issues/29754#issuecomment-2145283596)
> I can't repro this on Ubuntu 24.04:

I can reproduce the issue on a fresh Ubuntu 24.04 installation with the default `guix` package installed and `apparmor` package uninstalled.

Using the master branch @ 80bdd4b6beb878c95478b5623c9f9ff0b948ad57 and the following commit on top of it:
```diff
commit 371a379235211504f81d10e55953f0de4b91a442 (HEAD -> 240603-test-29754.0)
Author: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com>
Date: Mon Jun 3 14:40:05 2024 +0100

depe
...
💬 fanquake commented on issue "Guix builds are affected by external environment":
(https://github.com/bitcoin/bitcoin/issues/29754#issuecomment-2145287807)
Seems like this need more information then, for us to be able to do anything.
💬 maflcko commented on issue "Guix builds are affected by external environment":
(https://github.com/bitcoin/bitcoin/issues/29754#issuecomment-2145298857)
Do the shasums for the 22 LTS result differ from the 24 LTS result?
mzumsande closed an issue: "clang-format returns error"
(https://github.com/bitcoin/bitcoin/issues/29214)
💬 mzumsande commented on issue "clang-format returns error":
(https://github.com/bitcoin/bitcoin/issues/29214#issuecomment-2145311186)
> I'm guessing you switched to using a newer Clang here?

Well, lets's say I could have switched to a newer Clang if I wasn't so lazy about these kind of things and if I used clang-format more often...
But that's on me, and since there are multiple workarounds like the one by @TheCharlatan , I'll close.
💬 sr-gi commented on pull request "init: fixes file descriptor accounting":
(https://github.com/bitcoin/bitcoin/pull/30065#issuecomment-2145318356)
Nice catch @vasild, I've squashed that into one of the previous commits. It should be good now.
💬 andrewtoth commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#issuecomment-2145320870)
Rebased, and added a small optimization based on @achow101's suggestion. In `BatchWrite`, we also clear the flags when not erasing if the coin is unspent. This reduces the amount of flagged entries needed to be iterated in `Sync` to find the spent entries to delete. We can also throw a logic error now if we find a flagged entry in `Sync` that is not spent.
🤔 ismaelsadeeq reviewed a pull request: "policy: bump TX_MAX_STANDARD_VERSION to 3"
(https://github.com/bitcoin/bitcoin/pull/29496#pullrequestreview-2093926915)
Concept ACK
💬 ismaelsadeeq commented on pull request "policy: bump TX_MAX_STANDARD_VERSION to 3":
(https://github.com/bitcoin/bitcoin/pull/29496#discussion_r1624490836)
nit, I think the mention of TRUC transactions suffices, from the BIP specification we know they have nVersion=3?
```suggestion
// This module enforces rules for BIP 431 TRUC transactions which help make
```
💬 ismaelsadeeq commented on pull request "policy: bump TX_MAX_STANDARD_VERSION to 3":
(https://github.com/bitcoin/bitcoin/pull/29496#discussion_r1624500904)
Should we replace all v3 mentions with TRUC for clarity and uniformity.
The BIP specification refers to this policy rules as TRUC Transactions.
💬 hebasto commented on issue "Guix builds are affected by external environment":
(https://github.com/bitcoin/bitcoin/issues/29754#issuecomment-2145371780)
> Do the shasums for the 22 LTS result differ from the 24 LTS result?

Ubuntu 24.04, x86_64:
```
7ee2be332a9c7221280fa37c5720ee93077d6aa3f1730b75d748b3fa74efea5c guix-build-80bdd4b6beb8/output/arm64-apple-darwin/SHA256SUMS.part
bac5e4d4e4f823ce020ff82caf9fed0d00006204c3fd1207db3f6642db25306a guix-build-80bdd4b6beb8/output/arm64-apple-darwin/bitcoin-80bdd4b6beb8-arm64-apple-darwin-unsigned.tar.gz
eba8ad10522dd1420436c4f1560bb8cc4c00a40db5af5cc1da782f8636603719 guix-build-80bdd4b6beb8/out
...
💬 sr-gi commented on issue "Erlay Project Tracking":
(https://github.com/bitcoin/bitcoin/issues/28646#issuecomment-2145371987)
> I've updated the PR to review, at the top of the issue here. @sr-gi are you going to open your own tracking issue going forward? That'll likely be easier to keep up to date.

Sure, if there is review traction, I'll open a new tracking issue once the current target PR gets merged
💬 furszy commented on pull request "wallet: Migrate legacy wallets to descriptor wallets without requiring BDB":
(https://github.com/bitcoin/bitcoin/pull/26596#discussion_r1624612271)
Loading the entire wallet just to obtain the descriptors flag shouldn't be needed. Could just check the file type instead. We want a bdb file here. So, something like https://github.com/furszy/bitcoin-core/commit/7a03a039ae99e69504fe9df95b04f5e326c45a70 would work well. Yet, the code isn't the best but it includes a test that verifies the behavior.
📝 theStack opened a pull request: "refactor: remove unused `CKey::Negate` method"
(https://github.com/bitcoin/bitcoin/pull/30218)
This method was introduced as a pre-requirement for the v2 transport protocol back then (see PR #14047, commit 463921bb), when it was still BIP151. With the replacement BIP324, this is not needed anymore, and it's also unlikely that any other proposal we'd ever need to negate private keys at this abstraction level. I'd argue that this operation is usually something that should happen within a secp256k1 module (like e.g. done in MuSig2, Silent Payments...).

(If there is really demand in the fu
...
👍 theStack approved a pull request: "rename TransactionErrors: MISSING_INPUTS and ALREADY_IN_CHAIN"
(https://github.com/bitcoin/bitcoin/pull/30212#pullrequestreview-2094224853)
Concept and code-review ACK aa422bcb898b7e3aa8dca912d92016bd22cf243d

I agree that the proposed names are more to the point and less confusing.
💬 hebasto commented on issue "`WalletCreate{Encrypted/Plain}` benchmark crash on Windows":
(https://github.com/bitcoin/bitcoin/issues/29816#issuecomment-2145549461)
Can be closed as fixed by https://github.com/bitcoin/bitcoin/pull/30122.
fanquake closed an issue: "`WalletCreate{Encrypted/Plain}` benchmark crash on Windows"
(https://github.com/bitcoin/bitcoin/issues/29816)
💬 achow101 commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#issuecomment-2145621189)
> @achow101 I think this is what you might not be clear about. `BatchWrite` iterates the entire cache, but this patch does not. After this patch `BatchWrite` only iterates flagged entries, which is a much smaller subset of the cache.

Yes, that was what I was missing. It just wasn't clear to me from the description that this was implemented as another performance improvement in addition to clearing dirty and spent UTXOs.
👍 ryanofsky approved a pull request: "Encapsulate warnings in generalized node::Warnings and remove globals"
(https://github.com/bitcoin/bitcoin/pull/30058#pullrequestreview-2094278459)
Code review ACK 468cf536eb6473b0fbc1d0f9763c8d6caedeb849. This PR is a nice change making behavior more consistent, cleaning up the kernel interface, and removing globals. Main changes since last review are switching from string to enum ids and documenting behavior changes.