💬 laanwj commented on pull request "miniscript: convert non-critical asserts to Assumes":
(https://github.com/bitcoin/bitcoin/pull/28678#issuecomment-1787266550)
Concept ACK. Not having these potentially crash the application is an improvement.
Also agree with @darosior though, we don't want error conditions that can be caused by user input to go by silently, either.
(https://github.com/bitcoin/bitcoin/pull/28678#issuecomment-1787266550)
Concept ACK. Not having these potentially crash the application is an improvement.
Also agree with @darosior though, we don't want error conditions that can be caused by user input to go by silently, either.
📝 fanquake converted_to_draft a pull request: "build: Bump `native_clang` up to 17.0.2"
(https://github.com/bitcoin/bitcoin/pull/28732)
This PR makes it possible to build the `qt` package with macOS 14 SDK (Xcode 15.0).
For more details, please refer to https://github.com/bitcoin/bitcoin/pull/28622.
(https://github.com/bitcoin/bitcoin/pull/28732)
This PR makes it possible to build the `qt` package with macOS 14 SDK (Xcode 15.0).
For more details, please refer to https://github.com/bitcoin/bitcoin/pull/28622.
💬 fanquake commented on pull request "build: Bump `native_clang` up to 17.0.2":
(https://github.com/bitcoin/bitcoin/pull/28732#issuecomment-1787275533)
Discussed further offline. Moving this to draft for now. As mentioned, bumping Clang isn't required to use C++20, because bitcoind and other utils can already be compiled using C++20, against the newer SDK, so the assumption is that this is something that will need to be patched in Qt?
(https://github.com/bitcoin/bitcoin/pull/28732#issuecomment-1787275533)
Discussed further offline. Moving this to draft for now. As mentioned, bumping Clang isn't required to use C++20, because bitcoind and other utils can already be compiled using C++20, against the newer SDK, so the assumption is that this is something that will need to be patched in Qt?
💬 fanquake commented on pull request "guix: Zip needs to include all files and set time to SOURCE_DATE_EPOCH":
(https://github.com/bitcoin/bitcoin/pull/28757#issuecomment-1787279028)
Yea, we should nuke the `dist/` here, otherwise, this looks good to me.
(https://github.com/bitcoin/bitcoin/pull/28757#issuecomment-1787279028)
Yea, we should nuke the `dist/` here, otherwise, this looks good to me.
🤔 TheCharlatan reviewed a pull request: "wallet: Fix wallet directory initialization"
(https://github.com/bitcoin/bitcoin/pull/28514#pullrequestreview-1706431124)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/28514#pullrequestreview-1706431124)
Concept ACK
💬 furszy commented on pull request "p2p: adaptive connections services flags":
(https://github.com/bitcoin/bitcoin/pull/28170#discussion_r1377694801)
> In that case, i'd probably say that explicitly in the function's name. Like SeedsServiceFlags.
Sounds good. Will rename it to `SeedsServiceFlags`.
I was thinking we would use this function for other cases in the future, but agree that this can be simplified for now.
> Then, a comment saying something like these flags should probably be in sync with GetDesirableServiceFlags, but not strictly.
I'm not sure about this comment. We could have a much more complex logic at the PeerManager l
...
(https://github.com/bitcoin/bitcoin/pull/28170#discussion_r1377694801)
> In that case, i'd probably say that explicitly in the function's name. Like SeedsServiceFlags.
Sounds good. Will rename it to `SeedsServiceFlags`.
I was thinking we would use this function for other cases in the future, but agree that this can be simplified for now.
> Then, a comment saying something like these flags should probably be in sync with GetDesirableServiceFlags, but not strictly.
I'm not sure about this comment. We could have a much more complex logic at the PeerManager l
...
💬 glozow commented on pull request "refactors for subpackage evaluation":
(https://github.com/bitcoin/bitcoin/pull/28758#discussion_r1377697411)
Renamed to `IsConsistentPackage`
(https://github.com/bitcoin/bitcoin/pull/28758#discussion_r1377697411)
Renamed to `IsConsistentPackage`
💬 laanwj commented on pull request "Do the SOCKS5 handshake reliably":
(https://github.com/bitcoin/bitcoin/pull/28649#issuecomment-1787362292)
Concept ACK. Good catch!
(https://github.com/bitcoin/bitcoin/pull/28649#issuecomment-1787362292)
Concept ACK. Good catch!
📝 glozow opened a pull request: "MiniMiner changes for package linearization"
(https://github.com/bitcoin/bitcoin/pull/28762)
This is part of #27463. It splits off the `MiniMiner`-specific changes from #26711 for ease of review, as suggested in https://github.com/bitcoin/bitcoin/pull/26711#issuecomment-1786392253.
- Allow using `MiniMiner` on transactions that aren't in the mempool.
- Make `target_feerate` param of `BuildMockTemplate` optional, meaning "don't stop building the template until all the transactions have been selected."
- Add clarification for how this is different from `target_feerate=0` (https://g
...
(https://github.com/bitcoin/bitcoin/pull/28762)
This is part of #27463. It splits off the `MiniMiner`-specific changes from #26711 for ease of review, as suggested in https://github.com/bitcoin/bitcoin/pull/26711#issuecomment-1786392253.
- Allow using `MiniMiner` on transactions that aren't in the mempool.
- Make `target_feerate` param of `BuildMockTemplate` optional, meaning "don't stop building the template until all the transactions have been selected."
- Add clarification for how this is different from `target_feerate=0` (https://g
...
💬 glozow commented on pull request "validate package transactions with their in-package ancestor sets":
(https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1377711137)
Changed in #28762 to `{fee,vsize}_{self,ancestor}`. Just trying to make them compact-ish
(https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1377711137)
Changed in #28762 to `{fee,vsize}_{self,ancestor}`. Just trying to make them compact-ish
🤔 furszy reviewed a pull request: "p2p: adaptive connections services flags"
(https://github.com/bitcoin/bitcoin/pull/28170#pullrequestreview-1706509820)
Updated per feedback. Thanks again.
* Added coverage for the introduced PeerManager's adaptive connections service flags.
* Renamed StatelessServiceFlags function to SeedsServiceFlags.
* Re-introduced the "initial sync completed" flag update on `UpdatedBlockTip`.
This is to not depend solely on the stale check inside `CheckForStaleTipAndEvictPeers` which is executed every 10 minutes (`STALE_CHECK_INTERVAL`). See code comments for more information.
(https://github.com/bitcoin/bitcoin/pull/28170#pullrequestreview-1706509820)
Updated per feedback. Thanks again.
* Added coverage for the introduced PeerManager's adaptive connections service flags.
* Renamed StatelessServiceFlags function to SeedsServiceFlags.
* Re-introduced the "initial sync completed" flag update on `UpdatedBlockTip`.
This is to not depend solely on the stale check inside `CheckForStaleTipAndEvictPeers` which is executed every 10 minutes (`STALE_CHECK_INTERVAL`). See code comments for more information.
💬 glozow commented on pull request "validate package transactions with their in-package ancestor sets":
(https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1377713661)
Fees can be negative since we use modified fees, so 0 is not the same thing as turning it off. I've added a comment + test for this in #28762.
(https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1377713661)
Fees can be negative since we use modified fees, so 0 is not the same thing as turning it off. I've added a comment + test for this in #28762.
💬 glozow commented on pull request "validate package transactions with their in-package ancestor sets":
(https://github.com/bitcoin/bitcoin/pull/26711#issuecomment-1787375144)
> Shouldn't this PR be split up at this point?
Makes sense
> *refactors + test additions that don't change functionality changes (first 3 commits plus CleanupTemporaryCoins?)
^I've split this off into #28758
> [edit: miniminer patch could probably be its own PR too, which would probably be easier to review for people familiar with that particular code but not everything else]
^I've split this off into #28762. I also expanded it into smaller commits, added more tests, and add
...
(https://github.com/bitcoin/bitcoin/pull/26711#issuecomment-1787375144)
> Shouldn't this PR be split up at this point?
Makes sense
> *refactors + test additions that don't change functionality changes (first 3 commits plus CleanupTemporaryCoins?)
^I've split this off into #28758
> [edit: miniminer patch could probably be its own PR too, which would probably be easier to review for people familiar with that particular code but not everything else]
^I've split this off into #28762. I also expanded it into smaller commits, added more tests, and add
...
💬 sr-gi commented on pull request "net: improves addnode / m_added_nodes logic":
(https://github.com/bitcoin/bitcoin/pull/28155#discussion_r1377718949)
That works for me. @jonatack if you add rebase it I'll cherry-pick it again.
(https://github.com/bitcoin/bitcoin/pull/28155#discussion_r1377718949)
That works for me. @jonatack if you add rebase it I'll cherry-pick it again.
💬 toolsopen commented on issue "sendrawtransaction takes too long":
(https://github.com/bitcoin/bitcoin/issues/28745#issuecomment-1787422963)
@maflcko Will wallet.dat 9.3G affect performance? Is there any way to clear the used transactions in it?
(https://github.com/bitcoin/bitcoin/issues/28745#issuecomment-1787422963)
@maflcko Will wallet.dat 9.3G affect performance? Is there any way to clear the used transactions in it?
👋 fanquake's pull request is ready for review: "guix: update signapple to latest master"
(https://github.com/bitcoin/bitcoin/pull/28759)
(https://github.com/bitcoin/bitcoin/pull/28759)
💬 fanquake commented on pull request "guix: update signapple to latest master":
(https://github.com/bitcoin/bitcoin/pull/28759#issuecomment-1787429202)
The difference in bootstrapping is significant enough, that I'd also like to backport this to the 26.x branch. cc @theuni @darosior.
(https://github.com/bitcoin/bitcoin/pull/28759#issuecomment-1787429202)
The difference in bootstrapping is significant enough, that I'd also like to backport this to the 26.x branch. cc @theuni @darosior.
💬 willcl-ark commented on issue "sendrawtransaction takes too long":
(https://github.com/bitcoin/bitcoin/issues/28745#issuecomment-1787440296)
Hi @toolsopen
Bitcoin Core v22 is outside of maintenance window now, you should probably consider upgrading (to at least 24.2, or better still 25.1)
A few questions:
1) what do these config values do?
```
stake=0
staking=0
gen=0
```
2) How are you creating the sqlite wallet?
I can create a (new) sqlite descriptor wallet and import a wpkh pubkey on v25.1, after compiling with sqlite like this:
```sh
will@ubuntu in ~/src/bitcoin on master [$?⇡] : C v16.0.
...
(https://github.com/bitcoin/bitcoin/issues/28745#issuecomment-1787440296)
Hi @toolsopen
Bitcoin Core v22 is outside of maintenance window now, you should probably consider upgrading (to at least 24.2, or better still 25.1)
A few questions:
1) what do these config values do?
```
stake=0
staking=0
gen=0
```
2) How are you creating the sqlite wallet?
I can create a (new) sqlite descriptor wallet and import a wpkh pubkey on v25.1, after compiling with sqlite like this:
```sh
will@ubuntu in ~/src/bitcoin on master [$?⇡] : C v16.0.
...
💬 andrewtoth commented on pull request "p2p: adaptive connections services flags":
(https://github.com/bitcoin/bitcoin/pull/28170#discussion_r1377784027)
Can we first check that we don't go back to ibd if we are stale but at the threshold, then we do go back to ibd once we step over the threshold?
(https://github.com/bitcoin/bitcoin/pull/28170#discussion_r1377784027)
Can we first check that we don't go back to ibd if we are stale but at the threshold, then we do go back to ibd once we step over the threshold?
💬 achow101 commented on issue "sendrawtransaction takes too long":
(https://github.com/bitcoin/bitcoin/issues/28745#issuecomment-1787466576)
> Will wallet.dat 9.3G affect performance?
Yes, significantly.
> Is there any way to clear the used transactions in it?
Not automatically. However you can use `removeprunedfunds` and delete old transactions manually.
(https://github.com/bitcoin/bitcoin/issues/28745#issuecomment-1787466576)
> Will wallet.dat 9.3G affect performance?
Yes, significantly.
> Is there any way to clear the used transactions in it?
Not automatically. However you can use `removeprunedfunds` and delete old transactions manually.