💬 achow101 commented on pull request "doc: Extend developer-notes with file-name-only debugging fix":
(https://github.com/bitcoin/bitcoin/pull/30670#issuecomment-2455681847)
ACK 1b0b9b4c7873ff0c6323de0c7f439466eed06049

🚀 achow101 merged a pull request: "doc: Extend developer-notes with file-name-only debugging fix"
(https://github.com/bitcoin/bitcoin/pull/30670)

💬 achow101 commented on pull request "wallet: add coin selection parameter `add_excess_to_recipient_position` for changeless txs with excess that would be added to fees":
(https://github.com/bitcoin/bitcoin/pull/30080#issuecomment-2455717534)
It's not entirely clear to me why BnB should care about whether the excess is going to the recipient or not. I don't think that should change the algorithm at all, so the changes to `SelectCoinsBnB` seem unnecessary.

Additionally, this implementation modifies the target, and I think that's rather dangerous to do. I think the obvious way to implement this feature is to just detect when the fee paid is greater than the fee needed, and send the excess to the recipient specified, just as we alrea
...

💬 ismaelsadeeq commented on pull request "BlockAssembler: return selected packages vsize and feerate":
(https://github.com/bitcoin/bitcoin/pull/30391#discussion_r1828414346)
@glozow, is the idea of making `CFeeRate` a wrapper around `FeeFrac` better, as suggested https://github.com/bitcoin/bitcoin/pull/30535#issuecomment-2254263266? If so, then no changes are needed, and we can proceed with this approach while pursuing that direction and mark this comment as resolved.
Otherwise, I'm happy to change this to `FeeFrac` as you suggested if it's a blocking comment.

⚠️ 1440000bytes opened an issue: "Authentication with KYC"
(https://github.com/bitcoin/bitcoin/issues/31217)
### Please describe the feature you'd like to see added.

I want to see nodes doing KYC like https://github.com/cashubtc/nuts/pull/106 (account model)

### Is your feature related to a problem, if so please describe it.

Yes, the problem is everyone in the world thinks core has done nothing. But I think they have done a lot.

### Describe the solution you'd like

Close this issue.

### Describe any alternatives you've considered

_No response_

### Please leave any additional context

_No respon
...

⚠️ Sandra-Amina-Boss opened an issue: "Hm"
(https://github.com/bitcoin/bitcoin/issues/31218)
https://github.com/Axe-gift/lib-exchange-client/tree/master/javascript

✅ fanquake closed an issue: "Hm"
(https://github.com/bitcoin/bitcoin/issues/31218)

:lock: fanquake locked an issue: "Hm"
(https://github.com/bitcoin/bitcoin/issues/31218)

💬 sr-gi commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1828421634)
I'll squash in the next force push

✅ fanquake closed an issue: "Authentication with KYC"
(https://github.com/bitcoin/bitcoin/issues/31217)

💬 achow101 commented on pull request "fuzz: wallet: add target for spkm migration":
(https://github.com/bitcoin/bitcoin/pull/29694#discussion_r1828420886)
I think it would also be good to cover hd chain before hd chain split.

💬 achow101 commented on pull request "fuzz: wallet: add target for spkm migration":
(https://github.com/bitcoin/bitcoin/pull/29694#discussion_r1828423452)
It would be nice to have multisig scripts here as the handling of ismine for that is quite complicated and having fuzz testing of it would be great.

💬 achow101 commented on pull request "fuzz: wallet: add target for spkm migration":
(https://github.com/bitcoin/bitcoin/pull/29694#discussion_r1828421887)
Would it be possible for `script_count` to really be watchonly script count and just have it increment only when a watchonly script is added?

💬 achow101 commented on pull request "fuzz: wallet: add target for spkm migration":
(https://github.com/bitcoin/bitcoin/pull/29694#discussion_r1828422802)
If watchonly script count can be tracked, then this check should be able to be an exact check as `desc_spkms` should be `load_key_count` plus the number of spendable scripts.

💬 achow101 commented on pull request "wallet: fix unrelated parent conflict doesn't cause child tx to be marked as conflict":
(https://github.com/bitcoin/bitcoin/pull/29680#discussion_r1828448683)
In 7eba56accbd1b4ce3c09656c8d69b5aa437a706f "Handle double-spending of unrelated parents to wallet txs"

The replacement could still be sending to us, and in that case, it isn't unrelated.

💬 achow101 commented on pull request "wallet: fix unrelated parent conflict doesn't cause child tx to be marked as conflict":
(https://github.com/bitcoin/bitcoin/pull/29680#discussion_r1828472777)
In 7eba56accbd1b4ce3c09656c8d69b5aa437a706f "Handle double-spending of unrelated parents to wallet txs"

nit: whitespace.

Also, I think it would be better to clarify that the key is the txid of the replacing transaction which may be unrelated to the wallet, and the value is the replaced transaction which is related to this wallet.

💬 achow101 commented on pull request "wallet: fix unrelated parent conflict doesn't cause child tx to be marked as conflict":
(https://github.com/bitcoin/bitcoin/pull/29680#discussion_r1828455481)
In 7eba56accbd1b4ce3c09656c8d69b5aa437a706f "Handle double-spending of unrelated parents to wallet txs"

The extra scoping is not necessary, just name `it` something else.

🤔 ryanofsky reviewed a pull request: "tinyformat: Add compile-time checking for literal format strings"
(https://github.com/bitcoin/bitcoin/pull/31174#pullrequestreview-2414174938)
Updated e53829d3952c6ed275507a66e77636aad67d106b -> ecc5cb9a89c6b001df839675b23d8fc1f7ac69ba ([`pr/tcheck.3`](https://github.com/ryanofsky/bitcoin/commits/pr/tcheck.3) -> [`pr/tcheck.4`](https://github.com/ryanofsky/bitcoin/commits/pr/tcheck.4), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/tcheck.3..pr/tcheck.4)) with review suggestions.

re: https://github.com/bitcoin/bitcoin/pull/31174#pullrequestreview-2405166236

> Could document non-parity like so (unless you prefer I do it
...

💬 ryanofsky commented on pull request "tinyformat: Add compile-time checking for literal format strings":
(https://github.com/bitcoin/bitcoin/pull/31174#discussion_r1828424828)
re: https://github.com/bitcoin/bitcoin/pull/31174#discussion_r1822780992

> Should this be removed?

I think it's useful to document the format of the specifier, and it's still true that the remainder of the specifier (length and type) is not checked. Happy to update this if there's a specific suggestion, but I think it's still helpful and accurate so would not want to remove it.

💬 ryanofsky commented on pull request "tinyformat: Add compile-time checking for literal format strings":
(https://github.com/bitcoin/bitcoin/pull/31174#discussion_r1828424763)
re: https://github.com/bitcoin/bitcoin/pull/31174#discussion_r1822782509

> (One could expect that at least one precision-digit was required after '.' but it is not in tinyformat so this behavior is consistent).

That's good to know, I was just trying to make the parsing as simple as possible, but this syntax does seem to be commonly accepted (it works in python too) and I added test cases for it following your other suggestion