💬 Sjors commented on pull request "Show transactions as not fully confirmed during background validation":
(https://github.com/bitcoin/bitcoin/pull/28616#issuecomment-1798410685)
Before the background sync is finished, we can only check the proof-of-work for new blocks and few other rules. It's better than SPV, but not the same as knowing the transaction is in a fully validated block. We should display that difference somehow.

💬 maflcko commented on pull request "Show transactions as not fully confirmed during background validation":
(https://github.com/bitcoin/bitcoin/pull/28616#discussion_r1384865925)
I think wallet tests should be separate from consensus tests. Also, it would be good to check what happens if loading a wallet file from a backup, whose transactions happened before the utxo snapshot.

💬 aureleoules commented on pull request "rpc: Add script verification flags to getdeploymentinfo":
(https://github.com/bitcoin/bitcoin/pull/28806#discussion_r1384873465)
Thanks for the ping, I re-ran the job. AWS preempted the job and I haven't figured out how to rerun them automatically yet.
Anyone in this org can re-run corecheck jobs by signing in as well!

💬 dergoegge commented on issue "fuzz: Left over tmp files when fuzzing with afl++":
(https://github.com/bitcoin/bitcoin/issues/28811#issuecomment-1798477310)
The fact that timeouts or crashes lead to left over files on disk is a bug and we should fix that. Otherwise it isn't really possible to conduct long running fuzzing campaigns using afl++ (or other engines that use the fork-server model) without running out of disk space.

We should also fix the timeouts and crashes themselves but that is not what I wanted to document with this issue.

💬 sipa commented on pull request "test: python cryptography required for BIP 324 functional tests":
(https://github.com/bitcoin/bitcoin/pull/28374#issuecomment-1798520540)
utACK c534c0871038ded72dc9078cc91e030ceb746196

⚠️ maflcko opened an issue: "fuzz: Fix timeouts"
(https://github.com/bitcoin/bitcoin/issues/28812)
Fuzz timeouts may hint at bugs or performance issues. Also, they may block the fuzz CI and fuzz farms from making progress in a reasonable time.

Thus, they should be fixed.

One timeout was fixed today in commit 2b3f43b96ef0b674bf350f50f317477b8d3e1e56.

Currently OSS-Fuzz reports other timeouts (as non-deterministic):


Someone should investigate them
...

💬 maflcko commented on issue "fuzz: Left over tmp files when fuzzing with afl++":
(https://github.com/bitcoin/bitcoin/issues/28811#issuecomment-1798543387)
> We should also fix the timeouts and crashes themselves but that is not what I wanted to document with this issue.

Ok. I've opened https://github.com/bitcoin/bitcoin/issues/28812 to investigate and fix them. OSS-Fuzz only has 5 targets affected, and it would be good if you checked and reported any timeouts and crashes you found as well.

💬 maflcko commented on issue "fuzz: Fix timeouts":
(https://github.com/bitcoin/bitcoin/issues/28812#issuecomment-1798565712)
The files are:

[clusterfuzz-testcase-mocked_descriptor_parse-4571796256718848.not.txt.not.txt](https://github.com/bitcoin/bitcoin/files/13280644/clusterfuzz-testcase-mocked_descriptor_parse-4571796256718848.not.txt.not.txt)
[clusterfuzz-testcase-bloom_filter-5968259782148096.dmp.not.txt](https://github.com/bitcoin/bitcoin/files/13280645/clusterfuzz-testcase-bloom_filter-5968259782148096.dmp.not.txt)
[clusterfuzz-testcase-policy_estimator-4689779316621312.dmp.not.txt](https://github.com/bitc
...

💬 glozow commented on pull request "validate package transactions with their in-package ancestor sets":
(https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1384955796)
Using `Txid` for this now

💬 glozow commented on pull request "validate package transactions with their in-package ancestor sets":
(https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1384958797)
Added doc.

💬 kashifs commented on pull request "test: Make existing functional tests compatible with --v2transport":
(https://github.com/bitcoin/bitcoin/pull/28805#issuecomment-1798591280)
tACK [6b77f9](https://github.com/bitcoin/bitcoin/pull/28805/commits/6b77f9f96a923a70a246697827fcaf5551d77331)

Read through all updated code, pulled branch, compiled from source, ran:

`python3 wallet_multiwallet.py --usecli --v2transport`

💬 furszy commented on pull request "p2p: adaptive connections services flags":
(https://github.com/bitcoin/bitcoin/pull/28170#discussion_r1384972762)
> I'm wondering, does it even make sense to keep such flag at this point? What it would take to adjust all code to going back-and-forth w.r.t. superstale?

I think that would be an oxymoron. We would be adding code to put the node back into IBD state, because of a stale tip check, when the node is updating the tip.

The `fInitialDownload` flag usage is just a speedup. We know for sure that when the flag is true, it is because the node is in IBD. So we don't need to check anything else in suc
...

💬 furszy commented on pull request "p2p: adaptive connections services flags":
(https://github.com/bitcoin/bitcoin/pull/28170#discussion_r1384991649)
> The word `initial` here is indeed confusing... Thinking about something happening only at node bootstrap, which is a more obvious version i think. Can we find a different word now that you're changing it to certainly not mean that?

Hmm, first names coming to my mind are `m_close_to_synced` or `m_close_to_tip`? or, could use the existing term `m_can_direct_fetch`?

Still, maybe better to leave it for a follow-up because, with the back-and-forth functionality, we could also use this flag in
...

📝 glozow opened a pull request: "rpc: permit any ancestor package through submitpackage"
(https://github.com/bitcoin/bitcoin/pull/28813)
After #26711, validation should be able to handle any ancestor package properly. We can remove the child-with-unconfirmed-parents restriction and the `IsTree` restriction.

In draft while #26711 is still open.

💬 glozow commented on pull request "validate package transactions with their in-package ancestor sets":
(https://github.com/bitcoin/bitcoin/pull/26711#issuecomment-1798634990)
> The "linearize using fees" commit message seems to have an extra line at the end left over from a rebase? The algorithm description seems like it would be better as a code comment rather than a commit message.

Added the algo description to the documentation of the `AcceptPackage` function

> - linearisation + miniminer + unit/fuzz test and bench
> - acceptpackage loop + subpackage linearization + unit/fuzz tests?

I'd prefer to keep the `AncestorPackage` changes (along with its unit/be
...

👍 willcl-ark approved a pull request: "net: support unix domain sockets for -proxy and -onion"
(https://github.com/bitcoin/bitcoin/pull/27375#pullrequestreview-1716369031)
tACK 0c5f8c36c90fab4fb39ce39d1b4c204a2edd25c4

Left a few nits which IMO don't need addressing. Tested out new socket type for proxy and onion and found it working as expected.

💬 willcl-ark commented on pull request "net: support unix domain sockets for -proxy and -onion":
(https://github.com/bitcoin/bitcoin/pull/27375#discussion_r1384082654)
nit: in c079e29c266a413de1859c0cbc8ebb55ea6a3203

I get a compilation failure:

```log
net.cpp:481:109: error: no member named 'ToString' in 'Proxy'
LogPrintLevel(BCLog::PROXY, BCLog::Level::Debug, "Using proxy: %s to connect to %s:%s\n", proxy.ToString(), addrConnect.ToStringAddr(), addrConnect.GetPort());
~~~~~ ^
./logging.h:257:45: note: expanded from macro 'LogPrintLevel'
...

💬 willcl-ark commented on pull request "net: support unix domain sockets for -proxy and -onion":
(https://github.com/bitcoin/bitcoin/pull/27375#discussion_r1384854138)
In c079e29

`ConnectDirectly` and `ConnectThroughProxy` now differ in their primary args, with Directly taking a `CService` and Proxy remaining with `std::string&` (for host and port) which results in:

_src/net.cpp_

```cpp
} else if (use_proxy) {
LogPrintLevel(BCLog::PROXY, BCLog::Level::Debug, "Using proxy: %s to connect to %s:%s\n", proxy.proxy.ToStringAddrPort(), addrConnect.ToStringAddr(), addrConnect.GetPort());
sock = ConnectThroughProxy(proxy, ad
...

💬 glozow commented on pull request "validate package transactions with their in-package ancestor sets":
(https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1385000534)
This is an `Assume` now

💬 furszy commented on pull request "p2p: adaptive connections services flags":
(https://github.com/bitcoin/bitcoin/pull/28170#discussion_r1385001110)
Hmm, naming is hard.
As the function is called `TipMayBeStale()`, maybe calling it `SyncState` could also work. Thoughts? Or do you have anything else in mind?