💬 MarcoFalke commented on issue "Why is Bitcoin Core structured as a wallet? I know that it began as a wallet, but is it possible to make it like a GUI block explorer instead?":
(https://github.com/bitcoin/bitcoin/issues/27110#issuecomment-1433194111)
Usually the issue tracker is used to track technical issues related to the Bitcoin Core code base. General bitcoin questions and/or support requests are best directed to the [Bitcoin StackExchange](https://bitcoin.stackexchange.com) or the `#bitcoin` IRC channel on Libera Chat.
(https://github.com/bitcoin/bitcoin/issues/27110#issuecomment-1433194111)
Usually the issue tracker is used to track technical issues related to the Bitcoin Core code base. General bitcoin questions and/or support requests are best directed to the [Bitcoin StackExchange](https://bitcoin.stackexchange.com) or the `#bitcoin` IRC channel on Libera Chat.
🚀 fanquake merged a pull request: "doc: FreeBSD build doc updates to reflect removal of install_db4.sh"
(https://github.com/bitcoin/bitcoin/pull/26773)
(https://github.com/bitcoin/bitcoin/pull/26773)
💬 yancyribbens commented on pull request "refactor: Move coin_control variable to test setup section":
(https://github.com/bitcoin/bitcoin/pull/26154#issuecomment-1433228883)
@achow101
> Yes, it could be a problem if the test is removed, but it's unlikely that the test will be removed, and if it is removed, the line can be just as easily moved at that time.
I did not mean to imply that a test might be removed. I was only pointing out that this kind of coding practice is confusing because the variable definition doesn't belong _only_ to the test.
> While this isn't incorrect, it's not meaningfully helpful. It doesn't really add anything useful to the codeba
...
(https://github.com/bitcoin/bitcoin/pull/26154#issuecomment-1433228883)
@achow101
> Yes, it could be a problem if the test is removed, but it's unlikely that the test will be removed, and if it is removed, the line can be just as easily moved at that time.
I did not mean to imply that a test might be removed. I was only pointing out that this kind of coding practice is confusing because the variable definition doesn't belong _only_ to the test.
> While this isn't incorrect, it's not meaningfully helpful. It doesn't really add anything useful to the codeba
...
💬 TheCharlatan commented on pull request "refactor / kernel: Move non-gArgs chainparams functionality to kernel":
(https://github.com/bitcoin/bitcoin/pull/26177#discussion_r1108602508)
Yes, also keeping it the same as in: https://github.com/bitcoin/bitcoin/pull/25862/files#diff-4e268aeb074a176689aace31957a889c1f39c909e6678ca7be8707bda1a7be56R11
(https://github.com/bitcoin/bitcoin/pull/26177#discussion_r1108602508)
Yes, also keeping it the same as in: https://github.com/bitcoin/bitcoin/pull/25862/files#diff-4e268aeb074a176689aace31957a889c1f39c909e6678ca7be8707bda1a7be56R11
💬 instagibbs commented on pull request "[22.x] Backports":
(https://github.com/bitcoin/bitcoin/pull/26927#issuecomment-1433230124)
ACK https://github.com/bitcoin/bitcoin/pull/26927/commits/ea584a617c6853eb1f9740600cd9db75d77948eb
(https://github.com/bitcoin/bitcoin/pull/26927#issuecomment-1433230124)
ACK https://github.com/bitcoin/bitcoin/pull/26927/commits/ea584a617c6853eb1f9740600cd9db75d77948eb
💬 prusnak commented on pull request "doc: remove mention of "proper signing key"":
(https://github.com/bitcoin/bitcoin/pull/27107#issuecomment-1433230441)
ACK 304ae6d
(https://github.com/bitcoin/bitcoin/pull/27107#issuecomment-1433230441)
ACK 304ae6d
💬 TheCharlatan commented on pull request "refactor / kernel: Move non-gArgs chainparams functionality to kernel":
(https://github.com/bitcoin/bitcoin/pull/26177#discussion_r1108608610)
I see, I'll use `dep`, because that's what is used elsewhere in the code.
(https://github.com/bitcoin/bitcoin/pull/26177#discussion_r1108608610)
I see, I'll use `dep`, because that's what is used elsewhere in the code.
💬 Willtech commented on issue "Setting `onlynet=onion` still makes some IPv4 connections.":
(https://github.com/bitcoin/bitcoin/issues/12344#issuecomment-1433251695)
I only sometimes look just for interest and occasionally find this issue unless network has been updated. It is not rare it just happens when it does.
(https://github.com/bitcoin/bitcoin/issues/12344#issuecomment-1433251695)
I only sometimes look just for interest and occasionally find this issue unless network has been updated. It is not rare it just happens when it does.
💬 fanquake commented on pull request "test/BIP324: functional tests for v2 P2P encryption":
(https://github.com/bitcoin/bitcoin/pull/24748#issuecomment-1433257883)
Moved to draft given it's based on multiple other PRs.
(https://github.com/bitcoin/bitcoin/pull/24748#issuecomment-1433257883)
Moved to draft given it's based on multiple other PRs.
📝 fanquake converted_to_draft a pull request: "test/BIP324: functional tests for v2 P2P encryption"
(https://github.com/bitcoin/bitcoin/pull/24748)
This PR introduces support for v2 P2P encryption(BIP 324) in the existing functional test framework and adds functional tests for the same.
It's built on top of:
* #24545
* #24005
The first 2 commits help the CI pass since the secp256k1 subtree directory is touched by the parent PR 24545.
The next 2 commits bring in the mentioned parent PRs. They'll all be removed when the parent PRs get merged.
### commits overview
1. Introducing cryptographic constructs(HKDF, ECDH, ChaCha20, Poly
...
(https://github.com/bitcoin/bitcoin/pull/24748)
This PR introduces support for v2 P2P encryption(BIP 324) in the existing functional test framework and adds functional tests for the same.
It's built on top of:
* #24545
* #24005
The first 2 commits help the CI pass since the secp256k1 subtree directory is touched by the parent PR 24545.
The next 2 commits bring in the mentioned parent PRs. They'll all be removed when the parent PRs get merged.
### commits overview
1. Introducing cryptographic constructs(HKDF, ECDH, ChaCha20, Poly
...
💬 instagibbs commented on pull request "[23.x] Backports":
(https://github.com/bitcoin/bitcoin/pull/26921#issuecomment-1433258574)
Only extra commit/change is af862661654966d5de614755ab9bd1b5913e0959
ACK https://github.com/bitcoin/bitcoin/pull/26921/commits/52376d9217060ce84e992e374d5dc2beae40bb06
(https://github.com/bitcoin/bitcoin/pull/26921#issuecomment-1433258574)
Only extra commit/change is af862661654966d5de614755ab9bd1b5913e0959
ACK https://github.com/bitcoin/bitcoin/pull/26921/commits/52376d9217060ce84e992e374d5dc2beae40bb06
💬 fanquake commented on pull request "Descriptor unit tests and simplifications":
(https://github.com/bitcoin/bitcoin/pull/24361#issuecomment-1433259806)
Drafted for now given it's (maybe) waiting on #26076.
(https://github.com/bitcoin/bitcoin/pull/24361#issuecomment-1433259806)
Drafted for now given it's (maybe) waiting on #26076.
📝 fanquake converted_to_draft a pull request: "Descriptor unit tests and simplifications"
(https://github.com/bitcoin/bitcoin/pull/24361)
Builds on top of #24343.
Adds additional tests, and makes `ToPrivateString()` always succeed, using pubkeys in case privkeys are unavailable.
(https://github.com/bitcoin/bitcoin/pull/24361)
Builds on top of #24343.
Adds additional tests, and makes `ToPrivateString()` always succeed, using pubkeys in case privkeys are unavailable.
💬 jamesob commented on pull request "script: add explanation for the functionality of each opcode":
(https://github.com/bitcoin/bitcoin/pull/27109#issuecomment-1433262848)
Concept ACK
Probably worth just squashing this down to 1 commit.
(https://github.com/bitcoin/bitcoin/pull/27109#issuecomment-1433262848)
Concept ACK
Probably worth just squashing this down to 1 commit.
⚠️ instagibbs opened an issue: "decodescript miniscript functionality tops out at 520 bytes"
(https://github.com/bitcoin/bitcoin/issues/27111)
After https://github.com/bitcoin/bitcoin/pull/27037 miniscript is extracted from compatible scripts, unless the `FillableSigningProvider::AddCScript` check for `MAX_SCRIPT_ELEMENT_SIZE` fails, which then it falls back to legacy behavior.
@achow101
(https://github.com/bitcoin/bitcoin/issues/27111)
After https://github.com/bitcoin/bitcoin/pull/27037 miniscript is extracted from compatible scripts, unless the `FillableSigningProvider::AddCScript` check for `MAX_SCRIPT_ELEMENT_SIZE` fails, which then it falls back to legacy behavior.
@achow101
💬 instagibbs commented on issue "decodescript miniscript functionality tops out at 520 bytes":
(https://github.com/bitcoin/bitcoin/issues/27111#issuecomment-1433265740)
Was testing out with a 11-of-15 script from mainnet:
```
5b21020e0338c96a8870479f2396c373cc7696ba124e8635d41b0ea581112b678172612102675333a4e4b8fb51d9d4e22fa5a8eaced3fdac8a8cbf9be8c030f75712e6af992102896807d54bc55c24981f24a453c60ad3e8993d693732288068a23df3d9f50d4821029e51a5ef5db3137051de8323b001749932f2ff0d34c82e96a2c2461de96ae56c2102a4e1a9638d46923272c266631d94d36bdb03a64ee0e14c7518e49d2f29bc401021031c41fdbcebe17bec8d49816e00ca1b5ac34766b91c9f2ac37d39c63e5e008afb2103079e252e85abffd3c401a69b087
...
(https://github.com/bitcoin/bitcoin/issues/27111#issuecomment-1433265740)
Was testing out with a 11-of-15 script from mainnet:
```
5b21020e0338c96a8870479f2396c373cc7696ba124e8635d41b0ea581112b678172612102675333a4e4b8fb51d9d4e22fa5a8eaced3fdac8a8cbf9be8c030f75712e6af992102896807d54bc55c24981f24a453c60ad3e8993d693732288068a23df3d9f50d4821029e51a5ef5db3137051de8323b001749932f2ff0d34c82e96a2c2461de96ae56c2102a4e1a9638d46923272c266631d94d36bdb03a64ee0e14c7518e49d2f29bc401021031c41fdbcebe17bec8d49816e00ca1b5ac34766b91c9f2ac37d39c63e5e008afb2103079e252e85abffd3c401a69b087
...
👍 ryanofsky approved a pull request: "validation: Improve error handling when VerifyDB fails due to insufficient dbcache"
(https://github.com/bitcoin/bitcoin/pull/25574)
(https://github.com/bitcoin/bitcoin/pull/25574)
💬 ryanofsky commented on pull request "validation: Improve error handling when VerifyDB fails due to insufficient dbcache":
(https://github.com/bitcoin/bitcoin/pull/25574#discussion_r1107700526)
In commit "init, validation: Improve handling if VerifyDB() fails due to insufficient dbcache" (cf307244db97e7b9e29a3d0ff948a6fa7877b548)
This commit seems to change behavior by no longer logging `No coin database inconsistencies in last %i blocks` in the case where `skipped_l3_checks` is true. I think it would be still be good to log the number of blocks checked and that no inconsistencies were found. Perhaps the easiest way to do that would be to revert the other changes to this function in
...
(https://github.com/bitcoin/bitcoin/pull/25574#discussion_r1107700526)
In commit "init, validation: Improve handling if VerifyDB() fails due to insufficient dbcache" (cf307244db97e7b9e29a3d0ff948a6fa7877b548)
This commit seems to change behavior by no longer logging `No coin database inconsistencies in last %i blocks` in the case where `skipped_l3_checks` is true. I think it would be still be good to log the number of blocks checked and that no inconsistencies were found. Perhaps the easiest way to do that would be to revert the other changes to this function in
...
💬 ryanofsky commented on pull request "validation: Improve error handling when VerifyDB fails due to insufficient dbcache":
(https://github.com/bitcoin/bitcoin/pull/25574#discussion_r1108588225)
42b192f0cbf9c04da111145c921344b0881b3ce3
Would suggest dropping two `ERROR_` prefixes here, because reason for inconsistent prefixes is not very clear, and the decision about whether to treat different conditions as errors should depend on the caller, not the implementation.
I think a simple `{ SUCCESS, CORRUPTED_BLOCK_DB, INSUFFICIENT_CACHE, INSUFFICIENT_BLOCK_DATA }` result would suffice.
You could also go further and rename `INSUFFICIENT_CACHE` to `SKIPPED_L3_CHECKS` and rename `INSU
...
(https://github.com/bitcoin/bitcoin/pull/25574#discussion_r1108588225)
42b192f0cbf9c04da111145c921344b0881b3ce3
Would suggest dropping two `ERROR_` prefixes here, because reason for inconsistent prefixes is not very clear, and the decision about whether to treat different conditions as errors should depend on the caller, not the implementation.
I think a simple `{ SUCCESS, CORRUPTED_BLOCK_DB, INSUFFICIENT_CACHE, INSUFFICIENT_BLOCK_DATA }` result would suffice.
You could also go further and rename `INSUFFICIENT_CACHE` to `SKIPPED_L3_CHECKS` and rename `INSU
...
💬 ryanofsky commented on pull request "validation: Improve error handling when VerifyDB fails due to insufficient dbcache":
(https://github.com/bitcoin/bitcoin/pull/25574#discussion_r1108613248)
In commit "validation: Change return value of VerifyDB to enum type" (422127b696f390e7cd34e897e7d74f09e3dae2cb)
In the interest of preventing future bugs where shutdown could be interpreted as success, it seems like it would be less misleading (and easy) to return a `VerifyDBResult::INTERRUPTED` error code here, analogous to `ChainstateLoadStatus::INTERRUPTED`.
(https://github.com/bitcoin/bitcoin/pull/25574#discussion_r1108613248)
In commit "validation: Change return value of VerifyDB to enum type" (422127b696f390e7cd34e897e7d74f09e3dae2cb)
In the interest of preventing future bugs where shutdown could be interpreted as success, it seems like it would be less misleading (and easy) to return a `VerifyDBResult::INTERRUPTED` error code here, analogous to `ChainstateLoadStatus::INTERRUPTED`.
💬 ryanofsky commented on pull request "validation: Improve error handling when VerifyDB fails due to insufficient dbcache":
(https://github.com/bitcoin/bitcoin/pull/25574#discussion_r1108607226)
In commit "init, validation: Improve handling if VerifyDB() fails due to insufficient dbcache" (cf307244db97e7b9e29a3d0ff948a6fa7877b548)
This block of code seems to duplicate a block of code in `AppInitMain`, but in the `AppInitMain` block this PR is setting `fail_on_insufficient_dbcache` to `args.IsArgSet("-checkblocks") || args.IsArgSet("-checklevel")`, while here it is being set to `false`. Probably both blocks of code should be deduplicated and call a shared function later. But for now i
...
(https://github.com/bitcoin/bitcoin/pull/25574#discussion_r1108607226)
In commit "init, validation: Improve handling if VerifyDB() fails due to insufficient dbcache" (cf307244db97e7b9e29a3d0ff948a6fa7877b548)
This block of code seems to duplicate a block of code in `AppInitMain`, but in the `AppInitMain` block this PR is setting `fail_on_insufficient_dbcache` to `args.IsArgSet("-checkblocks") || args.IsArgSet("-checklevel")`, while here it is being set to `false`. Probably both blocks of code should be deduplicated and call a shared function later. But for now i
...