💬 darosior commented on pull request "qa: clarify and document assumeutxo tests":
(https://github.com/bitcoin/bitcoin/pull/31907#issuecomment-2672163304)
Updated the commit message to remove the reference to later modification i had in my branch.
(https://github.com/bitcoin/bitcoin/pull/31907#issuecomment-2672163304)
Updated the commit message to remove the reference to later modification i had in my branch.
🚀 fanquake merged a pull request: "cmake: Check `-Wno-*` compiler options for `leveldb` target"
(https://github.com/bitcoin/bitcoin/pull/31366)
(https://github.com/bitcoin/bitcoin/pull/31366)
💬 hebasto commented on pull request "cmake: Set top-level target output locations":
(https://github.com/bitcoin/bitcoin/pull/31161#issuecomment-2672176764)
Rebased due to the conflict with the merged bitcoin/bitcoin#31884.
(https://github.com/bitcoin/bitcoin/pull/31161#issuecomment-2672176764)
Rebased due to the conflict with the merged bitcoin/bitcoin#31884.
💬 sipa commented on pull request "cluster mempool: introduce TxGraph":
(https://github.com/bitcoin/bitcoin/pull/31363#issuecomment-2672180797)
I have modified (in "add staging support" and further) the TxGraph functions relating to the normalization steps to all take a level argument as input (`ApplyRemovals`, `SplitAll`, `GroupClusters`, `ApplyDependencies`, and `MakeAllAcceptable`). This meant a number of repeated sequences like:
```c++
SplitAll(0);
if (m_clustersets.size() == 1) ApplyDependencies();
```
into just
```
ApplyDependencies(0);
```
(https://github.com/bitcoin/bitcoin/pull/31363#issuecomment-2672180797)
I have modified (in "add staging support" and further) the TxGraph functions relating to the normalization steps to all take a level argument as input (`ApplyRemovals`, `SplitAll`, `GroupClusters`, `ApplyDependencies`, and `MakeAllAcceptable`). This meant a number of repeated sequences like:
```c++
SplitAll(0);
if (m_clustersets.size() == 1) ApplyDependencies();
```
into just
```
ApplyDependencies(0);
```
📝 fanquake converted_to_draft a pull request: "doc: Fix gen-manpages to check build options"
(https://github.com/bitcoin/bitcoin/pull/29457)
Fixes https://github.com/bitcoin/bitcoin/issues/17506
gen-manapages.py should check enabled build options when generating docs
(https://github.com/bitcoin/bitcoin/pull/29457)
Fixes https://github.com/bitcoin/bitcoin/issues/17506
gen-manapages.py should check enabled build options when generating docs
💬 vasild commented on pull request "Add waitNext() to BlockTemplate interface":
(https://github.com/bitcoin/bitcoin/pull/31283#discussion_r1964055908)
`clang-format` will not break long lines itself because `src/.clang-format` contains `ColumnLimit: 0` (unlimited). However, if you break the line yourself it will do the alignment according to the other rulez.
I have a `clang-format` wrapper shell script that replaces `ColumnLimit: 0` with `ColumnLimit: 110` in `src/.clang-format`, runs the actual `clang-format` and then restores the file (so that I do not commit it accidentally).
(https://github.com/bitcoin/bitcoin/pull/31283#discussion_r1964055908)
`clang-format` will not break long lines itself because `src/.clang-format` contains `ColumnLimit: 0` (unlimited). However, if you break the line yourself it will do the alignment according to the other rulez.
I have a `clang-format` wrapper shell script that replaces `ColumnLimit: 0` with `ColumnLimit: 110` in `src/.clang-format`, runs the actual `clang-format` and then restores the file (so that I do not commit it accidentally).
💬 fanquake commented on pull request "doc: Fix gen-manpages to check build options":
(https://github.com/bitcoin/bitcoin/pull/29457#issuecomment-2672186549)
Drafted for now. @BrandonOdiwuor are you still working on this?
(https://github.com/bitcoin/bitcoin/pull/29457#issuecomment-2672186549)
Drafted for now. @BrandonOdiwuor are you still working on this?
📝 fanquake converted_to_draft a pull request: "test: create assert_not_equal util"
(https://github.com/bitcoin/bitcoin/pull/29500)
In the functional tests there are lots of cases where we assert != which we now swap with assert_not_equal to be more readable
This is motivated/uses logic from this PR which was closed https://github.com/bitcoin/bitcoin/pull/28528
This partially helps https://github.com/bitcoin/bitcoin/issues/23119
I've broken it up to just `assert_not_equal` to keep the PR smaller as suggested in https://github.com/bitcoin/bitcoin/pull/28528#issuecomment-1959945805
I can create follow up PR's if this
...
(https://github.com/bitcoin/bitcoin/pull/29500)
In the functional tests there are lots of cases where we assert != which we now swap with assert_not_equal to be more readable
This is motivated/uses logic from this PR which was closed https://github.com/bitcoin/bitcoin/pull/28528
This partially helps https://github.com/bitcoin/bitcoin/issues/23119
I've broken it up to just `assert_not_equal` to keep the PR smaller as suggested in https://github.com/bitcoin/bitcoin/pull/28528#issuecomment-1959945805
I can create follow up PR's if this
...
💬 fanquake commented on pull request "test: create assert_not_equal util":
(https://github.com/bitcoin/bitcoin/pull/29500#issuecomment-2672191460)
@kevkevinpal what's the status of this?
(https://github.com/bitcoin/bitcoin/pull/29500#issuecomment-2672191460)
@kevkevinpal what's the status of this?
💬 fanquake commented on pull request "contrib: fix BUILDDIR in gen-bitcoin-conf script and gen-manpages.py":
(https://github.com/bitcoin/bitcoin/pull/31742#issuecomment-2672195966)
Added to 29.0 as at least the in-tree build mentions should be removed.
(https://github.com/bitcoin/bitcoin/pull/31742#issuecomment-2672195966)
Added to 29.0 as at least the in-tree build mentions should be removed.
👍 vasild approved a pull request: "Add waitNext() to BlockTemplate interface"
(https://github.com/bitcoin/bitcoin/pull/31283#pullrequestreview-2630681525)
ACK cadbd4137d84b71be26effd6a2ae177d5031345e
(https://github.com/bitcoin/bitcoin/pull/31283#pullrequestreview-2630681525)
ACK cadbd4137d84b71be26effd6a2ae177d5031345e
👍 ryanofsky approved a pull request: "build: Enhance Ccache performance across worktrees and build trees"
(https://github.com/bitcoin/bitcoin/pull/30861#pullrequestreview-2630570206)
Code review ACK 82a9f24f49b30c9462716cf0cbf207499be13edf since this seems like a reasonable change, but I'm confused about why it actually doing anything and agree with stickies concern https://github.com/bitcoin/bitcoin/pull/30861#pullrequestreview-2586373527 that forcing CCACHE_NOHASHDIR=1 might not be ideal, especially because according to documentation for [hash_dir](https://ccache.dev/manual/4.0.html#config_hash_dir) it sounds like if CWD is not hashed and `-fdebug-prefix-map` is not used,
...
(https://github.com/bitcoin/bitcoin/pull/30861#pullrequestreview-2630570206)
Code review ACK 82a9f24f49b30c9462716cf0cbf207499be13edf since this seems like a reasonable change, but I'm confused about why it actually doing anything and agree with stickies concern https://github.com/bitcoin/bitcoin/pull/30861#pullrequestreview-2586373527 that forcing CCACHE_NOHASHDIR=1 might not be ideal, especially because according to documentation for [hash_dir](https://ccache.dev/manual/4.0.html#config_hash_dir) it sounds like if CWD is not hashed and `-fdebug-prefix-map` is not used,
...
💬 ryanofsky commented on pull request "build: Enhance Ccache performance across worktrees and build trees":
(https://github.com/bitcoin/bitcoin/pull/30861#discussion_r1963996314)
re: https://github.com/bitcoin/bitcoin/pull/30861#discussion_r1947720890
In commit "cmake: Clarify ccache support for different generators" (c19a187c42fe867d61ca5dbd48ae18f15201839f)
> Thanks! [Reworked](https://github.com/bitcoin/bitcoin/pull/30861#issuecomment-2645380742) per your feedback.
It is hard to tell from reading this thread if the "I think we could drop this check entirely" suggestion and other suggestions have been addressed or not. The check appears to be unchanged. Would
...
(https://github.com/bitcoin/bitcoin/pull/30861#discussion_r1963996314)
re: https://github.com/bitcoin/bitcoin/pull/30861#discussion_r1947720890
In commit "cmake: Clarify ccache support for different generators" (c19a187c42fe867d61ca5dbd48ae18f15201839f)
> Thanks! [Reworked](https://github.com/bitcoin/bitcoin/pull/30861#issuecomment-2645380742) per your feedback.
It is hard to tell from reading this thread if the "I think we could drop this check entirely" suggestion and other suggestions have been addressed or not. The check appears to be unchanged. Would
...
💬 brunoerg commented on pull request "fuzz: provide more realistic values to the base58(check) decoders":
(https://github.com/bitcoin/bitcoin/pull/31917#discussion_r1964065868)
I think we use this function on the RPC target and we limit the length to 64.
(https://github.com/bitcoin/bitcoin/pull/31917#discussion_r1964065868)
I think we use this function on the RPC target and we limit the length to 64.
💬 fanquake commented on pull request "doc: corrected lockunspent rpc quoting":
(https://github.com/bitcoin/bitcoin/pull/31275#issuecomment-2672209936)
Please [squash your commits](https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#squashing-commits). You can improve the commit message at the same time. i.e "doc: ...."
(https://github.com/bitcoin/bitcoin/pull/31275#issuecomment-2672209936)
Please [squash your commits](https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#squashing-commits). You can improve the commit message at the same time. i.e "doc: ...."
✅ fanquake closed a pull request: "Enhance signet chain configuration in bitcoin.conf"
(https://github.com/bitcoin/bitcoin/pull/30203)
(https://github.com/bitcoin/bitcoin/pull/30203)
💬 fanquake commented on pull request "Enhance signet chain configuration in bitcoin.conf":
(https://github.com/bitcoin/bitcoin/pull/30203#issuecomment-2672213545)
Closing this given it hasn't received any feedback. I think discussion can continue in (or this branched can be linked to from) #29838.
(https://github.com/bitcoin/bitcoin/pull/30203#issuecomment-2672213545)
Closing this given it hasn't received any feedback. I think discussion can continue in (or this branched can be linked to from) #29838.
✅ fanquake closed a pull request: "Add OP_TXHASH and OP_CHECKTXHASHVERIFY opcodes"
(https://github.com/bitcoin/bitcoin/pull/29050)
(https://github.com/bitcoin/bitcoin/pull/29050)
💬 fanquake commented on pull request "Add OP_TXHASH and OP_CHECKTXHASHVERIFY opcodes":
(https://github.com/bitcoin/bitcoin/pull/29050#issuecomment-2672218933)
Closing this for now. Overall I think it'll be better open a new PR, after further discussion in https://github.com/bitcoin/bips/pull/1500.
(https://github.com/bitcoin/bitcoin/pull/29050#issuecomment-2672218933)
Closing this for now. Overall I think it'll be better open a new PR, after further discussion in https://github.com/bitcoin/bips/pull/1500.
💬 hebasto commented on pull request "build: Enhance Ccache performance across worktrees and build trees":
(https://github.com/bitcoin/bitcoin/pull/30861#issuecomment-2672224569)
> Also, I'm confused by why we need to require manually setting [base_dir](https://ccache.dev/manual/4.0.html#config_base_dir) instead of setting it automatically to CMAKE_SOURCE_DIR.
Setting `base_dir` to `CMAKE_SOURCE_DIR` won't be helpful in cases when a worktree or a build directory resides outside of the repository root, for example, it is a sibling directory.
(https://github.com/bitcoin/bitcoin/pull/30861#issuecomment-2672224569)
> Also, I'm confused by why we need to require manually setting [base_dir](https://ccache.dev/manual/4.0.html#config_base_dir) instead of setting it automatically to CMAKE_SOURCE_DIR.
Setting `base_dir` to `CMAKE_SOURCE_DIR` won't be helpful in cases when a worktree or a build directory resides outside of the repository root, for example, it is a sibling directory.