💬 Sjors commented on pull request "wallet: Implement independent BDB parser":
(https://github.com/bitcoin/bitcoin/pull/26606#discussion_r1279468544)
Not in the fuzz docs, but just having it in the test docs is probably fine.

📝 theStack opened a pull request: "test: add script compression coverage for not-on-curve P2PK outputs"
(https://github.com/bitcoin/bitcoin/pull/28193)
This PR adds unit test coverage for the script compression functions `{Compress,Decompress}Script` in the special case of uncompressed P2PK outputs (scriptPubKey: OP_PUSH65 <0x04 ....> OP_CHECKSIG) with [pubkeys that are not fully valid](https://github.com/bitcoin/bitcoin/blob/44b05bf3fef2468783dcebf651654fdd30717e7e/src/pubkey.cpp#L297-L302), i.e. where the encoded point is not on the secp256k1 curve. For those outputs, script compression is not possible, as the y coordinate of the pubkey can't
...

💬 fanquake commented on pull request "rpc: Add test-only RPC getaddrmaninfo for new/tried table address count":
(https://github.com/bitcoin/bitcoin/pull/27511#discussion_r1279500464)
Maybe relevant here, I think it's easier to just avoid (explicitly) copy-pasting ChatGPT or similar output into the project, for now, see #28175.

💬 achow101 commented on pull request "ParseHDKeypath: support h as hardened marker":
(https://github.com/bitcoin/bitcoin/pull/28192#issuecomment-1658730282)
I'm not sure if there's any software that enforces the consistent usage of a hardened marker.

💬 ryanofsky commented on pull request "CONTRIBUTING: Caution against using AI/LLMs (ChatGPT, Copilot, etc)":
(https://github.com/bitcoin/bitcoin/pull/28175#issuecomment-1658798341)
NACK from me, because I think legal questions like this are essentially political questions, and you make absurd legal outcomes more likely to happen by expecting them to happen, and by writing official documentation which gives them credence.

If the risk is that openai or microsoft could claim copyright over parts of the bitcoin codebase, that would be absurd because their usage agreements assign away their rights to the output and say it can be used for any purpose.

If the risk is that s
...

💬 kristapsk commented on pull request "policy: Enable full-rbf by default":
(https://github.com/bitcoin/bitcoin/pull/28132#issuecomment-1658851397)
> 2\. Users who do care and want full-RBF to be **off** and rely on defaults not changing.

Users should check release notes before upgrading Bitcoin node. Various defaults changes from time to time, this won't be the first time.

📝 jonatack opened a pull request: "test, script: python E721 and flake8 updates"
(https://github.com/bitcoin/bitcoin/pull/28194)
Update our functional tests per [E721](https://www.flake8rules.com/rules/E721.html) enforced by [flake8 6.1.0](https://flake8.pycqa.org/en/latest/release-notes/6.1.0.html), and update our CI lint task to use that release. This makes the following linter output on current master with flake8 6.1.0 green.

```
$ ./test/lint/lint-python.py ; ./test/lint/lint-spelling.py
test/functional/p2p_invalid_locator.py:35:16: E721 do not compare types, for exact checks use `is` / `is not`, for instance c
...

💬 luke-jr commented on pull request "Allow configuring target block time for a signet":
(https://github.com/bitcoin/bitcoin/pull/27446#issuecomment-1658950229)
>...the purpose of including signet in this repo is about making it easier to test bitcoin, not about building your own altcoin with different parameters. How does using a 30s signet do anything but encourage developers to build software that will only work well on altcoins (like liquid...

Just thought I should point out that Liquid is Bitcoin, not an altcoin.

💬 jonatack commented on pull request "test, script: python E721 and flake8 updates":
(https://github.com/bitcoin/bitcoin/pull/28194#issuecomment-1658963090)
CI lint task report looks good https://cirrus-ci.com/task/6522164438892544.

👋 jonatack's pull request is ready for review: "test, script: python E721 and flake8 updates"
(https://github.com/bitcoin/bitcoin/pull/28194)

🤔 pinheadmz reviewed a pull request: "Relay own transactions only via short-lived Tor or I2P connections"
(https://github.com/bitcoin/bitcoin/pull/27509#pullrequestreview-1555409244)
code review re-ACK 2541f09439099ec3e73f7c5a12f809f190e6af1d

confirmed changes since last ack were rebase and small review comments addressed. Found a typo, and played with the feature on signet (longer report about that to follow)

💬 pinheadmz commented on pull request "Relay own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/27509#discussion_r1279655081)
nit: s/send/sent

💬 willcl-ark commented on pull request "contrib: add tool to convert compact-serialized UTXO set to SQLite database":
(https://github.com/bitcoin/bitcoin/pull/27432#discussion_r1279739808)
nit: should we `sys.exit(1)` here too?

👍 willcl-ark approved a pull request: "contrib: add tool to convert compact-serialized UTXO set to SQLite database"
(https://github.com/bitcoin/bitcoin/pull/27432#pullrequestreview-1376557345)
tACK 3ce180ac2d

Left two nits which don't need addressing unless being re-touched, but overall this works well in testing and seems like a useful contrib script. Converting the output to json also worked as described in the comments above.

💬 willcl-ark commented on pull request "contrib: add tool to convert compact-serialized UTXO set to SQLite database":
(https://github.com/bitcoin/bitcoin/pull/27432#discussion_r1160947835)
Whitepsace nit (unless it was deliberate to align with following line):

```suggestion
parser.add_argument('infile', help='filename of compact-serialized UTXO set (input)')
```

🤔 stickies-v reviewed a pull request: "ci: Integrate `bitcoin-tidy` clang-tidy plugin"
(https://github.com/bitcoin/bitcoin/pull/26296#pullrequestreview-1555593455)
Approach ACK 3a727cd7ee02c83efcd57d004e6fca8d8e1bb33b

Did a code review too but both the build stuff as well as the clang-tidy syntax is still very new to me so doesn't mean too much.

Are the tests being run in CI with this PR? And perhaps some tidy-test for multiline strings would be helpful too?

💬 pinheadmz commented on pull request "Relay own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/27509#issuecomment-1659008138)
Ran with this feature on signet and found a few unexpected behaviors:

1. `-sensitiverelay=1` and `-onlynet=ipv4` should probably throw an init error. In this configuration, sensitive stuff seems to be ignored and new wallet TXs are relayed via ipv4 peers like normal. Is "only use tor for sensitive relay" a possible configuration?

2. Rapidly sending multiple transactions `"...incremented the number of connections to open from 20 to 25..."` this number grows rapidly and decreases slowly (esp
...

💬 achow101 commented on pull request "Package Relay 1/3: Introduce TxPackageTracker as Orphan Resolution Module":
(https://github.com/bitcoin/bitcoin/pull/28031#discussion_r1279730123)
In 29d9d326d5193bb9a410a8881eabc93de5dd6266 "[txorphanage] track size of stored orphans, total and by peer"

This function is unimplemented.

💬 achow101 commented on pull request "Package Relay 1/3: Introduce TxPackageTracker as Orphan Resolution Module":
(https://github.com/bitcoin/bitcoin/pull/28031#discussion_r1279714521)
In 543273d96e896adf5531ed961856aa0eb70cbe57 "[log] log ProcessOrphanTx() events by wtxid"

Perhaps log both txid and wtxid?

💬 achow101 commented on pull request "Package Relay 1/3: Introduce TxPackageTracker as Orphan Resolution Module":
(https://github.com/bitcoin/bitcoin/pull/28031#discussion_r1279743325)
In 974d864419dd98be6e32dec3ee11f5082b060b1b "[refactor] make TxPackageTracker responsible for EraseTx and AddChildrenToWorkset"

This line seems a bit unrelated to this commit as there is no pre-existing `EraseTx`.

💬 achow101 commented on pull request "Package Relay 1/3: Introduce TxPackageTracker as Orphan Resolution Module":
(https://github.com/bitcoin/bitcoin/pull/28031#discussion_r1279747416)
In b5ab45e595bbcedbd602b6385b83e9ffd983f216 "[p2p/refactor] make TxPackageTracker responsible for orphan resolution"

nit: indentation