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

💬 maflcko commented on pull request "test: Add test case for spending bare multisig":
(https://github.com/bitcoin/bitcoin/pull/29120#issuecomment-2074251700)
Are you still working on this?

💬 laanwj commented on pull request "cleanse: switch to SecureZeroMemory for Windows cross-compile, check for usage":
(https://github.com/bitcoin/bitcoin/pull/26950#issuecomment-2074260637)
Concept ACK

💬 laanwj commented on pull request "cleanse: switch to SecureZeroMemory for Windows cross-compile, check for usage":
(https://github.com/bitcoin/bitcoin/pull/26950#discussion_r1577416299)
This checks that the function is there, not whether it is used, in the right places. But it may be enough as a start.

💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#issuecomment-2074267427)
`cc760207b8...ea1ca3715e`: adjust `feature_config_args.py` after forbidding `-walletbroadcast` when `-privatebroadcast` is enabled.

⚠️ KonradStaniec opened an issue: "Calling `walletprocesspsbt` to sign multisig containing `OP_GREATERTHANOREQUAL` op_code"
(https://github.com/bitcoin/bitcoin/issues/29949)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

I want to create signature by calling `walletprocesspsbt` endpoint to spend from multisig output. It is worth stating that this is taproot output with one leaf.

Wallet contains one of the key from multisig. Key was created by first calling `getnewaddress` and then calling `getaddressinfo` and retrieving public key.

Now the issue is that if multisig script is in format:
`<key1> OP_C
...

💬 laanwj commented on pull request "contrib: list other binaries in manpage output":
(https://github.com/bitcoin/bitcoin/pull/29585#discussion_r1577434913)
Could use `os.path.basename` it's slightly more readable and will always pick the last path item.

💬 laanwj commented on pull request "contrib: list other binaries in manpage output":
(https://github.com/bitcoin/bitcoin/pull/29585#issuecomment-2074293842)
Good idea, concept ACK.

👍 laanwj approved a pull request: "contrib: list other binaries in manpage output"
(https://github.com/bitcoin/bitcoin/pull/29585#pullrequestreview-2019127833)
Code review ACK 7c3ac598dd9a1f1a506c4931249ff6c9f1c949ba