💬 maflcko commented on pull request "bitcoin-cli: Add -ipcconnect option":
(https://github.com/bitcoin/bitcoin/pull/32297#issuecomment-2818291536)
Looks like those are real typos that should be fixed:

> ### LLM Linter (✨ experimental)
>
> Possible typos and grammar issues:
>
> The following typos/grammar issues were found in added lines:
>
> 1. Extra “unix’” in the –ipcconnect help text
> Replace
> “…node.sock unix’ but fall back to TCP…”
> with
> “…node.sock’ but fall back to TCP…”
>
> 2. Double negative in JSONErrorReply comment
> Replace
> “…when a request cannot not
...

💬 maflcko commented on pull request "bench: close wallets after migration":
(https://github.com/bitcoin/bitcoin/pull/32309#discussion_r2052293562)
q in e0c95cc123351681ab4326adb746debba33129f0: What are the exact steps to reproduce this "dangling-pointer"? Also, what is the explanation where it is destructed early and where it is used after free?

👍 rkrux approved a pull request: "descriptor: Move filling of keys from `DescriptorImpl::MakeScripts` to `PubkeyProvider::GetPubKey`"
(https://github.com/bitcoin/bitcoin/pull/31243#pullrequestreview-2781144870)
ACK acee5c5

> Was that an ACK an you just forgot the word or did you just want to say you reviewed? :)

I wanted to convey I code reviewed & almost ack. The changes made sense to me intuitively but there was one thing that I had not completely reasoned about, so was hesitant in acking earlier: https://github.com/bitcoin/bitcoin/pull/31243#discussion_r2047087551

💬 rkrux commented on pull request "descriptor: Move filling of keys from `DescriptorImpl::MakeScripts` to `PubkeyProvider::GetPubKey`":
(https://github.com/bitcoin/bitcoin/pull/31243#discussion_r2052327089)
I see, they were being added for `pkh`, `wpkh`, `combo`, `tr`, `miniscript` ones in `MakeScript` but not for `multisig` & the like. I debugged this flow in on master while importing a `multisig` descriptor and found it quite weird that the origins of all the `pubkeys` involved in the multisig were being added in the `out.origins` inside `ExpandHelper` but not all the `pubkeys` were added in `out.pubkeys`!
Also, `pubkeys` being set inside `MakeScript` but `origins` being set in `ExpandHelper` ma
...

💬 rkrux commented on pull request "descriptor: Move filling of keys from `DescriptorImpl::MakeScripts` to `PubkeyProvider::GetPubKey`":
(https://github.com/bitcoin/bitcoin/pull/31243#discussion_r2052331986)
Probably in some follow up: The documentation of the `MakeScripts` function in `DescriptorImpl` class can be updated to get rid of the following line because the origin info is added in the `GetPubKey` functions now and was added in the `ExpandHelper` function previously.

```
The origin info of the provided pubkeys is automatically added.
```

⚠️ Aria-saeid opened an issue: "Double Spending and Its Impact on Digital Security"
(https://github.com/bitcoin/bitcoin/issues/32316)
### Is there an existing issue for this?

- [x] I have searched the existing issues

### Current behaviour

"Tell us what’s wrong" (مشکل چیست؟): 🔹 Double spending is a security threat in digital currencies where a unit of Bitcoin is spent more than once. To prevent this issue, it is proposed to convert even-numbered values to binary, improving transaction security and blockchain integrity.Currently, double spending can occur in certain digital currency transactions, potentially compromising sec
...

✅ willcl-ark closed an issue: "Double Spending and Its Impact on Digital Security"
(https://github.com/bitcoin/bitcoin/issues/32316)

💬 TheCharlatan commented on pull request "test: test MAX_SCRIPT_SIZE for block validity":
(https://github.com/bitcoin/bitcoin/pull/32304#discussion_r2052325094)
I think it would be good to mention that this split is done for pushdata accounting and to use `MAX_SCRIPT_ELEMENT_SIZE` instead of 520. Maybe something like
```
# (OP_PUSHDATA2 + 2 byte length + b'\x01' * 520) * 19 + (1 byte length + b'\x01' * 62)
```

💬 darosior commented on pull request "test: adds coverage to src/validation for invalid tx coinbase":
(https://github.com/bitcoin/bitcoin/pull/32253#issuecomment-2818354053)
I don't think there is any use in keeping this open? The test is strictly redundant, as Marco demonstrated it's already covered both in unit and functional tests.

💬 kevkevinpal commented on pull request "test: adds coverage to src/validation for invalid tx coinbase":
(https://github.com/bitcoin/bitcoin/pull/32253#issuecomment-2818358588)
> I don't think there is any use in keeping this open? The test is strictly redundant, as Marco demonstrated it's already covered both in unit and functional tests.

Sounds I can go ahead and close it

✅ kevkevinpal closed a pull request: "test: adds coverage to src/validation for invalid tx coinbase"
(https://github.com/bitcoin/bitcoin/pull/32253)

📝 TheCharlatan opened a pull request: "kernel: Seperate UTXO set access from validation functions"
(https://github.com/bitcoin/bitcoin/pull/32317)
The current code makes it hard to call functions for checking the consensus-correctness of a block without having a full UTXO set at hand. This makes implementing features such as Utreexo and non-assumevalid swiftsync, where maintaining a full UTXO set defeats their purpose, through the kernel API difficult. Checks like the coinbase subsidy, or block sigops are only available through `ConnectBlock`, which requires a complete coins cache.

Solve this by moving accessing coins and the responsibi
...

💬 pinheadmz commented on pull request "docs: remove passing CI section in guidelines for PR merging as it's common sense":
(https://github.com/bitcoin/bitcoin/pull/32006#issuecomment-2818394106)
NACK, it may not be common sense for new contributors. Also the tip about adding passing tests before code changes in commit history is useful. I'm not gonna push back if others really want to merge this change, it just doesn't seem useful.

💬 EthanHeilman commented on pull request "tests: improves tapscript unit tests":
(https://github.com/bitcoin/bitcoin/pull/31640#issuecomment-2818410640)
Any reason this hasn't been merged despite 2 approvals?

💬 instagibbs commented on pull request "test: test MAX_SCRIPT_SIZE for block validity":
(https://github.com/bitcoin/bitcoin/pull/32304#issuecomment-2818420304)
@Sjors It doesn't apply because the first clause of the `IsUnspendable` check is if it starts with an OP_RETURN.

💬 andrewtoth commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2052396373)
nit: could be `const` method.

💬 instagibbs commented on pull request "test: test MAX_SCRIPT_SIZE for block validity":
(https://github.com/bitcoin/bitcoin/pull/32304#discussion_r2052398185)
Pushed a change to use `MAX_SCRIPT_ELEMENT_SIZE`, but I feel that the code with asserts is already a good explanation?

👍 rkrux approved a pull request: "test: add test for decoding PSBT with MuSig2 PSBT key types (BIP 373)"
(https://github.com/bitcoin/bitcoin/pull/32305#pullrequestreview-2781251768)
tACK 23ee07dd0f81930f46d2f4910cc92cb0eab61cc3

I believe this is a value add as it sets the base for the MuSig2 functional testing. I left few minor suggestions, I would be quick to re-ack if they are implemented.

💬 rkrux commented on pull request "test: add test for decoding PSBT with MuSig2 PSBT key types (BIP 373)":
(https://github.com/bitcoin/bitcoin/pull/32305#discussion_r2052391023)
Since this is a new check that has been added & the current use case is of a list of only bytes (in participant pubkeys), maybe we can assert this on all instead of any to keep the checking strict to start with that can be eased in the future if needed?

💬 rkrux commented on pull request "test: add test for decoding PSBT with MuSig2 PSBT key types (BIP 373)":
(https://github.com/bitcoin/bitcoin/pull/32305#discussion_r2052391602)
Are asserts like on this line really needed? If the decoded psbt is malformed then the test would fail in the following assert anyway.