🚀 fanquake merged a pull request: "psbt: add non-default sighash types to PSBTs and unify sighash type match checking"
(https://github.com/bitcoin/bitcoin/pull/31622)
(https://github.com/bitcoin/bitcoin/pull/31622)
💬 fanquake commented on pull request "random: Use modern Windows randomness functions":
(https://github.com/bitcoin/bitcoin/pull/32400#issuecomment-2897179032)
> But, it probably would still make sense to close https://github.com/bitcoin/bitcoin/issues/32391.
Yea, on our side, this should be enough to close #32391.
(https://github.com/bitcoin/bitcoin/pull/32400#issuecomment-2897179032)
> But, it probably would still make sense to close https://github.com/bitcoin/bitcoin/issues/32391.
Yea, on our side, this should be enough to close #32391.
💬 fanquake commented on pull request "build: Use clang-cl to build on Windows natively":
(https://github.com/bitcoin/bitcoin/pull/31507#issuecomment-2897204778)
Reading the description, I'm still not really clear on the goals here. It seems like this is just adding even more ways to compile for Windows, which we have to add more CI / and more work arounds for, but doesn't actually improve the Windows binaries we are shipping to end users in any way. This might make more sense if we were actually going to switch to shipping Windows binaries using Clang.
(https://github.com/bitcoin/bitcoin/pull/31507#issuecomment-2897204778)
Reading the description, I'm still not really clear on the goals here. It seems like this is just adding even more ways to compile for Windows, which we have to add more CI / and more work arounds for, but doesn't actually improve the Windows binaries we are shipping to end users in any way. This might make more sense if we were actually going to switch to shipping Windows binaries using Clang.
🚀 fanquake merged a pull request: "qt: drop unused watch-only functionality"
(https://github.com/bitcoin/bitcoin/pull/32459)
(https://github.com/bitcoin/bitcoin/pull/32459)
✅ maflcko closed an issue: "Dropping unused legacy wallet code"
(https://github.com/bitcoin-core/gui/issues/873)
(https://github.com/bitcoin-core/gui/issues/873)
💬 maflcko commented on issue "Dropping unused legacy wallet code":
(https://github.com/bitcoin-core/gui/issues/873#issuecomment-2897268477)
> In that case I can close this once [bitcoin/bitcoin#32459](https://github.com/bitcoin/bitcoin/pull/32459) is merged, unless there's another issue found by then.
(https://github.com/bitcoin-core/gui/issues/873#issuecomment-2897268477)
> In that case I can close this once [bitcoin/bitcoin#32459](https://github.com/bitcoin/bitcoin/pull/32459) is merged, unless there's another issue found by then.
🤔 janb84 reviewed a pull request: "qa: feature_framework_startup_failures.py fixes & improvements (#30660 follow-up)"
(https://github.com/bitcoin/bitcoin/pull/32509#pullrequestreview-2857005161)
LGTM ACK https://github.com/bitcoin/bitcoin/commit/bf950c4544d3a8478b46faf0b93c0dc647274c9b
- code review ✅
- build and tested ✅
Improved code readability and addressed open issues from #30660
(https://github.com/bitcoin/bitcoin/pull/32509#pullrequestreview-2857005161)
LGTM ACK https://github.com/bitcoin/bitcoin/commit/bf950c4544d3a8478b46faf0b93c0dc647274c9b
- code review ✅
- build and tested ✅
Improved code readability and addressed open issues from #30660
🤔 janb84 reviewed a pull request: "ci: Avoid && dropping errors"
(https://github.com/bitcoin/bitcoin/pull/32573#pullrequestreview-2857038264)
Code review ACK https://github.com/bitcoin/bitcoin/commit/fab97f583f119f43da352774479dd78e39729632
(https://github.com/bitcoin/bitcoin/pull/32573#pullrequestreview-2857038264)
Code review ACK https://github.com/bitcoin/bitcoin/commit/fab97f583f119f43da352774479dd78e39729632
💬 romanz commented on pull request "index: store per-block transaction locations for efficient lookups":
(https://github.com/bitcoin/bitcoin/pull/32541#issuecomment-2897330791)
I have used `ApacheBench 2.3` for benchmarking REST API, and the following Rust client for `getrawtransaction` RPC:
# fetching using the new index
```
$ ab -k -c 1 -n 100000 http://localhost:8332/rest/txfromblock/0000000000000000000083a0cff38278aae196d6d923a7e8ee7e5a0e371226fe-42.bin
Document Path: /rest/txfromblock/0000000000000000000083a0cff38278aae196d6d923a7e8ee7e5a0e371226fe-42.bin
Document Length: 301 bytes
Concurrency Level: 1
Time taken for tests: 13.7
...
(https://github.com/bitcoin/bitcoin/pull/32541#issuecomment-2897330791)
I have used `ApacheBench 2.3` for benchmarking REST API, and the following Rust client for `getrawtransaction` RPC:
# fetching using the new index
```
$ ab -k -c 1 -n 100000 http://localhost:8332/rest/txfromblock/0000000000000000000083a0cff38278aae196d6d923a7e8ee7e5a0e371226fe-42.bin
Document Path: /rest/txfromblock/0000000000000000000083a0cff38278aae196d6d923a7e8ee7e5a0e371226fe-42.bin
Document Length: 301 bytes
Concurrency Level: 1
Time taken for tests: 13.7
...
👍 hebasto approved a pull request: "ci: Avoid && dropping errors"
(https://github.com/bitcoin/bitcoin/pull/32573#pullrequestreview-2857077463)
ACK fab97f583f119f43da352774479dd78e39729632.
Could this approach be forced by a linter?
(https://github.com/bitcoin/bitcoin/pull/32573#pullrequestreview-2857077463)
ACK fab97f583f119f43da352774479dd78e39729632.
Could this approach be forced by a linter?
👍 Sjors approved a pull request: "policy: make pathological transactions packed with legacy sigops non-standard"
(https://github.com/bitcoin/bitcoin/pull/32521#pullrequestreview-2857047398)
ACK 29e8fe71440a7e27ee89527a49753be615f205c7
(https://github.com/bitcoin/bitcoin/pull/32521#pullrequestreview-2857047398)
ACK 29e8fe71440a7e27ee89527a49753be615f205c7
💬 Sjors commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2099855247)
29e8fe71440a7e27ee89527a49753be615f205c7: note to other reviewers, yes `generate` checks for errors, as you can see if you duplicate `nonstd_tx.serialize().hex()`
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2099855247)
29e8fe71440a7e27ee89527a49753be615f205c7: note to other reviewers, yes `generate` checks for errors, as you can see if you duplicate `nonstd_tx.serialize().hex()`
💬 Sjors commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2099858669)
29e8fe71440a7e27ee89527a49753be615f205c7: `MAX_TX_LEGACY_SIGOPS` would be consistent with the c++ code
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2099858669)
29e8fe71440a7e27ee89527a49753be615f205c7: `MAX_TX_LEGACY_SIGOPS` would be consistent with the c++ code
💬 Sjors commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2099863081)
a139a538c298621674ba622e63971704cbb7ceff: what's this magic `424242` value?
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2099863081)
a139a538c298621674ba622e63971704cbb7ceff: what's this magic `424242` value?
💬 Sjors commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2099872077)
a139a538c298621674ba622e63971704cbb7ceff: could insert a segwit spend to show that it doesn't count.
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2099872077)
a139a538c298621674ba622e63971704cbb7ceff: could insert a segwit spend to show that it doesn't count.
💬 Sjors commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2099865784)
a139a538c298621674ba622e63971704cbb7ceff: it seems more clear if you define it as an integer instead of auto.
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2099865784)
a139a538c298621674ba622e63971704cbb7ceff: it seems more clear if you define it as an integer instead of auto.
💬 Sjors commented on issue "Dropping unused legacy wallet code":
(https://github.com/bitcoin-core/gui/issues/873#issuecomment-2897367471)
@achow101 also checked that there doesn't appear to be more lingering stuff: https://github.com/bitcoin/bitcoin/pull/32459#issuecomment-2887600076
(https://github.com/bitcoin-core/gui/issues/873#issuecomment-2897367471)
@achow101 also checked that there doesn't appear to be more lingering stuff: https://github.com/bitcoin/bitcoin/pull/32459#issuecomment-2887600076
⚠️ fanquake opened an issue: "test: `tool_wallet.py` references no-longer used CI"
(https://github.com/bitcoin/bitcoin/issues/32576)
We no longer use Appveyor, so any code that existed to support it, should be dropped:
https://github.com/bitcoin/bitcoin/blob/54e406a3055ad0454a576f14b51def34ab8e9209/test/functional/tool_wallet.py#L160
https://github.com/bitcoin/bitcoin/blob/54e406a3055ad0454a576f14b51def34ab8e9209/test/functional/tool_wallet.py#L171
(https://github.com/bitcoin/bitcoin/issues/32576)
We no longer use Appveyor, so any code that existed to support it, should be dropped:
https://github.com/bitcoin/bitcoin/blob/54e406a3055ad0454a576f14b51def34ab8e9209/test/functional/tool_wallet.py#L160
https://github.com/bitcoin/bitcoin/blob/54e406a3055ad0454a576f14b51def34ab8e9209/test/functional/tool_wallet.py#L171
💬 fanquake commented on pull request "build: remove need to test for endianness":
(https://github.com/bitcoin/bitcoin/pull/29852#issuecomment-2897381770)
I'll come back to this once we land the changes upstream.
(https://github.com/bitcoin/bitcoin/pull/29852#issuecomment-2897381770)
I'll come back to this once we land the changes upstream.
✅ fanquake closed a pull request: "build: remove need to test for endianness"
(https://github.com/bitcoin/bitcoin/pull/29852)
(https://github.com/bitcoin/bitcoin/pull/29852)