Bitcoin Core Github
42 subscribers
125K links
Download Telegram
💬 0xB10C commented on pull request "ci: Switch to gcr.io mirror to avoid rate limits":
(https://github.com/bitcoin/bitcoin/pull/31906#issuecomment-2672063270)
Agree that this is the better than having to modify all CI machines and deployments. I did some light testing on my runners and didn't see any problems in fetching the images. The CI passing here indicate the same. While switching the GitHub Actions tasks to the new images probably isn't necessary due to being exempt from the rate-limiting (https://github.com/bitcoin/bitcoin/pull/31906#discussion_r1961960848), I don't think it hurts.


ACK fa8de4706a01322fd8ab8e491d297b97f001ecff
💬 achow101 commented on pull request "guix: Notarize MacOS app bundle and codesign all MacOS and Windows binaries":
(https://github.com/bitcoin/bitcoin/pull/31407#issuecomment-2672063512)
According to https://developer.apple.com/forums/thread/706379, starting a command line tool from Finder just doesn't work in general.
💬 maflcko commented on pull request "qa: fix an off-by-one in utxo snapshot fuzz target and sanity check its snapshot data":
(https://github.com/bitcoin/bitcoin/pull/31910#discussion_r1963985718)
> The fuzz mock `hash[31] & 0x80 == 0` is equivalent to the pow check against regtest min difficulty used in the unit test

Just for reference, I don't think this is true. It simply happens to pass because it is the most likely outcome, given the number of regtest blocks, but that is not guaranteed. Block hashes do exist where this is not true. For example:

```cpp

{
const auto consensus = CreateChainParams(*m_node.args, ChainType::REGTEST)->GetConsensus();
unsigned int nBits{0x
...
👍 theuni approved a pull request: "cmake: Do not modify `CMAKE_TRY_COMPILE_TARGET_TYPE` globally"
(https://github.com/bitcoin/bitcoin/pull/31662#pullrequestreview-2630560726)
utACK 2c4b229c906de6250500d3af2b44808e90b9ce0b

I didn't re-review each commit in great detail, but the diff from before is as I'd expect.
🤔 mzumsande reviewed a pull request: "qa: fix an off-by-one in utxo snapshot fuzz target and sanity check its snapshot data"
(https://github.com/bitcoin/bitcoin/pull/31910#pullrequestreview-2630585725)

> I did not generate a full coverage report but did check it was still possible for the target to find a valid snapshot and also check the cov: value reported by libfuzzer was the same on master and this branch (4200 without sanitizers).

More strictly, I'd expect that a fuzz seed that generated a valid snapshot before this change would also generate one with the change - the loop iteration that isn't done anymore doesn't make use of fuzzed_data_provider. So existing seeds shouldn't be inval
...
💬 pinheadmz commented on pull request "guix: Notarize MacOS app bundle and codesign all MacOS and Windows binaries":
(https://github.com/bitcoin/bitcoin/pull/31407#issuecomment-2672098930)
Hm! It does work if I build and codesign remotely then download with SFTP.

![Feb-20-2025 11-54-52](https://github.com/user-attachments/assets/4cb9b142-972f-4566-82b4-8ec3fa754355)
💬 darosior commented on pull request "qa: fix an off-by-one in utxo snapshot fuzz target and sanity check its snapshot data":
(https://github.com/bitcoin/bitcoin/pull/31910#discussion_r1964024171)
Ah right.
💬 maflcko commented on pull request "test: cover base[32|58|64] with symmetric roundtrip fuzz (and padding) tests":
(https://github.com/bitcoin/bitcoin/pull/30746#discussion_r1964025826)
> Are you suggesting the new functionality doesn't cover everything the old one did?

A unit test and a fuzz test are conceptually the same, but in practise they are slightly different:

* A unit test is a test where the test vectors are hard-coded. (Either in the source code, or enumerated, or provided by an external file).
* A fuzz test is a test where the test vectors are provided by an external environment dynamically. (Usually generated by a fuzz engine, or by hand, and collected in a
...
🚀 fanquake merged a pull request: "cmake: Make implicit `libbitcoinkernel` dependencies explicit"
(https://github.com/bitcoin/bitcoin/pull/31884)
📝 hebasto opened a pull request: "qt: Update `src/qt/locale/bitcoin_en.xlf` after string freeze"
(https://github.com/bitcoin-core/gui/pull/854)
This PR follows our [Release Process](https://github.com/bitcoin/bitcoin/blob/864386a7444fb5cf16613956ce8bf335f51b67d5/doc/release-process.md) and implements the ["Translation string freeze"](https://github.com/bitcoin/bitcoin/issues/31029) step.

Steps to reproduce the diff on Ubuntu:
```
$ cmake --preset dev-mode -DWITH_USDT=OFF -DWITH_MULTIPROCESS=OFF
$ cmake --build build_dev_mode --target translate
```

At the moment, the multiprocess-specific code does not introduce any new transla
...
💬 hebasto commented on pull request "qt: Update `src/qt/locale/bitcoin_en.xlf` after string freeze":
(https://github.com/bitcoin-core/gui/pull/854#issuecomment-2672141670)
cc @stickies-v @pablomartin4btc @johnny9 @jarolrod as regular reviewers of similar previous PRs.

cc @l0rinc as the author of https://github.com/bitcoin/bitcoin/pull/31731.

cc @glozow as the 29.0 release manager.
💬 l0rinc commented on pull request "test: cover base[32|58|64] with symmetric roundtrip fuzz (and padding) tests":
(https://github.com/bitcoin/bitcoin/pull/30746#discussion_r1964034510)
> checksum in the test vector, but the fuzz test does not

Is that the case? In both cases we were doing roundtrips. But in the code you're quoting it's also trying randomly - which, as you pointed out, usually fails. But the roundtrip part is still there at the end.

I've moved these randomized unit tests to the fuzz part since it was explicitly requested: https://github.com/bitcoin/bitcoin/pull/30746#discussion_r1736402266
👍 hebasto approved a pull request: "doc: update translation generation cmake example"
(https://github.com/bitcoin/bitcoin/pull/31731#pullrequestreview-2630633312)
ACK 758a93d6215c2fa4799741d721e610a8a7214c34.
fanquake closed a pull request: "p2p: Evict outbound peers with high minFeeRate"
(https://github.com/bitcoin/bitcoin/pull/28488)
💬 fanquake commented on pull request "p2p: Evict outbound peers with high minFeeRate":
(https://github.com/bitcoin/bitcoin/pull/28488#issuecomment-2672148214)
Closing for now, due to no followup. Please leave a comment, if you want this reopened.
💬 darosior commented on pull request "qa: fix an off-by-one in utxo snapshot fuzz target and sanity check its snapshot data":
(https://github.com/bitcoin/bitcoin/pull/31910#discussion_r1964036826)
> ```c++
> hash = uint256{"7fffff0000000000000000000000000000000000000000000000000000000001"};
> Assert(CheckProofOfWork(hash, nBits, consensus)); // fails in the unit test, passes in the fuzz test
> ```

I don't think this actually passes in the fuzz test. The hex representation is in reversed byte order. `hash[31]` here is `0x7f`. If it had its highest bit set (that is `hash[31] & 0x80 != 0` then it would be necessarily be higher than the regtest max target.

All values with th
...
💬 maflcko commented on pull request "test: cover base[32|58|64] with symmetric roundtrip fuzz (and padding) tests":
(https://github.com/bitcoin/bitcoin/pull/30746#discussion_r1964037815)
> `Access is denied to this issue`

[clusterfuzz-testcase-minimized-base58check_encode_decode-5171917854408704.not.txt](https://github.com/user-attachments/files/18892191/clusterfuzz-testcase-minimized-base58check_encode_decode-5171917854408704.not.txt)
[clusterfuzz-testcase-base58_encode_decode-6227427978444800.not.txt](https://github.com/user-attachments/files/18892195/clusterfuzz-testcase-base58_encode_decode-6227427978444800.not.txt)
💬 fanquake commented on pull request "net: update comment for service bit support info for seed.bitcoin.sipa.be":
(https://github.com/bitcoin/bitcoin/pull/29809#issuecomment-2672159460)
@naiyoma what is the status of this?
💬 l0rinc commented on pull request "test: cover base[32|58|64] with symmetric roundtrip fuzz (and padding) tests":
(https://github.com/bitcoin/bitcoin/pull/30746#discussion_r1964042412)
So the issue, as noticed by @marcofleon as well is that I [always need to restrict the input](https://github.com/bitcoin/bitcoin/pull/31917#discussion_r1963946848) - thanks, working on it!
🚀 fanquake merged a pull request: "doc: update translation generation cmake example"
(https://github.com/bitcoin/bitcoin/pull/31731)