💬 achow101 commented on pull request "miniscript: Use `ToIntegral` instead of `ParseInt64`":
(https://github.com/bitcoin/bitcoin/pull/30577#issuecomment-2272366647)
ACK 6714276d72244c2e2dbe9617f1341ba7fc06c0cc
(https://github.com/bitcoin/bitcoin/pull/30577#issuecomment-2272366647)
ACK 6714276d72244c2e2dbe9617f1341ba7fc06c0cc
🚀 achow101 merged a pull request: "miniscript: Use `ToIntegral` instead of `ParseInt64`"
(https://github.com/bitcoin/bitcoin/pull/30577)
(https://github.com/bitcoin/bitcoin/pull/30577)
💬 achow101 commented on pull request "descriptors: Be able to specify change and receiving in a single descriptor string":
(https://github.com/bitcoin/bitcoin/pull/22838#discussion_r1706262457)
Done, since I'm making changes anyways.
(https://github.com/bitcoin/bitcoin/pull/22838#discussion_r1706262457)
Done, since I'm making changes anyways.
💬 achow101 commented on pull request "descriptors: Be able to specify change and receiving in a single descriptor string":
(https://github.com/bitcoin/bitcoin/pull/22838#discussion_r1706262514)
Done
(https://github.com/bitcoin/bitcoin/pull/22838#discussion_r1706262514)
Done
💬 achow101 commented on pull request "descriptors: Be able to specify change and receiving in a single descriptor string":
(https://github.com/bitcoin/bitcoin/pull/22838#discussion_r1706262593)
Done
(https://github.com/bitcoin/bitcoin/pull/22838#discussion_r1706262593)
Done
💬 achow101 commented on pull request "descriptors: Be able to specify change and receiving in a single descriptor string":
(https://github.com/bitcoin/bitcoin/pull/22838#discussion_r1706262619)
Done
(https://github.com/bitcoin/bitcoin/pull/22838#discussion_r1706262619)
Done
💬 tdb3 commented on pull request "doc, rpc : `#30275` followups":
(https://github.com/bitcoin/bitcoin/pull/30525#discussion_r1704667895)
nitty nit: If this file ends up being changed again, could remove the extra space between the first `%s` and the newline.
(https://github.com/bitcoin/bitcoin/pull/30525#discussion_r1704667895)
nitty nit: If this file ends up being changed again, could remove the extra space between the first `%s` and the newline.
👍 tdb3 approved a pull request: "doc, rpc : `#30275` followups"
(https://github.com/bitcoin/bitcoin/pull/30525#pullrequestreview-2219921393)
re ACK fa2f26960ee084971ab09959b213a9b8104482e5
(https://github.com/bitcoin/bitcoin/pull/30525#pullrequestreview-2219921393)
re ACK fa2f26960ee084971ab09959b213a9b8104482e5
🤔 tdb3 reviewed a pull request: "docs: doc update for mempoolfullrbf default + log deprecation"
(https://github.com/bitcoin/bitcoin/pull/30594#pullrequestreview-2222478599)
Approach ACK
(https://github.com/bitcoin/bitcoin/pull/30594#pullrequestreview-2222478599)
Approach ACK
💬 tdb3 commented on pull request "docs: doc update for mempoolfullrbf default + log deprecation":
(https://github.com/bitcoin/bitcoin/pull/30594#discussion_r1706274236)
Maybe we can use `LogInfo` here?
(https://github.com/bitcoin/bitcoin/pull/30594#discussion_r1706274236)
Maybe we can use `LogInfo` here?
🚀 ryanofsky merged a pull request: "crypto, refactor: add new KeyPair class"
(https://github.com/bitcoin/bitcoin/pull/30051)
(https://github.com/bitcoin/bitcoin/pull/30051)
💬 achow101 commented on pull request "doc, rpc : `#30275` followups":
(https://github.com/bitcoin/bitcoin/pull/30525#issuecomment-2272523519)
ACK fa2f26960ee084971ab09959b213a9b8104482e5
(https://github.com/bitcoin/bitcoin/pull/30525#issuecomment-2272523519)
ACK fa2f26960ee084971ab09959b213a9b8104482e5
💬 ismaelsadeeq commented on pull request "doc, rpc : `#30275` followups":
(https://github.com/bitcoin/bitcoin/pull/30525#discussion_r1706382050)
Thank you.
I will address this and @willcl-ark nit if there is need to retouch.
(https://github.com/bitcoin/bitcoin/pull/30525#discussion_r1706382050)
Thank you.
I will address this and @willcl-ark nit if there is need to retouch.
🚀 achow101 merged a pull request: "doc, rpc : `#30275` followups"
(https://github.com/bitcoin/bitcoin/pull/30525)
(https://github.com/bitcoin/bitcoin/pull/30525)
💬 maflcko commented on pull request "test: Use Decimal for fee calculations in feature_fee_estimation.py":
(https://github.com/bitcoin/bitcoin/pull/29566#discussion_r1706456465)
Yes, sounds good to leave the functions completely untouched in this pull. When internally `double` is changed to something else, the Python code can also use something other than `float` (which has double precision). Thanks for providing context.
(https://github.com/bitcoin/bitcoin/pull/29566#discussion_r1706456465)
Yes, sounds good to leave the functions completely untouched in this pull. When internally `double` is changed to something else, the Python code can also use something other than `float` (which has double precision). Thanks for providing context.
💬 petertodd commented on pull request "Remove mempoolfullrbf":
(https://github.com/bitcoin/bitcoin/pull/30592#issuecomment-2272824670)
@ariard First of all, with ~100% of hash power mining full-rbf, `mempoolfullrbf=0` clearly provides no security benefit.
Users may have their own reasons to want a record of the first-seen transaction. But there's no reason why we in Core need to support such a niche use-case. It would make much more sense for users who need that functionality to get it directly, eg with some kind of "incoming transaction feed" hook.
For the use-case of zeroconf channels, I don't see how `mempoolfullrbf=0`
...
(https://github.com/bitcoin/bitcoin/pull/30592#issuecomment-2272824670)
@ariard First of all, with ~100% of hash power mining full-rbf, `mempoolfullrbf=0` clearly provides no security benefit.
Users may have their own reasons to want a record of the first-seen transaction. But there's no reason why we in Core need to support such a niche use-case. It would make much more sense for users who need that functionality to get it directly, eg with some kind of "incoming transaction feed" hook.
For the use-case of zeroconf channels, I don't see how `mempoolfullrbf=0`
...
💬 TheCharlatan commented on pull request "assumeutxo: Drop block height from metadata":
(https://github.com/bitcoin/bitcoin/pull/30598#issuecomment-2272838226)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/30598#issuecomment-2272838226)
Concept ACK
💬 paplorinc commented on pull request "crypto, refactor: add new KeyPair class":
(https://github.com/bitcoin/bitcoin/pull/30051#discussion_r1706546530)
This was the result of a change that was reverted since, where it was important to test the exact before/after structure.
(https://github.com/bitcoin/bitcoin/pull/30051#discussion_r1706546530)
This was the result of a change that was reverted since, where it was important to test the exact before/after structure.
💬 maflcko commented on pull request "Halt processing of unrequested transactions v2":
(https://github.com/bitcoin/bitcoin/pull/30572#issuecomment-2272848626)
> I'm particularly interested if any protocol v2 nodes engage in it, since I think that's a viable way to immediately enforce without breaking anything.
Just for context, there is an idea for a "BIP324 proxy" for legacy light clients: https://delvingbitcoin.org/t/bip324-proxy-easy-integration-of-v2-transport-protocol-for-light-clients-poc/678 . However, it is experimental, only works for outbound connections, requires the light client to be patched right now (and may have other design issues)
...
(https://github.com/bitcoin/bitcoin/pull/30572#issuecomment-2272848626)
> I'm particularly interested if any protocol v2 nodes engage in it, since I think that's a viable way to immediately enforce without breaking anything.
Just for context, there is an idea for a "BIP324 proxy" for legacy light clients: https://delvingbitcoin.org/t/bip324-proxy-easy-integration-of-v2-transport-protocol-for-light-clients-poc/678 . However, it is experimental, only works for outbound connections, requires the light client to be patched right now (and may have other design issues)
...
💬 paplorinc commented on pull request "crypto, refactor: add new KeyPair class":
(https://github.com/bitcoin/bitcoin/pull/30051#discussion_r1706563939)
We've had something like that before: https://github.com/bitcoin/bitcoin/pull/30051#discussion_r1681077911
But it was ambiguous, as to whether it can fail or not during execution as we can see here:
https://github.com/hebasto/bitcoin/blob/master/src/pubkey.cpp#L345-L350 or https://github.com/hebasto/bitcoin/blob/master/src/secp256k1/src/secp256k1.c#L685
But @josibake argued that they cannot fail at that point - if they do it's a bug, but we didn't want to ignore the return values either.
...
(https://github.com/bitcoin/bitcoin/pull/30051#discussion_r1706563939)
We've had something like that before: https://github.com/bitcoin/bitcoin/pull/30051#discussion_r1681077911
But it was ambiguous, as to whether it can fail or not during execution as we can see here:
https://github.com/hebasto/bitcoin/blob/master/src/pubkey.cpp#L345-L350 or https://github.com/hebasto/bitcoin/blob/master/src/secp256k1/src/secp256k1.c#L685
But @josibake argued that they cannot fail at that point - if they do it's a bug, but we didn't want to ignore the return values either.
...