📝 sedited opened a pull request: "net processing: Add ibd check before processing block for txdownloadman"
(https://github.com/bitcoin/bitcoin/pull/34054)
Calculating the rolling bloom filters takes some CPU time from the scheduler thread. This can be observed for example in [this flamegraph](https://bitcoin-dev-tools.github.io/benchcoin/results/pr-172/20066462508/mainnet-default-instrumented-base-flamegraph.svg?x=920203898521&y=780), where handling the filter takes about 2.6% of total time.
During ibd the entries in the tx download bloom filter are just continuously rolled over, and aren't consumed, since no mempool is maintained at the time.
...
(https://github.com/bitcoin/bitcoin/pull/34054)
Calculating the rolling bloom filters takes some CPU time from the scheduler thread. This can be observed for example in [this flamegraph](https://bitcoin-dev-tools.github.io/benchcoin/results/pr-172/20066462508/mainnet-default-instrumented-base-flamegraph.svg?x=920203898521&y=780), where handling the filter takes about 2.6% of total time.
During ibd the entries in the tx download bloom filter are just continuously rolled over, and aren't consumed, since no mempool is maintained at the time.
...
🤔 Eunovo reviewed a pull request: "Implementation of SwiftSync"
(https://github.com/bitcoin/bitcoin/pull/34004#pullrequestreview-3570977196)
Since this SwiftSync implementation uses `assumevalid` assumptions, it makes sense to reason about attacks that apply to `assumevalid`, here as well.
What happens if an attack gives a SwiftSync node a malicious hint file and eclipses the node so that it is denied an honest chain? Assumevalid uses:
- the minimumchain work check to ensure that scripts are verified when the node is denied access to "any chain at least as good as the expected chain"
- the equivalent time check to force the attacker
...
(https://github.com/bitcoin/bitcoin/pull/34004#pullrequestreview-3570977196)
Since this SwiftSync implementation uses `assumevalid` assumptions, it makes sense to reason about attacks that apply to `assumevalid`, here as well.
What happens if an attack gives a SwiftSync node a malicious hint file and eclipses the node so that it is denied an honest chain? Assumevalid uses:
- the minimumchain work check to ensure that scripts are verified when the node is denied access to "any chain at least as good as the expected chain"
- the equivalent time check to force the attacker
...
💬 Ataraxia009 commented on pull request "log: Remove brittle and confusing LogPrintLevel":
(https://github.com/bitcoin/bitcoin/pull/34051#discussion_r2613617565)
sounds good
(https://github.com/bitcoin/bitcoin/pull/34051#discussion_r2613617565)
sounds good
💬 maflcko commented on pull request "p2p: avoid traversing blocks (twice) during IBD":
(https://github.com/bitcoin/bitcoin/pull/32730#discussion_r2613636332)
this was picked in https://github.com/bitcoin/bitcoin/pull/34054, so i guess this can be closed again?
(https://github.com/bitcoin/bitcoin/pull/32730#discussion_r2613636332)
this was picked in https://github.com/bitcoin/bitcoin/pull/34054, so i guess this can be closed again?
💬 stratospher commented on issue "qa: Assertion in `p2p_v2_misbehaving.py`":
(https://github.com/bitcoin/bitcoin/issues/34035#issuecomment-3645779184)
ok looks like python event loop asynchronous behaviour.
- it's scheduling connection to `peer1` and then scheduling a connection to `peer2`
- but it is actually connecting to `peer2` first (which gets assigned peer id = 1 on c++ side) and only later connecting to the peer we're interested in `peer1` (which gets assigned peer id = 2 on c++ side) and we're seeing a log mismatch
ideally, we let the event loop do whatever it wants and then use the actual peer id in `expected_debug_message` - as in
...
(https://github.com/bitcoin/bitcoin/issues/34035#issuecomment-3645779184)
ok looks like python event loop asynchronous behaviour.
- it's scheduling connection to `peer1` and then scheduling a connection to `peer2`
- but it is actually connecting to `peer2` first (which gets assigned peer id = 1 on c++ side) and only later connecting to the peer we're interested in `peer1` (which gets assigned peer id = 2 on c++ side) and we're seeing a log mismatch
ideally, we let the event loop do whatever it wants and then use the actual peer id in `expected_debug_message` - as in
...
⚠️ Manishdudi700 opened an issue: "Showing error:- Error unlocking wallet: some keys decrypt but not all. Your wallet file may be corrupt."
(https://github.com/bitcoin/bitcoin/issues/34055)
When i am trying to transfer funds from my bitcoin core wallet it is showing a error which says:- Error unlocking wallet: some keys decrypt but not all. Your wallet file may be corrupt.
It is also showing the same error in console of bitcoin core app after entering passphrase when i tried to dumpwallet
1. Does it me
...
(https://github.com/bitcoin/bitcoin/issues/34055)
When i am trying to transfer funds from my bitcoin core wallet it is showing a error which says:- Error unlocking wallet: some keys decrypt but not all. Your wallet file may be corrupt.
It is also showing the same error in console of bitcoin core app after entering passphrase when i tried to dumpwallet
1. Does it me
...
💬 stickies-v commented on pull request "log: Remove brittle and confusing LogPrintLevel":
(https://github.com/bitcoin/bitcoin/pull/34051#discussion_r2613683807)
> I've added a new `LogDebug(BCLog::HTTP,` in the line below, to clarify this in the test itself.
That just made me wonder "wait why does this pass" rather than make it obvious that this was enabled in `LogSetup`. It's confusing because in normal operation `Info` is the default level. I think the below diff is all that's needed to make this test self-documenting.
<details>
<summary>git diff on fa35682637</summary>
```diff
diff --git a/src/test/logging_tests.cpp b/src/test/logging_test
...
(https://github.com/bitcoin/bitcoin/pull/34051#discussion_r2613683807)
> I've added a new `LogDebug(BCLog::HTTP,` in the line below, to clarify this in the test itself.
That just made me wonder "wait why does this pass" rather than make it obvious that this was enabled in `LogSetup`. It's confusing because in normal operation `Info` is the default level. I think the below diff is all that's needed to make this test self-documenting.
<details>
<summary>git diff on fa35682637</summary>
```diff
diff --git a/src/test/logging_tests.cpp b/src/test/logging_test
...
💬 maflcko commented on pull request "net processing: Add ibd check before processing block for txdownloadman":
(https://github.com/bitcoin/bitcoin/pull/34054#issuecomment-3645846544)
> During ibd the entries in the tx download bloom filter are just continuously rolled over, and aren't consumed, since no mempool is maintained at the time.
I think this could be clarified a bit, why this is safe. My understanding is that a mempool does exist and is maintained (albeit it will normally be empty). The mempool will also happily accept transactions via RPC, or the wallet. However, this is fine, because they don't interact with the orphanage and the txdownloadman, because adding a
...
(https://github.com/bitcoin/bitcoin/pull/34054#issuecomment-3645846544)
> During ibd the entries in the tx download bloom filter are just continuously rolled over, and aren't consumed, since no mempool is maintained at the time.
I think this could be clarified a bit, why this is safe. My understanding is that a mempool does exist and is maintained (albeit it will normally be empty). The mempool will also happily accept transactions via RPC, or the wallet. However, this is fine, because they don't interact with the orphanage and the txdownloadman, because adding a
...
💬 maflcko commented on pull request "log: Remove brittle and confusing LogPrintLevel":
(https://github.com/bitcoin/bitcoin/pull/34051#discussion_r2613733349)
Thanks for providing a diff. I've pushed a commit with that:
```diff
$ git diff fa35682637 fa33a9f5be -U0
diff --git a/src/test/logging_tests.cpp b/src/test/logging_tests.cpp
index 3d95c97426..2a34aefae1 100644
--- a/src/test/logging_tests.cpp
+++ b/src/test/logging_tests.cpp
@@ -187,0 +188 @@ BOOST_FIXTURE_TEST_CASE(logging_SeverityLevels, LogSetup)
+ LogInstance().SetLogLevel(BCLog::Level::Debug);
(https://github.com/bitcoin/bitcoin/pull/34051#discussion_r2613733349)
Thanks for providing a diff. I've pushed a commit with that:
```diff
$ git diff fa35682637 fa33a9f5be -U0
diff --git a/src/test/logging_tests.cpp b/src/test/logging_tests.cpp
index 3d95c97426..2a34aefae1 100644
--- a/src/test/logging_tests.cpp
+++ b/src/test/logging_tests.cpp
@@ -187,0 +188 @@ BOOST_FIXTURE_TEST_CASE(logging_SeverityLevels, LogSetup)
+ LogInstance().SetLogLevel(BCLog::Level::Debug);
💬 maflcko commented on issue "Showing error:- Error unlocking wallet: some keys decrypt but not all. Your wallet file may be corrupt.":
(https://github.com/bitcoin/bitcoin/issues/34055#issuecomment-3645935489)
> 1. Does it means mine wallet.dat is corrupt?
>
> 2. Password is wrong ?
>
> 3. bitcoin core is not synced fully ?
>
>
> Please Help me
When it says "Error unlocking wallet: some keys decrypt but not all. Your wallet file may be corrupt.", then it means "Error unlocking wallet: some keys decrypt but not all. Your wallet file may be corrupt.".
You can continue at your own risk, but my recommendation would be to backup your wallet.dat, and also restore an old backup and double che
...
(https://github.com/bitcoin/bitcoin/issues/34055#issuecomment-3645935489)
> 1. Does it means mine wallet.dat is corrupt?
>
> 2. Password is wrong ?
>
> 3. bitcoin core is not synced fully ?
>
>
> Please Help me
When it says "Error unlocking wallet: some keys decrypt but not all. Your wallet file may be corrupt.", then it means "Error unlocking wallet: some keys decrypt but not all. Your wallet file may be corrupt.".
You can continue at your own risk, but my recommendation would be to backup your wallet.dat, and also restore an old backup and double che
...
💬 maflcko commented on issue "Showing error:- Error unlocking wallet: some keys decrypt but not all. Your wallet file may be corrupt.":
(https://github.com/bitcoin/bitcoin/issues/34055#issuecomment-3645936203)
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, or one of the Bitcoin subreddits, or any other place that you feel is well suited.
(https://github.com/bitcoin/bitcoin/issues/34055#issuecomment-3645936203)
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, or one of the Bitcoin subreddits, or any other place that you feel is well suited.
✅ maflcko closed an issue: "Showing error:- Error unlocking wallet: some keys decrypt but not all. Your wallet file may be corrupt."
(https://github.com/bitcoin/bitcoin/issues/34055)
(https://github.com/bitcoin/bitcoin/issues/34055)
✅ fanquake closed a pull request: "p2p: avoid traversing blocks (twice) during IBD"
(https://github.com/bitcoin/bitcoin/pull/32730)
(https://github.com/bitcoin/bitcoin/pull/32730)
🚀 fanquake merged a pull request: "net: Waste less time in socket handling"
(https://github.com/bitcoin/bitcoin/pull/34025)
(https://github.com/bitcoin/bitcoin/pull/34025)
💬 stickies-v commented on pull request "log: Remove brittle and confusing LogPrintLevel":
(https://github.com/bitcoin/bitcoin/pull/34051#issuecomment-3645987726)
re-ACK fa33a9f5be1beabfee6c217d133cc51dea3b9a63
No changes except improved self-documentation of test, ty!
(https://github.com/bitcoin/bitcoin/pull/34051#issuecomment-3645987726)
re-ACK fa33a9f5be1beabfee6c217d133cc51dea3b9a63
No changes except improved self-documentation of test, ty!
👍 rkrux approved a pull request: "log: Remove brittle and confusing LogPrintLevel"
(https://github.com/bitcoin/bitcoin/pull/34051#pullrequestreview-3571354142)
ACK fa33a9f5be1beabfee6c217d133cc51dea3b9a63
This is helpful. Having gone through the logging code recently, I found myself confused a bit upon seeing different ways to log. Removal of `LogPrintLevel` helps in decreasing the confusion by just using `LOG<$LEVEL>()` macros.
(https://github.com/bitcoin/bitcoin/pull/34051#pullrequestreview-3571354142)
ACK fa33a9f5be1beabfee6c217d133cc51dea3b9a63
This is helpful. Having gone through the logging code recently, I found myself confused a bit upon seeing different ways to log. Removal of `LogPrintLevel` helps in decreasing the confusion by just using `LOG<$LEVEL>()` macros.
💬 stickies-v commented on pull request "lint: Remove confusing, redundant, and brittle lint-spelling":
(https://github.com/bitcoin/bitcoin/pull/34053#issuecomment-3645998330)
Concept ACK. Didn't seem to be a useful test, so best to remove it.
(https://github.com/bitcoin/bitcoin/pull/34053#issuecomment-3645998330)
Concept ACK. Didn't seem to be a useful test, so best to remove it.
📝 optout21 converted_to_draft a pull request: "test: Add test on skip heights in CBlockIndex"
(https://github.com/bitcoin/bitcoin/pull/33661)
The skip height values, used by `CBlockIndex::BuildSkip()` and `GetSkipHeight` are not tested and not well documented. (noticed while reviewing #33515, recently merged).
The motivation is to document the skip value computation through a test.
The first commit adds a test for the properties of the distribution of skip values, namely that they have non-uniform distribution: most values are small but there are some large ones as well.
The second commit adds low-level tests to the `GetSkipH
...
(https://github.com/bitcoin/bitcoin/pull/33661)
The skip height values, used by `CBlockIndex::BuildSkip()` and `GetSkipHeight` are not tested and not well documented. (noticed while reviewing #33515, recently merged).
The motivation is to document the skip value computation through a test.
The first commit adds a test for the properties of the distribution of skip values, namely that they have non-uniform distribution: most values are small but there are some large ones as well.
The second commit adds low-level tests to the `GetSkipH
...
⚠️ husstege-e opened an issue: "Wallet RPC semantics: gettransaction.confirmations returns large negative values for RBF-replaced transactions (undocumented behavior)"
(https://github.com/bitcoin/bitcoin/issues/34056)
### Is there an existing issue for this?
- [x] I have searched the existing issues
### Current behaviour
When using Replace-By-Fee (RBF) in combination with wallet rescans, the wallet RPC reports transactions with negative confirmation counts (e.g. -900, -1200), long after the replacing transaction has been confirmed.
These negative confirmation values persist even when:
- the replacement transaction is deeply confirmed
- the original transaction is fully conflicted and no longer in the memp
...
(https://github.com/bitcoin/bitcoin/issues/34056)
### Is there an existing issue for this?
- [x] I have searched the existing issues
### Current behaviour
When using Replace-By-Fee (RBF) in combination with wallet rescans, the wallet RPC reports transactions with negative confirmation counts (e.g. -900, -1200), long after the replacing transaction has been confirmed.
These negative confirmation values persist even when:
- the replacement transaction is deeply confirmed
- the original transaction is fully conflicted and no longer in the memp
...
👍 rkrux approved a pull request: "lint: Remove confusing, redundant, and brittle lint-spelling"
(https://github.com/bitcoin/bitcoin/pull/34053#pullrequestreview-3571429071)
ACK fa904fc683c0892eb800ddb986fdc0c646721077
(https://github.com/bitcoin/bitcoin/pull/34053#pullrequestreview-3571429071)
ACK fa904fc683c0892eb800ddb986fdc0c646721077
💬 Ataraxia009 commented on pull request "log: Remove brittle and confusing LogPrintLevel":
(https://github.com/bitcoin/bitcoin/pull/34051#discussion_r2613872871)
I still don't see the point of leaving the `Assume` in there since it seems to make no difference other than constricting somebody that would want to create a new macro for `LogInfo` w category. But i wont hold the PR back because of it.
(https://github.com/bitcoin/bitcoin/pull/34051#discussion_r2613872871)
I still don't see the point of leaving the `Assume` in there since it seems to make no difference other than constricting somebody that would want to create a new macro for `LogInfo` w category. But i wont hold the PR back because of it.