:lock: hebasto locked an issue: "Pull request Name: Jodie Gerber"
(https://github.com/bitcoin/bitcoin/issues/29927)
(https://github.com/bitcoin/bitcoin/issues/29927)
⚠️ Kino1994 opened an issue: "Signed Integer Overflow in GetBlockSubsidy at block height 2,147,483,647 (During Epoch 10,227, halving 10,226) could Increase Block Subsidy"
(https://github.com/bitcoin/bitcoin/issues/29928)
### Is there an existing issue for this?
- [X] I have searched the existing issues
### Current behaviour
The GetBlockSubsidy function could potentially encounter a signed integer overflow if nHeight reaches 2,147,483,648, causing it to wrap to -2,147,483,648. This overflow can disrupt the halving mechanism implemented in the function, particularly around the calculated 10,226th halving interval derived from INT_MAX / nSubsidyHalvingInterval.
Implications:
Such an overflow could lead to
...
(https://github.com/bitcoin/bitcoin/issues/29928)
### Is there an existing issue for this?
- [X] I have searched the existing issues
### Current behaviour
The GetBlockSubsidy function could potentially encounter a signed integer overflow if nHeight reaches 2,147,483,648, causing it to wrap to -2,147,483,648. This overflow can disrupt the halving mechanism implemented in the function, particularly around the calculated 10,226th halving interval derived from INT_MAX / nSubsidyHalvingInterval.
Implications:
Such an overflow could lead to
...
💬 laanwj commented on issue "[Linux] Add wayland support":
(https://github.com/bitcoin/bitcoin/issues/19950#issuecomment-2067951956)
See #29923, it is a first step to Wayland support in the release binaries without the earlier noted drawbacks of requiring extra build-time or run-time dependencies.
(https://github.com/bitcoin/bitcoin/issues/19950#issuecomment-2067951956)
See #29923, it is a first step to Wayland support in the release binaries without the earlier noted drawbacks of requiring extra build-time or run-time dependencies.
💬 laanwj commented on pull request "refactor: Use our own implementation of urlDecode":
(https://github.com/bitcoin/bitcoin/pull/29904#issuecomment-2067954555)
> It's far more likely there's a bugfix that we miss picking up on, than that there's malicious code inserted later.
Sometimes i'd agree with that reasoning. But let's face it, `urlDecode` is not more complex than say, `leftpad`, it's unlikely to need maintainance or bug fixes, and thus seems unnecessary to carry a dependency for.
(Also this isn't a direct copy of the C code! It's a C++ reimplementation that does `std::string` to `std::string`, so should handle embedded NULL bytes withou
...
(https://github.com/bitcoin/bitcoin/pull/29904#issuecomment-2067954555)
> It's far more likely there's a bugfix that we miss picking up on, than that there's malicious code inserted later.
Sometimes i'd agree with that reasoning. But let's face it, `urlDecode` is not more complex than say, `leftpad`, it's unlikely to need maintainance or bug fixes, and thus seems unnecessary to carry a dependency for.
(Also this isn't a direct copy of the C code! It's a C++ reimplementation that does `std::string` to `std::string`, so should handle embedded NULL bytes withou
...
💬 maflcko commented on issue "Apple Clang 14.0 lacks support for `std::is_eq`":
(https://github.com/bitcoin/bitcoin/issues/29918#issuecomment-2067959418)
> supported platforms
My understanding is that you can run Bitcoin Core on those platforms. However, compiling Bitcoin Core from source may not work out of the box.
If macOS does not offer an upgrade path for the system compiler, can you try to use a compiler provided by `brew`?
Another alternative would be to compile the compiler you need yourself.
(https://github.com/bitcoin/bitcoin/issues/29918#issuecomment-2067959418)
> supported platforms
My understanding is that you can run Bitcoin Core on those platforms. However, compiling Bitcoin Core from source may not work out of the box.
If macOS does not offer an upgrade path for the system compiler, can you try to use a compiler provided by `brew`?
Another alternative would be to compile the compiler you need yourself.
💬 laanwj commented on pull request "wallet: Implement independent BDB parser":
(https://github.com/bitcoin/bitcoin/pull/26606#discussion_r1573675369)
The check currently always fails for other-endian wallets, needs something like:
```diff
diff --git a/src/wallet/migrate.cpp b/src/wallet/migrate.cpp
index 15478e8bc5bfe39e04b400fa475215e25eef6452..0a41ae199e1950bf372bae90b8a88fccca2b88f0 100644
--- a/src/wallet/migrate.cpp
+++ b/src/wallet/migrate.cpp
@@ -566,6 +566,10 @@ void BerkeleyRODatabase::Open()
uint32_t offset;
SeekToPage(db_file, i, page_size);
db_file >> file >> offset;
+ if (outer_meta.oth
...
(https://github.com/bitcoin/bitcoin/pull/26606#discussion_r1573675369)
The check currently always fails for other-endian wallets, needs something like:
```diff
diff --git a/src/wallet/migrate.cpp b/src/wallet/migrate.cpp
index 15478e8bc5bfe39e04b400fa475215e25eef6452..0a41ae199e1950bf372bae90b8a88fccca2b88f0 100644
--- a/src/wallet/migrate.cpp
+++ b/src/wallet/migrate.cpp
@@ -566,6 +566,10 @@ void BerkeleyRODatabase::Open()
uint32_t offset;
SeekToPage(db_file, i, page_size);
db_file >> file >> offset;
+ if (outer_meta.oth
...
💬 maflcko commented on issue "Apple Clang 14.0 lacks support for `std::is_eq`":
(https://github.com/bitcoin/bitcoin/issues/29918#issuecomment-2067964609)
> Faced this too. Updating to 14.0.3 fixes the problem.
Just to clarify the confusing versioning here, this corresponds to Xcode 14.3, llvm 15, and Apple Clang 14.0.3
(https://github.com/bitcoin/bitcoin/issues/29918#issuecomment-2067964609)
> Faced this too. Updating to 14.0.3 fixes the problem.
Just to clarify the confusing versioning here, this corresponds to Xcode 14.3, llvm 15, and Apple Clang 14.0.3
💬 maflcko commented on pull request "wallet: Implement independent BDB parser":
(https://github.com/bitcoin/bitcoin/pull/26606#discussion_r1573680139)
nit in a0b4c1b7a3bfeaeae0277fe151cd5ef9b31ccf15: Is this used in this pull request? I wonder if it could make sense to return the error string to the caller, assuming this is called by RPC eventually?
(https://github.com/bitcoin/bitcoin/pull/26606#discussion_r1573680139)
nit in a0b4c1b7a3bfeaeae0277fe151cd5ef9b31ccf15: Is this used in this pull request? I wonder if it could make sense to return the error string to the caller, assuming this is called by RPC eventually?
💬 TheCharlatan commented on pull request "[refactor] Check CTxMemPool options in ctor":
(https://github.com/bitcoin/bitcoin/pull/28830#issuecomment-2067972727)
Reworked 364456f6598b135fcc0acab8a660658b4407f837 -> 30a1024b63105a97d04149e83ae2d8cf0830cf69 ([mempoolArgs_4](https://github.com/TheCharlatan/bitcoin/tree/mempoolArgs_4) -> [mempoolArgs_5](https://github.com/TheCharlatan/bitcoin/tree/mempoolArgs_5), [compare](https://github.com/TheCharlatan/bitcoin/compare/mempoolArgs_4..mempoolArgs_5))
* Reworked this after @ryanofsky's [comment](https://github.com/bitcoin/bitcoin/pull/28830#issuecomment-2059591077). I agree that the better option here is t
...
(https://github.com/bitcoin/bitcoin/pull/28830#issuecomment-2067972727)
Reworked 364456f6598b135fcc0acab8a660658b4407f837 -> 30a1024b63105a97d04149e83ae2d8cf0830cf69 ([mempoolArgs_4](https://github.com/TheCharlatan/bitcoin/tree/mempoolArgs_4) -> [mempoolArgs_5](https://github.com/TheCharlatan/bitcoin/tree/mempoolArgs_5), [compare](https://github.com/TheCharlatan/bitcoin/compare/mempoolArgs_4..mempoolArgs_5))
* Reworked this after @ryanofsky's [comment](https://github.com/bitcoin/bitcoin/pull/28830#issuecomment-2059591077). I agree that the better option here is t
...
💬 levantah commented on issue "Release schedule for 27.0":
(https://github.com/bitcoin/bitcoin/issues/29028#issuecomment-2067979559)
Excuse me,
the bitcoincore.org/bin was quite slow for me so here is a hopefully-faster mirror with no data limits: https://ln.anyone.eu.org/bin/bitcoin-core-27.0/
See https://cfpages-cache.pages.dev/ ( or full -log=(https://cfpages-cache.pages.dev/log.txt)) for the latest cache-fill.
HTH
(https://github.com/bitcoin/bitcoin/issues/29028#issuecomment-2067979559)
Excuse me,
the bitcoincore.org/bin was quite slow for me so here is a hopefully-faster mirror with no data limits: https://ln.anyone.eu.org/bin/bitcoin-core-27.0/
See https://cfpages-cache.pages.dev/ ( or full -log=(https://cfpages-cache.pages.dev/log.txt)) for the latest cache-fill.
HTH
💬 fanquake commented on issue "Release schedule for 27.0":
(https://github.com/bitcoin/bitcoin/issues/29028#issuecomment-2068002408)
@levantah thanks, however we'll refrain from linking to sources other than our own website.
(https://github.com/bitcoin/bitcoin/issues/29028#issuecomment-2068002408)
@levantah thanks, however we'll refrain from linking to sources other than our own website.
💬 fanquake commented on issue "Apple Clang 14.0 lacks support for `std::is_eq`":
(https://github.com/bitcoin/bitcoin/issues/29918#issuecomment-2068007481)
Yea. I think this can be closed with "use a compiler that better supports C++20". We still support the mentioned systems, and as far as I'm aware, installing and using a newer compiler would also work fine.
(https://github.com/bitcoin/bitcoin/issues/29918#issuecomment-2068007481)
Yea. I think this can be closed with "use a compiler that better supports C++20". We still support the mentioned systems, and as far as I'm aware, installing and using a newer compiler would also work fine.
📝 paplorinc converted_to_draft a pull request: "refactor: Reduce memory copying operations in bech32 encoding/decoding"
(https://github.com/bitcoin/bitcoin/pull/29607)
Started optimizing the base conversions in [TryParseHex](https://github.com/bitcoin/bitcoin/pull/29458), [Base58](https://github.com/bitcoin/bitcoin/pull/29473) and [IsSpace](https://github.com/bitcoin/bitcoin/pull/29602) - this is the next step.
Here I've reduced the memory reallocations and copying operations in bech32 encoding/decoding, resulting in decode being `~22%` faster, encode `~32%` faster.
Before:
```
| ns/byte | byte/s | err% | total | benchma
...
(https://github.com/bitcoin/bitcoin/pull/29607)
Started optimizing the base conversions in [TryParseHex](https://github.com/bitcoin/bitcoin/pull/29458), [Base58](https://github.com/bitcoin/bitcoin/pull/29473) and [IsSpace](https://github.com/bitcoin/bitcoin/pull/29602) - this is the next step.
Here I've reduced the memory reallocations and copying operations in bech32 encoding/decoding, resulting in decode being `~22%` faster, encode `~32%` faster.
Before:
```
| ns/byte | byte/s | err% | total | benchma
...
💬 paplorinc commented on pull request "refactor: Reduce memory copying operations in bech32 encoding/decoding":
(https://github.com/bitcoin/bitcoin/pull/29607#issuecomment-2068016788)
There seems to be a failure in https://github.com/bitcoin/bitcoin/pull/29607/checks?check_run_id=24070603728 that I can't reproduce locally, converting back to draft - how do I check at least what the failure message is?
(https://github.com/bitcoin/bitcoin/pull/29607#issuecomment-2068016788)
There seems to be a failure in https://github.com/bitcoin/bitcoin/pull/29607/checks?check_run_id=24070603728 that I can't reproduce locally, converting back to draft - how do I check at least what the failure message is?
💬 pinheadmz commented on issue "./bitcoin.conf file should not cause confusion with ./datadir/bitcoin.conf":
(https://github.com/bitcoin/bitcoin/issues/29139#issuecomment-2068038387)
After catching up on the discussion in #29903 my opinion now is that we close this issue and the pull request. The bitcoin.conf file is self-documented and the datadir location where it belongs is also well documented. If users are opening that file to change settings but ignoring the first line in that file that tells them how to use it, I'm not sure there's anything else the developers can do.
I'd recommend adding a README file in the tar but that probably won't prevent this user behavior e
...
(https://github.com/bitcoin/bitcoin/issues/29139#issuecomment-2068038387)
After catching up on the discussion in #29903 my opinion now is that we close this issue and the pull request. The bitcoin.conf file is self-documented and the datadir location where it belongs is also well documented. If users are opening that file to change settings but ignoring the first line in that file that tells them how to use it, I'm not sure there's anything else the developers can do.
I'd recommend adding a README file in the tar but that probably won't prevent this user behavior e
...
💬 hebasto commented on pull request "[DO NOT MERGE] cmake: Migrate CI scripts to CMake-based build system -- WIP":
(https://github.com/bitcoin/bitcoin/pull/29790#issuecomment-2068043063)
The recent push uses approach from the https://github.com/hebasto/bitcoin/pull/157.
(https://github.com/bitcoin/bitcoin/pull/29790#issuecomment-2068043063)
The recent push uses approach from the https://github.com/hebasto/bitcoin/pull/157.
📝 hebasto opened a pull request: "ci: Drop no longer needed `-I` flag in "tidy" task"
(https://github.com/bitcoin/bitcoin/pull/29929)
As title says.
(https://github.com/bitcoin/bitcoin/pull/29929)
As title says.
💬 maflcko commented on pull request "ci: Drop no longer needed `-I` flag in "tidy" task":
(https://github.com/bitcoin/bitcoin/pull/29929#issuecomment-2068055365)
lgtm ACK 6f5954acac2ced22dae7088e2b679bf663507d4c
(https://github.com/bitcoin/bitcoin/pull/29929#issuecomment-2068055365)
lgtm ACK 6f5954acac2ced22dae7088e2b679bf663507d4c
👍 cbergqvist approved a pull request: "test: Extends wait_for_getheaders so a specific block hash can be checked"
(https://github.com/bitcoin/bitcoin/pull/29736#pullrequestreview-2013448704)
ACK c4f857cc301d856f3c60acbe6271d3fe19441c7a
Nice to see the TODO in `p2p.py` resolved!
Searched through source for "getheaders" to make sure the function had been applied when suitable.
Reasoned through the modified test functions.
Modifications look good taking into account the new behavior of `wait_for_getheaders()` popping off the message and requiring the block hash to match the top one if provided.
Built and ran functional tests with `--extended --exclude=feature_dbcrash` wi
...
(https://github.com/bitcoin/bitcoin/pull/29736#pullrequestreview-2013448704)
ACK c4f857cc301d856f3c60acbe6271d3fe19441c7a
Nice to see the TODO in `p2p.py` resolved!
Searched through source for "getheaders" to make sure the function had been applied when suitable.
Reasoned through the modified test functions.
Modifications look good taking into account the new behavior of `wait_for_getheaders()` popping off the message and requiring the block hash to match the top one if provided.
Built and ran functional tests with `--extended --exclude=feature_dbcrash` wi
...
💬 fjahr commented on pull request "refactor: Use our own implementation of urlDecode":
(https://github.com/bitcoin/bitcoin/pull/29904#issuecomment-2068074835)
> Embedding copies of shared code just makes things more fragile, not safer. It's far more likely there's a bugfix that we miss picking up on, than that there's malicious code inserted later.
In addition to what @laanwj said, it's also a much simpler re-implementation. The libevent function provided further options that we didn't care about and hardcoded, that's how we could simplify. And as was discussed in the last IRC meeting, there is also interest in removing libevent in general, at whi
...
(https://github.com/bitcoin/bitcoin/pull/29904#issuecomment-2068074835)
> Embedding copies of shared code just makes things more fragile, not safer. It's far more likely there's a bugfix that we miss picking up on, than that there's malicious code inserted later.
In addition to what @laanwj said, it's also a much simpler re-implementation. The libevent function provided further options that we didn't care about and hardcoded, that's how we could simplify. And as was discussed in the last IRC meeting, there is also interest in removing libevent in general, at whi
...