💬 Sjors commented on pull request "wallet: Implement independent BDB parser":
(https://github.com/bitcoin/bitcoin/pull/26606#discussion_r1279447033)
Maybe add a recommendation to the fuzz doc to [mount /tmp to tmpfs](https://ubuntu.com/blog/data-driven-analysis-tmp-on-tmpfs), or ensure we don't use the file system.

💬 MarcoFalke commented on pull request "wallet: Implement independent BDB parser":
(https://github.com/bitcoin/bitcoin/pull/26606#discussion_r1279450655)
This is already in the test docs, no?

```
$ git grep tmpfs test/README.md
test/README.md:sudo mount -t tmpfs -o size=4g tmpfs /mnt/tmp/

💬 jonathanbier commented on pull request "policy: Enable full-rbf by default":
(https://github.com/bitcoin/bitcoin/pull/28132#issuecomment-1658569312)
When I first read this, I did not understand what those links to https://mempool.space/ were, as I did not know, but they seem to have a new RBF feature. This RBF information then seems to go after a few days. Therefore I have provided some screenshots below so people can see the feature and it wont disappear. It shows that the transactions were replaced and that there was no RBF flag. I hope this helps.

[Antpool](https://mempool.space/tx/53cec64b52989c531550ac4606bedf1ff83d5bfd90efdc4006f122
...

💬 Sjors commented on pull request "wallet: Implement independent BDB parser":
(https://github.com/bitcoin/bitcoin/pull/26606#discussion_r1279466130)
It would be useful here - if BDB is compiled - to also open it with BDB, dump and then compare both dumps.

I'm not sure what to do for cases where BDB and RO have slightly different opinions on what they're willing to open at all. It's probably fine to skip comparing files where _either_ refuses to open.

💬 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?