💬 davidgumberg commented on pull request "descriptor: remove unreachable verification for `pkh`":
(https://github.com/bitcoin/bitcoin/pull/31555#issuecomment-2565909356)
crACK https://github.com/bitcoin/bitcoin/commit/366ae00b779acd59a61719422f0597acb17fb3e0
Checked that `ParseScript()` is never called with `ParseScriptContext::P2WPKH`.
(https://github.com/bitcoin/bitcoin/pull/31555#issuecomment-2565909356)
crACK https://github.com/bitcoin/bitcoin/commit/366ae00b779acd59a61719422f0597acb17fb3e0
Checked that `ParseScript()` is never called with `ParseScriptContext::P2WPKH`.
💬 achow101 commented on pull request "descriptor: Add proper Clone function to miniscript::Node":
(https://github.com/bitcoin/bitcoin/pull/30866#discussion_r1899805412)
Yes, done and deleted the copy constructor.
(https://github.com/bitcoin/bitcoin/pull/30866#discussion_r1899805412)
Yes, done and deleted the copy constructor.
💬 achow101 commented on pull request "descriptor: Add proper Clone function to miniscript::Node":
(https://github.com/bitcoin/bitcoin/pull/30866#issuecomment-2565936197)
> I don't think anything in the Bitcoin Core codebase actually needs the `std::shared_ptr` way of storing miniscript subnodes, and `std::unique_ptr` would suffice. The `std::shared_ptr`s were inherited from the miniscript codebase, where the shared_ptrs matter for the policy compiler, but I could understand that Bitcoin Core doesn't want that burden. Using `std::unique_ptr` instead would not leave any chance for shallow duplication.
Indeed, the `std:;shared_ptr` wasn't really needed and I've
...
(https://github.com/bitcoin/bitcoin/pull/30866#issuecomment-2565936197)
> I don't think anything in the Bitcoin Core codebase actually needs the `std::shared_ptr` way of storing miniscript subnodes, and `std::unique_ptr` would suffice. The `std::shared_ptr`s were inherited from the miniscript codebase, where the shared_ptrs matter for the policy compiler, but I could understand that Bitcoin Core doesn't want that burden. Using `std::unique_ptr` instead would not leave any chance for shallow duplication.
Indeed, the `std:;shared_ptr` wasn't really needed and I've
...
💬 achow101 commented on pull request "descriptor: remove unreachable verification for `pkh`":
(https://github.com/bitcoin/bitcoin/pull/31555#issuecomment-2565938488)
ACK 366ae00b779acd59a61719422f0597acb17fb3e0
(https://github.com/bitcoin/bitcoin/pull/31555#issuecomment-2565938488)
ACK 366ae00b779acd59a61719422f0597acb17fb3e0
💬 yancyribbens commented on pull request "Remove unused variable assignment":
(https://github.com/bitcoin/bitcoin/pull/31497#issuecomment-2565938730)
Thanks for the reviews.
(https://github.com/bitcoin/bitcoin/pull/31497#issuecomment-2565938730)
Thanks for the reviews.
🚀 achow101 merged a pull request: "descriptor: remove unreachable verification for `pkh`"
(https://github.com/bitcoin/bitcoin/pull/31555)
(https://github.com/bitcoin/bitcoin/pull/31555)
💬 ismaelsadeeq commented on pull request "RPC: Add reserve member function to `UniValue` and use it in `getblock` RPC":
(https://github.com/bitcoin/bitcoin/pull/31179#issuecomment-2565964265)
Thanks for taking a look @Eunovo @l0rinc
> could you commit the diff you’ve provided and include the benchmark results in the commit message?
I've added 8d4823b383be4f8cd0152176a4f67b447d29549 For the benchmarks and instead update the PR title with the bench results.
> I ran your benchmarks from https://github.com/bitcoin/bitcoin/pull/31179#issuecomment-2446067820, but I only see a 7% speedup in the TxVerbosity::SHOW_TXID bench - while the rest were exactly the same as before.
I did
...
(https://github.com/bitcoin/bitcoin/pull/31179#issuecomment-2565964265)
Thanks for taking a look @Eunovo @l0rinc
> could you commit the diff you’ve provided and include the benchmark results in the commit message?
I've added 8d4823b383be4f8cd0152176a4f67b447d29549 For the benchmarks and instead update the PR title with the bench results.
> I ran your benchmarks from https://github.com/bitcoin/bitcoin/pull/31179#issuecomment-2446067820, but I only see a 7% speedup in the TxVerbosity::SHOW_TXID bench - while the rest were exactly the same as before.
I did
...
💬 l0rinc commented on pull request "RPC: Add reserve member function to `UniValue` and use it in `getblock` RPC":
(https://github.com/bitcoin/bitcoin/pull/31179#issuecomment-2565981600)
I can implement my suggestions in separate PRs if you say the RPC speedups would be welcome
(https://github.com/bitcoin/bitcoin/pull/31179#issuecomment-2565981600)
I can implement my suggestions in separate PRs if you say the RPC speedups would be welcome
💬 tdb3 commented on pull request "rpc: add target to getmininginfo and introduce gettarget":
(https://github.com/bitcoin/bitcoin/pull/31583#discussion_r1899866083)
nit: Since `7fffff0...` is used both here and in `mining_basic.py`, might be worth de-duplicating? Maybe into `test_framework/util.py`?
(https://github.com/bitcoin/bitcoin/pull/31583#discussion_r1899866083)
nit: Since `7fffff0...` is used both here and in `mining_basic.py`, might be worth de-duplicating? Maybe into `test_framework/util.py`?
🤔 tdb3 reviewed a pull request: "rpc: add target to getmininginfo and introduce gettarget"
(https://github.com/bitcoin/bitcoin/pull/31583#pullrequestreview-2526096356)
Approach ACK
Welcome addition
Sanity checked on mainnet (at block 200,000)
```
$ build/src/bitcoin-cli gettarget
00000000000005db8b0000000000000000000000000000000000000000000000
$ build/src/bitcoin-cli getmininginfo
{
"blocks": 200000,
"difficulty": 2864140.507810974,
"target": "00000000000005db8b0000000000000000000000000000000000000000000000",
"networkhashps": 26411459642253.2,
"pooledtx": 0,
"chain": "main",
"warnings": [
"This is a pre-release test build -
...
(https://github.com/bitcoin/bitcoin/pull/31583#pullrequestreview-2526096356)
Approach ACK
Welcome addition
Sanity checked on mainnet (at block 200,000)
```
$ build/src/bitcoin-cli gettarget
00000000000005db8b0000000000000000000000000000000000000000000000
$ build/src/bitcoin-cli getmininginfo
{
"blocks": 200000,
"difficulty": 2864140.507810974,
"target": "00000000000005db8b0000000000000000000000000000000000000000000000",
"networkhashps": 26411459642253.2,
"pooledtx": 0,
"chain": "main",
"warnings": [
"This is a pre-release test build -
...
💬 starius commented on pull request "Extend signetchallenge to set target block spacing":
(https://github.com/bitcoin/bitcoin/pull/29365#issuecomment-2566090827)
Rebased the PR, including https://github.com/bitcoin/bitcoin/pull/31468 and resolving the conflict in `test/functional/feature_signet.py`.
(https://github.com/bitcoin/bitcoin/pull/29365#issuecomment-2566090827)
Rebased the PR, including https://github.com/bitcoin/bitcoin/pull/31468 and resolving the conflict in `test/functional/feature_signet.py`.
💬 starius commented on pull request "Extend signetchallenge to set target block spacing":
(https://github.com/bitcoin/bitcoin/pull/29365#discussion_r1899891585)
Thanks for the proposal! I reworked the change of feature_signet.py test. Instead of modifying the existing test case, I added a new one with the wrapped signet challenge.
(https://github.com/bitcoin/bitcoin/pull/29365#discussion_r1899891585)
Thanks for the proposal! I reworked the change of feature_signet.py test. Instead of modifying the existing test case, I added a new one with the wrapped signet challenge.
📝 starius opened a pull request: "txmempool: fix typos in comments"
(https://github.com/bitcoin/bitcoin/pull/31584)
Fixed typos identified by codespell lint in CI.
(https://github.com/bitcoin/bitcoin/pull/31584)
Fixed typos identified by codespell lint in CI.
🤔 Pitan1993 requested changes to a pull request: "scripted-diff: Type-safe settings retrieval"
(https://github.com/bitcoin/bitcoin/pull/31260#pullrequestreview-2526132864)
2024-12-14T00:04:24.9747967Z Cleaning up orphan processes
(https://github.com/bitcoin/bitcoin/pull/31260#pullrequestreview-2526132864)
2024-12-14T00:04:24.9747967Z Cleaning up orphan processes
💬 Pitan1993 commented on pull request "scripted-diff: Type-safe settings retrieval":
(https://github.com/bitcoin/bitcoin/pull/31260#issuecomment-2566096091)
e12edf7661384f593b5868b3f3374613773019d9
(https://github.com/bitcoin/bitcoin/pull/31260#issuecomment-2566096091)
e12edf7661384f593b5868b3f3374613773019d9
📝 mikeknack86 opened a pull request: "Create docker-image.yml"
(https://github.com/bitcoin/bitcoin/pull/31585)
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:
* Any test improvements or new tests that improv
...
(https://github.com/bitcoin/bitcoin/pull/31585)
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:
* Any test improvements or new tests that improv
...
👍1
🤔 mikeknack86 reviewed a pull request: "Create docker-image.yml"
(https://github.com/bitcoin/bitcoin/pull/31585#pullrequestreview-2526149918)
[](url)
(https://github.com/bitcoin/bitcoin/pull/31585#pullrequestreview-2526149918)
[](url)
✅ achow101 closed a pull request: "Create docker-image.yml"
(https://github.com/bitcoin/bitcoin/pull/31585)
(https://github.com/bitcoin/bitcoin/pull/31585)
📝 achow101 locked a pull request: "."
(https://github.com/bitcoin/bitcoin/pull/31585)
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:
* Any test improvements or new tests that improv
...
(https://github.com/bitcoin/bitcoin/pull/31585)
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:
* Any test improvements or new tests that improv
...
💬 davidgumberg commented on pull request "refactor: Remove redundant edge case in fee rate rounding logic":
(https://github.com/bitcoin/bitcoin/pull/31572#discussion_r1899948765)
Why the second `if` statement?
(https://github.com/bitcoin/bitcoin/pull/31572#discussion_r1899948765)
Why the second `if` statement?