💬 fanquake commented on pull request "doc: use llvm-config for bitcoin-tidy example":
(https://github.com/bitcoin/bitcoin/pull/28245#issuecomment-1672982779)
Simplifed this further, given we can actually just ask for `llvm-config --cmakedir`.
(https://github.com/bitcoin/bitcoin/pull/28245#issuecomment-1672982779)
Simplifed this further, given we can actually just ask for `llvm-config --cmakedir`.
⚠️ Sjors opened an issue: "Raise maximum -dbcache setting"
(https://github.com/bitcoin/bitcoin/issues/28249)
### Please describe the feature you'd like to see added.
`nMaxDbCache` was set to 16 GB back in 2015: https://github.com/bitcoin/bitcoin/commit/b3ed4236beb7f68e1720ceb3da15e0c3682ef629#diff-d102b6032635ce90158c1e6e614f03b50e4449aa46ce23370da5387a658342fd
The UTXO set has been growing significantly over the past few months so we should probably raise it.
### Is your feature related to a problem, if so please describe it.
Doing IBD without flushing dbcache is faster. That said, I'm not sure
...
(https://github.com/bitcoin/bitcoin/issues/28249)
### Please describe the feature you'd like to see added.
`nMaxDbCache` was set to 16 GB back in 2015: https://github.com/bitcoin/bitcoin/commit/b3ed4236beb7f68e1720ceb3da15e0c3682ef629#diff-d102b6032635ce90158c1e6e614f03b50e4449aa46ce23370da5387a658342fd
The UTXO set has been growing significantly over the past few months so we should probably raise it.
### Is your feature related to a problem, if so please describe it.
Doing IBD without flushing dbcache is faster. That said, I'm not sure
...
💬 Sjors commented on issue "Raise maximum -dbcache setting":
(https://github.com/bitcoin/bitcoin/issues/28249#issuecomment-1672986348)
cc @sipa
(https://github.com/bitcoin/bitcoin/issues/28249#issuecomment-1672986348)
cc @sipa
💬 theStack commented on pull request "test: locked_wallet, skip default fee estimation":
(https://github.com/bitcoin/bitcoin/pull/28232#discussion_r1289938015)
nit: could use the `get_fee` helper (from test_framework.util) for not having to bother with manual fee-rate unit coversion and fee calculation:
```suggestion
value = inputs[0]["amount"] - get_fee(expected_tx_size, self.min_relay_tx_fee)
```
(https://github.com/bitcoin/bitcoin/pull/28232#discussion_r1289938015)
nit: could use the `get_fee` helper (from test_framework.util) for not having to bother with manual fee-rate unit coversion and fee calculation:
```suggestion
value = inputs[0]["amount"] - get_fee(expected_tx_size, self.min_relay_tx_fee)
```
👍 pinheadmz approved a pull request: "doc: Improve documentation of rpcallowip"
(https://github.com/bitcoin/bitcoin/pull/27480#pullrequestreview-1571626541)
ACK c8e066461b54d745b85411035fcc00a1a4044d76
reviewed code changes, built locally. tested the option with various (in)valid parameters to confirm the docs match the behavior
<details><summary>Show Signature</summary>
```
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256
ACK c8e066461b54d745b85411035fcc00a1a4044d76
-----BEGIN PGP SIGNATURE-----
iQIzBAEBCAAdFiEE5hdzzW4BBA4vG9eM5+KYS2KJyToFAmTUwzsACgkQ5+KYS2KJ
yTpDbhAAzRSmfx29YKWZDQ4gY4bCIKF4/PDAwX4DTo3hh7mOpKai7eKx3TPl2RAd
oy42t0
...
(https://github.com/bitcoin/bitcoin/pull/27480#pullrequestreview-1571626541)
ACK c8e066461b54d745b85411035fcc00a1a4044d76
reviewed code changes, built locally. tested the option with various (in)valid parameters to confirm the docs match the behavior
<details><summary>Show Signature</summary>
```
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256
ACK c8e066461b54d745b85411035fcc00a1a4044d76
-----BEGIN PGP SIGNATURE-----
iQIzBAEBCAAdFiEE5hdzzW4BBA4vG9eM5+KYS2KJyToFAmTUwzsACgkQ5+KYS2KJ
yTpDbhAAzRSmfx29YKWZDQ4gY4bCIKF4/PDAwX4DTo3hh7mOpKai7eKx3TPl2RAd
oy42t0
...
⚠️ Vasu-08 opened an issue: "RPC `createmultisig` outputs a `sh(addr(...))` descriptor when address_type field is "p2sh-segwit""
(https://github.com/bitcoin/bitcoin/issues/28250)
### Is there an existing issue for this?
- [X] I have searched the existing issues
### Current behaviour
While executing rpc createmultisig it outputs a addr descriptor wrapped around sh. Addr descriptor is supposed to be a top level descriptor. Please refer to this gist [https://gist.github.com/Vasu-08/57c1ff479baf2e70e7e2195d31575651](url) for further details.
### Expected behaviour
Not sure.
### Steps to reproduce

### Is there an existing issue for this?
- [X] I have searched the existing issues
### Current behaviour
While executing rpc createmultisig it outputs a addr descriptor wrapped around sh. Addr descriptor is supposed to be a top level descriptor. Please refer to this gist [https://gist.github.com/Vasu-08/57c1ff479baf2e70e7e2195d31575651](url) for further details.
### Expected behaviour
Not sure.
### Steps to reproduce

ACK fae405556d56f6f13ce57f69a06b9ec1e825422b, though I lack historical context to really judge the second commit fa8685597e7302fc136f21b6dd3a4b187fa8e251.
I was curious if it might be possible to easily make one of the `CBlockIndex*` in `BlockTreeDB` a `CBlockIndex&`, but the change became a bit sprawling.
(https://github.com/bitcoin/bitcoin/pull/28195#issuecomment-1673110008)
ACK fae405556d56f6f13ce57f69a06b9ec1e825422b, though I lack historical context to really judge the second commit fa8685597e7302fc136f21b6dd3a4b187fa8e251.
I was curious if it might be possible to easily make one of the `CBlockIndex*` in `BlockTreeDB` a `CBlockIndex&`, but the change became a bit sprawling.
👍 TheCharlatan approved a pull request: "doc: use llvm-config for bitcoin-tidy example"
(https://github.com/bitcoin/bitcoin/pull/28245#pullrequestreview-1571755564)
Nice, Re-ACK d82bb90a5b9dc1fd48b10514bdcd8f425aced256
(https://github.com/bitcoin/bitcoin/pull/28245#pullrequestreview-1571755564)
Nice, Re-ACK d82bb90a5b9dc1fd48b10514bdcd8f425aced256
💬 furszy commented on pull request "test: locked_wallet, skip default fee estimation":
(https://github.com/bitcoin/bitcoin/pull/28232#discussion_r1290085370)
sure, pushed. Thanks.
would be nice to have a document somewhere mentioning the existence of these utility functions.
(https://github.com/bitcoin/bitcoin/pull/28232#discussion_r1290085370)
sure, pushed. Thanks.
would be nice to have a document somewhere mentioning the existence of these utility functions.
💬 furszy commented on pull request "test: locked_wallet, skip default fee estimation":
(https://github.com/bitcoin/bitcoin/pull/28232#issuecomment-1673173304)
Updated per feedback. Thanks theStack.
Tiny change. Instead of calculating the fee manually, the code now uses
the `get_fee()` util function.
(https://github.com/bitcoin/bitcoin/pull/28232#issuecomment-1673173304)
Updated per feedback. Thanks theStack.
Tiny change. Instead of calculating the fee manually, the code now uses
the `get_fee()` util function.
💬 brunoerg commented on pull request "fuzz: fix a couple incorrect assertions in the `coins_view` target":
(https://github.com/bitcoin/bitcoin/pull/28215#discussion_r1290088658)
In https://github.com/bitcoin/bitcoin/pull/28215/commits/c5f6b1db56f67f529377bfb61f58c0a8c17b0127:
```
Note this was never hit because `exists_using_have_coin_in_backend` is
currently never `true` (it's the default implementation of `CCoinsView`.
However this might change if we were to add a target where the backend
is a `CCoinsViewDB`.
```
What do you mean by "add a target"? Because if `exists_using_have_coin_in_backend` is always false, `(!coin_using_access_coin.IsSpent() && exists_u
...
(https://github.com/bitcoin/bitcoin/pull/28215#discussion_r1290088658)
In https://github.com/bitcoin/bitcoin/pull/28215/commits/c5f6b1db56f67f529377bfb61f58c0a8c17b0127:
```
Note this was never hit because `exists_using_have_coin_in_backend` is
currently never `true` (it's the default implementation of `CCoinsView`.
However this might change if we were to add a target where the backend
is a `CCoinsViewDB`.
```
What do you mean by "add a target"? Because if `exists_using_have_coin_in_backend` is always false, `(!coin_using_access_coin.IsSpent() && exists_u
...
💬 dergoegge commented on pull request "fuzz: fix a couple incorrect assertions in the `coins_view` target":
(https://github.com/bitcoin/bitcoin/pull/28215#discussion_r1290090216)
See #28216
(https://github.com/bitcoin/bitcoin/pull/28215#discussion_r1290090216)
See #28216
📝 glozow opened a pull request: "validation: avoid mempool eviction mid-package evaluation"
(https://github.com/bitcoin/bitcoin/pull/28251)
While we are evaluating a package, we split it into "subpackages" for evaluation (currently subpackages all have size 1 except the last one). If a subpackage has size 1, we may add a tx to mempool and call `LimitMempoolSize()`, which evicts transactions if the mempool gets full. We handle the case where the just-submitted transaction is evicted immediately, but we don't handle the case in which a transaction from a previous subpackage (either just submitted or already in mempool) is evicted. Mai
...
(https://github.com/bitcoin/bitcoin/pull/28251)
While we are evaluating a package, we split it into "subpackages" for evaluation (currently subpackages all have size 1 except the last one). If a subpackage has size 1, we may add a tx to mempool and call `LimitMempoolSize()`, which evicts transactions if the mempool gets full. We handle the case where the just-submitted transaction is evicted immediately, but we don't handle the case in which a transaction from a previous subpackage (either just submitted or already in mempool) is evicted. Mai
...
💬 glozow commented on pull request "validation: avoid mempool eviction mid-package evaluation":
(https://github.com/bitcoin/bitcoin/pull/28251#issuecomment-1673181801)
cc @instagibbs
(https://github.com/bitcoin/bitcoin/pull/28251#issuecomment-1673181801)
cc @instagibbs
🤔 TheCharlatan reviewed a pull request: "refactor: Move IsInitialBlockDownload & NotifyHeaderTip to ChainstateManager"
(https://github.com/bitcoin/bitcoin/pull/28218#pullrequestreview-1571859693)
Why not make `NotifyHeaderTip` a private `ChainstateManager` member? The PR title seems to already hint as much.
(https://github.com/bitcoin/bitcoin/pull/28218#pullrequestreview-1571859693)
Why not make `NotifyHeaderTip` a private `ChainstateManager` member? The PR title seems to already hint as much.
💬 achow101 commented on issue "RPC `createmultisig` outputs a `sh(addr(...))` descriptor when address_type field is "p2sh-segwit"":
(https://github.com/bitcoin/bitcoin/issues/28250#issuecomment-1673195541)
I think this was fixed by #28067
(https://github.com/bitcoin/bitcoin/issues/28250#issuecomment-1673195541)
I think this was fixed by #28067
💬 brunoerg commented on pull request "fuzz: fix a couple incorrect assertions in the `coins_view` target":
(https://github.com/bitcoin/bitcoin/pull/28215#discussion_r1290112536)
Thanks, @dergoegge
(https://github.com/bitcoin/bitcoin/pull/28215#discussion_r1290112536)
Thanks, @dergoegge
💬 instagibbs commented on pull request "validation: avoid mempool eviction mid-package evaluation":
(https://github.com/bitcoin/bitcoin/pull/28251#issuecomment-1673202074)
@sipa you might want to weigh in as well
(https://github.com/bitcoin/bitcoin/pull/28251#issuecomment-1673202074)
@sipa you might want to weigh in as well
💬 fanquake commented on issue "BIP324 tracking issue":
(https://github.com/bitcoin/bitcoin/issues/27634#issuecomment-1673203112)
Review comments from #28008 that could be incorporated into future changes:
- [ ] https://github.com/bitcoin/bitcoin/pull/28008#pullrequestreview-1554883653
- [ ] https://github.com/bitcoin/bitcoin/pull/28008#discussion_r1277460913
- [ ] https://github.com/bitcoin/bitcoin/pull/28008#discussion_r1284724692
- [ ] https://github.com/bitcoin/bitcoin/pull/28008#discussion_r1284733846
- [ ] https://github.com/bitcoin/bitcoin/pull/28008#discussion_r1284753030
- [ ] https://github.com/bitcoin/bi
...
(https://github.com/bitcoin/bitcoin/issues/27634#issuecomment-1673203112)
Review comments from #28008 that could be incorporated into future changes:
- [ ] https://github.com/bitcoin/bitcoin/pull/28008#pullrequestreview-1554883653
- [ ] https://github.com/bitcoin/bitcoin/pull/28008#discussion_r1277460913
- [ ] https://github.com/bitcoin/bitcoin/pull/28008#discussion_r1284724692
- [ ] https://github.com/bitcoin/bitcoin/pull/28008#discussion_r1284733846
- [ ] https://github.com/bitcoin/bitcoin/pull/28008#discussion_r1284753030
- [ ] https://github.com/bitcoin/bi
...
💬 fanquake commented on pull request "Remove arbitrary restrictions on OP_RETURN by default":
(https://github.com/bitcoin/bitcoin/pull/28130#issuecomment-1673204546)
I locked this thread, because the discussion here was clearly not achieving anything (other than spamming the thousands of subscribers to this repository).
I'd suggest that anyone who's interested in continuing this discussion, take their thoughts to the mailing list: https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2023-August/021840.html.
In the mean time, I think it's unlikely that Bitcoin Core is going to make any changes in regards to `OP_RETURN`. For now, I'm going to close th
...
(https://github.com/bitcoin/bitcoin/pull/28130#issuecomment-1673204546)
I locked this thread, because the discussion here was clearly not achieving anything (other than spamming the thousands of subscribers to this repository).
I'd suggest that anyone who's interested in continuing this discussion, take their thoughts to the mailing list: https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2023-August/021840.html.
In the mean time, I think it's unlikely that Bitcoin Core is going to make any changes in regards to `OP_RETURN`. For now, I'm going to close th
...