Bitcoin Core Github
44 subscribers
121K links
Download Telegram
⚠️ Sjors opened an issue: "v27 node thinks my v28 / master block database is corrupted"
(https://github.com/bitcoin/bitcoin/issues/31286)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

I'm unable to start Bitcoin Core v27.1 or v27.2 (haven't tried older) on my newly installed node. The node works with v28.0 and on recent master (2b33322169bc).

I never set `-blocksxor`.

I did use assume utxo during the initial sync. I haven't tried (yet) to sync again and see if that matters.

The initial sync was done on master, I haven't tried doing the sync with the release ins
...
πŸ’¬ Sjors commented on issue "v27 node thinks my v28 / master block database is corrupted":
(https://github.com/bitcoin/bitcoin/issues/31286#issuecomment-2474736673)
Although I did not use `-blocksxor` there is a `xor.dat` file in my blocks dir.

Here's the block, undo and xor file: https://download.sprovoost.nl/download.php?id=10&token=34c0e1dcec8e6d096c2f25ebfe8f59f2
πŸ’¬ maflcko commented on issue "v27 node thinks my v28 / master block database is corrupted":
(https://github.com/bitcoin/bitcoin/issues/31286#issuecomment-2474746949)
> Although I did not use `-blocksxor` there is a `xor.dat` file in my blocks dir.

This is expected. You can print the help (and the default values) of args with the `-help` arg.
πŸ’¬ maflcko commented on issue "v27 node thinks my v28 / master block database is corrupted":
(https://github.com/bitcoin/bitcoin/issues/31286#issuecomment-2474750672)
I can see the point of delaying the default value by one release, but creating a blocksdir with the most recent release, only to downgrade seems an edge case.
πŸ’¬ Sjors commented on issue "v27 node thinks my v28 / master block database is corrupted":
(https://github.com/bitcoin/bitcoin/issues/31286#issuecomment-2474753134)
Too late now, but I think the release notes should have made it more clear that you can't downgrade.

https://bitcoincore.org/en/releases/28.0/

Burried under "Low-level Changes":

> Blockstorage

> Block files are now XOR’d by default with a key stored in the blocksdir. Previous releases of Bitcoin Core or previous external software will not be able to read the blocksdir with a non-zero XOR-key.
βœ… Sjors closed an issue: "v27 node thinks my v28 / master block database is corrupted"
(https://github.com/bitcoin/bitcoin/issues/31286)
πŸ’¬ Sjors commented on issue "v27 node thinks my v28 / master block database is corrupted":
(https://github.com/bitcoin/bitcoin/issues/31286#issuecomment-2474756367)
Oh never mind, that's only "for a freshly initialized blocksdir". That seems reasonable.
πŸ’¬ sr-gi commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1841164936)
`2==true` would mean that we had the transaction in both places, which should never happen (I'd mean that a transaction is both ready and delayed at the same time).

If that were to happen in production for whatever reason, this would return false with the suggested changes, which may break something?
πŸ’¬ sr-gi commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1841223757)
In that case, no transaction would be moved from the reconciliation set to `invs_to_send`, given no transaction would be removed:

```cpp
auto it = std::find(fanout_with_ancestors.begin(), fanout_with_ancestors.end(), peer_id);
if (it != fanout_with_ancestors.end()) {
for (const auto wtxid: parents) {
m_txreconciliation->TryRemovingFromSet(peer_id, wtxid);
invs_to_send.push_back(wtxid);
}
} else {
// If the peer is registered for set reconciliation, maybe pi
...
πŸ’¬ sr-gi commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1841243963)
Done
πŸ’¬ sr-gi commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1841245788)
Done
πŸ’¬ sr-gi commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1841245945)
Done
πŸ‘ hodlinator approved a pull request: "tinyformat: Add compile-time checking for literal format strings"
(https://github.com/bitcoin/bitcoin/pull/31174#pullrequestreview-2434652082)
re-ACK fe39acf88ff552bfc4a502c99774375b91824bb1

`git range-diff master ecc5cb9 fe39acf`

- Added more `FailFmtWithError`-tests ([maflcko](https://github.com/bitcoin/bitcoin/pull/31174#pullrequestreview-2423906139)).
- Terser skipping of `%%` ([l0rinc](https://github.com/bitcoin/bitcoin/pull/31174#discussion_r1835770569)).
- Comment regarding format string components updated ([me](https://github.com/bitcoin/bitcoin/pull/31174#discussion_r1834472950)).
πŸ’¬ hodlinator commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1841318869)
Thanks for clarifying when modified fees are ignored.

> We're assuming node operators/miners care about dust. If they do not, they can already set their dust relay rate to 0 and avoid all of these mechanisms entirely, ephemeral dust becomes a no-op. (e.g., MARA reportedly doesn't enforce dust limits and runs an accelerator already)
>
> If we end up being wrong and miners are ok putting one dust output in the utxo set but not multiple or whatever, we can revisit?

My assumption would be th
...
πŸ’¬ achow101 commented on pull request "bench: add support for custom data directory":
(https://github.com/bitcoin/bitcoin/pull/31000#issuecomment-2475050409)
ACK fa66e0887ca1a1445d8b18ba1fadb12b2d911048
πŸ€” hodlinator reviewed a pull request: "descriptor: Add proper Clone function to miniscript::Node"
(https://github.com/bitcoin/bitcoin/pull/30866#pullrequestreview-2434704657)
I've experimented with a change on top of this PR that removes `shared_ptr` usage from *src/script/* altogether in order to clarify ownership.

My change is in 80fca845b5f28677207a8fea4a173baaef23036f. It helped uncover another place where the `node` needs to be cloned (at the bottom of the outer loop):

```diff
diff --git a/src/script/descriptor.cpp b/src/script/descriptor.cpp
index b099fdfc3f..f0294df72a 100644
--- a/src/script/descriptor.cpp
+++ b/src/script/descriptor.cpp
@@ -2154,7
...
πŸš€ achow101 merged a pull request: "bench: add support for custom data directory"
(https://github.com/bitcoin/bitcoin/pull/31000)
πŸ’¬ achow101 commented on pull request "descriptor: Add proper Clone function to miniscript::Node":
(https://github.com/bitcoin/bitcoin/pull/30866#issuecomment-2475109762)
> My change is in [80fca84](https://github.com/bitcoin/bitcoin/commit/80fca845b5f28677207a8fea4a173baaef23036f). It helped uncover another place where the `node` needs to be cloned (at the bottom of the outer loop):

Good catch, added that.

Do you have a particular test case that triggered there?
πŸ‘ ajtowns approved a pull request: "validation: Remove RECENT_CONSENSUS_CHANGE validation result"
(https://github.com/bitcoin/bitcoin/pull/31269#pullrequestreview-2434890591)
ACK e80e4c6ff91e27d7d40f099a2d7942c29085234c
πŸ’¬ andremralves commented on pull request "contrib: skip missing binaries in gen-manpages":
(https://github.com/bitcoin/bitcoin/pull/30986#discussion_r1841534621)
Good suggestion. Thanks!