🤔 tdb3 reviewed a pull request: "test: test_bitcoin: allow -testdatadir=<datadir>"
(https://github.com/bitcoin/bitcoin/pull/26564#pullrequestreview-1887381174)
Nice feature. Concept ACK.

Ran a few practical tests on 7b81dea7eb6b14a939230477835159dad6b4b75b. Unless I'm overlooking something, seems like there may be an issue with some tests when `testdatadir` is provided a relative path (i.e. non-absolute path).

1. Ran all unit tests, without `-testdatadir`. All passed. Verified `/tmp/test_common_Bitcoin Core` was empty afterward.
1. Ran all unit tests, with `-testdatadir=/absolute/path/to/mytestdir/newlycreateddir/`, where `mytestdir` was cre
...

📝 ariard opened a pull request: "Add issuer-selected opt-in txn / pckg policy checks"
(https://github.com/bitcoin/bitcoin/pull/29448)
All the values selected by the transaction issuers are implemented to respect current hard policy limits as of 27.0
Bitcoin Core version (e.g `MAX_PACKAGE_COUNT` or `MAX_STANDARD_TX_WEIGHT`). As such introducing a new distinction in the Bitcoin ecosystem among tx-relay policy checks enforced by full-nodes hosts of a said full-node implementation/version and the the policy-check opt-ed by any transaction or second-layers use-cases. That way any significant on-chain economic traffic can be still
...

💬 ariard commented on pull request "Add imbued v3 based on template-matching":
(https://github.com/bitcoin/bitcoin/pull/29427#issuecomment-1951637283)
can you precise where the imbuance mechanism should be applied? like in `AcceptSingleTransaction` or `AcceptMultipleTransaction` or even earlier at the transaction-relay level in `ProcessMessages` ?
i think it’s good thing to have a generic imbuance mechanism though i would say the earlier in the net processing stack the better.

hardcoding a template in the imbuance mechanism is a dumb idea even for today Lightning, the “must have exactly 2 330-satoshi outputs” i can just add another HTLC p2
...

💬 ariard commented on pull request "[WIP] Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#issuecomment-1951640940)
do you have a simulation environment or script like you’ve done for #25717 to observe the computational complexity diff compared to today’s mempool against different chain of transaction performance payload ?
interesting what is the lowest performance linux host assumed for decentralization of the tx-relay network.
assume always 24/7 internet and on a vpcu instance, not baremetal.

💬 ariard commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#issuecomment-1951641928)
> are you still interested if I do a proof-of-concept bitcoin core branch or idea draft of a) moving the “package limit” on the use-case side and b) make it dynamic (in the limit of absolute core DoS robustness) ? I think it can be backup in future bip331 changes. v3 is your work and this is your PR making anti-pinning claims after-all.

did it with #29448

💬 stratospher commented on pull request "test/BIP324: disconnection scenarios during v2 handshake":
(https://github.com/bitcoin/bitcoin/pull/29431#discussion_r1494116658)
`test_earlykeyresponse` sends handshake bytes etc.. in the `MainThread` and I feel it's better to isolate that behaviour in a different test.

also, if you grep for `def test_` in `test/functional`, you'll see many [examples](https://github.com/bitcoin/bitcoin/blob/3cbc8cbc71d3d6ecfaf41164ce59c24ac94bae99/test/functional/feature_asmap.py#L128) of different tests listed in `run_test()` using the pattern which is currently used.

💬 stratospher commented on pull request "test/BIP324: disconnection scenarios during v2 handshake":
(https://github.com/bitcoin/bitcoin/pull/29431#discussion_r1494131410)
yeah! `expected_debug_message` on [L180](https://github.com/bitcoin/bitcoin/blob/ec9005ca4be088dfa2d247bbfe964a9c98e4f29d/test/functional/p2p_v2_misbehaving.py#L180) was written with the assumption that `test_v2disconnection` is run after `test_earlykeyresponse`. so peer=0 happens in `test_earlykeyresponse` and peer=1,2,3,4,5 happens in `test_v2disconnection`.

💬 Sjors commented on pull request "Silent payment index (for light wallets and consistency check)":
(https://github.com/bitcoin/bitcoin/pull/28241#discussion_r1494191408)
Because I want to drop that in #26201. Though perhaps Silent Payments itself is a reason to keep it.

💬 paplorinc commented on pull request "doc: Bump copyright years to present (headers only)":
(https://github.com/bitcoin/bitcoin/pull/26817#issuecomment-1951977757)
I've opened a similar issue proposing to remove all of these inconsistent and hard to maintain headers and replace them with simple SPDX-License-Identifier https://github.com/bitcoin/bitcoin/issues/29445

💬 naumenkogs commented on pull request "net: attempts to connect to all resolved addresses when connecting to a node":
(https://github.com/bitcoin/bitcoin/pull/28834#discussion_r1494207563)
Yeah, after looking more into the current PR, it achieves nearly the same goal...
I'm fine with not doing anything here.

But it would be cool to understand better what it achieves :)

Here's what i think is going on. A DNS implementation may (or may not) return results in the same order, and then all the nodes will end up making connections with the same result. Then it's useful for load balancing, and to avoid DNS influencing the connectivity in general.
Might be worth adding this commen
...

💬 Sjors commented on pull request "Silent payment index (for light wallets and consistency check)":
(https://github.com/bitcoin/bitcoin/pull/28241#discussion_r1494213677)
(though I think the specific reason here was because I was in a hurry)

💬 Sjors commented on pull request "Silent payment index (for light wallets and consistency check)":
(https://github.com/bitcoin/bitcoin/pull/28241#discussion_r1494217939)
Brought it back.

💬 Sjors commented on pull request "Silent payment index (for light wallets and consistency check)":
(https://github.com/bitcoin/bitcoin/pull/28241#discussion_r1494226737)
Done

💬 Sjors commented on pull request "Silent payment index (for light wallets and consistency check)":
(https://github.com/bitcoin/bitcoin/pull/28241#issuecomment-1952006804)
Applied @fjahr's patches and running the indexer again...

💬 maflcko commented on issue "getJsonToken assumes underlying string is null-terminated but requires end pointer":
(https://github.com/bitcoin/bitcoin/issues/28260#issuecomment-1952037763)
> My changes have a few orthogonal, but related fixes and test additions. We could make different pull-requests for them. Let me know what you prefer.

According to the dev notes, they should be different commits. Ideally bugfixes and features in a different PR. See https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#committing-patches (The doc, and it's links to the dev notes and productivity notes should explain all your questions)

💬 maflcko commented on issue "[CI/CD]Release channels?":
(https://github.com/bitcoin/bitcoin/issues/29446#issuecomment-1952055714)
Yes, they already exist. I presume "canary" means nightly, which is the `master` branch. Otherwise, you can compile from `rc`s (release candidates), which are the "beta". Finally, if you compile from the latest tag, it can be considered "stable".

💬 recursive-rat4 commented on issue "Proposal: Adopt simple SPDX-License-Identifier Across Bitcoin Codebase":
(https://github.com/bitcoin/bitcoin/issues/29445#issuecomment-1952060401)
>version control already preserves historical data and legal integrity

Version control history is often not preserved in practice for whatever reasons. A fresh example of this is https://github.com/testng-team/testng-ant splitted up in https://github.com/testng-team/testng/issues/3033

💬 maflcko commented on pull request "rpc: Fixed signed integer overflow for large feerates":
(https://github.com/bitcoin/bitcoin/pull/29434#discussion_r1494273142)
Sure, may do if I re-touch.

💬 fanquake commented on issue "test: Write assumeutxo tests":
(https://github.com/bitcoin/bitcoin/issues/28648#issuecomment-1952071633)
There are more PRs open. i.e #29428.

🤔 Sjors reviewed a pull request: "test: add end-to-end tests for CConnman and PeerManager"
(https://github.com/bitcoin/bitcoin/pull/26812#pullrequestreview-1888057050)
Reviewed the fist commit bee6bdf0a5084b2d5749ff06ad63c0e77816c733. Added a question for the second.