💬 maflcko commented on pull request "Improve parallel script validation error debug logging":
(https://github.com/bitcoin/bitcoin/pull/31112#discussion_r1867208630)
nano nit in 492e1f0994 (only if you retouch): Could remove the unused `f`?

💬 Sjors commented on pull request "Miner: never create a template which exploits the timewarp bug":
(https://github.com/bitcoin/bitcoin/pull/31376#discussion_r1867245362)
So why not drop `- MAX_TIMEWARP` and just make sure that we never propose a block with an earlier timestamp?

Or go beyond that and never allow time to go backwards on _any_ block?

💬 Sjors commented on pull request "Miner: never create a template which exploits the timewarp bug":
(https://github.com/bitcoin/bitcoin/pull/31376#issuecomment-2513848116)
> But this PR does not make any block invalid. This is therefore not a concern with this PR

No, but it's adding complexity that might not match the actual soft fork. See my inline comment for a more generic approach (but I'm not sure if it's safe).

💬 vasild commented on pull request "ci: detect outbound internet traffic generated while running tests":
(https://github.com/bitcoin/bitcoin/pull/31349#discussion_r1867271307)
> ... explicitly list the required (or removed) caps ...

Done, `NET_RAW` is required to run `tcpdump`: https://www.tcpdump.org/manpages/pcap.3pcap.html

Plus, the ASAN job requires that `tcpdump -w` runs and creates the file, otherwise it will be red.

I think that resolves the concerns from this thread, so I am closing it. Feel free to comment / reopen if there is more to this.

💬 ajtowns commented on pull request "p2p: Increase tx relay rate":
(https://github.com/bitcoin/bitcoin/pull/28592#issuecomment-2513884424)
Rebased for fresh CI

📝 maflcko opened a pull request: "test: Avoid logging error when logging error"
(https://github.com/bitcoin/bitcoin/pull/31408)
Currently a logging error in the form of `--- Logging error ---` happens when an error is logged in the `_on_data` helper.

Fix it by properly logging the error.

Also, treat pylint errors as errors, to avoid this problem in the future.

Can be tested by running `p2p_addrv2_relay.py` with the following example diff:

```diff
diff --git a/test/functional/test_framework/p2p.py b/test/functional/test_framework/p2p.py
index 523e1bd068..0f1eb29d13 100755
--- a/test/functional/test_framewor
...

👍 i-am-yuvi approved a pull request: "test: fix `test_invalid_tx_in_compactblock` in `p2p_compactblocks`"
(https://github.com/bitcoin/bitcoin/pull/31406#pullrequestreview-2475052056)
Tested ACK 9e278f7f13395219ab056ba8c2810d3ce5c41d65

- The test works correctly
- For those who didn't understand why its better to use `!=` instead of `is not` here, since we are converting the tip hex to int and `is not` checks for object identity (if two references point to different objects in memory) which might not accurately capture intended comparison whereas `!=` checks the numerical values directly.

💬 willcl-ark commented on issue "macOS App Notarization & Stapling":
(https://github.com/bitcoin/bitcoin/issues/15774#issuecomment-2513978792)
Awesome!

> Also, it turns out anyone enrolled in the Apple Developer Program can submit any app to Apple for notarization even if they weren't the code signer...

This is surprising, and potentially annoying. It reads to me from [the docs](https://developer.apple.com/documentation/security/resolving-common-notarization-issues#Ensure-a-valid-code-signature) that one could only notarize a codesigned binary. I hope, that the notarisation authentication must match the codesignature in some way?
...

💬 maflcko commented on pull request "refactor: Move GuessVerificationProgress into ChainstateManager":
(https://github.com/bitcoin/bitcoin/pull/31393#discussion_r1867360742)
Thanks, done

💬 Sjors commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#issuecomment-2514050392)
I also tried @bigspider's MooSig demo which worked. I then crafted a multisig between A and the device: `tr(musig(A,L)/<0;1>/*)`. I managed to register the policy (after several mistakes, it's very tedious to do this manually).

I wanted to try using HWI to display the address, but I would have to modify it to work with the test app. I just yolo funded it.

I then created a withdrawal PSBT and pasted it in the Moosig script, modifying it to only add its public nonce. I also hardcoded the reg
...

💬 Prabhat1308 commented on pull request "wallet, rpc: Settxfeerate":
(https://github.com/bitcoin/bitcoin/pull/31278#discussion_r1867364249)
```suggestion
self.log.debug("Test that settxfeerate set the feerate in sat/vB")
```

👍 dergoegge approved a pull request: "Improve parallel script validation error debug logging"
(https://github.com/bitcoin/bitcoin/pull/31112#pullrequestreview-2475144960)
ACK 492e1f09943fcb6145c21d470299305a19e17d8b

💬 willcl-ark commented on issue "Crash upon RPC v1 connection in v28.0.0":
(https://github.com/bitcoin/bitcoin/issues/31041#issuecomment-2514072312)
I took a look with heaptrack on a debug build of 28.0 (output file: https://tmp.256k1.dev/heaptrack.bitcoind.3546996.zst )


Whilst number of allocations increase over time as expected:


Consumed heap memory stays pretty stable:

![image](https://github.com/user-attachments/assets/528a529c-7f35-4eb0-a6d9-59
...

💬 Sjors commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#issuecomment-2514087290)
Looking at the functional tests in 3649c2eb2053a0c166c68beb310c9c64ddc5b273 it seems the way this is designed to work is by swapping out the active `tr([m/86'/1'/0']xpriv/<0;1>)/*` descriptor for `tr(musig([m/86'/1'/0']xpriv,other,other)/<0;1>)/*`.

I guess that's fine for the purpose of getting MuSig2 functionality in for experimental use, but it seems a bit unsafe and confusing for general use.

💬 maflcko commented on issue "Crash upon RPC v1 connection in v28.0.0":
(https://github.com/bitcoin/bitcoin/issues/31041#issuecomment-2514089741)
Thanks! I don't expect a memory leak either, but I'd find it interesting to see where the heap usage came from. I guess it is all Univalue?

💬 maflcko commented on issue "fuzz, parse_iso8601: attempt to dereference an end-of-stream istreambuf_iterator":
(https://github.com/bitcoin/bitcoin/issues/28917#issuecomment-2514119343)
Fixed in https://github.com/bitcoin/bitcoin/pull/31391

💬 willcl-ark commented on issue "Crash upon RPC v1 connection in v28.0.0":
(https://github.com/bitcoin/bitcoin/issues/31041#issuecomment-2514123767)
Actually seems to be a mixture, and UniValue doesn't particularly stand out to me:


👍 dergoegge approved a pull request: "util: Drop boost posix_time in ParseISO8601DateTime"
(https://github.com/bitcoin/bitcoin/pull/31391#pullrequestreview-2475255194)
utACK faf70cc9941ce2b0ce4fd48ecfdbe28194adb8ba

💬 vasild commented on pull request "ci: detect outbound internet traffic generated while running tests":
(https://github.com/bitcoin/bitcoin/pull/31349#issuecomment-2514198467)
Ready for review. I updated the OP with some details.

👋 vasild's pull request is ready for review: "ci: detect outbound internet traffic generated while running tests"
(https://github.com/bitcoin/bitcoin/pull/31349)