Bitcoin Core Github
44 subscribers
120K links
Download Telegram
💬 fanquake commented on pull request "cmake: Explicitly specify `Boost_ROOT` for Homebrew's package":
(https://github.com/bitcoin/bitcoin/pull/32814#issuecomment-3008616277)
Backported to 29.x in #32810.
💬 Sjors commented on issue "`dumptxoutset` rollback feature does not take forks into account":
(https://github.com/bitcoin/bitcoin/issues/32817#issuecomment-3008617353)
cc @fjahr
fanquake closed a pull request: "Fix build on macOS when `qt@6` is installed"
(https://github.com/bitcoin/bitcoin/pull/32804)
💬 fanquake commented on pull request "Fix build on macOS when `qt@6` is installed":
(https://github.com/bitcoin/bitcoin/pull/32804#issuecomment-3008618922)
Both of these changes are now in #32810, so if someone would like to test them, they can use that branch.
💬 fanquake commented on issue "29.x Having qt(@6) breaks build for qt@5 on macOS 15.0 and 13.7":
(https://github.com/bitcoin/bitcoin/issues/31009#issuecomment-3008621035)
Can be tested with #32810 now.
⚠️ fanquake reopened an issue: "29.x Having qt(@6) breaks build for qt@5 on macOS 15.0 and 13.7"
(https://github.com/bitcoin/bitcoin/issues/31009)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

Building bitcoin-qt produces a flood of errors:

```
In file included from /Users/sjors/dev/bitcoin/src/qt/csvmodelwriter.cpp:5:
In file included from /Users/sjors/dev/bitcoin/src/qt/csvmodelwriter.h:8:
In file included from /usr/local/opt/qt@5/lib/QtCore.framework/Headers/QList:1:
In file included from /usr/local/opt/qt@5/lib/QtCore.framework/Headers/qlist.h:48:
/usr/local/include/
...
📝 willcl-ark opened a pull request: "Add read-only mode to sqlite db and use in `bitcoin-wallet`"
(https://github.com/bitcoin/bitcoin/pull/32818)
Currently our `wallet-tool` performs a few "read-only" operations (`info`, `dump`) on sqlite databases. However, currently this involves opening the wallet in RW mode, with the side-effects of modifying the wallet file, and failing to open a "read-only" wallet file.

See #15608 for more context.

Since we have dropped the BDB wallet, this change got a lot simpler; the BDB parser is now custom and only operates in read-only mode anyway.

This change adds a `read_only` bool to `DatabaseOptio
...
💬 willcl-ark commented on issue "Feature request: bitcoin-wallet tool: don't modify files unless requested":
(https://github.com/bitcoin/bitcoin/issues/15608#issuecomment-3008635500)
Opened #32818 to address this.
💬 leopardracer commented on pull request "correct variable name in p2p_monitor.py":
(https://github.com/bitcoin/bitcoin/pull/32816#issuecomment-3008638712)
> This just fixes a typo in AVALIABLE -> AVAILABLE. I don't think this is good use of reviewer time. Functionally is not affected.

@0xB10C totally get that this seems minor, but I think even small things like typos matter, especially in a repo like Bitcoin. It's such a high-profile project that sets the tone for the whole ecosystem
💬 musaHaruna commented on pull request "rpc: Distinguish between vsize and sigop adjusted mempool vsize":
(https://github.com/bitcoin/bitcoin/pull/32800#discussion_r2169183653)
Q1. Yeah there's no comment about that in [#27591](https://github.com/bitcoin/bitcoin/pull/27591#discussion_r1376980882), Am still skeptical about conditionally reporting the `sigosize` , the initial idea's to check when txSize which is the transaction in the mempool (which might be sigops adjusted from `entry->GetTxSize()`) is less than vSize `GetVirtualTransactionSize(txSize, 0, 0)` which is not sigop adjusted. But I think it will make more sense to remove the codition and report the` sigopsi
...
💬 Sjors commented on pull request "[29.x] More backports":
(https://github.com/bitcoin/bitcoin/pull/32810#issuecomment-3008658529)
Tested on M4 macOS 15.5 that with 5987c1b6abaefad61d8d2cca605349354432398a having qt5 and qt6 (`qt@5` and `qt` via Homebrew) leads to the issues described in #31009, but with fe8034b09c53f49d02b54f4e55cfe11bbd113fed it's happy.

Will test on Intel later.
💬 musaHaruna commented on pull request "rpc: Distinguish between vsize and sigop adjusted mempool vsize":
(https://github.com/bitcoin/bitcoin/pull/32800#discussion_r2169188242)
Yeah, I don't think it's necessary that's why I decide to report it (sigopsize) only in `getrawtransaction` RPC
💬 darosior commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#issuecomment-3008736607)
Extracted the BIP54-specific sigop counting into a separate function that can be moved into `consensus/tx_verify.cpp` with no modification if/when this is implemented in consensus, as suggested by @instagibbs.
💬 polespinasa commented on pull request "refactor: CFeeRate encapsulates FeeFrac internally":
(https://github.com/bitcoin/bitcoin/pull/32750#discussion_r2169246397)
@davidgumberg I'm having doubts about this. FeeFrac takes an int32 for `size` attribute. I'm not sure it is a good idea to mix types between functions.
💬 0xB10C commented on pull request "correct variable name in p2p_monitor.py":
(https://github.com/bitcoin/bitcoin/pull/32816#issuecomment-3008752031)
Why doesn't the description say it fixes a typo?

> * Replaced hardcoded row calculations with the `ROWS_AVAILABLE_FOR_LIST` variable for improved readability and maintainability.
💬 sipa commented on pull request "refactor: CFeeRate encapsulates FeeFrac internally":
(https://github.com/bitcoin/bitcoin/pull/32750#discussion_r2169251427)
If you change `at_size` to be uint32, I don't think this check is still correct.
💬 leopardracer commented on pull request "correct variable name in p2p_monitor.py":
(https://github.com/bitcoin/bitcoin/pull/32816#issuecomment-3008781449)
> Why doesn't the description say it fixes a typo?
>
> > * Replaced hardcoded row calculations with the `ROWS_AVAILABLE_FOR_LIST` variable for improved readability and maintainability.

@0xB10C description updated
💬 Sjors commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#issuecomment-3008785284)
Although it's more readable, the downside of this approach is that we're looping over tx.vin twice and, probably much worse, calling `AccessCoin` twice.
💬 musaHaruna commented on pull request "rpc: Distinguish between vsize and sigop adjusted mempool vsize":
(https://github.com/bitcoin/bitcoin/pull/32800#discussion_r2169298815)
Done I used `GetVirtualTransactionSize(*tx)`, check [83b6253](https://github.com/bitcoin/bitcoin/pull/32800/commits/83b6253eb3921a79231f450bb43e1977ea835b5e). Thanks!
💬 musaHaruna commented on pull request "rpc: Distinguish between vsize and sigop adjusted mempool vsize":
(https://github.com/bitcoin/bitcoin/pull/32800#discussion_r2169301179)
Done in [83b6253](https://github.com/bitcoin/bitcoin/pull/32800/commits/83b6253eb3921a79231f450bb43e1977ea835b5e). Thanks!