💬 darosior commented on pull request "MiniTapscript: port Miniscript to Tapscript":
(https://github.com/bitcoin/bitcoin/pull/27255#discussion_r1195519598)
Reproduced and fixed!
(https://github.com/bitcoin/bitcoin/pull/27255#discussion_r1195519598)
Reproduced and fixed!
👍 hebasto approved a pull request: "ci: Run iwyu on all src files"
(https://github.com/bitcoin/bitcoin/pull/27571#pullrequestreview-1429129896)
ACK ddddf4957b02c83ed9b6c46b35d8ae1e137889d2
(https://github.com/bitcoin/bitcoin/pull/27571#pullrequestreview-1429129896)
ACK ddddf4957b02c83ed9b6c46b35d8ae1e137889d2
💬 MarcoFalke commented on pull request "build: Drop support for g++-8":
(https://github.com/bitcoin/bitcoin/pull/27662#issuecomment-1550157077)
> Could pull in this commit, to remove the GCC 8 -lstdc++fs check: https://github.com/fanquake/bitcoin/commit/b2632e7c7b7dfc5487629d80e11a8cdeaa649ed9.
My preference would be to also bump clang and then just remove the whole check instead of fiddling with it, when all supported distros ship clang-10 or higher already.
(https://github.com/bitcoin/bitcoin/pull/27662#issuecomment-1550157077)
> Could pull in this commit, to remove the GCC 8 -lstdc++fs check: https://github.com/fanquake/bitcoin/commit/b2632e7c7b7dfc5487629d80e11a8cdeaa649ed9.
My preference would be to also bump clang and then just remove the whole check instead of fiddling with it, when all supported distros ship clang-10 or higher already.
💬 darosior commented on pull request "MiniTapscript: port Miniscript to Tapscript":
(https://github.com/bitcoin/bitcoin/pull/27255#issuecomment-1550158814)
Rebased and updated.
- I addressed @josibake's comments. (thanks for the review!)
- I fixed the `multi_a` satisfaction to use an algorithm similar to `multi`'s.
- Modified the maximum script size to account for some breathing room for the spending transaction size.
- Made the execution size accounting easier by not tracking the size of the stack after execution of a fragment, instead assuming the maximum amount of stack elements the fragment can consume were consumed. The number of items pus
...
(https://github.com/bitcoin/bitcoin/pull/27255#issuecomment-1550158814)
Rebased and updated.
- I addressed @josibake's comments. (thanks for the review!)
- I fixed the `multi_a` satisfaction to use an algorithm similar to `multi`'s.
- Modified the maximum script size to account for some breathing room for the spending transaction size.
- Made the execution size accounting easier by not tracking the size of the stack after execution of a fragment, instead assuming the maximum amount of stack elements the fragment can consume were consumed. The number of items pus
...
💬 fanquake commented on pull request "build: Drop support for g++-8":
(https://github.com/bitcoin/bitcoin/pull/27662#issuecomment-1550173042)
Are you going to pull a Clang bump into this PR? I don't see the point of leaving redundant code, especially when the diff is trivial to review.
(https://github.com/bitcoin/bitcoin/pull/27662#issuecomment-1550173042)
Are you going to pull a Clang bump into this PR? I don't see the point of leaving redundant code, especially when the diff is trivial to review.
💬 MarcoFalke commented on pull request "Raise on invalid -debug and -loglevel config options":
(https://github.com/bitcoin/bitcoin/pull/27632#discussion_r1195604095)
unrelated: A minimal (but hacky) way to add support for `void` to `Result` would be:
```diff
diff --git a/src/util/result.h b/src/util/result.h
index 972b1aada0..b99995c7e5 100644
--- a/src/util/result.h
+++ b/src/util/result.h
@@ -31,16 +31,19 @@ struct Error {
//! `std::optional<T>` can be updated to return `util::Result<T>` and return
//! error strings usually just replacing `return std::nullopt;` with `return
//! util::Error{error_string};`.
-template <class T>
+template <c
...
(https://github.com/bitcoin/bitcoin/pull/27632#discussion_r1195604095)
unrelated: A minimal (but hacky) way to add support for `void` to `Result` would be:
```diff
diff --git a/src/util/result.h b/src/util/result.h
index 972b1aada0..b99995c7e5 100644
--- a/src/util/result.h
+++ b/src/util/result.h
@@ -31,16 +31,19 @@ struct Error {
//! `std::optional<T>` can be updated to return `util::Result<T>` and return
//! error strings usually just replacing `return std::nullopt;` with `return
//! util::Error{error_string};`.
-template <class T>
+template <c
...
💬 theuni commented on pull request "macOS: Bump minimum required runtime version and prepare for building with upstream LLVM":
(https://github.com/bitcoin/bitcoin/pull/27676#issuecomment-1550239909)
> Note that you'll also need to update our minimum version check in:
>
> https://github.com/bitcoin/bitcoin/blob/904631e0fc00ac9c8a03d1ce226d071bf88c00db/contrib/devtools/symbol-check.py#L235
Thanks. Done. I used #22993 as a template for bumping, we'll see if c-i is happy.
(https://github.com/bitcoin/bitcoin/pull/27676#issuecomment-1550239909)
> Note that you'll also need to update our minimum version check in:
>
> https://github.com/bitcoin/bitcoin/blob/904631e0fc00ac9c8a03d1ce226d071bf88c00db/contrib/devtools/symbol-check.py#L235
Thanks. Done. I used #22993 as a template for bumping, we'll see if c-i is happy.
📝 brunoerg opened a pull request: "fuzz: net, add `recv_flood_size`, `prefer_evict` in `ConsumeNode`"
(https://github.com/bitcoin/bitcoin/pull/27678)
This PR adds `recv_flood_size` and `prefer_evict` in `CNodeOptions` when creating a new CNode in `ConsumeNode`. I noticed they were missing while working on an improvement for net fuzzing.
Checked that #27324 added `recv_flood_size` into `CNodeOptions` and #25962 added`prefer_evict`.
(https://github.com/bitcoin/bitcoin/pull/27678)
This PR adds `recv_flood_size` and `prefer_evict` in `CNodeOptions` when creating a new CNode in `ConsumeNode`. I noticed they were missing while working on an improvement for net fuzzing.
Checked that #27324 added `recv_flood_size` into `CNodeOptions` and #25962 added`prefer_evict`.
💬 TheCharlatan commented on pull request "kernel: Remove interface_ui, util/system from kernel library":
(https://github.com/bitcoin/bitcoin/pull/27636#issuecomment-1550261815)
Thank you for the review,
Updated 2c58fbf816d73395167a3046c4ce957829bf45f9 -> b29eab3fc68de28fe2aa67700fd99c6744e37f61 ([chainstateRmKernelUiInterface_4](https://github.com/TheCharlatan/bitcoin/commits/chainstateRmKernelUiInterface_4) -> [chainstateRmKernelUiInterface_5](https://github.com/TheCharlatan/bitcoin/commits/chainstateRmKernelUiInterface_5), [compare](https://github.com/TheCharlatan/bitcoin/compare/chainstateRmKernelUiInterface_4..chainstateRmKernelUiInterface_5)).
* Addressed @r
...
(https://github.com/bitcoin/bitcoin/pull/27636#issuecomment-1550261815)
Thank you for the review,
Updated 2c58fbf816d73395167a3046c4ce957829bf45f9 -> b29eab3fc68de28fe2aa67700fd99c6744e37f61 ([chainstateRmKernelUiInterface_4](https://github.com/TheCharlatan/bitcoin/commits/chainstateRmKernelUiInterface_4) -> [chainstateRmKernelUiInterface_5](https://github.com/TheCharlatan/bitcoin/commits/chainstateRmKernelUiInterface_5), [compare](https://github.com/TheCharlatan/bitcoin/compare/chainstateRmKernelUiInterface_4..chainstateRmKernelUiInterface_5)).
* Addressed @r
...
👍 theStack approved a pull request: "test: Return dict in MiniWallet::send_to"
(https://github.com/bitcoin/bitcoin/pull/27640#pullrequestreview-1429300073)
Code-review ACK faf4315c88d8c81c2ff84870bc81aef3cf719816
(https://github.com/bitcoin/bitcoin/pull/27640#pullrequestreview-1429300073)
Code-review ACK faf4315c88d8c81c2ff84870bc81aef3cf719816
💬 joostjager commented on pull request "validate package transactions with their in-package ancestor sets":
(https://github.com/bitcoin/bitcoin/pull/26711#issuecomment-1550285007)
What are the current use cases for transaction packages enabled by this PR that are more complicated than child-with-parents-tree-only?
(https://github.com/bitcoin/bitcoin/pull/26711#issuecomment-1550285007)
What are the current use cases for transaction packages enabled by this PR that are more complicated than child-with-parents-tree-only?
📝 pinheadmz opened a pull request: "ZMQ: Support UNIX domain sockets"
(https://github.com/bitcoin/bitcoin/pull/27679)
This is a follow-up to https://github.com/bitcoin/bitcoin/pull/27375, allowing ZMQ notifications to be published to a UNIX domain socket, adding one more commit.
Fortunately, libzmq handles unix sockets already, all we really have to do to support it is allow the format in the actual option. libzmq uses the prefix `ipc://` as opposed to `unix:` which is used by Tor and by #27375 so, for now I'm testing with both being allowed.
(https://github.com/bitcoin/bitcoin/pull/27679)
This is a follow-up to https://github.com/bitcoin/bitcoin/pull/27375, allowing ZMQ notifications to be published to a UNIX domain socket, adding one more commit.
Fortunately, libzmq handles unix sockets already, all we really have to do to support it is allow the format in the actual option. libzmq uses the prefix `ipc://` as opposed to `unix:` which is used by Tor and by #27375 so, for now I'm testing with both being allowed.
💬 instagibbs commented on pull request "validate package transactions with their in-package ancestor sets":
(https://github.com/bitcoin/bitcoin/pull/26711#issuecomment-1550292949)
@joostjager could be as simple as any wallet that does CPFP bumping in their coin selection algorithm(chaining unconfirmed spends). The ideal is to support as many usage patters exist in practice, ancestor packages is just a large-ish subset of that that from a CPFP point of view.
(https://github.com/bitcoin/bitcoin/pull/26711#issuecomment-1550292949)
@joostjager could be as simple as any wallet that does CPFP bumping in their coin selection algorithm(chaining unconfirmed spends). The ideal is to support as many usage patters exist in practice, ancestor packages is just a large-ish subset of that that from a CPFP point of view.
💬 joostjager commented on pull request "validate package transactions with their in-package ancestor sets":
(https://github.com/bitcoin/bitcoin/pull/26711#issuecomment-1550315212)
I am trying to get a feel for how big of a problem it is that this PR solves. In Lightning the problem is very real and potentially exposing lots of users to coin loss because of the combination of pre-signed transactions and time sensitivity, but the topology is of the simplest kind. I am not sure if the wallet example that you give is of the same order because RBF is also an option and timing may not be as critical?
Of course it is great to support as many patterns as possible, but it also
...
(https://github.com/bitcoin/bitcoin/pull/26711#issuecomment-1550315212)
I am trying to get a feel for how big of a problem it is that this PR solves. In Lightning the problem is very real and potentially exposing lots of users to coin loss because of the combination of pre-signed transactions and time sensitivity, but the topology is of the simplest kind. I am not sure if the wallet example that you give is of the same order because RBF is also an option and timing may not be as critical?
Of course it is great to support as many patterns as possible, but it also
...
💬 willcl-ark commented on issue "v25.0 testing":
(https://github.com/bitcoin/bitcoin/issues/27621#issuecomment-1550329283)
You could query the repo with curl?
```sh
$ curl -s "https://api.github.com/repos/bitcoin-core/guix.sigs/contents/25.0rc2" | jq -r '.[] | select(.type=="dir") | .name'
CoinForensics
Emzy
Sjors
TheCharlatan
achow101
benthecarman
cfields
darosior
fanquake
glozow
guggero
hebasto
jackielove4u
josibake
laanwj
svanstaa
theStack
vertiond
```
(https://github.com/bitcoin/bitcoin/issues/27621#issuecomment-1550329283)
You could query the repo with curl?
```sh
$ curl -s "https://api.github.com/repos/bitcoin-core/guix.sigs/contents/25.0rc2" | jq -r '.[] | select(.type=="dir") | .name'
CoinForensics
Emzy
Sjors
TheCharlatan
achow101
benthecarman
cfields
darosior
fanquake
glozow
guggero
hebasto
jackielove4u
josibake
laanwj
svanstaa
theStack
vertiond
```
💬 kroese commented on issue "v25.0 testing":
(https://github.com/bitcoin/bitcoin/issues/27621#issuecomment-1550341233)
@willcl-ark Thank you very much. I had already opened an issue ( https://github.com/bitcoin-core/guix.sigs/issues/696 ) where TheCharlatan came up with a very similar suggestion.
(https://github.com/bitcoin/bitcoin/issues/27621#issuecomment-1550341233)
@willcl-ark Thank you very much. I had already opened an issue ( https://github.com/bitcoin-core/guix.sigs/issues/696 ) where TheCharlatan came up with a very similar suggestion.
💬 TheCharlatan commented on pull request "ZMQ: Support UNIX domain sockets":
(https://github.com/bitcoin/bitcoin/pull/27679#issuecomment-1550363562)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/27679#issuecomment-1550363562)
Concept ACK
⚠️ DanM3rcurius opened an issue: "Can't compile v24.0.1"
(https://github.com/bitcoin/bitcoin/issues/27680)
### Is there an existing issue for this?
- [X] I have searched the existing issues
### Current behaviour
I'm trying to install bitcoin core headless with wallet on Raspi4 with Raspi OS installed on a bootable SSD.
Berkeley DB 4.8 is installed as per this tutorial https://raspnode.com/diyBitcoin.html#swap
When I try to compile the 24.0.1 branch, i only get as far as the configure command here:
When I enter (in the cloned bitcoin folder):
`./configure CPPFLAGS="-I/usr/local/Berkeley
...
(https://github.com/bitcoin/bitcoin/issues/27680)
### Is there an existing issue for this?
- [X] I have searched the existing issues
### Current behaviour
I'm trying to install bitcoin core headless with wallet on Raspi4 with Raspi OS installed on a bootable SSD.
Berkeley DB 4.8 is installed as per this tutorial https://raspnode.com/diyBitcoin.html#swap
When I try to compile the 24.0.1 branch, i only get as far as the configure command here:
When I enter (in the cloned bitcoin folder):
`./configure CPPFLAGS="-I/usr/local/Berkeley
...
🤔 mzumsande reviewed a pull request: "init: verify blocks data existence only once for all the indexers"
(https://github.com/bitcoin/bitcoin/pull/27607#pullrequestreview-1429474726)
I can't see how the current approach could work, even if the problems discussed above were solved:
With this PR, the newly introduced function `update_indexes_start_block` is executed right after creating the index, but before executing `Start()`. But the index is basically just an empty object at this point, because its constructor doesn't interact with the database stored on disk at all. So calling `GetSummary()` can't possibly give any meaningful data about the best block at this stage. Th
...
(https://github.com/bitcoin/bitcoin/pull/27607#pullrequestreview-1429474726)
I can't see how the current approach could work, even if the problems discussed above were solved:
With this PR, the newly introduced function `update_indexes_start_block` is executed right after creating the index, but before executing `Start()`. But the index is basically just an empty object at this point, because its constructor doesn't interact with the database stored on disk at all. So calling `GetSummary()` can't possibly give any meaningful data about the best block at this stage. Th
...
💬 hernanmarino commented on pull request "Debug Console implementation of generate method":
(https://github.com/bitcoin-core/gui/pull/692#issuecomment-1550418936)
As requested by @hebasto i splitted this PR in 2 commits :
ad5642ae91beb522b6ae806f28cb015a759d1d19 refactors the code to prepare for the new functionality implemented in the following commit, as well as for future console-only commands to be added.
411b1da407f78f2f973f90d48676ce1ae26734a7 adds the new "generate" command, as well as the updated "help generate"
(https://github.com/bitcoin-core/gui/pull/692#issuecomment-1550418936)
As requested by @hebasto i splitted this PR in 2 commits :
ad5642ae91beb522b6ae806f28cb015a759d1d19 refactors the code to prepare for the new functionality implemented in the following commit, as well as for future console-only commands to be added.
411b1da407f78f2f973f90d48676ce1ae26734a7 adds the new "generate" command, as well as the updated "help generate"