⚠️ lcharles123 opened an issue: "Feature request: Backup of datadir state"
(https://github.com/bitcoin/bitcoin/issues/31324)
### Please describe the feature you'd like to see added.

It will be good to have a option included in bitcoind to backup and restore the current state of the datadir periodically.

### Is your feature related to a problem, if so please describe it.

Not a software problem, but its the environment. Many events can corrupt data inside datadir, to name some: power outages, out of resources (RAM), network fail (in case of network storage).
Once corrupted, it's costly to re-sync it again from scra
...

💬 achow101 commented on pull request "wallet: Remove IsMine from migration code":
(https://github.com/bitcoin/bitcoin/pull/30328#discussion_r1848782223)
Hmm, the test that is supposed to fail here is passing because the P2WSH scriptPubKey also ends up in the `setWatchOnly` so it is added when we iterate those.

I think if the wallet was used normally, this loop probably isn't necessary. However, I am hesitant to remove it as in theory, if a P2WSH were in `mapScripts` and not in `setWatchOnly`, we would miss it but `IsMine()` would've still allowed it. I think the only way to get that though is through direct modification of the wallet file or
...

💬 achow101 commented on pull request "wallet: Remove IsMine from migration code":
(https://github.com/bitcoin/bitcoin/pull/30328#discussion_r1848782387)
Done

💬 achow101 commented on pull request "wallet: Remove IsMine from migration code":
(https://github.com/bitcoin/bitcoin/pull/30328#discussion_r1848782476)
Done

💬 achow101 commented on pull request "wallet: Remove IsMine from migration code":
(https://github.com/bitcoin/bitcoin/pull/30328#discussion_r1848782538)
Done

💬 achow101 commented on pull request "wallet: Remove IsMine from migration code":
(https://github.com/bitcoin/bitcoin/pull/30328#discussion_r1848782692)
Done

💬 achow101 commented on pull request "wallet: Remove IsMine from migration code":
(https://github.com/bitcoin/bitcoin/pull/30328#discussion_r1848782930)
Renamed

💬 pinheadmz commented on issue "Feature request: Backup of datadir state":
(https://github.com/bitcoin/bitcoin/issues/31324#issuecomment-2486339380)
I think backups should be the users job not bitcoind. Especially putting backups in datadir -- wouldn't it make more sense to be on a completely different drive ?

💬 Sjors commented on issue "v27.2 guix build fails with hash mismatch":
(https://github.com/bitcoin/bitcoin/issues/31266#issuecomment-2486349343)
Noticed while setting up an new guix environment for #31323 that I needed a few more (starting at 1.4.0 installed on Ubuntu in a VM):

```
guix download https://www.gnupg.org/ftp/gcrypt/gnutls/v3.7/gnutls-3.7.2.tar.xz
guix download https://gnupg.org/ftp/gcrypt/libgcrypt/libgcrypt-1.8.8.tar.bz2
guix download https://gnupg.org/ftp/gcrypt/libgpg-error/libgpg-error-1.42.tar.bz2
```

(will add more here if needed, still building)

💬 Sjors commented on pull request "guix: swap `moreutils` for just `sponge`":
(https://github.com/bitcoin/bitcoin/pull/31323#issuecomment-2486363936)
On my x86_64 machine I get the same guix hashes as @fanquake.

💬 Sjors commented on issue "v27.2 guix build fails with hash mismatch":
(https://github.com/bitcoin/bitcoin/issues/31266#issuecomment-2486378063)
This could be a problem... I can't find `net-tools-1.60-0.479bb4a.zip.drv` anywhere on the internet, so I had to copy it from my other guix machine instead.

So the problem may not be limited to backport releases like v27.2

💬 ryanofsky commented on pull request "mining: add early return to waitTipChanged()":
(https://github.com/bitcoin/bitcoin/pull/31297#discussion_r1848821236)
Are you are seeing a difference in practice? At least according to https://en.cppreference.com/w/cpp/thread/condition_variable/wait_for, `wait_for(lock, rel_time, pred)` is equivalent to `wait_until(lock, rel_time + now, pred)` and according to https://en.cppreference.com/w/cpp/thread/condition_variable/wait_until that should be equivalent to `while(!pred()) wait...` checking the predicate before there is any waiting.

💬 ryanofsky commented on pull request "mining: add early return to waitTipChanged()":
(https://github.com/bitcoin/bitcoin/pull/31297#issuecomment-2486457573)
re: https://github.com/bitcoin/bitcoin/pull/31297#issuecomment-2485712161

> Otherwise we'd never get past this point:

I think we typically get past that point on line 1795 in `AppInitMain` even though `m_tip_block` is still null at that point, because when the node restarted the `chainman.ActiveTip() == nullptr` check will normally be false. This is because of the `InitAndLoadChainstate` call on line 1624, which calls `Chainstate::LoadChainTip()` and sets the active tip to `coins_cache.Get
...

👍 ryanofsky approved a pull request: "Add destroy to BlockTemplate schema"
(https://github.com/bitcoin/bitcoin/pull/31288#pullrequestreview-2446300488)
Code review ACK 9aa50152c1cfa1c41215b2b51ed7a516aa67137a

💬 achow101 commented on pull request "test: Introduce ensure_for helper":
(https://github.com/bitcoin/bitcoin/pull/30893#issuecomment-2486474269)
ACK 111465d72dd35e42361fc2a089036f652417ed37

💬 Sjors commented on pull request "mining: add early return to waitTipChanged()":
(https://github.com/bitcoin/bitcoin/pull/31297#discussion_r1848870979)
I thought I did, but give what the documentation says, it indeed shouldn't be needed. Will test again.

📝 Sjors converted_to_draft a pull request: "mining: add early return to waitTipChanged()"
(https://github.com/bitcoin/bitcoin/pull/31297)
It's useful to be able to wait for an active chaintip to appear, by calling ` waitTipChanged(uint256::ZERO);`.

Unfortunately this doesn't work out of the box, because `notifications().m_tip_block` is `ZERO` until a new block arrives.

Additionally we need an early return before calling `wait_for`.

👍 ryanofsky approved a pull request: "Drop script_pub_key arg from createNewBlock"
(https://github.com/bitcoin/bitcoin/pull/31318#pullrequestreview-2446313068)
Concept ACK . It seems like a nice improvement to not require specifying an option that won't typically be used. But I'm a little hazy on the details of this. I think better documentation could help. Left a suggestion and some questions below.

💬 ryanofsky commented on pull request "Drop script_pub_key arg from createNewBlock":
(https://github.com/bitcoin/bitcoin/pull/31318#discussion_r1848878221)
re: https://github.com/bitcoin/bitcoin/pull/31318#discussion_r1846813317

Would it make sense to declare the field as `CScript coinbase_output_script{CScript{} << OP_TRUE}` instead of `std::optional<CScript> coinbase_output_script{};` just to be clearer about what the default value is? Or maybe just add documentation to say what happens if this is not set?

I think the drawback of using std::optional here is that it's not really clear what happens if the option is not set. It's also not clea
...

🚀 achow101 merged a pull request: "test: Introduce ensure_for helper"
(https://github.com/bitcoin/bitcoin/pull/30893)