📝 Xekyo opened a pull request: "[Fuzz/Bugfix] Mitigate timeout in CalculateTotalBumpFees"
(https://github.com/bitcoin/bitcoin/pull/27803)
The slow fuzz seed described in #27799 was just slower than expected, not an endless loop. Ensuring that every anscestor is only processed once speeds up the termination of the graph traversal.

Fixes #27799

💬 Xekyo commented on issue "fuzz: mini_miner: Timeout in mini_miner":
(https://github.com/bitcoin/bitcoin/issues/27799#issuecomment-1572852886)
Letting the timeout fuzz seed run longer results in it terminating eventually. I propose a fix in #27803

💬 denavila commented on pull request "wallet: Deniability API (Unilateral Transaction Meta-Privacy)":
(https://github.com/bitcoin/bitcoin/pull/27792#issuecomment-1572869964)
> See for example https://github.com/achow101/wallet-fingerprinting/blob/main/fingerprints.md

Thanks!
At a quick glance, it looks like Core's transactions are somewhat close to Electrum's.
I'm not familiar with some of the items, but assuming it's possible, I can look into inject some variety in the "deniabilized" transactions to make them resemble a variety of wallets (eg Electrum as a start).
Is there prior work in this by any chance? That would help me for sure.
Also, should I pursu
...

💬 ismaelsadeeq commented on pull request "Fee estimation: avoid serving stale fee estimate":
(https://github.com/bitcoin/bitcoin/pull/27622#discussion_r1213739677)
Yes, I agree with you. stale estimates should not be read. I have updated the MAX_FILE_AGE to 60 hours, which 2.5 days, as @instagibbs suggested that longer would be okay?

💬 denavila commented on pull request "wallet: Deniability API (Unilateral Transaction Meta-Privacy)":
(https://github.com/bitcoin/bitcoin/pull/27792#issuecomment-1572892485)
> It doesn't improve privacy.

Can you please elaborate why you think so?
Paul's blog post had some pretty compelling arguments why this scheme should work and I think it has a lot of merit.

> False sense of privacy implemented in this pull request only helps miners with fees.

It's important to point out that this PR doesn't add functionality that couldn't be achieved manually before.
If a user wanted to split a coin and send it to themselves, they could do it with the existing Bitcoi
...

💬 ryanofsky commented on pull request "kernel: Remove shutdown from kernel library":
(https://github.com/bitcoin/bitcoin/pull/27711#discussion_r1213768416)
> Just for clarity: This would imply passing a reference of the kernel context to the validation and blockstorage code? Giving the kernel ownership over this interrupt state seems a bit weird to me at first glance, since it is shared with all the other components that we have.

I think the idea is to give the kernel ownership of it's own interrupt state, and not have to rely on an external mechanism. If other components that are already depending on the kernel want to share the same interrupt
...

👍 theStack approved a pull request: "wallet: Add tracing for sqlite statements"
(https://github.com/bitcoin/bitcoin/pull/27801#pullrequestreview-1456464007)
tACK f45e0669dd1f1bdb243e645691e44993905a3919

Tested on OpenBSD 7.3, sqlite 3.41.0. Started bitcoind with `debug=walletdb -loglevel=walletdb:trace`, ran RPCs `createwallet`, `getnewaddress` and `setlabel` and observed the debug output emiting sqlite statements (`SQLite Statement: INSERT or REPLACE into main values(.....)`.

💬 furszy commented on pull request "walletdb: Add PrefixCursor":
(https://github.com/bitcoin/bitcoin/pull/27790#discussion_r1213799284)
This isn't covered by the tests.

In the test, all prefixes are serialized into a `DataStream` and then provided to `GetNewPrefixCursor`, so the size is always part of the data. Therefore, we never get up to this point.

Still, it doesn't seems to be reachable right now; As the db handler write function always serializes data, the size is always there.

But.. for the sake of test coverage completeness and leave nothing to chance, made a commit that exercises it: https://github.com/furszy/b
...

💬 furszy commented on pull request "walletdb: Add PrefixCursor":
(https://github.com/bitcoin/bitcoin/pull/27790#discussion_r1213702653)
Could:
```c++
for (const DatabaseFormat& db_format : DATABASE_FORMATS) {
dbs.emplace_back(MakeDatabase(path_root / strprintf("%d", db_format).c_str(), options, status, error));
BOOST_ASSERT(status == DatabaseStatus::SUCCESS);
}
```

💬 ryanofsky commented on pull request "walletdb: Add PrefixCursor":
(https://github.com/bitcoin/bitcoin/pull/27790#discussion_r1213849864)
> Noticed that this isn't covered by the tests.

If this is true, it seems like a bug in the test because two of the CheckPrefix lines were written specifically to cover this case.

https://github.com/bitcoin/bitcoin/blob/ba616b932cb9e9adb7eb9f1826caa62ce422a22d/src/wallet/test/db_tests.cpp#L204-L205

I think there is no need for a RawWrite method because spans of bytes should be serialized by just appending the bytes.

💬 furszy commented on pull request "walletdb: Add PrefixCursor":
(https://github.com/bitcoin/bitcoin/pull/27790#discussion_r1213883772)
oh well, I noticed while was checking the first test, and went deeper without noticing the second test existence :man_facepalming:.
What a day.. forget all what I said above. Thanks for the quick heads up.

💬 ajtowns commented on pull request "net_processing: Drop m_recently_announced_invs bloom filter":
(https://github.com/bitcoin/bitcoin/pull/27675#issuecomment-1573150818)
> What's the rationale regarding optimizing for memory instead of privacy here? I can see how this simplifies the design, however, I fail to see this being an obvious pick given we'd be potentially even more exposed to fingerprinting than before (as is now it is trivial for every connection to probe data on the node's mempool).

This isn't trading off privacy -- that's the point of [this comment](https://github.com/bitcoin/bitcoin/pull/27675#issuecomment-1549711784). It does mean that someone
...

💬 MarcoFalke commented on pull request "walletdb: Add PrefixCursor":
(https://github.com/bitcoin/bitcoin/pull/27790#discussion_r1213968128)
> But I think I would still like to avoid overloading Serialize for std::byte spans. The whole point of std::byte is that it's supposed to be a very safe type which requires you to be deliberate and explicit about conversions.

Agree on this. For raw C-Arrays where the length is denoted in the type, it shouldn't cause any issues. Probably the same for C++20 spans that are fixed size (but our `Span` doesn't have that feature).

> No because `Span<char>` is fixed length

Are you sure? One ca
...

👍 MarcoFalke approved a pull request: "streams: Drop confusing DataStream::Serialize method and << operator"
(https://github.com/bitcoin/bitcoin/pull/27800#pullrequestreview-1456744923)
Makes sense even absent the confusion, because Span also avoids a temporary copy to a `std::vector` (in DataStream).

lgtm ACK 5cd0717a54ce7a2065b29d90717aa2eb1c5e302d 🌙

<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}"
RUTRmVTMeKV5npGrKx1nqXCw
...

💬 MarcoFalke commented on pull request "streams: Drop confusing DataStream::Serialize method and << operator":
(https://github.com/bitcoin/bitcoin/pull/27800#discussion_r1213984067)
`+=` removed in faa96f841fe45bc49ebb6e07ac82a129fa9c40bf

💬 kylezs commented on issue "-fallbackfee should apply to estimatesmartfee":
(https://github.com/bitcoin/bitcoin/issues/27415#issuecomment-1573276009)
> If your test isn't concerned with the fee estimator's exact behavior, as @instagibbs says, you can mock it or load a placeholder fee_estimates.dat file to populate the estimator with data.

This might be the way to go, is there one that exists that can be downloaded and loaded in? I don't really mind what the exact behaviour of the estimator is.

💬 ajtowns commented on pull request "p2p: Stop relaying non-mempool txs":
(https://github.com/bitcoin/bitcoin/pull/27625#issuecomment-1573281471)
I've cleaned up and rebased this as https://github.com/ajtowns/bitcoin/commits/202306-dropmaprelay if that's any help.

💬 ccdle12 commented on pull request "net, refactor: extract Network and BIP155Network logic to node/network":
(https://github.com/bitcoin/bitcoin/pull/27385#issuecomment-1573387265)
> Rebased 3rd_place_medal
>
> @ccdle12 Thanks! Mind re-acking? As to motivation, I think it's a mix of both reasons you mentioned, with the first one being the direction to go when doing so -- see `doc/design/libraries.md` and merged changes like [c741d74](https://github.com/bitcoin/bitcoin/commit/c741d748d4d9836940b99091cc7be09c65efcb79).

Of course! re-tACK at 8efd76b, thanks for explaining the motivation and pointing out the design/doc :)

💬 willcl-ark commented on issue "Remove Ambiguity of Script ASM Hex and Decimal Integer Representations":
(https://github.com/bitcoin/bitcoin/issues/27795#issuecomment-1573396315)
It's a little unclear to me how the ASM output is used on the consumer side, and therefore how breaking any changes to it might be. However I still think we have a few options to resolve this:

1) Fully hex-encoded output in the ASM repr:

```shell
$ /home/will/src/bitcoin/src/bitcoin-cli -regtest decodescript 0512121212120457c74942
{
"asm": "1212121212 57c74942",
```

🚀 fanquake merged a pull request: "Update .style.yapf"
(https://github.com/bitcoin/bitcoin/pull/27802)