⚠️ 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
...
✅ fanquake closed a pull request: "Remove arbitrary restrictions on OP_RETURN by default"
(https://github.com/bitcoin/bitcoin/pull/28130)
(https://github.com/bitcoin/bitcoin/pull/28130)
💬 luke-jr commented on pull request "p2p: ensure mapBlockSource is removed from in ProcessBlock":
(https://github.com/bitcoin/bitcoin/pull/28235#issuecomment-1673229566)
Is there a case where we really need to punish for this anyway? We already bypass the punishment for compact-block relaying...
(https://github.com/bitcoin/bitcoin/pull/28235#issuecomment-1673229566)
Is there a case where we really need to punish for this anyway? We already bypass the punishment for compact-block relaying...
💬 ryanofsky commented on pull request "refactor: Move IsInitialBlockDownload & NotifyHeaderTip to ChainstateManager":
(https://github.com/bitcoin/bitcoin/pull/28218#issuecomment-1673234551)
re: https://github.com/bitcoin/bitcoin/pull/28218#pullrequestreview-1571859693
> Why not make `NotifyHeaderTip` a private `ChainstateManager` member?
Because it doesn't access private state. I think the point of classes in c++ is to encapsulate private state. If there is a group of functions operating on public state, they should be standalone functions, so they can be split up into files and organized by functionality. This way we can avoid being permanently stuck with a sprawling mess of
...
(https://github.com/bitcoin/bitcoin/pull/28218#issuecomment-1673234551)
re: https://github.com/bitcoin/bitcoin/pull/28218#pullrequestreview-1571859693
> Why not make `NotifyHeaderTip` a private `ChainstateManager` member?
Because it doesn't access private state. I think the point of classes in c++ is to encapsulate private state. If there is a group of functions operating on public state, they should be standalone functions, so they can be split up into files and organized by functionality. This way we can avoid being permanently stuck with a sprawling mess of
...
💬 ChrisCho-H commented on pull request "fix: bad opcode err msg includes reserved opcode":
(https://github.com/bitcoin/bitcoin/pull/28234#issuecomment-1673244113)
closed as it would be better to integrate this update here https://github.com/bitcoin/bitcoin/pull/28169
(https://github.com/bitcoin/bitcoin/pull/28234#issuecomment-1673244113)
closed as it would be better to integrate this update here https://github.com/bitcoin/bitcoin/pull/28169
✅ ChrisCho-H closed a pull request: "fix: bad opcode err msg includes reserved opcode"
(https://github.com/bitcoin/bitcoin/pull/28234)
(https://github.com/bitcoin/bitcoin/pull/28234)