💬 kevkevinpal commented on pull request "doc: fixed inconsistencies in documentation between autotools to cmake change":
(https://github.com/bitcoin/bitcoin/pull/30875#discussion_r1759769962)
thank you! Updated in [4f72eca](https://github.com/bitcoin/bitcoin/pull/30875/commits/4f72ecae2b542dbc052fae78cdc5b64a0da8b534)
(https://github.com/bitcoin/bitcoin/pull/30875#discussion_r1759769962)
thank you! Updated in [4f72eca](https://github.com/bitcoin/bitcoin/pull/30875/commits/4f72ecae2b542dbc052fae78cdc5b64a0da8b534)
💬 kevkevinpal commented on pull request "doc: fixed inconsistencies in documentation between autotools to cmake change":
(https://github.com/bitcoin/bitcoin/pull/30875#discussion_r1759769972)
thank you! Updated in [4f72eca](https://github.com/bitcoin/bitcoin/pull/30875/commits/4f72ecae2b542dbc052fae78cdc5b64a0da8b534)
(https://github.com/bitcoin/bitcoin/pull/30875#discussion_r1759769972)
thank you! Updated in [4f72eca](https://github.com/bitcoin/bitcoin/pull/30875/commits/4f72ecae2b542dbc052fae78cdc5b64a0da8b534)
💬 kevkevinpal commented on pull request "doc: fixed inconsistencies in documentation between autotools to cmake change":
(https://github.com/bitcoin/bitcoin/pull/30875#discussion_r1759769979)
thank you! Updated in [4f72eca](https://github.com/bitcoin/bitcoin/pull/30875/commits/4f72ecae2b542dbc052fae78cdc5b64a0da8b534)
(https://github.com/bitcoin/bitcoin/pull/30875#discussion_r1759769979)
thank you! Updated in [4f72eca](https://github.com/bitcoin/bitcoin/pull/30875/commits/4f72ecae2b542dbc052fae78cdc5b64a0da8b534)
💬 kevkevinpal commented on pull request "doc: fixed inconsistencies in documentation between autotools to cmake change":
(https://github.com/bitcoin/bitcoin/pull/30875#issuecomment-2351067786)
> Also in `test_deterministic_coverage.sh`: `Run \"./configure --enable-lcov\"`
thank you! Updated in [4f72eca](https://github.com/bitcoin/bitcoin/pull/30875/commits/4f72ecae2b542dbc052fae78cdc5b64a0da8b534)
(https://github.com/bitcoin/bitcoin/pull/30875#issuecomment-2351067786)
> Also in `test_deterministic_coverage.sh`: `Run \"./configure --enable-lcov\"`
thank you! Updated in [4f72eca](https://github.com/bitcoin/bitcoin/pull/30875/commits/4f72ecae2b542dbc052fae78cdc5b64a0da8b534)
💬 kevkevinpal commented on pull request "doc: fixed inconsistencies in documentation between autotools to cmake change":
(https://github.com/bitcoin/bitcoin/pull/30875#issuecomment-2351067954)
> Since you are here, you could update the translate instructions as well (I've tested them), if @hebasto agrees with the changes:
>
> * `doc/translation_process.md`
> ```diff
> @@ -18,8 +18,8 @@ We use automated scripts to help extract translations in both Qt, and non-Qt sou
>
> To automatically regenerate the `bitcoin_en.ts` file, run the following commands:
> ```sh
> -cd src/
> -make translate
> +cmake -B build -DWITH_BDB=ON -
...
(https://github.com/bitcoin/bitcoin/pull/30875#issuecomment-2351067954)
> Since you are here, you could update the translate instructions as well (I've tested them), if @hebasto agrees with the changes:
>
> * `doc/translation_process.md`
> ```diff
> @@ -18,8 +18,8 @@ We use automated scripts to help extract translations in both Qt, and non-Qt sou
>
> To automatically regenerate the `bitcoin_en.ts` file, run the following commands:
> ```sh
> -cd src/
> -make translate
> +cmake -B build -DWITH_BDB=ON -
...
💬 kevkevinpal commented on pull request "doc: fixed inconsistencies in documentation between autotools to cmake change and removed libtools from CI":
(https://github.com/bitcoin/bitcoin/pull/30875#issuecomment-2351069254)
From these two comments these are the two that have not been addressed yet in this PR
https://github.com/bitcoin/bitcoin/pull/30664#issuecomment-2320892444 https://github.com/bitcoin/bitcoin/pull/30664#issuecomment-2321610186
```
test/get_previous_releases.py: './configure {}'.format(config_flags),
contrib/devtools/check-deps.sh:# Output makefile targets, converting library .a paths to libtool .la targets
```
(https://github.com/bitcoin/bitcoin/pull/30875#issuecomment-2351069254)
From these two comments these are the two that have not been addressed yet in this PR
https://github.com/bitcoin/bitcoin/pull/30664#issuecomment-2320892444 https://github.com/bitcoin/bitcoin/pull/30664#issuecomment-2321610186
```
test/get_previous_releases.py: './configure {}'.format(config_flags),
contrib/devtools/check-deps.sh:# Output makefile targets, converting library .a paths to libtool .la targets
```
👍 itornaza approved a pull request: "Stratum v2 Noise Protocol"
(https://github.com/bitcoin/bitcoin/pull/29346#pullrequestreview-2304920988)
re ACK c8d9d84e21ba091be6be83261b31684ac693d016
Run all tests locally once again and all of them pass including the extended.
(https://github.com/bitcoin/bitcoin/pull/29346#pullrequestreview-2304920988)
re ACK c8d9d84e21ba091be6be83261b31684ac693d016
Run all tests locally once again and all of them pass including the extended.
💬 itornaza commented on pull request "Stratum v2 Noise Protocol":
(https://github.com/bitcoin/bitcoin/pull/29346#discussion_r1759782079)
If you consider updating for other reasons, could you format this in the same way as PROTOCOL_NAME_HASH?
(https://github.com/bitcoin/bitcoin/pull/29346#discussion_r1759782079)
If you consider updating for other reasons, could you format this in the same way as PROTOCOL_NAME_HASH?
💬 tdb3 commented on pull request "doc: fixed inconsistencies in documentation between autotools to cmake change and removed libtools from CI":
(https://github.com/bitcoin/bitcoin/pull/30875#discussion_r1759786194)
Wouldn't we want to keep `bitcoin-node` (part of multiprocess) rather than change to `bitcoind`? Am I missing something?
Thoughts @ryanofsky ?
(https://github.com/bitcoin/bitcoin/pull/30875#discussion_r1759786194)
Wouldn't we want to keep `bitcoin-node` (part of multiprocess) rather than change to `bitcoind`? Am I missing something?
Thoughts @ryanofsky ?
💬 ismaelsadeeq commented on issue "Mempool + Validation Code Organization":
(https://github.com/bitcoin/bitcoin/issues/25584#issuecomment-2351096923)
If I understand correctly as is right now
- Validation houses to all of validation logic(block, transaction, and mempool)
- txmempool defines the data structure of mempool transactions managing removal and addition.
```mermaid
graph TD
A[Validation] --> B[Block Validation]
A[Validation] --> C[Mempool Validation]
A[Validation] --> D[Chainstate Manager]
A[Validation] --> E[MemPool]
E --> F[CTxMempool]
D --> E
```
> I agree, but my point is that the relatio
...
(https://github.com/bitcoin/bitcoin/issues/25584#issuecomment-2351096923)
If I understand correctly as is right now
- Validation houses to all of validation logic(block, transaction, and mempool)
- txmempool defines the data structure of mempool transactions managing removal and addition.
```mermaid
graph TD
A[Validation] --> B[Block Validation]
A[Validation] --> C[Mempool Validation]
A[Validation] --> D[Chainstate Manager]
A[Validation] --> E[MemPool]
E --> F[CTxMempool]
D --> E
```
> I agree, but my point is that the relatio
...
💬 ismaelsadeeq commented on issue "Improve documentation of estimatesmartfee":
(https://github.com/bitcoin/bitcoin/issues/18217#issuecomment-2351102167)
The previous attempt to fix in #30525 in commit https://github.com/bitcoin/bitcoin/commit/e387642f36ba01e45656875d8bde7b021ed28556 was not successful, so I am bringing this here as an attempt to move this issue forward i.e closed/fixed
> _@glozow said in https://github.com/bitcoin/bitcoin/pull/30525#discussion_r1705470107:
I think the previous text is clear and not inaccurate, so I'm wondering whether this last commit is necessary._
(https://github.com/bitcoin/bitcoin/issues/18217#issuecomment-2351102167)
The previous attempt to fix in #30525 in commit https://github.com/bitcoin/bitcoin/commit/e387642f36ba01e45656875d8bde7b021ed28556 was not successful, so I am bringing this here as an attempt to move this issue forward i.e closed/fixed
> _@glozow said in https://github.com/bitcoin/bitcoin/pull/30525#discussion_r1705470107:
I think the previous text is clear and not inaccurate, so I'm wondering whether this last commit is necessary._
📝 hebasto opened a pull request: "cmake: Switch to crc32c upstream build system"
(https://github.com/bitcoin/bitcoin/pull/30905)
A few things still need to be addressed:
- The minimum supported version 3.1 is too low and raises a warning.
- Some compiler flags (hardening, sanitizers etc) have to be passed to the subtree build system.
- https://github.com/google/crc32c/pull/61.
(https://github.com/bitcoin/bitcoin/pull/30905)
A few things still need to be addressed:
- The minimum supported version 3.1 is too low and raises a warning.
- Some compiler flags (hardening, sanitizers etc) have to be passed to the subtree build system.
- https://github.com/google/crc32c/pull/61.
💬 hebasto commented on pull request "doc: fixed inconsistencies in documentation between autotools to cmake change and removed libtools from CI":
(https://github.com/bitcoin/bitcoin/pull/30875#issuecomment-2351112478)
@kevkevinpal
I didn't notice that https://github.com/bitcoin/bitcoin/pull/30902 overlaps with this PR. Feel free to pick all changes from https://github.com/bitcoin/bitcoin/pull/30902 to let me close it.
(https://github.com/bitcoin/bitcoin/pull/30875#issuecomment-2351112478)
@kevkevinpal
I didn't notice that https://github.com/bitcoin/bitcoin/pull/30902 overlaps with this PR. Feel free to pick all changes from https://github.com/bitcoin/bitcoin/pull/30902 to let me close it.
💬 hebasto commented on pull request "Remove Autotools packages from depends and CI":
(https://github.com/bitcoin/bitcoin/pull/30902#issuecomment-2351113099)
About to close it in favour of https://github.com/bitcoin/bitcoin/pull/30875.
(https://github.com/bitcoin/bitcoin/pull/30902#issuecomment-2351113099)
About to close it in favour of https://github.com/bitcoin/bitcoin/pull/30875.
💬 edilmedeiros commented on issue "contrib/signet/miner: grind will fail for high difficulty chain":
(https://github.com/bitcoin/bitcoin/issues/30102#issuecomment-2351114889)
Could not reproduce this using `--min-nbits` with the new miner script from #28417. Seems like the difficulty won't increase as fast as I would expect, even after 10 difficulty adjustment periods. I'll leave it running more time to be sure and also try again using a custom difficulty as initially reported.
(https://github.com/bitcoin/bitcoin/issues/30102#issuecomment-2351114889)
Could not reproduce this using `--min-nbits` with the new miner script from #28417. Seems like the difficulty won't increase as fast as I would expect, even after 10 difficulty adjustment periods. I'll leave it running more time to be sure and also try again using a custom difficulty as initially reported.
💬 hebasto commented on pull request "kernel: Create usable static kernel library":
(https://github.com/bitcoin/bitcoin/pull/30814#discussion_r1760005637)
Should this be a part of the ["Kernel" component](https://github.com/bitcoin/bitcoin/pull/30835)?
(https://github.com/bitcoin/bitcoin/pull/30814#discussion_r1760005637)
Should this be a part of the ["Kernel" component](https://github.com/bitcoin/bitcoin/pull/30835)?
💬 TheCharlatan commented on pull request "kernel: Create usable static kernel library":
(https://github.com/bitcoin/bitcoin/pull/30814#discussion_r1760007655)
Yeah, I missed this. Already patched in #30595 , but could also patch it independently if you want to.
(https://github.com/bitcoin/bitcoin/pull/30814#discussion_r1760007655)
Yeah, I missed this. Already patched in #30595 , but could also patch it independently if you want to.
💬 hebasto commented on pull request "kernel: Create usable static kernel library":
(https://github.com/bitcoin/bitcoin/pull/30814#discussion_r1760009962)
> Already patched in #30595 , but could also patch it independently if you want to.
Great!
Feel free to consider another refactor commit to get rid off `foreach()` loop: https://github.com/hebasto/bitcoin/commits/240915-kernel/.
(https://github.com/bitcoin/bitcoin/pull/30814#discussion_r1760009962)
> Already patched in #30595 , but could also patch it independently if you want to.
Great!
Feel free to consider another refactor commit to get rid off `foreach()` loop: https://github.com/hebasto/bitcoin/commits/240915-kernel/.
💬 fjahr commented on pull request "test: Introduce ensure helper":
(https://github.com/bitcoin/bitcoin/pull/30893#issuecomment-2351534412)
Since a few reviewers seem to prefer a clearer naming, I have now changed it to `ensure_for(duration=..., f=...)`.
(https://github.com/bitcoin/bitcoin/pull/30893#issuecomment-2351534412)
Since a few reviewers seem to prefer a clearer naming, I have now changed it to `ensure_for(duration=..., f=...)`.
💬 danielabrozzoni commented on pull request "signet: fixing mining for OP_TRUE challenge":
(https://github.com/bitcoin/bitcoin/pull/29032#discussion_r1760032207)
I think walletprocesspsbt is trying to sign the PSBT using `SIGHASH_DEFAULT`, while the PSBT specifies to use `SIGHASH_ALL`.
Using `$CLI -signet walletprocesspsbt $PSBT true ALL` works for me.
(https://github.com/bitcoin/bitcoin/pull/29032#discussion_r1760032207)
I think walletprocesspsbt is trying to sign the PSBT using `SIGHASH_DEFAULT`, while the PSBT specifies to use `SIGHASH_ALL`.
Using `$CLI -signet walletprocesspsbt $PSBT true ALL` works for me.