Bitcoin Core Github
44 subscribers
119K links
Download Telegram
💬 ryanofsky commented on pull request "rename TransactionErrors: MISSING_INPUTS and ALREADY_IN_CHAIN":
(https://github.com/bitcoin/bitcoin/pull/30212#discussion_r1651844249)
In commit "refactor: PSBTError::MISSING_INPUTS" (77ef75d3ce4bff76a0db730b57896dcf7e8f3988):

I think it would be a little better if this message said just "Invalid inputs" instead of "Invalid inputs specified" since the input indices are internal to the PSBT data structure, not really specified by the user.

Someone more familiar with PSBT code than me might be able to suggest a better error code and message than this to use, but at least with this change the error should be more accurate.

...
👍 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)
👍 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.
⚠️ 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_
fanquake closed an issue: "BTC address change bc1qee7hk4a3k3km7j8hwclm0pkl76dhhgxay5nevu"
(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.
💬 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
💬 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.
🤔 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.
💬 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.
💬 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
💬 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
...
💬 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
...
💬 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
...
💬 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.
💬 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
💬 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?
💬 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?
💬 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.
🤔 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