💬 mzumsande commented on issue "Stuck in Endless Pre-Syncing Headers Loop":
(https://github.com/bitcoin/bitcoin/issues/26391#issuecomment-2654838081)
> A question is whether we could detect this during the pre-sync phase instead, which wouldn't stop the lack of progress, but would avoid the bandwidth waste on repeated presyncs with everyone. The answer is yes - it wouldn't be hard to check for known-invalid headers in the presync phase as well, however, I don't think we want to do that because of fingerprinting reasons: it would permit an attacker to feed you an invalid, low-PoW, block during IBD, and then later follow you around the network
...
(https://github.com/bitcoin/bitcoin/issues/26391#issuecomment-2654838081)
> A question is whether we could detect this during the pre-sync phase instead, which wouldn't stop the lack of progress, but would avoid the bandwidth waste on repeated presyncs with everyone. The answer is yes - it wouldn't be hard to check for known-invalid headers in the presync phase as well, however, I don't think we want to do that because of fingerprinting reasons: it would permit an attacker to feed you an invalid, low-PoW, block during IBD, and then later follow you around the network
...
💬 sipa commented on issue "Stuck in Endless Pre-Syncing Headers Loop":
(https://github.com/bitcoin/bitcoin/issues/26391#issuecomment-2654843751)
@mzumsande Agreed, reading back what I wrote, I don't see why I thought fingerprinting would be an issue.
I think the most useful course of actual here is detecting the presence of a high-PoW header-invalid chain, and reporting it to the user as a sign of likely corruption. Unsure what that would mean for anything other than the GUI, though.
(https://github.com/bitcoin/bitcoin/issues/26391#issuecomment-2654843751)
@mzumsande Agreed, reading back what I wrote, I don't see why I thought fingerprinting would be an issue.
I think the most useful course of actual here is detecting the presence of a high-PoW header-invalid chain, and reporting it to the user as a sign of likely corruption. Unsure what that would mean for anything other than the GUI, though.
🤔 brunoerg reviewed a pull request: "descriptors: inference process, do not return unparsable multisig descriptors"
(https://github.com/bitcoin/bitcoin/pull/31404#pullrequestreview-2613204326)
It seems `InferDescriptor` can still produce invalid descriptors due to invalid keys. e.g.:
Hex script: `5141046161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161610051ae`
descriptor (invalid): `multi(1,0461616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616100)#jqqp7208`.
(https://github.com/bitcoin/bitcoin/pull/31404#pullrequestreview-2613204326)
It seems `InferDescriptor` can still produce invalid descriptors due to invalid keys. e.g.:
Hex script: `5141046161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161610051ae`
descriptor (invalid): `multi(1,0461616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616161616100)#jqqp7208`.
💬 ismaelsadeeq commented on pull request "cluster mempool: introduce TxGraph":
(https://github.com/bitcoin/bitcoin/pull/31363#discussion_r1953242843)
In "txgraph: (tests) add simulation fuzz test" 1f06bc1e4a8108f1430bcd20fc391c9f663a2e4b
why add 1 and then subtract again when using the choices value?
(https://github.com/bitcoin/bitcoin/pull/31363#discussion_r1953242843)
In "txgraph: (tests) add simulation fuzz test" 1f06bc1e4a8108f1430bcd20fc391c9f663a2e4b
why add 1 and then subtract again when using the choices value?
💬 ismaelsadeeq commented on pull request "cluster mempool: introduce TxGraph":
(https://github.com/bitcoin/bitcoin/pull/31363#discussion_r1953271770)
In "txgraph: (tests) add simulation fuzz test" 1f06bc1e4a8108f1430bcd20fc391c9f663a2e4b
Should we return early when parent ref is the same as ref child? This will prevent a possibility of having a cycle.
As such, we don’t have to check before adding dependency in the fuzz test.
(https://github.com/bitcoin/bitcoin/pull/31363#discussion_r1953271770)
In "txgraph: (tests) add simulation fuzz test" 1f06bc1e4a8108f1430bcd20fc391c9f663a2e4b
Should we return early when parent ref is the same as ref child? This will prevent a possibility of having a cycle.
As such, we don’t have to check before adding dependency in the fuzz test.
💬 ismaelsadeeq commented on pull request "cluster mempool: introduce TxGraph":
(https://github.com/bitcoin/bitcoin/pull/31363#discussion_r1953307646)
In "txgraph: (tests) add simulation fuzz test" 1f06bc1e4a8108f1430bcd20fc391c9f663a2e4b
This loop is quite big, has series of steps that are depending if behavior of the if else branches.
Will be nice to add an overview of how the test works, just like it was done in `cluster_linearize.cpp`
(https://github.com/bitcoin/bitcoin/pull/31363#discussion_r1953307646)
In "txgraph: (tests) add simulation fuzz test" 1f06bc1e4a8108f1430bcd20fc391c9f663a2e4b
This loop is quite big, has series of steps that are depending if behavior of the if else branches.
Will be nice to add an overview of how the test works, just like it was done in `cluster_linearize.cpp`
💬 ismaelsadeeq commented on pull request "cluster mempool: introduce TxGraph":
(https://github.com/bitcoin/bitcoin/pull/31363#discussion_r1953392682)
In "txgraph: (tests) add internal sanity check function" 741a6a8c4d851cb10ecee810a09187bcbfa5af4c
Should we also compare that the list of transactions in chunk 0 matches `m_linearization` subset from from previous hit of the conditional statement to `lin_pos`?
(https://github.com/bitcoin/bitcoin/pull/31363#discussion_r1953392682)
In "txgraph: (tests) add internal sanity check function" 741a6a8c4d851cb10ecee810a09187bcbfa5af4c
Should we also compare that the list of transactions in chunk 0 matches `m_linearization` subset from from previous hit of the conditional statement to `lin_pos`?
💬 ismaelsadeeq commented on pull request "cluster mempool: introduce TxGraph":
(https://github.com/bitcoin/bitcoin/pull/31363#discussion_r1953384628)
In "txgraph: (tests) add internal sanity check function" 741a6a8c4d851cb10ecee810a09187bcbfa5af4c
What is `m_wiped`
(https://github.com/bitcoin/bitcoin/pull/31363#discussion_r1953384628)
In "txgraph: (tests) add internal sanity check function" 741a6a8c4d851cb10ecee810a09187bcbfa5af4c
What is `m_wiped`
💬 ismaelsadeeq commented on pull request "cluster mempool: introduce TxGraph":
(https://github.com/bitcoin/bitcoin/pull/31363#discussion_r1953022822)
In "txgraph: (tests) add simulation fuzz test" 1f06bc1e4a8108f1430bcd20fc391c9f663a2e4b
using real in the comments here is a bit confusing as you represent the actual tx graph as real
(https://github.com/bitcoin/bitcoin/pull/31363#discussion_r1953022822)
In "txgraph: (tests) add simulation fuzz test" 1f06bc1e4a8108f1430bcd20fc391c9f663a2e4b
using real in the comments here is a bit confusing as you represent the actual tx graph as real
💬 sipa commented on pull request "cluster mempool: introduce TxGraph":
(https://github.com/bitcoin/bitcoin/pull/31363#discussion_r1953411421)
It follows from the property that PostLinearize results in chunks that do not consist of multiple disconnected components: if a higher-feerate chunk is appended to a lower-feerate one, it must mean they get separated.
I'm happy to elaborate briefly in this comment, but perhaps this is also not the place to provide deeper more theoretical reasons why the properties hold.
(https://github.com/bitcoin/bitcoin/pull/31363#discussion_r1953411421)
It follows from the property that PostLinearize results in chunks that do not consist of multiple disconnected components: if a higher-feerate chunk is appended to a lower-feerate one, it must mean they get separated.
I'm happy to elaborate briefly in this comment, but perhaps this is also not the place to provide deeper more theoretical reasons why the properties hold.
💬 andrewtoth commented on pull request "Add -pruneduringinit option to temporarily use another prune target during IBD":
(https://github.com/bitcoin/bitcoin/pull/31845#issuecomment-2654905595)
Can't we just do an IBD without pruning, then shutdown and restart with a prune height set? Or just call `pruneblockchain` after we are done IBD?
(https://github.com/bitcoin/bitcoin/pull/31845#issuecomment-2654905595)
Can't we just do an IBD without pruning, then shutdown and restart with a prune height set? Or just call `pruneblockchain` after we are done IBD?
💬 murchandamus commented on issue "Update BnB upper bound to use `min_viable_change`":
(https://github.com/bitcoin/bitcoin/issues/26466#issuecomment-2654913988)
That’s a great idea!
(https://github.com/bitcoin/bitcoin/issues/26466#issuecomment-2654913988)
That’s a great idea!
💬 sipa commented on pull request "cluster mempool: introduce TxGraph":
(https://github.com/bitcoin/bitcoin/pull/31363#discussion_r1953458099)
If we're going to expend run-time cost to perform the check, we might as well drop the requirement that Ref refers to this graph entirely. Given that we're not expecting to ever have multiple TxGraph objects in production, this feels like overkill.
(https://github.com/bitcoin/bitcoin/pull/31363#discussion_r1953458099)
If we're going to expend run-time cost to perform the check, we might as well drop the requirement that Ref refers to this graph entirely. Given that we're not expecting to ever have multiple TxGraph objects in production, this feels like overkill.
💬 sipa commented on pull request "cluster mempool: introduce TxGraph":
(https://github.com/bitcoin/bitcoin/pull/31363#discussion_r1953460271)
I'd rather not refer to implementation details in the public interface.
(https://github.com/bitcoin/bitcoin/pull/31363#discussion_r1953460271)
I'd rather not refer to implementation details in the public interface.
💬 murchandamus commented on issue "wallet: Branch and Bound producing change":
(https://github.com/bitcoin/bitcoin/issues/31830#issuecomment-2654938578)
The issue appears to have been introduced by myself in #28366 in the commit https://github.com/bitcoin/bitcoin/pull/28366/commits/7aa7e30441fe77bf8e8092916e36b004bbbfe2a7#diff-d473ed8396f9451afb848923cfcfaa630c9811a78e07f3ae1ffd3a65da218accR796.
Branch and Bound uses `target + cost_of_change` as the upper bound inclusive of the solution space. In the mentioned PR, it also uses "cost_of_change` as the value for `min_viable_change` in the `RecalculateWaste` call. However, `min_viable_change` den
...
(https://github.com/bitcoin/bitcoin/issues/31830#issuecomment-2654938578)
The issue appears to have been introduced by myself in #28366 in the commit https://github.com/bitcoin/bitcoin/pull/28366/commits/7aa7e30441fe77bf8e8092916e36b004bbbfe2a7#diff-d473ed8396f9451afb848923cfcfaa630c9811a78e07f3ae1ffd3a65da218accR796.
Branch and Bound uses `target + cost_of_change` as the upper bound inclusive of the solution space. In the mentioned PR, it also uses "cost_of_change` as the value for `min_viable_change` in the `RecalculateWaste` call. However, `min_viable_change` den
...
💬 mzumsande commented on issue "Stuck in Endless Pre-Syncing Headers Loop":
(https://github.com/bitcoin/bitcoin/issues/26391#issuecomment-2654943412)
> I think the most useful course of actual here is detecting the presence of a high-PoW header-invalid chain, and reporting it to the user as a sign of likely corruption. Unsure what that would mean for anything other than the GUI, though.
We actually have this kind of check (`CheckForkWarningConditions`) but have it disabled during IBD for no good reason I can see (at least after #19905). I'll work on a PR suggesting to use this check in the pre-sync header loop situation (and also rework it,
...
(https://github.com/bitcoin/bitcoin/issues/26391#issuecomment-2654943412)
> I think the most useful course of actual here is detecting the presence of a high-PoW header-invalid chain, and reporting it to the user as a sign of likely corruption. Unsure what that would mean for anything other than the GUI, though.
We actually have this kind of check (`CheckForkWarningConditions`) but have it disabled during IBD for no good reason I can see (at least after #19905). I'll work on a PR suggesting to use this check in the pre-sync header loop situation (and also rework it,
...
💬 fanquake commented on pull request "ci: switch MSAN to use prebuilt Clang binaries":
(https://github.com/bitcoin/bitcoin/pull/31850#discussion_r1953473331)
https://github.com/llvm/llvm-project/releases/tag/llvmorg-20.1.0-rc2
(https://github.com/bitcoin/bitcoin/pull/31850#discussion_r1953473331)
https://github.com/llvm/llvm-project/releases/tag/llvmorg-20.1.0-rc2
💬 sipa commented on pull request "cluster mempool: introduce TxGraph":
(https://github.com/bitcoin/bitcoin/pull/31363#discussion_r1953481542)
Indeed.
(https://github.com/bitcoin/bitcoin/pull/31363#discussion_r1953481542)
Indeed.
💬 sipa commented on pull request "cluster mempool: introduce TxGraph":
(https://github.com/bitcoin/bitcoin/pull/31363#discussion_r1953484318)
It's a tiny number, as it's bounded by $\mathcal{O}(\log n)$, so it didn't feel right to use the same `DepGraphIndex` or `GraphIndex` type.
(https://github.com/bitcoin/bitcoin/pull/31363#discussion_r1953484318)
It's a tiny number, as it's bounded by $\mathcal{O}(\log n)$, so it didn't feel right to use the same `DepGraphIndex` or `GraphIndex` type.
⚠️ InnDe opened an issue: "Wallet passpharse"
(https://github.com/bitcoin/bitcoin/issues/31852)
### Is there an existing issue for this?
- [x] I have searched the existing issues
### Current behaviour
I’m using a passphrase for my encrypted wallets—8 in total.
This passphrase is: (space “thekey” \ \ \ \ \ \ \ \ \ \ \ \ \)
“ \ \ \ \ \ \ \ \ \ \ \ \ \)”
I use this passphrase for all my wallets (total: 2.4 BTC), but now it doesn’t work as usual. I tried creating a new wallet with the same passphrase, and I encountered the same issue.
You can review my question and all the comments her
...
(https://github.com/bitcoin/bitcoin/issues/31852)
### Is there an existing issue for this?
- [x] I have searched the existing issues
### Current behaviour
I’m using a passphrase for my encrypted wallets—8 in total.
This passphrase is: (space “thekey” \ \ \ \ \ \ \ \ \ \ \ \ \)
“ \ \ \ \ \ \ \ \ \ \ \ \ \)”
I use this passphrase for all my wallets (total: 2.4 BTC), but now it doesn’t work as usual. I tried creating a new wallet with the same passphrase, and I encountered the same issue.
You can review my question and all the comments her
...
👍1