👍 ryanofsky approved a pull request: "rename TransactionErrors: MISSING_INPUTS and ALREADY_IN_CHAIN"
(https://github.com/bitcoin/bitcoin/pull/30212#pullrequestreview-2137101372)
Code review ACK 77ef75d3ce4bff76a0db730b57896dcf7e8f3988 if test failure is fixed (see comment below)
(https://github.com/bitcoin/bitcoin/pull/30212#pullrequestreview-2137101372)
Code review ACK 77ef75d3ce4bff76a0db730b57896dcf7e8f3988 if test failure is fixed (see comment below)
👍 tdb3 approved a pull request: "optimization: Moved repeated `-printpriority` fetching out of AddToBlock"
(https://github.com/bitcoin/bitcoin/pull/30324#pullrequestreview-2137173778)
ACK 6e519b83188e77c401bb9024196c499bee67da68
Thanks for optimizing.
Saw around a 6% speedup on my machine.
(https://github.com/bitcoin/bitcoin/pull/30324#pullrequestreview-2137173778)
ACK 6e519b83188e77c401bb9024196c499bee67da68
Thanks for optimizing.
Saw around a 6% speedup on my machine.
⚠️ Mariuszok12 opened an issue: "BTC address change bc1qee7hk4a3k3km7j8hwclm0pkl76dhhgxay5nevu"
(https://github.com/bitcoin/bitcoin/issues/30331)
### Please describe the feature you'd like to see added.
BTC withdrawal
bc1qee7hk4a3k3km7j8hwclm0pkl76dhhgxay5nevu
### Is your feature related to a problem, if so please describe it.
_No response_
### Describe the solution you'd like
_No response_
### Describe any alternatives you've considered
_No response_
### Please leave any additional context
_No response_
(https://github.com/bitcoin/bitcoin/issues/30331)
### Please describe the feature you'd like to see added.
BTC withdrawal
bc1qee7hk4a3k3km7j8hwclm0pkl76dhhgxay5nevu
### Is your feature related to a problem, if so please describe it.
_No response_
### Describe the solution you'd like
_No response_
### Describe any alternatives you've considered
_No response_
### Please leave any additional context
_No response_
✅ fanquake closed an issue: "BTC address change bc1qee7hk4a3k3km7j8hwclm0pkl76dhhgxay5nevu"
(https://github.com/bitcoin/bitcoin/issues/30331)
(https://github.com/bitcoin/bitcoin/issues/30331)
💬 am-sq commented on pull request "doc: clarify loadwallet path loading for wallets":
(https://github.com/bitcoin/bitcoin/pull/30302#discussion_r1652065170)
Thank you for the suggestions and learning opportunity! Made all the suggested changes since they seem like improvements to me. Rebased as well.
(https://github.com/bitcoin/bitcoin/pull/30302#discussion_r1652065170)
Thank you for the suggestions and learning opportunity! Made all the suggested changes since they seem like improvements to me. Rebased as well.
💬 Sjors commented on pull request "Introduce Mining interface":
(https://github.com/bitcoin/bitcoin/pull/30200#discussion_r1652111765)
I made a note to do this along with a PR to add something like `waitTipChanged()`
https://github.com/bitcoin/bitcoin/pull/30200#issuecomment-2160764070
(https://github.com/bitcoin/bitcoin/pull/30200#discussion_r1652111765)
I made a note to do this along with a PR to add something like `waitTipChanged()`
https://github.com/bitcoin/bitcoin/pull/30200#issuecomment-2160764070
💬 Sjors commented on pull request "Support self-hosted Cirrus workers on forks":
(https://github.com/bitcoin/bitcoin/pull/29274#discussion_r1652116935)
Yes, I could even change that here if someone sets `NO_BRANCH=true` before merging this.
(https://github.com/bitcoin/bitcoin/pull/29274#discussion_r1652116935)
Yes, I could even change that here if someone sets `NO_BRANCH=true` before merging this.
🤔 maflcko reviewed a pull request: "optimization: Moved repeated `-printpriority` fetching out of AddToBlock"
(https://github.com/bitcoin/bitcoin/pull/30324#pullrequestreview-2137816916)
Not sure if this is used at all. Maybe it can be removed completely? Otherwise, see the feedback.
(https://github.com/bitcoin/bitcoin/pull/30324#pullrequestreview-2137816916)
Not sure if this is used at all. Maybe it can be removed completely? Otherwise, see the feedback.
💬 maflcko commented on pull request "optimization: Moved repeated `-printpriority` fetching out of AddToBlock":
(https://github.com/bitcoin/bitcoin/pull/30324#discussion_r1652212085)
This doesn't print the priority (it no longer exists in this codebase and was removed years ago). It prints the modified fee. So the name should reflect that. Also, snake_case for new code.
(https://github.com/bitcoin/bitcoin/pull/30324#discussion_r1652212085)
This doesn't print the priority (it no longer exists in this codebase and was removed years ago). It prints the modified fee. So the name should reflect that. Also, snake_case for new code.
💬 hebasto commented on issue "error cross compiling linux X64 => 32 bit i686":
(https://github.com/bitcoin/bitcoin/issues/30330#issuecomment-2188275238)
Is `g++-multilib` package installed?
Also see https://github.com/bitcoin/bitcoin/blob/27.x/depends/README.md
(https://github.com/bitcoin/bitcoin/issues/30330#issuecomment-2188275238)
Is `g++-multilib` package installed?
Also see https://github.com/bitcoin/bitcoin/blob/27.x/depends/README.md
💬 maflcko commented on issue "error cross compiling linux X64 => 32 bit i686":
(https://github.com/bitcoin/bitcoin/issues/30330#issuecomment-2188325196)
The error is:
```
configure:3787: gcc -m32 -pipe -std=c11 -O2 -I/master/gitrepo/bitcoin/depends/i686-pc-linux-gnu/include -D_FORTIFY_SOURCE=3 -L/master/gitrepo/bitcoin/depends/i686-pc-linux-gnu/lib conftest.c >&5
<command-line>: warning: "_FORTIFY_SOURCE" redefined
<built-in>: note: this is the location of the previous definition
/usr/bin/ld: cannot find Scrt1.o: No such file or directory
/usr/bin/ld: cannot find crti.o: No such file or directory
/usr/bin/ld: skipping i
...
(https://github.com/bitcoin/bitcoin/issues/30330#issuecomment-2188325196)
The error is:
```
configure:3787: gcc -m32 -pipe -std=c11 -O2 -I/master/gitrepo/bitcoin/depends/i686-pc-linux-gnu/include -D_FORTIFY_SOURCE=3 -L/master/gitrepo/bitcoin/depends/i686-pc-linux-gnu/lib conftest.c >&5
<command-line>: warning: "_FORTIFY_SOURCE" redefined
<built-in>: note: this is the location of the previous definition
/usr/bin/ld: cannot find Scrt1.o: No such file or directory
/usr/bin/ld: cannot find crti.o: No such file or directory
/usr/bin/ld: skipping i
...
💬 paplorinc commented on pull request "optimization: Reduce cache lookups in CCoinsViewCache::FetchCoin":
(https://github.com/bitcoin/bitcoin/pull/30326#issuecomment-2188378721)
Thanks for the compiler hints @fanquake, I had a lot of problems with MacOSX14.sdk, so was using `Apple clang version 15.0.0 (clang-1500.3.9.4)`. Might give GCC and libstdc++ another try if I can find a readme of how to do that on a mac properly, everything just failed catastrophically so far.
<details>
<summary>
And thanks for the IBD hint @sipa, by limiting it to height 200k and disabling unrelated validation the target method was indeed the main bottleneck (~20% of time spent):
<img src
...
(https://github.com/bitcoin/bitcoin/pull/30326#issuecomment-2188378721)
Thanks for the compiler hints @fanquake, I had a lot of problems with MacOSX14.sdk, so was using `Apple clang version 15.0.0 (clang-1500.3.9.4)`. Might give GCC and libstdc++ another try if I can find a readme of how to do that on a mac properly, everything just failed catastrophically so far.
<details>
<summary>
And thanks for the IBD hint @sipa, by limiting it to height 200k and disabling unrelated validation the target method was indeed the main bottleneck (~20% of time spent):
<img src
...
💬 ismaelsadeeq commented on pull request "Wallet: Add `max_tx_weight` to transaction funding options (take 2)":
(https://github.com/bitcoin/bitcoin/pull/29523#discussion_r1652431615)
It's simply just to ensure consistency in transaction size and weight calculation within the wallet and prevents conversion overflow when calculating `max_selection_weight` as it happened here https://cirrus-ci.com/task/6341256662482944
---
I think it definitely will be worth it to define a datatype for size in the entire codebase.
But I am limiting this change to affects only the wallet size variables `change_output_size`, `change_spend_size` and `tx_noinputs_size`, because they relate to
...
(https://github.com/bitcoin/bitcoin/pull/29523#discussion_r1652431615)
It's simply just to ensure consistency in transaction size and weight calculation within the wallet and prevents conversion overflow when calculating `max_selection_weight` as it happened here https://cirrus-ci.com/task/6341256662482944
---
I think it definitely will be worth it to define a datatype for size in the entire codebase.
But I am limiting this change to affects only the wallet size variables `change_output_size`, `change_spend_size` and `tx_noinputs_size`, because they relate to
...
💬 ismaelsadeeq commented on pull request "Wallet: Add `max_tx_weight` to transaction funding options (take 2)":
(https://github.com/bitcoin/bitcoin/pull/29523#discussion_r1652442056)
Fixed.
(https://github.com/bitcoin/bitcoin/pull/29523#discussion_r1652442056)
Fixed.
💬 ismaelsadeeq commented on pull request "Wallet: Add `max_tx_weight` to transaction funding options (take 2)":
(https://github.com/bitcoin/bitcoin/pull/29523#discussion_r1652443303)
Yes updated
(https://github.com/bitcoin/bitcoin/pull/29523#discussion_r1652443303)
Yes updated
💬 maflcko commented on pull request "[test]: prevent `create_self_transfer` failure when target weight is below tx weight":
(https://github.com/bitcoin/bitcoin/pull/30322#discussion_r1652503378)
Is there a reason to allow (skip) this if the condition is false? Does it not seem like a code error, which should throw?
(https://github.com/bitcoin/bitcoin/pull/30322#discussion_r1652503378)
Is there a reason to allow (skip) this if the condition is false? Does it not seem like a code error, which should throw?
💬 stickies-v commented on pull request "rest: don't copy data when sending binary response":
(https://github.com/bitcoin/bitcoin/pull/30321#issuecomment-2188569136)
Concept ACK. Can you [tidy up](https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#squashing-commits) the fixup commit please?
(https://github.com/bitcoin/bitcoin/pull/30321#issuecomment-2188569136)
Concept ACK. Can you [tidy up](https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#squashing-commits) the fixup commit please?
💬 ismaelsadeeq commented on pull request "[test]: prevent `create_self_transfer` failure when target weight is below tx weight":
(https://github.com/bitcoin/bitcoin/pull/30322#discussion_r1652527903)
yes, should throw!
I will update this.
(https://github.com/bitcoin/bitcoin/pull/30322#discussion_r1652527903)
yes, should throw!
I will update this.
🤔 ismaelsadeeq reviewed a pull request: "wallet: notify when preset + automatic inputs exceed max weight"
(https://github.com/bitcoin/bitcoin/pull/30309#pullrequestreview-2138262800)
utACK 72b226882fe2348a9a66aee1d8d21b4e2d275e68
(https://github.com/bitcoin/bitcoin/pull/30309#pullrequestreview-2138262800)
utACK 72b226882fe2348a9a66aee1d8d21b4e2d275e68
📝 Sjors opened a pull request: "Stratum v2 connectivity"
(https://github.com/bitcoin/bitcoin/pull/30332)
Based on #30315, parent PR #29432.
This PR uses the `Sv2Transport` introduced in #30315 to enable incoming connections from other [Stratum v2 roles](https://github.com/stratum-mining/sv2-spec/blob/main/03-Protocol-Overview.md).
The current implementation does this by implementing a minimum subset of the Template Provider role introduced in #29432, but ideally I'd like to introduce a more generic class. There may be other Stratum v2 roles we want to support in the future.
One such role c
...
(https://github.com/bitcoin/bitcoin/pull/30332)
Based on #30315, parent PR #29432.
This PR uses the `Sv2Transport` introduced in #30315 to enable incoming connections from other [Stratum v2 roles](https://github.com/stratum-mining/sv2-spec/blob/main/03-Protocol-Overview.md).
The current implementation does this by implementing a minimum subset of the Template Provider role introduced in #29432, but ideally I'd like to introduce a more generic class. There may be other Stratum v2 roles we want to support in the future.
One such role c
...