💬 rkrux commented on pull request "wallet: Identify transactions spending 0-value outputs, and add tests for anchor outputs in a wallet":
(https://github.com/bitcoin/bitcoin/pull/33268#discussion_r2318929377)
In https://github.com/bitcoin/bitcoin/commit/69eeb11bc585e92a3b56811c3251f13dba05aa5f "wallet: Add m_cached_from_me to cache "from me" status"

nit to shorten:
```diff
diff --git a/src/wallet/receive.cpp b/src/wallet/receive.cpp
index 4ed98c6d55..13a25b3632 100644
--- a/src/wallet/receive.cpp
+++ b/src/wallet/receive.cpp
@@ -195,8 +195,9 @@ void CachedTxGetAmounts(const CWallet& wallet, const CWalletTx& wtx,

bool CachedTxIsFromMe(const CWallet& wallet, const CWalletTx& wtx)
{
-
...

💬 rkrux commented on pull request "wallet: Identify transactions spending 0-value outputs, and add tests for anchor outputs in a wallet":
(https://github.com/bitcoin/bitcoin/pull/33268#discussion_r2318869615)
In https://github.com/bitcoin/bitcoin/commit/1cd57c67b10a771d2eb1c2324b241533c0b1f287 "test: Add a test for anchor outputs in the wallet"

While passing the `inputs` in this RPC is fine, I feel another frequently used way of calling `sendall` RPC could be to not pass the `inputs` in the request, in which case the RPC throws `Internal bug detected`.

```zsh
test_framework.authproxy.JSONRPCException: Internal bug detected: output.input_bytes > 0
wallet/rpc/spend.cpp:1524 (operator())
Bitcoi
...

💬 rkrux commented on pull request "wallet: Identify transactions spending 0-value outputs, and add tests for anchor outputs in a wallet":
(https://github.com/bitcoin/bitcoin/pull/33268#discussion_r2318733172)
In 1cd57c67b10a771d2eb1c2324b241533c0b1f287 "test: Add a test for anchor outputs in the wallet"

supernit:

```diff
index 1d1435145a..a47f3c6b3c 100755
--- a/test/functional/wallet_anchor.py
+++ b/test/functional/wallet_anchor.py
@@ -71,13 +71,13 @@ class WalletAnchorTest(BitcoinTestFramework):
assert_equal(utxos[0]["txid"], anchor_txid)
assert_equal(utxos[0]["address"], ANCHOR_ADDRESS)
assert_equal(utxos[0]["amount"], 0)
- wallet.gettransaction(anch
...

💬 rkrux commented on pull request "wallet: Identify transactions spending 0-value outputs, and add tests for anchor outputs in a wallet":
(https://github.com/bitcoin/bitcoin/pull/33268#discussion_r2318809119)
In 1cd57c67b10a771d2eb1c2324b241533c0b1f287 "test: Add a test for anchor outputs in the wallet"

There is a bit of duplication in all 3 tests. Maybe can remove this test altogether and put some of its functionality in the below test.
```diff
diff --git a/test/functional/wallet_anchor.py b/test/functional/wallet_anchor.py
index 1d1435145a..e6872deba0 100755
--- a/test/functional/wallet_anchor.py
+++ b/test/functional/wallet_anchor.py
@@ -79,40 +79,22 @@ class WalletAnchorTest(BitcoinTestF
...

💬 rkrux commented on pull request "wallet: Identify transactions spending 0-value outputs, and add tests for anchor outputs in a wallet":
(https://github.com/bitcoin/bitcoin/pull/33268#discussion_r2318709095)
In 1cd57c67b10a771d2eb1c2324b241533c0b1f287 "test: Add a test for anchor outputs in the wallet"

Nit: Probably can stay in the util file where `PAY_TO_ANCHOR` is defined.

💬 0xB10C commented on issue "tracing: test `interface_usdt_net.py` fails due to garbage message type in `net:outbound_message` tracepoint":
(https://github.com/bitcoin/bitcoin/issues/33246#issuecomment-3249215214)
As per https://github.com/iovisor/bcc/blob/791bf81ad9ec18ce2899f79509e4de96ddc21e9d/src/cc/usdt/usdt_args.cc#L228-L231, bcc on `aarch64` claims to support parsing `8@[x0]` via `[-]<size>@[<reg>]`.

```c
// Support the following argument patterns:
// [-]<size>@<value>, [-]<size>@<reg>, [-]<size>@[<reg>], or
// [-]<size>@[<reg>,<offset>]
// [-]<size>@[<reg>,<index_reg>]
```

🤔 janb84 reviewed a pull request: "build: suggest -DENABLE_IPC=OFF when missing capnp"
(https://github.com/bitcoin/bitcoin/pull/33290#pullrequestreview-3180629451)
ACK f39a782ba8629bbca1587229f91ba2318d6b765c

PR adds warning text to cmake configure step if CAPNP is not installed. The CAPNP dependency is new, and this warning helps people that do not read the Releasenotes how to solve the error.

Although I do not recommend to upgrade without reading the Release notes / changelog, this warning can reduce the number of issues and guides (non-technical) users to avoid errors .

- code review ✅
- build with and without capnp to test warning. ✅

<de
...

👍 vasild approved a pull request: "Replace libevent with our own HTTP and socket-handling implementation"
(https://github.com/bitcoin/bitcoin/pull/32061#pullrequestreview-3180206398)
ACK e531a7cd2c17dfb8d075d02865dbc25f8a832b3a

All my suggestions above have been addressed or some reasoning has been given why not.

💬 vasild commented on pull request "Replace libevent with our own HTTP and socket-handling implementation":
(https://github.com/bitcoin/bitcoin/pull/32061#discussion_r2318674837)
nit: I think that can be a bit shorter:

```suggestion
[](char c) {
// Avoid implicit-integer-sign-change UB by only
// processing ASCII.
if (c < 0) return c;
```

💬 vasild commented on pull request "Replace libevent with our own HTTP and socket-handling implementation":
(https://github.com/bitcoin/bitcoin/pull/32061#discussion_r2318700631)
Maybe also check that the total length of all headers is as expected. That is, no extra, unexpected, stuff has been returned:

```diff
BOOST_REQUIRE(headers_string.ends_with("\r\n\r\n"));
+ BOOST_CHECK_EQUAL(headers_string.length(), 67);
}
```

💬 vasild commented on pull request "Replace libevent with our own HTTP and socket-handling implementation":
(https://github.com/bitcoin/bitcoin/pull/32061#discussion_r2318877575)
Similar to above:

```suggestion
actual.find("Date: Wed, 11 Dec 2024 00:47:09 GMT\r\n") != std::string::npos &&
actual.length() == 146
```

💬 vasild commented on pull request "Replace libevent with our own HTTP and socket-handling implementation":
(https://github.com/bitcoin/bitcoin/pull/32061#discussion_r2318891980)
nit: no need for trailing `;` and some missing whitespace:

```suggestion
HTTPRequestMethod GetRequestMethod() const { return m_method; }
```

💬 rkrux commented on pull request "wallet: Identify transactions spending 0-value outputs, and add tests for anchor outputs in a wallet":
(https://github.com/bitcoin/bitcoin/pull/33268#discussion_r2318999432)
I think this particular error is appropriately specific. The error in `sendtoaddress`, while easier to parse, is thrown because it also tries to sign the transaction, though at an earlier stage in the RPC flow. Perhaps this particular test can avoid testing for this specific error because the different error is conspicuous.

💬 glozow commented on pull request "Revert compact block cache inefficiencies":
(https://github.com/bitcoin/bitcoin/pull/33253#issuecomment-3249315224)
Should this be backported?

👍 dergoegge approved a pull request: "p2p: add `DifferenceFormatter` fuzz target and invariant check"
(https://github.com/bitcoin/bitcoin/pull/33252#pullrequestreview-3180811360)
Code review ACK 65a10fc3c52ea09a4794345bcf607dff908c783a

📝 m3dwards opened a pull request: "added qa assets yml"
(https://github.com/bitcoin/bitcoin/pull/33292)

✅ m3dwards closed a pull request: "added qa assets yml"
(https://github.com/bitcoin/bitcoin/pull/33292)

🤔 glozow reviewed a pull request: "doc: truc packages allow sub min feerate transactions"
(https://github.com/bitcoin/bitcoin/pull/33220#pullrequestreview-3180848706)
ACK 7270839af425adddb3ed436a58a41b5bc843dab5

✅ glozow closed an issue: "doc: Mempool Policy documentation Outdated since TRUC"
(https://github.com/bitcoin/bitcoin/issues/32067)

🚀 glozow merged a pull request: "doc: truc packages allow sub min feerate transactions"
(https://github.com/bitcoin/bitcoin/pull/33220)