💬 Umiiii commented on pull request "test: add missing comparison of node1's mempool in MempoolPackagesTest":
(https://github.com/bitcoin/bitcoin/pull/29941#discussion_r1577099806)
Could you please clarify what you meant by "indented wrong"? I believe these lines are intended to follow the same indentation pattern as the rest of the file. If there's a specific indentation style that should be applied here, I'd appreciate your guidance.

🤔 tdb3 reviewed a pull request: "contrib: rpcauth.py - Add new option (-json) to output text in json format"
(https://github.com/bitcoin/bitcoin/pull/29433#pullrequestreview-2018620939)
Concept ACK.

Looks like these commits (4f746716c008216ff01da48ccfda559308b4232e, 1606590bbffa46f88fa6e4333960e82b919efd8a, and f2fb13bb1ac392abc4ccd656b025aa017b12060e) should be squashed to clean up and tell a concise story of update.

Seems to work well:
```
$ ./rpcauth.py
usage: rpcauth.py [-h] [-j] username [password]
rpcauth.py: error: the following arguments are required: username

$ ./rpcauth.py -h
usage: rpcauth.py [-h] [-j] username [password]

Create login credentials for
...

💬 Umiiii commented on pull request "test: add missing comparison of node1's mempool in MempoolPackagesTest":
(https://github.com/bitcoin/bitcoin/pull/29941#discussion_r1577104003)
But I agreed with you. Those lines are self explanatory. I removed the comments.

✅ Umiiii closed a pull request: "test: add missing comparison of node1's mempool in MempoolPackagesTest"
(https://github.com/bitcoin/bitcoin/pull/29941)

📝 Umiiii opened a pull request: "test: add missing comparison of node1's mempool in MempoolPackagesTest"
(https://github.com/bitcoin/bitcoin/pull/29948)
#29941 Recreated a pull request because there was a conflict. Trying to resolve the conflict but the old one automatically closed.

Add missing comparison for TODO comments in `mempool_packages.py`

Also, notice that the ancestor size limits and descendant size limits actually implemented in #21800 , so I removed the todo for those two size limits.

@glozow @instagibbs

:lock: fanquake locked an issue: "help i think my core is hacked"
(https://github.com/bitcoin/bitcoin/issues/29944)

💬 fanquake commented on pull request "JSON-RPC request Content-Type is application/json":
(https://github.com/bitcoin/bitcoin/pull/29946#issuecomment-2073990266)
Removed bugfix given this is a doc/test only change.

💬 maflcko commented on pull request "contrib: rpcauth.py - Add new option (-json) to output text in json format":
(https://github.com/bitcoin/bitcoin/pull/29433#issuecomment-2074059554)
Please squash your commits according to https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#squashing-commits

💬 maflcko commented on issue "Crashes in v27 on Windows 10 and 11":
(https://github.com/bitcoin/bitcoin/issues/29916#issuecomment-2074064411)
It could make sense to run this in a debugger (such as gdb) to get a stacktrace, but I am not sure what debuggers exist on Windows.

💬 maflcko commented on issue "~/.bitcoin (which is a softlink to a separate vmware virtual drive) dir is now almost 1tb":
(https://github.com/bitcoin/bitcoin/issues/29909#issuecomment-2074070581)
> Now i've set it up without that "txindex" parameter = enabled... still the same issue...

Unsetting txindex does not delete the txindex from storage.

Can you please share the sizes of the largest folders in the datadir?

💬 maflcko commented on pull request "Remove redundant `-datacarrier` option":
(https://github.com/bitcoin/bitcoin/pull/29942#issuecomment-2074103437)
> Another difference from `-chain` is that it is possible to set both `-datacarrier=0` and a positive `-datacarriersize` without so much as a warning (`-chain` immediately crashes when in conflict with another option).

Right. However, after this pull request, the same is true if `-datacarrier=0` is set in a config file. (See #15021). That is, it is possible that this is silently ignored for a user that has it intentionally set.

> Here are also a few examples of users confused about the two
...

💬 maflcko commented on pull request "doc: Suggest only necessary Qt packages for installation on OpenBSD":
(https://github.com/bitcoin/bitcoin/pull/29947#issuecomment-2074119271)
> Verified on a fresh install of OpenBSD 7.5 that these two packages are the minimum requirement to build the GUI:

Please update the version number in line 3 of the doc as well. I don't know if there is a difference between 7.4 and 7.5, but it can't hurt to be accurate here.

💬 maflcko commented on pull request "script: throw disabled err for op_ver and its variants":
(https://github.com/bitcoin/bitcoin/pull/28169#issuecomment-2074193707)
I think it is hard to follow this pull request. The motivation (pull request description) starts with "Edit:", and then has a list of reasons, where the second one is struck through. I don't think it makes it easy for reviewers, if they have to re-construct themselves why this pull request was created, and which parts of it were based on misunderstandings. The pull request description ends up in the merge commit, so it should be accurate and on point, not be used as a scratch-pad.

I personall
...

💬 maflcko commented on pull request "test: Assumeutxo: snapshots with less work should not be loaded":
(https://github.com/bitcoin/bitcoin/pull/29428#issuecomment-2074217332)
Please delete the hidden comment in the pull description, because this will end up in the merge commit.

💬 hanmz commented on pull request "Fix typos in description.md and wallet_util.py":
(https://github.com/bitcoin/bitcoin/pull/29938#issuecomment-2074218722)
> Also:
>
> ```
> test/functional/test_framework/wallet_util.py:168: lenghts ==> lengths
> ```

Good catch, I fixed.

💬 laanwj commented on pull request "security-check: test for `_FORTIFY_SOURCE` usage in release binaries":
(https://github.com/bitcoin/bitcoin/pull/27038#issuecomment-2074237343)
Concept ACK

💬 maflcko commented on pull request "Fix typos in description.md and wallet_util.py":
(https://github.com/bitcoin/bitcoin/pull/29938#issuecomment-2074243896)
Please squash your commits according to https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#squashing-commits

💬 laanwj commented on pull request "security-check: test for `_FORTIFY_SOURCE` usage in release binaries":
(https://github.com/bitcoin/bitcoin/pull/27038#discussion_r1577400140)
Might want to check `.imported` to make sure it's an imported symbol, just to be sure.

💬 laanwj commented on pull request "security-check: test for `_FORTIFY_SOURCE` usage in release binaries":
(https://github.com/bitcoin/bitcoin/pull/27038#issuecomment-2074248144)
i'd be okay with skipping the check for `bitcoin-util`: it's the least relevant binary for fortification (no network access, not even file format access). Could reconsider it later if it actually gains some useful functionality 😄

💬 maflcko commented on pull request "doc: Fix gen-manpages to check build options":
(https://github.com/bitcoin/bitcoin/pull/29457#discussion_r1577404978)
> This seems arbitrary and prone to breakage, giving the hardcoding of specific components.

I don't think there is a better alternative, is there?