Bitcoin Core Github
43 subscribers
123K links
Download Telegram
💬 fanquake commented on pull request "refactor: Remove redundant and confusing calls to IsArgSet":
(https://github.com/bitcoin/bitcoin/pull/31896#issuecomment-2742558962)
@ryanofsky Want to take another look?
💬 fanquake commented on pull request "wallet: removed duplicate call to GetDescriptorScriptPubKeyMan":
(https://github.com/bitcoin/bitcoin/pull/32023#issuecomment-2742562905)
https://cirrus-ci.com/task/5112314648592384?logs=ci#L5356:
```bash
[01:20:03.641] Run scriptpubkeyman with args ['/ci_container_base/ci/scratch/build-x86_64-pc-linux-gnu/bin/fuzz', '-runs=1', PosixPath('/ci_container_base/ci/scratch/qa-assets/fuzz_corpora/scriptpubkeyman')]INFO: Running with entropic power schedule (0xFF, 100).
[01:20:03.641] INFO: Seed: 2231869594
[01:20:03.641] INFO: Loaded 1 modules (623423 inline 8-bit counters): 623423 [0x556e6d276398, 0x556e6d30e6d7),
[01:20:03.641
...
🚀 fanquake merged a pull request: "torcontrol: Limit reconnect timeout to max seconds and log delay in whole seconds"
(https://github.com/bitcoin/bitcoin/pull/31979)
💬 maflcko commented on issue "Source code mapping for debugger has changed since cmake":
(https://github.com/bitcoin/bitcoin/issues/31204#issuecomment-2742587668)
> I find it okay to have to set a mapping for the root directory, but I currently need things like:
>
> ```
> "/home/hodlinator/b2/build/src/test/util/test/util/": "/home/hodlinator/b2/src/test/util/"
> ```
>
> Note how the subdirectory needs to be repeated for it to work: `test/util/test/util/`
>
> Same thing happened for rkrux [above](https://github.com/bitcoin/bitcoin/issues/31204#issuecomment-2681660181):
>
> ```
> ...valid location was found at /Users/rkrux/projects/bitcoin/build/src/wa
...
💬 ajtowns commented on pull request "cluster mempool: introduce TxGraph":
(https://github.com/bitcoin/bitcoin/pull/31363#issuecomment-2742606528)
> Can this get surfaced in the fuzz coverage with something that would previously fail?

As long as you actually run the fuzz binary with your changes, and not the fuzz binary in the old location that you compiled a few days ago (*cough*), the following should work:

```diff
--- a/src/test/fuzz/txgraph.cpp
+++ b/src/test/fuzz/txgraph.cpp
@@ -723,4 +723,6 @@ FUZZ_TARGET(txgraph)

// Sanity check again (because invoking inspectors may modify internal unobservable state).
real->
...
🚀 fanquake merged 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)
💬 fanquake commented on pull request "fuzz: split `coinselection` harness":
(https://github.com/bitcoin/bitcoin/pull/31870#issuecomment-2742718818)
cc @dergoegge
💬 fanquake commented on pull request "fuzz: coinselection: cover `SetBumpFeeDiscount`":
(https://github.com/bitcoin/bitcoin/pull/31806#issuecomment-2742720048)
cc @marcofleon
💬 fanquake commented on pull request "miniscript: account for all `StringType` variants in `Miniscriptdescriptor::ToString()`":
(https://github.com/bitcoin/bitcoin/pull/31734#issuecomment-2742725629)
cc @darosior
💬 maflcko commented on pull request "fuzz: split `coinselection` harness":
(https://github.com/bitcoin/bitcoin/pull/31870#issuecomment-2742747821)
review ACK ba82240553ddd534287845e10bc76b46b45329fe 👐

<details><summary>Show signature</summary>

Signature:

```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: review ACK ba82240553dd
...
💬 BrandonOdiwuor commented on pull request "Feature: Use different datadirs for different signets":
(https://github.com/bitcoin/bitcoin/pull/29838#issuecomment-2742830552)
@maflcko The assumption was, providing default signet challenge is not so useful as it would result in the loss of default the signet configurations, hence did not include the check

https://github.com/bitcoin/bitcoin/blob/2db00278ea571d0f734609f9fefecfa75c16ee34/src/kernel/chainparams.cpp#L393-L408
willcl-ark closed an issue: "Vytvořit žádost o podporu | Kapesní možnost"
(https://github.com/bitcoin/bitcoin/issues/32108)
:lock: fanquake locked an issue: "Vytvořit žádost o podporu | Kapesní možnost"
(https://github.com/bitcoin/bitcoin/issues/32108)
🤔 Sjors reviewed a pull request: "multiprocess: add bitcoin-mine test program"
(https://github.com/bitcoin/bitcoin/pull/30437#pullrequestreview-2705328965)
Code reviewed 5554f9cb4abf7e1852dd79d2d75e714fa8cbeadd.

Rebase looks good, but found a bug in the test.
💬 Sjors commented on pull request "multiprocess: add bitcoin-mine test program":
(https://github.com/bitcoin/bitcoin/pull/30437#discussion_r2007217890)
In 5554f9cb4abf7e1852dd79d2d75e714fa8cbeadd "test: add interface_ipc_mining.py test calling bitcoin-mine":

On macOS the limit is 104, which I run into when using a RAM disk with `--tmpdir=/Volumes/RAMDisk/tmp`.

Additionally, our temp path has funny characters, e.g. `/tmp/test_runner_₿_🏃_20250321_104521` so we need to length in bytes.

```diff
diff --git a/test/functional/interface_ipc_mining.py b/test/functional/interface_ipc_mining.py
index c9eedfba5d..73b9a3fb6b 100755
--- a/test/f
...
📝 vasild opened a pull request: "fuzz: avoid returning non-conforming results from FuzzedSock::GetSockName()"
(https://github.com/bitcoin/bitcoin/pull/32109)
It would be a bug in `getsockname(2)` if it returns a result that is smaller than the returned socket address family. For example, if it indicates that the result is less than `sizeof(sockaddr_in6)` and sets `sa_family` equal to `AF_INET6` in the output.

In other words, the `name->sa_family` in the output should be consistent with the returned `*name_len`.

The current code could fail to do that if:
* the caller provides `sockaddr_in6` and an input value of `*name_len=28`
* `ConsumeRandom
...
💬 vasild commented on pull request "fuzz: add targets for PCP and NAT-PMP port mapping requests":
(https://github.com/bitcoin/bitcoin/pull/31676#discussion_r2007258156)
Thanks for fixing this! Much better this way, but not enough - it could still return e.g. IPv6 address that is smaller than the expected size for an IPv6 address, possibly confusing callers to read data past the end of the buffer. Also, minor thing, it forgot to set `errno` when returning an error (`-1`).

Addressed in https://github.com/bitcoin/bitcoin/pull/32109
💬 vasild commented on pull request "fuzz: avoid returning non-conforming results from FuzzedSock::GetSockName()":
(https://github.com/bitcoin/bitcoin/pull/32109#issuecomment-2742918691)
cc @darosior, @marcofleon
📝 jurraca opened a pull request: "contrib: document asmap-tool commands more thoroughly"
(https://github.com/bitcoin/bitcoin/pull/32110)
This README was a little sparse in my opinion, and was missing a mention of the `diff-addrs` command. This PR also adds detail to the help text for the `--fill` flag.

The README updates add background and examples for each command, split in two sections (encode/decode and diff/diff-addrs). This is intended to help people know how and when to run the commands available in the `asmap-tool.py` script.

However, I could use some confirmation on the behavior of the `--fill` flag. It's true that
...