π€ TheCharlatan reviewed a pull request: "Add checkBlock() to Mining interface"
(https://github.com/bitcoin/bitcoin/pull/31981#pullrequestreview-2870950393)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/31981#pullrequestreview-2870950393)
Concept ACK
π¬ TheCharlatan commented on pull request "Add checkBlock() to Mining interface":
(https://github.com/bitcoin/bitcoin/pull/31981#discussion_r2109107660)
In commit 981853ab25dc6f967f393ecba57965afb8a822f3
I would have suggested the same. It is not clear to me why you are introducing a new `blockCoins` (camel case?) view here. Maybe do some of these `TestBlockValidity` refactors and documentation in a separate commit. It is not quite clear to me why this should be changed.
(https://github.com/bitcoin/bitcoin/pull/31981#discussion_r2109107660)
In commit 981853ab25dc6f967f393ecba57965afb8a822f3
I would have suggested the same. It is not clear to me why you are introducing a new `blockCoins` (camel case?) view here. Maybe do some of these `TestBlockValidity` refactors and documentation in a separate commit. It is not quite clear to me why this should be changed.
π¬ maflcko commented on pull request "rpc: generateblock to allow multiple outputs":
(https://github.com/bitcoin/bitcoin/pull/32468#discussion_r2109111837)
βinto a OP_RETURN output.β
-> βinto an OP_RETURN output.β [article before vowel sound βOβ]
I am also thinking about splitting up the "make this arg optional". Seems like a small and easy preparatory pull request?
(https://github.com/bitcoin/bitcoin/pull/32468#discussion_r2109111837)
βinto a OP_RETURN output.β
-> βinto an OP_RETURN output.β [article before vowel sound βOβ]
I am also thinking about splitting up the "make this arg optional". Seems like a small and easy preparatory pull request?
π¬ maflcko commented on pull request "descriptors: MuSig2":
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2109115291)
participatns -> participants [correct misspelling in error message]
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2109115291)
participatns -> participants [correct misspelling in error message]
π¬ brunoerg commented on pull request "fuzz: wallet: add target for `MigrateToDescriptor`":
(https://github.com/bitcoin/bitcoin/pull/32624#discussion_r2109117905)
Just removed it. Redudant.
(https://github.com/bitcoin/bitcoin/pull/32624#discussion_r2109117905)
Just removed it. Redudant.
π¬ brunoerg commented on pull request "fuzz: wallet: add target for `MigrateToDescriptor`":
(https://github.com/bitcoin/bitcoin/pull/32624#issuecomment-2912418699)
08b574dde2c5f13a4c9dadcb9d8bded158c59623..5f2fb4dd333f8d1db27c78414afc84454e6ee4a5: Address https://github.com/bitcoin/bitcoin/pull/32624#discussion_r2109086986
(https://github.com/bitcoin/bitcoin/pull/32624#issuecomment-2912418699)
08b574dde2c5f13a4c9dadcb9d8bded158c59623..5f2fb4dd333f8d1db27c78414afc84454e6ee4a5: Address https://github.com/bitcoin/bitcoin/pull/32624#discussion_r2109086986
π€ naiyoma reviewed a pull request: "rpc: generateblock to allow multiple outputs"
(https://github.com/bitcoin/bitcoin/pull/32468#pullrequestreview-2870982878)
A summary of my understanding of this PR so far, regarding the `generateblock` changes: :
- [x] Outputs and transactions are now optional.
- [x] `generateblock` can now take an array of addresses and distribute the reward equally among them.
- [x] `generateblock` with an empty array - in this case fallback to OP_TRUE
- [ ] I think there's an intention to add a remainder β that way, it's possible to specify part of the reward to specific addresses and then split the remainder among the oth
...
(https://github.com/bitcoin/bitcoin/pull/32468#pullrequestreview-2870982878)
A summary of my understanding of this PR so far, regarding the `generateblock` changes: :
- [x] Outputs and transactions are now optional.
- [x] `generateblock` can now take an array of addresses and distribute the reward equally among them.
- [x] `generateblock` with an empty array - in this case fallback to OP_TRUE
- [ ] I think there's an intention to add a remainder β that way, it's possible to specify part of the reward to specific addresses and then split the remainder among the oth
...
π¬ maflcko commented on pull request "improve MallocUsage() accuracy":
(https://github.com/bitcoin/bitcoin/pull/28531#discussion_r2109129254)
https://cirrus-ci.com/task/4825274484785152?logs=ci#L3624
```
in sync_mempools
[23:01:19.556] raise AssertionError("Mempool sync timed out after {}s:{}".format(
[23:01:19.556] AssertionError: Mempool sync timed out after 2400s:
[23:01:19.556]
(https://github.com/bitcoin/bitcoin/pull/28531#discussion_r2109129254)
https://cirrus-ci.com/task/4825274484785152?logs=ci#L3624
```
in sync_mempools
[23:01:19.556] raise AssertionError("Mempool sync timed out after {}s:{}".format(
[23:01:19.556] AssertionError: Mempool sync timed out after 2400s:
[23:01:19.556]
π¬ instagibbs commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#discussion_r2109129595)
weight / WITNESS_SCALE_FACTOR = vbytes
A bit confusing
(https://github.com/bitcoin/bitcoin/pull/32406#discussion_r2109129595)
weight / WITNESS_SCALE_FACTOR = vbytes
A bit confusing
π¬ naiyoma commented on pull request "rpc: generateblock to allow multiple outputs":
(https://github.com/bitcoin/bitcoin/pull/32468#discussion_r2109130072)
Delete this instead of commenting it out.
(https://github.com/bitcoin/bitcoin/pull/32468#discussion_r2109130072)
Delete this instead of commenting it out.
π¬ maflcko commented on pull request "improve MallocUsage() accuracy":
(https://github.com/bitcoin/bitcoin/pull/28531#discussion_r2109130959)
that size -> than size [incorrect preposition]
beyojnd -> beyond [typographical error]
(https://github.com/bitcoin/bitcoin/pull/28531#discussion_r2109130959)
that size -> than size [incorrect preposition]
beyojnd -> beyond [typographical error]
π¬ naiyoma commented on pull request "rpc: generateblock to allow multiple outputs":
(https://github.com/bitcoin/bitcoin/pull/32468#discussion_r2109136833)
nit: Snake case is preferred for variables. https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#coding-style-c
(https://github.com/bitcoin/bitcoin/pull/32468#discussion_r2109136833)
nit: Snake case is preferred for variables. https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#coding-style-c
π¬ Crypt-iQ commented on issue "compact block fingerprinting":
(https://github.com/bitcoin/bitcoin/issues/28272#issuecomment-2912452626)
> Dropping seemingly-unsolicited compact blocks seems fine, we already do that for the parallel portion, this should be a change just to the first one?
Yup. You wrote the code so I'm sure you know, but for viewers at home, just the first "slot" would need to be accounted for as the other two slots are [guaranteed to be HB.](https://github.com/bitcoin/bitcoin/blob/87860143be792d219aac7f4a04e79d00016df627/src/net_processing.cpp#L4506)
> So I think the CB probing, in fact, is an additional inform
...
(https://github.com/bitcoin/bitcoin/issues/28272#issuecomment-2912452626)
> Dropping seemingly-unsolicited compact blocks seems fine, we already do that for the parallel portion, this should be a change just to the first one?
Yup. You wrote the code so I'm sure you know, but for viewers at home, just the first "slot" would need to be accounted for as the other two slots are [guaranteed to be HB.](https://github.com/bitcoin/bitcoin/blob/87860143be792d219aac7f4a04e79d00016df627/src/net_processing.cpp#L4506)
> So I think the CB probing, in fact, is an additional inform
...
π¬ instagibbs commented on issue "compact block fingerprinting":
(https://github.com/bitcoin/bitcoin/issues/28272#issuecomment-2912461827)
> I don't think this is true anymore.
It's ostensibly forwarded once PoW/merkle checks pass, but not promising utxo/script/etc checks.
(https://github.com/bitcoin/bitcoin/issues/28272#issuecomment-2912461827)
> I don't think this is true anymore.
It's ostensibly forwarded once PoW/merkle checks pass, but not promising utxo/script/etc checks.
π¬ fanquake commented on pull request "guix: accomodate migration to codeberg":
(https://github.com/bitcoin/bitcoin/pull/32439#issuecomment-2912474792)
Guix Build
```bash
8e7798bc42e611c15022d90cfb525fdaccb0eda0753f3d9a1f85f5d947bcced1 guix-build-c8d9baae942c/output/aarch64-linux-gnu/SHA256SUMS.part
e7b1f02d7f0a8390f596e4ba857e503041c2b8f51099763f2a94527493456d81 guix-build-c8d9baae942c/output/aarch64-linux-gnu/bitcoin-c8d9baae942c-aarch64-linux-gnu-debug.tar.gz
6a29f3b05a22a6c52468218ad0e62a38743ebe67c7b05754f90016ee1046e330 guix-build-c8d9baae942c/output/aarch64-linux-gnu/bitcoin-c8d9baae942c-aarch64-linux-gnu.tar.gz
9b624000937c03f2e
...
(https://github.com/bitcoin/bitcoin/pull/32439#issuecomment-2912474792)
Guix Build
```bash
8e7798bc42e611c15022d90cfb525fdaccb0eda0753f3d9a1f85f5d947bcced1 guix-build-c8d9baae942c/output/aarch64-linux-gnu/SHA256SUMS.part
e7b1f02d7f0a8390f596e4ba857e503041c2b8f51099763f2a94527493456d81 guix-build-c8d9baae942c/output/aarch64-linux-gnu/bitcoin-c8d9baae942c-aarch64-linux-gnu-debug.tar.gz
6a29f3b05a22a6c52468218ad0e62a38743ebe67c7b05754f90016ee1046e330 guix-build-c8d9baae942c/output/aarch64-linux-gnu/bitcoin-c8d9baae942c-aarch64-linux-gnu.tar.gz
9b624000937c03f2e
...
π¬ hodlinator commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#discussion_r2109164428)
True, I wasn't clear on vbytes. Seems bytes would still be less fuzzy but hardly worth changing even if you were forced to touch for other reasons.
(https://github.com/bitcoin/bitcoin/pull/32406#discussion_r2109164428)
True, I wasn't clear on vbytes. Seems bytes would still be less fuzzy but hardly worth changing even if you were forced to touch for other reasons.
π hebasto approved a pull request: "guix: accomodate migration to codeberg"
(https://github.com/bitcoin/bitcoin/pull/32439#pullrequestreview-2871055732)
My Guix build:
```
aarch64
8e7798bc42e611c15022d90cfb525fdaccb0eda0753f3d9a1f85f5d947bcced1 guix-build-c8d9baae942c/output/aarch64-linux-gnu/SHA256SUMS.part
e7b1f02d7f0a8390f596e4ba857e503041c2b8f51099763f2a94527493456d81 guix-build-c8d9baae942c/output/aarch64-linux-gnu/bitcoin-c8d9baae942c-aarch64-linux-gnu-debug.tar.gz
6a29f3b05a22a6c52468218ad0e62a38743ebe67c7b05754f90016ee1046e330 guix-build-c8d9baae942c/output/aarch64-linux-gnu/bitcoin-c8d9baae942c-aarch64-linux-gnu.tar.gz
9b624000
...
(https://github.com/bitcoin/bitcoin/pull/32439#pullrequestreview-2871055732)
My Guix build:
```
aarch64
8e7798bc42e611c15022d90cfb525fdaccb0eda0753f3d9a1f85f5d947bcced1 guix-build-c8d9baae942c/output/aarch64-linux-gnu/SHA256SUMS.part
e7b1f02d7f0a8390f596e4ba857e503041c2b8f51099763f2a94527493456d81 guix-build-c8d9baae942c/output/aarch64-linux-gnu/bitcoin-c8d9baae942c-aarch64-linux-gnu-debug.tar.gz
6a29f3b05a22a6c52468218ad0e62a38743ebe67c7b05754f90016ee1046e330 guix-build-c8d9baae942c/output/aarch64-linux-gnu/bitcoin-c8d9baae942c-aarch64-linux-gnu.tar.gz
9b624000
...
π fanquake merged a pull request: "test: fix and augment block tests of invalid_txs"
(https://github.com/bitcoin/bitcoin/pull/32591)
(https://github.com/bitcoin/bitcoin/pull/32591)
π€ rkrux reviewed a pull request: "wallet, rpc: Return normalized descriptor in parent_descs"
(https://github.com/bitcoin/bitcoin/pull/32594#pullrequestreview-2870949997)
ACK a759a22d59e805834d077a28c95695e4834983a9
Agree that normalized descriptors are more useful as no further hardened derivation is required.
Not suggesting any change, only a question: Specific for the `listunspent` RPC though where both `parent_descs` and `desc` fields are present, is `parent_descs` field useful only when the unspent is not solvable? Because `parent_descs` seem like the superset of `desc` from few values I checked and also by its name.
(https://github.com/bitcoin/bitcoin/pull/32594#pullrequestreview-2870949997)
ACK a759a22d59e805834d077a28c95695e4834983a9
Agree that normalized descriptors are more useful as no further hardened derivation is required.
Not suggesting any change, only a question: Specific for the `listunspent` RPC though where both `parent_descs` and `desc` fields are present, is `parent_descs` field useful only when the unspent is not solvable? Because `parent_descs` seem like the superset of `desc` from few values I checked and also by its name.
π¬ rkrux commented on pull request "wallet, rpc: Return normalized descriptor in parent_descs":
(https://github.com/bitcoin/bitcoin/pull/32594#discussion_r2109239705)
IIUC, this is just a dummy signing provider being passed here and essentially the last hardened xpub would be retrieved from the desc cache because I don't see this provider being populated in the call stack.
https://github.com/bitcoin/bitcoin/blob/f7cc7f6468afeb20b01ee86575c7b6329ed2faf9/src/script/descriptor.cpp#L533-L536
Ideally, passing a null for the provider arg could have been better to emphasise on this but I see the function signature accepts a reference instead.
https://github
...
(https://github.com/bitcoin/bitcoin/pull/32594#discussion_r2109239705)
IIUC, this is just a dummy signing provider being passed here and essentially the last hardened xpub would be retrieved from the desc cache because I don't see this provider being populated in the call stack.
https://github.com/bitcoin/bitcoin/blob/f7cc7f6468afeb20b01ee86575c7b6329ed2faf9/src/script/descriptor.cpp#L533-L536
Ideally, passing a null for the provider arg could have been better to emphasise on this but I see the function signature accepts a reference instead.
https://github
...