💬 TheCharlatan commented on issue "clang-format returns error":
(https://github.com/bitcoin/bitcoin/issues/29214#issuecomment-2145163638)
Might be good to note that since #29251 it is also possible to manually specify a clang format binary with the `-binary` option. It should be trivial to install a newer clang format on ubuntu, so I think this can be closed?
(https://github.com/bitcoin/bitcoin/issues/29214#issuecomment-2145163638)
Might be good to note that since #29251 it is also possible to manually specify a clang format binary with the `-binary` option. It should be trivial to install a newer clang format on ubuntu, so I think this can be closed?
👍 instagibbs approved a pull request: "policy: bump TX_MAX_STANDARD_VERSION to 3"
(https://github.com/bitcoin/bitcoin/pull/29496#pullrequestreview-2093899899)
utACK 30a01134cdec37e7467fcd6eee8b0ae3890a131c
(https://github.com/bitcoin/bitcoin/pull/29496#pullrequestreview-2093899899)
utACK 30a01134cdec37e7467fcd6eee8b0ae3890a131c
🤔 furszy reviewed a pull request: "wallet: optimize migration process, batch db transactions"
(https://github.com/bitcoin/bitcoin/pull/28574#pullrequestreview-2093904324)
Rebased due a hidden conflict with #26606.
(https://github.com/bitcoin/bitcoin/pull/28574#pullrequestreview-2093904324)
Rebased due a hidden conflict with #26606.
✅ maflcko closed an issue: "make cov fails with lcov-2"
(https://github.com/bitcoin/bitcoin/issues/28468)
(https://github.com/bitcoin/bitcoin/issues/28468)
🚀 fanquake merged a pull request: "doc: JSON-RPC request Content-Type is application/json"
(https://github.com/bitcoin/bitcoin/pull/30215)
(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
...
(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.
(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?
(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)
(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.
(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.
(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.
(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
(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
```
(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.
(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
...
(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
(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.
(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
...
(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.
(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.