Bitcoin Core Github
43 subscribers
122K links
Download Telegram
💬 TheCharlatan commented on pull request "kernel: Move block tree db open to block manager":
(https://github.com/bitcoin/bitcoin/pull/30965#issuecomment-2580192438)
Updated 182945e946ac6a92daad012d6579e5441b9641cc -> 384c73d4fcda4af7c2b4bfb945a248cb93dba47d ([blockmanDB_5](https://github.com/TheCharlatan/bitcoin/tree/blockmanDB_5) -> [blockmanDB_6](https://github.com/TheCharlatan/bitcoin/tree/blockmanDB_6), [compare](https://github.com/TheCharlatan/bitcoin/compare/blockmanDB_5..blockmanDB_6))

* Addressed @ryanofsky's [comment](https://github.com/bitcoin/bitcoin/pull/30965#discussion_r1906052767), added back interrupt check.
* Addressed @ryanofsky's [com
...
📝 hebasto opened a pull request: "depends: Fix spacing issue"
(https://github.com/bitcoin/bitcoin/pull/31627)
This PR resolves an issue where a missing space caused the value of the `build_AR` variable to be concatenated with the "NM=" string. This resulted in subsequent calls to `${AR}` and `${NM}` failing.

Here is a diff for the `make -C depends print-build_id DEBUG=1` output:
```diff
@@ -110,50 +110,18 @@
CXX_STANDARD=c++20
END CXX
BEGIN AR
-ar: invalid option -- '='
-Usage: ar [emulation options] [-]{dmpqrstx}[abcDfilMNoOPsSTuvV] [--plugin <name>] [member-name] [count] archive-file file
...
💬 maflcko commented on pull request "validation: write chainstate to disk every hour":
(https://github.com/bitcoin/bitcoin/pull/30611#discussion_r1908825505)
Are you sure? rand_uniform_delay should cast the range to the duration type of the timepoint, which is the system clocks type.

See also the unit test:

```cpp
src/test/random_tests.cpp: BOOST_CHECK_EQUAL(26443, ctx.rand_uniform_delay(time_point, 9h).time_since_epoch().count());
💬 hebasto commented on pull request "depends: add *FLAGS to gen_id":
(https://github.com/bitcoin/bitcoin/pull/31125#discussion_r1908827904)
It should be
```suggestion
AR='$(build_AR)' NM='$(build_NM)' RANLIB='$(build_RANLIB)' STRIP='$(build_STRIP)' SHA256SUM='$(build_SHA256SUM)' \
```

Fixed in https://github.com/bitcoin/bitcoin/pull/31627.
💬 glozow commented on pull request "p2p: track and use all potential peers for orphan resolution":
(https://github.com/bitcoin/bitcoin/pull/31397#discussion_r1908833307)
Ah I see what you mean, yes!
💬 fanquake commented on pull request "cmake: Add `CheckLinkerSupportsPIE` module":
(https://github.com/bitcoin/bitcoin/pull/31359#issuecomment-2580228775)
What is the status of this
👍 theStack approved a pull request: "test: raise an error in `_bulk_tx_` when `target_vsize` is too low"
(https://github.com/bitcoin/bitcoin/pull/30322#pullrequestreview-2539977783)
ACK 92787dd52cd4ddc378cf1bcb7e92a649916a0f42

Tested the new error raising code paths manually by introducing `create_self_transfer` calls (in an arbitrary functional test), one with too-high-fee and another one with too-low-target-vsize:

```
wallet.create_self_transfer(fee=Decimal("50.00000001"))
...
=> RuntimeError: UTXO value 50.00000000 is too small to cover fees 50.00000001


wallet.create_self_transfer(utxo_to_spend=utxo, target_vsize=100)
...
=> RuntimeError: target_vsize 100
...
💬 theStack commented on pull request "test: raise an error in `_bulk_tx_` when `target_vsize` is too low":
(https://github.com/bitcoin/bitcoin/pull/30322#discussion_r1908841262)
nit: could deduplicate the expression for calculating the final fee in sats
💬 l0rinc commented on pull request "validation: write chainstate to disk every hour":
(https://github.com/bitcoin/bitcoin/pull/30611#discussion_r1908874529)
I've checked
```patch
diff --git a/src/validation.cpp b/src/validation.cpp
--- a/src/validation.cpp (revision 1ac02221219a2ae6a8605dfddff4390a60712d56)
+++ b/src/validation.cpp (date 1736431426006)
@@ -2869,6 +2869,10 @@

if (should_write || m_next_write == NodeClock::time_point::max()) {
constexpr auto range{DATABASE_WRITE_INTERVAL_MAX - DATABASE_WRITE_INTERVAL_MIN + 1min};
+ for (int i = 0; i < 10; ++i) {
+ auto random_delay = FastRand
...
💬 pablomartin4btc commented on pull request "test: raise explicit error if any of the needed release binaries is missing":
(https://github.com/bitcoin/bitcoin/pull/31462#discussion_r1908883714)
Oh, I see... yeah, ok... in the case of `WalletMigrationTest` I can see the required versions defined in the setup. Thank you!
🤔 pablomartin4btc reviewed a pull request: "test: raise explicit error if any of the needed release binaries is missing"
(https://github.com/bitcoin/bitcoin/pull/31462#pullrequestreview-2540045704)
tACK 1ea7e45a1f445d32a2b690d52befb2e63418653b
💬 kehiy commented on pull request "doc: upgrade license to 2025.":
(https://github.com/bitcoin/bitcoin/pull/31611#issuecomment-2580352679)
> lgtm ACK [b537a2c](https://github.com/bitcoin/bitcoin/commit/b537a2c02a9921235d1ecf8c3c7dc1836ec68131)
>
> Looks similar to what was done in commit [1f8450f](https://github.com/bitcoin/bitcoin/commit/1f8450f066724dfbb5c5bc4060843e2f3340ed88) last year.

@maflcko thanks. yes, every year we had these changes on this repo.
💬 sipa commented on pull request "validation: write chainstate to disk every hour":
(https://github.com/bitcoin/bitcoin/pull/30611#discussion_r1908927814)
@maflcko @l0rinc You're both right. No problem in that case.

I think the `+ 1min` still needs to go, though.
💬 l0rinc commented on pull request "validation: write chainstate to disk every hour":
(https://github.com/bitcoin/bitcoin/pull/30611#discussion_r1908961700)
Won't that make it asymmetric to the hour?
💬 Pttn commented on pull request "descriptors: Try pubkeys of both evenness when retrieving the private keys for an xonly pubkey in a descriptor":
(https://github.com/bitcoin/bitcoin/pull/31590#issuecomment-2580503897)
ACK c0045e6cee06bc0029fb79b5a531aa1f2b817424
Solves the issue, looks good to me, thank you for working on this!
💬 sipa commented on pull request "Safegcd-based modular inverses in MuHash3072":
(https://github.com/bitcoin/bitcoin/pull/21590#discussion_r1908979740)
Which ones in particular are missing?
💬 andrewtoth commented on pull request "validation: write chainstate to disk every hour":
(https://github.com/bitcoin/bitcoin/pull/30611#discussion_r1908981550)
Hmm yeah the `+ 1min` is just noise. If we want to increase the range by 1 minute, we could just update `DATABASE_WRITE_INTERVAL_MAX = 71min`.
💬 andrewtoth commented on pull request "validation: write chainstate to disk every hour":
(https://github.com/bitcoin/bitcoin/pull/30611#discussion_r1908983240)
Removed the `+ 1min`.
💬 sipa commented on pull request "Safegcd-based modular inverses in MuHash3072":
(https://github.com/bitcoin/bitcoin/pull/21590#discussion_r1908985488)
Yes and no. They aren't skipped, the carry step is the equivalent of processing the inner limbs. They're just easier, because the modulus here can be represented as [1 << FINAL_LIMB_MODULUS_BITS, 0, 0, 0, ..., 0, 0, -MAX_PRIME_DIFF]. In the `modinv32_impl.h` code, the modulus is treated as generic, where any limb can be nonzero.
💬 furszy commented on pull request "wallet: fix crash during migration due to invalid multisig descriptors":
(https://github.com/bitcoin/bitcoin/pull/31378#issuecomment-2580524635)
> > we are enforcing this rules within the descriptors parsing logic.
>
> That sounds dumb. Why are we?

I assume this was simply because there was no need to represent them in the descriptors' language. They've been non-standard for at least the past 10 years now.

However, there's no documentation about it. This restriction has always been present; it was introduced in the first commit that added descriptors in #13697.

We could relax the parsing restriction if a real use case arises.
...