👍 andrewtoth approved a pull request: "rpc, rest: Improve block rpc error handling, check header before attempting to read block data."
(https://github.com/bitcoin/bitcoin/pull/30410#pullrequestreview-2222373701)
utACK 02f6a42be3dfc57998304801c0d909bfa96745e4
(https://github.com/bitcoin/bitcoin/pull/30410#pullrequestreview-2222373701)
utACK 02f6a42be3dfc57998304801c0d909bfa96745e4
💬 mzumsande commented on pull request "Assumeutxo: Sanitize block height in metadata":
(https://github.com/bitcoin/bitcoin/pull/30516#issuecomment-2272318013)
>The height in the metadata clearly tells us which height the hash is expected at. Not having the same hash at that height clearly tells us that the node is on a different chain than was expected by the metadata. That is better than just saying "we don't know this hash for whatever reason".
Ah, ok, my point is that the height is implied by the block hash, together with the block index db of the node. If the block tree db doesn't contain a header with that block hash, then we abort anyway. If
...
(https://github.com/bitcoin/bitcoin/pull/30516#issuecomment-2272318013)
>The height in the metadata clearly tells us which height the hash is expected at. Not having the same hash at that height clearly tells us that the node is on a different chain than was expected by the metadata. That is better than just saying "we don't know this hash for whatever reason".
Ah, ok, my point is that the height is implied by the block hash, together with the block index db of the node. If the block tree db doesn't contain a header with that block hash, then we abort anyway. If
...
💬 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.