Merge bitcoin/bitcoin#35752: wallet: make encryption state updates atomic

### src/interfaces/wallet.h
@@ -76,14 +76,14 @@ class Wallet
     virtual bool lock() = 0;
 
     //! Unlock wallet.
-    virtual bool unlock(const SecureString& wallet_passphrase) = 0;
+    virtual util::Expected<void, wallet::WalletError> unlock(const SecureString& wallet_passphrase) = 0;
 
     //! Return whether wallet is locked.
     virtual bool isLocked() = 0;
 
     //! Change wallet passphrase.
-    virtual bool changeWalletPassphrase(const SecureString& old_wallet_passphrase,
-        const SecureString& new_wallet_passphrase) = 0;
+    virtual util::Expected<void, wallet::WalletError> changeWalletPassphrase(const SecureString& old_wallet_passphrase,
+                                                                             const SecureString& new_wallet_passphrase) = 0;
 
     //! Abort a rescan.
     virtual void abortRescan() = 0;

### src/qt/askpassphrasedialog.cpp
@@ -10,6 +10,7 @@
 #include <qt/walletmodel.h>
 
 #include <support/allocators/secure.h>
+#include <wallet/types.h>
 
 #include <QKeyEvent>
 #include <QMessageBox>
@@ -158,20 +159,8 @@ void AskPassphraseDialog::accept()
     } break;
     case Unlock:
         try {
-            if (!model->setWalletLocked(false, oldpass)) {
-                // Check if the passphrase has a null character (see #27067 for details)
-                if (oldpass.find('\0') == std::string::npos) {
-                    QMessageBox::critical(this, tr("Wallet unlock failed"),
-                                          tr("The passphrase entered for the wallet decryption was incorrect."));
-                } else {
-                    QMessageBox::critical(this, tr("Wallet unlock failed"),
-                                          tr("The passphrase entered for the wallet decryption is incorrect. "
-                                             "It contains a null character (ie - a zero byte). "
-                                             "If the passphrase was set with a version of this software prior to 25.0, "
-                                             "please try again with only the characters up to — but not including — "
-                                             "the first null character. If this is successful, please set a new "
-                                             "passphrase to avoid this issue in the future."));
-                }
+            if (auto unlocked{model->wallet().unlock(oldpass)}; !unlocked) {
+                QMessageBox::critical(this, tr("Wallet unlock failed"), QString::fromStdString(unlocked.error().message.translated));
             } else {
                 if (m_passphrase_out) {
                     m_passphrase_out->assign(oldpass);
@@ -189,26 +178,12 @@ void AskPassphraseDialog::accept()
     case ChangePass:
         if(newpass1 == newpass2)
         {
-            if(model->changePassphrase(oldpass, newpass1))
-            {
+            if (auto changed{model->changePassphrase(oldpass, newpass1)}) {
                 QMessageBox::information(this, tr("Wallet encrypted"),
                                      tr("Wallet passphrase was successfully changed."));
                 QDialog::accept(); // Success
-            }
-            else
-            {
-                // Check if the old passphrase had a null character (see #27067 for details)
-                if (oldpass.find('\0') == std::string::npos) {
-                    QMessageBox::critical(this, tr("Passphrase change failed"),
-                                          tr("The passphrase entered for the wallet decryption was incorrect."));
-                } else {
-                    QMessageBox::critical(this, tr("Passphrase change failed"),
-                                          tr("The old passphrase entered for the wallet decryption is incorrect. "
-                                             "It contains a null character (ie - a zero byte). "
-                                             "If the passphrase was set with a version of this software prior to 25.0, "
-                                             "please try again with only the characters up to — but not including — "
-                                             "the first null character."));
-                }
+            } else {
+                QMessageBox::critical(this, tr("Passphrase change failed"), QString::fromStdString(changed.error().message.translated));
             }
         }
         else

### src/qt/walletmodel.cpp
@@ -328,21 +328,7 @@ bool WalletModel::setWalletEncrypted(const SecureString& passphrase)
     return m_wallet->encryptWallet(passphrase);
 }
 
-bool WalletModel::setWalletLocked(bool locked, const SecureString &passPhrase)
-{
-    if(locked)
-    {
-        // Lock
-        return m_wallet->lock();
-    }
-    else
-    {
-        // Unlock
-        return m_wallet->unlock(passPhrase);
-    }
-}
-
-bool WalletModel::changePassphrase(const SecureString &oldPass, const SecureString &newPass)
+util::Expected<void, wallet::WalletError> WalletModel::changePassphrase(const SecureString& oldPass, const SecureString& newPass)
 {
     m_wallet->lock(); // Make sure wallet is locked before attempting pass change
     return m_wallet->changeWalletPassphrase(oldPass, newPass);
@@ -458,7 +444,7 @@ WalletModel::UnlockContext::~UnlockContext()
 {
     if(valid && relock)
     {
-        wallet->setWalletLocked(true);
+        wallet->wallet().lock();
     }
 }
 

### src/qt/walletmodel.h
@@ -102,9 +102,7 @@ class WalletModel : public QObject
 
     // Wallet encryption
     bool setWalletEncrypted(const SecureString& passphrase);
-    // Passphrase only needed when unlocking
-    bool setWalletLocked(bool locked, const SecureString &passPhrase=SecureString());
-    bool changePassphrase(const SecureString &oldPass, const SecureString &newPass);
+    util::Expected<void, wallet::WalletError> changePassphrase(const SecureString& oldPass, const SecureString& newPass);
 
     // RAII object for unlocking wallet, returned by requestUnlock()
     class UnlockContext

### src/wallet/interfaces.cpp
@@ -147,10 +147,10 @@ class WalletImpl : public Wallet
     }
     bool isCrypted() override { return m_wallet->HasEncryptionKeys(); }
     bool lock() override { return m_wallet->Lock(); }
-    bool unlock(const SecureString& wallet_passphrase) override { return m_wallet->Unlock(wallet_passphrase); }
+    util::Expected<void, WalletError> unlock(const SecureString& wallet_passphrase) override { return m_wallet->Unlock(wallet_passphrase); }
     bool isLocked() override { return m_wallet->IsLocked(); }
-    bool changeWalletPassphrase(const SecureString& old_wallet_passphrase,
-        const SecureString& new_wallet_passphrase) override
+    util::Expected<void, WalletError> changeWalletPassphrase(const SecureString& old_wallet_passphrase,
+                                                             const SecureString& new_wallet_passphrase) override
     {
         return m_wallet->ChangeWalletPassphrase(old_wallet_passphrase, new_wallet_passphrase);
     }

### src/wallet/rpc/encrypt.cpp
@@ -71,18 +71,8 @@ RPCMethod walletpassphrase()
             throw JSONRPCError(RPC_INVALID_PARAMETER, "passphrase cannot be empty");
         }
 
-        if (!pwallet->Unlock(strWalletPass)) {
-            // Check if the passphrase has a null character (see #27067 for details)
-            if (strWalletPass.find('\0') == std::string::npos) {
-                throw JSONRPCError(RPC_WALLET_PASSPHRASE_INCORRECT, "Error: The wallet passphrase entered was incorrect.");
-            } else {
-                throw JSONRPCError(RPC_WALLET_PASSPHRASE_INCORRECT, "Error: The wallet passphrase entered is incorrect. "
-                                                                    "It contains a null character (ie - a zero byte). "
-                                                                    "If the passphrase was set with a version of this software prior to 25.0, "
-                                                                    "please try again with only the characters up to — but not including — "
-                                                                    "the first null character. If this is successful, please set a new "
-                                                                    "passphrase to avoid this issue in the future.");
-            }
+        if (auto unlocked{pwallet->Unlock(strWalletPass)}; !unlocked) {
+            throw JSONRPCError(HandleWalletErrorCode(unlocked.error().code), unlocked.error().message.original);
         }
 
         pwallet->TopUpKeyPool();
@@ -157,17 +147,8 @@ RPCMethod walletpassphrasechange()
         throw JSONRPCError(RPC_INVALID_PARAMETER, "passphrase cannot be empty");
     }
 
-    if (!pwallet->ChangeWalletPassphrase(strOldWalletPass, strNewWalletPass)) {
-        // Check if the old passphrase had a null character (see #27067 for details)
-        if (strOldWalletPass.find('\0') == std::string::npos) {
-            throw JSONRPCError(RPC_WALLET_PASSPHRASE_INCORRECT, "Error: The wallet passphrase entered was incorrect.");
-        } else {
-            throw JSONRPCError(RPC_WALLET_PASSPHRASE_INCORRECT, "Error: The old wallet passphrase entered is incorrect. "
-                                                                "It contains a null character (ie - a zero byte). "
-                                                                "If the old passphrase was set with a version of this software prior to 25.0, "
-                                                                "please try again with only the characters up to — but not including — "
-                                                                "the first null character.");
-        }
+    if (auto changed{pwallet->ChangeWalletPassphrase(strOldWalletPass, strNewWalletPass)}; !changed) {
+        throw JSONRPCError(HandleWalletErrorCode(changed.error().code), changed.error().message.original);
     }
 
     return UniValue::VNULL;

### src/wallet/rpc/util.cpp
@@ -155,24 +155,14 @@ void HandleWalletError(const std::shared_ptr<CWallet>& wallet, DatabaseStatus& s
 
 RPCErrorCode HandleWalletErrorCode(const WalletErrorCode code)
 {
-    RPCErrorCode res = RPC_WALLET_ERROR;
     switch(code) {
-        case WalletErrorCode::UnlockNeeded:
-            res = RPC_WALLET_UNLOCK_NEEDED;
-            break;
-        case WalletErrorCode::InvalidDescriptor:
-            res = RPC_INVALID_ADDRESS_OR_KEY;
-            break;
-        case WalletErrorCode::InvalidParameter:
-            res = RPC_INVALID_PARAMETER;
-            break;
-        case WalletErrorCode::MiscError:
-            res = RPC_MISC_ERROR;
-            break;
-        default: // RPC_WALLET_ERROR is returned for all other cases.
-            break;
+        case WalletErrorCode::UnlockNeeded: return RPC_WALLET_UNLOCK_NEEDED;
+        case WalletErrorCode::PassphraseIncorrect: return RPC_WALLET_PASSPHRASE_INCORRECT;
+        case WalletErrorCode::InvalidDescriptor: return RPC_INVALID_ADDRESS_OR_KEY;
+        case WalletErrorCode::InvalidParameter: return RPC_INVALID_PARAMETER;
+        case WalletErrorCode::MiscError: return RPC_MISC_ERROR;
+        default: return RPC_WALLET_ERROR;
     }
-    return res;
 }
 
 void AppendLastProcessedBlock(UniValue& entry, const CWallet& wallet)

### src/wallet/scriptpubkeyman.cpp
@@ -1014,6 +1014,7 @@ bool DescriptorScriptPubKeyMan::Encrypt(const CKeyingMaterial& master_key, Walle
         return false;
     }
 
+    CryptedKeyMap crypted_keys;
     for (const KeyMap::value_type& key_in : m_map_keys)
     {
         const CKey &key = key_in.second;
@@ -1023,10 +1024,19 @@ bool DescriptorScriptPubKeyMan::Encrypt(const CKeyingMaterial& master_key, Walle
         if (!EncryptSecret(master_key, secret, pubkey.GetHash(), crypted_secret)) {
             return false;
         }
-        m_map_crypted_keys[pubkey.GetID()] = make_pair(pubkey, crypted_secret);
-        batch->WriteCryptedDescriptorKey(GetID(), pubkey, crypted_secret);
+        if (!batch->WriteCryptedDescriptorKey(GetID(), pubkey, crypted_secret)) {
+            return false;
+        }
+        crypted_keys[pubkey.GetID()] = make_pair(pubkey, std::move(crypted_secret));
     }
-    m_map_keys.clear();
+
+    batch->RegisterTxnListener({
+        .on_commit = [this, keys = std::move(crypted_keys)]() mutable {
+            LOCK(cs_desc_man);
+            m_map_crypted_keys = std::move(keys);
+            m_map_keys.clear();
+        },
+        .on_abort = [] {}});
     return true;
 }
 
@@ -1220,12 +1230,17 @@ bool DescriptorScriptPubKeyMan::AddDescriptorKeyWithDB(WalletBatch& batch, const
             return false;
         }
 
-        m_map_crypted_keys[pubkey.GetID()] = make_pair(pubkey, crypted_secret);
-        return batch.WriteCryptedDescriptorKey(GetID(), pubkey, crypted_secret);
+        if (!batch.WriteCryptedDescriptorKey(GetID(), pubkey, crypted_secret)) {
+            return false;
+        }
+        m_map_crypted_keys[pubkey.GetID()] = make_pair(pubkey, std::move(crypted_secret));
     } else {
+        if (!batch.WriteDescriptorKey(GetID(), pubkey, key.GetPrivKey())) {
+            return false;
+        }
         m_map_keys[pubkey.GetID()] = key;
-        return batch.WriteDescriptorKey(GetID(), pubkey, key.GetPrivKey());
     }
+    return true;
 }
 
 bool DescriptorScriptPubKeyMan::IsHDEnabled() const

### src/wallet/scriptpubkeyman.h
@@ -89,6 +89,7 @@ class ScriptPubKeyMan
 
     //! Check that the given decryption key is valid for this ScriptPubKeyMan, i.e. it decrypts all of the keys handled by it.
     virtual bool CheckDecryptionKey(const CKeyingMaterial& master_key) { return false; }
+    //! Encrypt keys and write them to a batch with an active transaction. Update in-memory keys only after the transaction commits.
     virtual bool Encrypt(const CKeyingMaterial& master_key, WalletBatch* batch) { return false; }
 
     virtual util::Result<CTxDestination> GetReservedDestination(const OutputType type, bool internal, int64_t& index) { return util::Error{Untranslated("Not supported")}; }

### src/wallet/test/util.h
@@ -6,11 +6,15 @@
 #define BITCOIN_WALLET_TEST_UTIL_H
 
 #include <addresstype.h>
+#include <streams.h>
 #include <wallet/db.h>
 #include <wallet/scriptpubkeyman.h>
 #include <wallet/sqlite.h>
 
 #include <memory>
+#include <optional>
+#include <string>
+#include <utility>
 
 class ArgsManager;
 class CChain;
@@ -68,6 +72,82 @@ class MockableSQLiteDatabase : public InMemoryWalletDatabase
     std::unique_ptr<DatabaseBatch> MakeBatch() override { return std::make_unique<MockableSQLiteBatch>(*this); }
 };
 
+/** A SQLite wallet database that can fail selected operations for testing */
+class FaultInjectingDatabase : public MockableSQLiteDatabase
+{
+public:
+    void FailNextWrite(std::string record_type, size_t match_skip_count = 0)
+    {
+        m_fail_write = {std::move(record_type), match_skip_count};
+    }
+    void FailNextErase(std::string record_type) { m_fail_erase = {std::move(record_type)}; }
+    void FailNextCommit() { m_fail_commit = true; }
+
+    std::optional<SerializeData> GetRecordValue(const std::string& record_type)
+    {
+        auto batch{MakeBatch()};
+        if (auto cursor{batch->GetNewPrefixCursor(DataStream() << record_type)}) {
+            DataStream key, value;
+            if (cursor->Next(key, value) == DatabaseCursor::Status::MORE) {
+                return SerializeData{value.begin(), value.end()};
+            }
+        }
+        return std::nullopt;
+    }
+
+    bool HasRecordType(const std::string& record_type) { return GetRecordValue(record_type).has_value(); }
+
+    std::unique_ptr<DatabaseBatch> MakeBatch() override { return std::make_unique<Batch>(*this); }
+
+private:
+    struct Failure {
+        std::string record_type;
+        size_t match_skip_count{0};
+    };
+
+    static bool ShouldFail(std::optional<Failure>& failure, const DataStream& key)
+    {
+        if (!failure) return false;
+        std::string record_type;
+        SpanReader{MakeByteSpan(key)} >> record_type;
+        if (failure->record_type != record_type) return false;
+        if (failure->match_skip_count > 0) {
+            --failure->match_skip_count;
+            return false;
+        }
+        failure.reset();
+        return true;
+    }
+
+    struct Batch : SQLiteBatch {
+        explicit Batch(FaultInjectingDatabase& database) : SQLiteBatch(database), m_owner{database} {}
+
+        bool TxnCommit() override
+        {
+            if (std::exchange(m_owner.m_fail_commit, false)) return false;
+            return SQLiteBatch::TxnCommit();
+        }
+
+        bool WriteKey(DataStream&& key, DataStream&& value, bool overwrite = true) override
+        {
+            if (ShouldFail(m_owner.m_fail_write, key)) return false;
+            return SQLiteBatch::WriteKey(std::move(key), std::move(value), overwrite);
+        }
+
+        bool EraseKey(DataStream&& key) override
+        {
+            if (ShouldFail(m_owner.m_fail_erase, key)) return false;
+            return SQLiteBatch::EraseKey(std::move(key));
+        }
+
+        FaultInjectingDatabase& m_owner;
+    };
+
+    std::optional<Failure> m_fail_write;
+    std::optional<Failure> m_fail_erase;
+    bool m_fail_commit{false};
+};
+
 std::unique_ptr<WalletDatabase> CreateMockableWalletDatabase();
 MockableSQLiteDatabase& GetMockableDatabase(CWallet& wallet);
 

### src/wallet/test/wallet_tests.cpp
@@ -12,6 +12,7 @@
 #include <limits>
 #include <memory>
 #include <optional>
+#include <stdexcept>
 #include <string>
 #include <utility>
 #include <vector>
@@ -34,7 +35,10 @@
 #include <test/util/logging.h>
 #include <test/util/random.h>
 #include <test/util/setup_common.h>
+#include <test/util/time.h>
 #include <util/byte_units.h>
+#include <util/check.h>
+#include <util/strencodings.h>
 #include <util/translation.h>
 #include <validation.h>
 #include <validationinterface.h>
@@ -127,6 +131,182 @@ BOOST_AUTO_TEST_CASE(reject_invalid_descriptor_ranges)
     }
 }
 
+namespace {
+struct EncryptionFailureSetup : TestingSetup {
+    WalletContext context;
+    FaultInjectingDatabase* fail_db{nullptr};
+    std::shared_ptr<CWallet> wallet;
+    FakeNodeClock clock; // Frozen time makes EncryptMasterKey use the default KDF iteration count
+
+    EncryptionFailureSetup()
+    {
+        context.args = &m_args;
+        m_args.ForceSetArg("-keypool", "1"); // Failure injection does not depend on keypool depth
+        context.chain = m_node.chain.get();
+        RecreateWallet(WALLET_FLAG_DESCRIPTORS);
+    }
+
+    void RecreateWallet(uint64_t create_flags)
+    {
+        if (wallet) TestUnloadWallet(std::move(wallet));
+        auto database{std::make_unique<FaultInjectingDatabase>()};
+        fail_db = database.get();
+        wallet = TestCreateWallet(std::move(database), context, create_flags);
+    }
+
+    ~EncryptionFailureSetup() { TestUnloadWallet(std::move(wallet)); }
+};
+} // namespace
+
+BOOST_FIXTURE_TEST_CASE(encrypt_wallet_master_key_write_failure, EncryptionFailureSetup)
+{
+    AddKey(*wallet, GenerateRandomKey());
+
+    fail_db->FailNextWrite(DBKeys::MASTER_KEY); // The injected failure affects only the first attempt
+    for (bool success : {false, true}) {
+        BOOST_CHECK_EQUAL(wallet->EncryptWallet("passphrase"), success);
+        BOOST_CHECK_EQUAL(wallet->HasEncryptionKeys(), success);
+        BOOST_CHECK_EQUAL(wallet->HaveCryptedKeys(), success);
+        BOOST_CHECK_EQUAL(fail_db->HasRecordType(DBKeys::MASTER_KEY), success);
+        BOOST_CHECK_EQUAL(fail_db->HasRecordType(DBKeys::WALLETDESCRIPTORKEY), !success);
+        BOOST_CHECK_EQUAL(fail_db->HasRecordType(DBKeys::WALLETDESCRIPTORCKEY), success);
+    }
+}
+
+BOOST_FIXTURE_TEST_CASE(encrypt_wallet_commit_failure, EncryptionFailureSetup)
+{
+    AddKey(*wallet, GenerateRandomKey());
+
+    fail_db->FailNextCommit(); // The injected failure affects only the first attempt
+    test_only_CheckFailuresAreExceptionsNotAborts mock_checks; // Keep abort regressions observable
+    BOOST_CHECK(!wallet->EncryptWallet("passphrase"));
+    BOOST_CHECK(!wallet->HasEncryptionKeys());
+    BOOST_CHECK(!wallet->HaveCryptedKeys());
+    BOOST_CHECK(!fail_db->HasRecordType(DBKeys::MASTER_KEY));
+    BOOST_CHECK( fail_db->HasRecordType(DBKeys::WALLETDESCRIPTORKEY));
+    BOOST_CHECK(!fail_db->HasRecordType(DBKeys::WALLETDESCRIPTORCKEY));
+    BOOST_CHECK( wallet->EncryptWallet("passphrase"));
+    BOOST_CHECK( wallet->HasEncryptionKeys());
+    BOOST_CHECK( wallet->HaveCryptedKeys());
+    BOOST_CHECK( fail_db->HasRecordType(DBKeys::MASTER_KEY));
+    BOOST_CHECK(!fail_db->HasRecordType(DBKeys::WALLETDESCRIPTORKEY));
+    BOOST_CHECK( fail_db->HasRecordType(DBKeys::WALLETDESCRIPTORCKEY));
+}
+
+BOOST_FIXTURE_TEST_CASE(encrypt_wallet_descriptor_key_write_failure, EncryptionFailureSetup)
+{
+    AddKey(*wallet, GenerateRandomKey());
+
+    fail_db->FailNextWrite(DBKeys::WALLETDESCRIPTORCKEY, /*match_skip_count=*/1); // Only one write fails
+    for (bool success : {false, true}) {
+        BOOST_CHECK_EQUAL(wallet->EncryptWallet("passphrase"), success);
+        BOOST_CHECK_EQUAL(wallet->HasEncryptionKeys(), success);
+        BOOST_CHECK_EQUAL(wallet->HaveCryptedKeys(), success);
+        BOOST_CHECK_EQUAL(fail_db->HasRecordType(DBKeys::MASTER_KEY), success);
+        BOOST_CHECK_EQUAL(fail_db->HasRecordType(DBKeys::WALLETDESCRIPTORKEY), !success);
+        BOOST_CHECK_EQUAL(fail_db->HasRecordType(DBKeys::WALLETDESCRIPTORCKEY), success);
+    }
+}
+
+BOOST_FIXTURE_TEST_CASE(encrypt_wallet_descriptor_key_erase_failure, EncryptionFailureSetup)
+{
+    AddKey(*wallet, GenerateRandomKey());
+
+    fail_db->FailNextErase(DBKeys::WALLETDESCRIPTORKEY); // Only one erase fails
+    for (bool success : {false, true}) {
+        BOOST_CHECK_EQUAL(wallet->EncryptWallet("passphrase"), success);
+        BOOST_CHECK_EQUAL(wallet->HasEncryptionKeys(), success);
+        BOOST_CHECK_EQUAL(wallet->HaveCryptedKeys(), success);
+        BOOST_CHECK_EQUAL(fail_db->HasRecordType(DBKeys::MASTER_KEY), success);
+        BOOST_CHECK_EQUAL(fail_db->HasRecordType(DBKeys::WALLETDESCRIPTORKEY), !success);
+        BOOST_CHECK_EQUAL(fail_db->HasRecordType(DBKeys::WALLETDESCRIPTORCKEY), success);
+    }
+}
+
+BOOST_FIXTURE_TEST_CASE(change_passphrase_master_key_write_failure, EncryptionFailureSetup)
+{
+    AddKey(*wallet, GenerateRandomKey());
+    BOOST_REQUIRE(wallet->EncryptWallet("old_pass"));
+    BOOST_REQUIRE(wallet->IsLocked());
+    const auto master_key_record{fail_db->GetRecordValue(DBKeys::MASTER_KEY)};
+    BOOST_REQUIRE(master_key_record);
+
+    fail_db->FailNextWrite(DBKeys::MASTER_KEY); // The injected failure affects only the first attempt
+    const auto changed{wallet->ChangeWalletPassphrase("old_pass", "new_pass")};
+    BOOST_REQUIRE(!changed);
+    BOOST_CHECK_EQUAL(changed.error().code, WalletErrorCode::GenericError);
+    BOOST_CHECK_EQUAL(changed.error().message.original, "Error: Writing the new encryption key to the wallet database failed");
+    BOOST_CHECK( wallet->IsLocked());
+    BOOST_CHECK( fail_db->GetRecordValue(DBKeys::MASTER_KEY) == master_key_record);
+    BOOST_CHECK( wallet->Unlock("old_pass"));
+    wallet->Lock();
+    BOOST_CHECK(!wallet->Unlock("new_pass"));
+    BOOST_CHECK( wallet->ChangeWalletPassphrase("old_pass", "new_pass"));
+    BOOST_CHECK( wallet->IsLocked());
+    BOOST_CHECK( fail_db->GetRecordValue(DBKeys::MASTER_KEY) != master_key_record);
+    BOOST_CHECK( wallet->Unlock("new_pass"));
+    wallet->Lock();
+    const auto unlocked{wallet->Unlock("old_pass")};
+    BOOST_REQUIRE(!unlocked);
+    BOOST_CHECK_EQUAL(unlocked.error().code, WalletErrorCode::PassphraseIncorrect);
+    BOOST_CHECK_EQUAL(unlocked.error().message.original, "Error: The wallet passphrase entered was incorrect.");
+}
+
+BOOST_FIXTURE_TEST_CASE(add_encrypted_descriptor_key_without_plaintext_record, EncryptionFailureSetup)
+{
+    RecreateWallet(WALLET_FLAG_DESCRIPTORS | WALLET_FLAG_BLANK_WALLET);
+    BOOST_REQUIRE(wallet->EncryptWallet("passphrase"));
+    BOOST_REQUIRE(wallet->Unlock("passphrase"));
+
+    AddKey(*wallet, GenerateRandomKey());
+    BOOST_CHECK( wallet->HaveCryptedKeys());
+    BOOST_CHECK( fail_db->HasRecordType(DBKeys::WALLETDESCRIPTORCKEY));
+    BOOST_CHECK(!fail_db->HasRecordType(DBKeys::WALLETDESCRIPTORKEY));
+}
+
+BOOST_FIXTURE_TEST_CASE(add_descriptor_key_database_failure, EncryptionFailureSetup)
+{
+    enum class Failure { PlaintextWrite, EncryptedWrite, Erase, Commit };
+    for (auto failure : {Failure::PlaintextWrite, Failure::EncryptedWrite, Failure::Erase, Failure::Commit}) {
+        const bool encrypted{failure != Failure::PlaintextWrite};
+        RecreateWallet(WALLET_FLAG_DESCRIPTORS | WALLET_FLAG_BLANK_WALLET);
+        CKey key{GenerateRandomKey()};
+        // Add a public descriptor first so the private-key update exercises an existing live manager
+        auto* spkm{CreateDescriptor(*wallet, strprintf("combo(%s)", HexStr(key.GetPubKey())), /*success=*/true)};
+        WalletDescriptor descriptor{WITH_LOCK(spkm->cs_desc_man, return spkm->GetWalletDescriptor())};
+        FlatSigningProvider provider;
+        provider.keys.emplace(key.GetPubKey().GetID(), key);
+        auto add_key{[&] {
+            LOCK(wallet->cs_wallet);
+            return wallet->AddWalletDescriptor(descriptor, provider, /*label=*/"", /*internal=*/false);
+        }};
+        auto has_key{[&] {
+            LOCK(wallet->cs_wallet);
+            return wallet->GetKey(key.GetPubKey().GetID()).has_value();
+        }};
+        if (encrypted) {
+            BOOST_REQUIRE(wallet->EncryptWallet("passphrase"));
+            BOOST_REQUIRE(wallet->Unlock("passphrase"));
+        }
+        BOOST_CHECK(!has_key());
+
+        const std::string record_type{encrypted ? DBKeys::WALLETDESCRIPTORCKEY : DBKeys::WALLETDESCRIPTORKEY};
+        if (failure == Failure::Erase) {
+            fail_db->FailNextErase(DBKeys::WALLETDESCRIPTORKEY);
+        } else if (failure == Failure::Commit) {
+            fail_db->FailNextCommit();
+        } else {
+            fail_db->FailNextWrite(record_type);
+        }
+        BOOST_CHECK_EXCEPTION((void)add_key(), std::runtime_error, HasReason{"UpdateWithSigningProvider: writing descriptor private key failed"});
+        BOOST_CHECK(!has_key());
+        BOOST_CHECK(!fail_db->HasRecordType(record_type));
+        BOOST_CHECK( add_key());
+        BOOST_CHECK( has_key());
+        BOOST_CHECK( fail_db->HasRecordType(record_type));
+    }
+}
+
 BOOST_FIXTURE_TEST_CASE(update_non_range_descriptor, TestingSetup)
 {
     CWallet wallet(m_node.chain.get(), "", CreateMockableWalletDatabase());

### src/wallet/types.h
@@ -72,6 +72,9 @@ enum class WalletErrorCode {
     //! Callers may ask the user to unlock the wallet and retry the operation.
     UnlockNeeded,
 
+    //! The passphrase does not decrypt the wallet. Callers may ask the user to enter it again.
+    PassphraseIncorrect,
+
     //! TODO Add correct descriptions to each error.
     //! At the moment only used by ImportDescriptors.
     InvalidDescriptor,

### src/wallet/wallet.cpp
@@ -587,7 +587,26 @@ static bool DecryptMasterKey(const SecureString& wallet_passphrase, const CMaste
     return true;
 }
 
-bool CWallet::Unlock(const SecureString& strWalletPassphrase)
+static util::Unexpected<WalletError> UnlockPassphraseError(const SecureString& passphrase)
+{
+    bilingual_str message;
+    if (passphrase.find('\0') != std::string::npos) {
+        // The passphrase has a null character (see #27067 for details)
+        message = _("Error: The wallet passphrase entered is incorrect. "
+                    "It contains a null character (ie - a zero byte). "
+                    "If the passphrase was set with a version of this software prior to 25.0, "
+                    "please try again with only the characters up to — but not including — "
+                    "the first null character. If this is successful, please set a new "
+                    "passphrase to avoid this issue in the future.");
+    } else if (passphrase.empty()) {
+        message = _("Error: The wallet passphrase was not provided");
+    } else {
+        message = _("Error: The wallet passphrase entered was incorrect.");
+    }
+    return util::Unexpected{WalletError{WalletErrorCode::PassphraseIncorrect, std::move(message)}};
+}
+
+util::Expected<void, WalletError> CWallet::Unlock(const SecureString& strWalletPassphrase)
 {
     CKeyingMaterial plain_master_key;
 
@@ -601,14 +620,14 @@ bool CWallet::Unlock(const SecureString& strWalletPassphrase)
             if (Unlock(plain_master_key)) {
                 // Now that we've unlocked, upgrade the descriptor cache
                 UpgradeDescriptorCache();
-                return true;
+                return {};
             }
         }
     }
-    return false;
+    return UnlockPassphraseError(strWalletPassphrase);
 }
 
-bool CWallet::ChangeWalletPassphrase(const SecureString& strOldWalletPassphrase, const SecureString& strNewWalletPassphrase)
+util::Expected<void, WalletError> CWallet::ChangeWalletPassphrase(const SecureString& strOldWalletPassphrase, const SecureString& strNewWalletPassphrase)
 {
     bool fWasLocked = IsLocked();
 
@@ -620,24 +639,26 @@ bool CWallet::ChangeWalletPassphrase(const SecureString& strOldWalletPassphrase,
         for (auto& [master_key_id, master_key] : mapMasterKeys)
         {
             if (!DecryptMasterKey(strOldWalletPassphrase, master_key, plain_master_key)) {
-                return false;
+                return UnlockPassphraseError(strOldWalletPassphrase);
             }
             if (Unlock(plain_master_key))
             {
-                if (!EncryptMasterKey(strNewWalletPassphrase, plain_master_key, master_key)) {
-                    return false;
+                if (fWasLocked) Lock();
+                CMasterKey new_master_key{master_key};
+                if (!EncryptMasterKey(strNewWalletPassphrase, plain_master_key, new_master_key)) {
+                    return util::Unexpected{WalletError{WalletErrorCode::GenericError, _("Error: Unable to encrypt encryption key with new passphrase")}};
                 }
-                WalletLogPrintf("Wallet passphrase changed to an nDeriveIterations of %i\n", master_key.nDeriveIterations);
-
-                WalletBatch(GetDatabase()).WriteMasterKey(master_key_id, master_key);
-                if (fWasLocked)
-                    Lock();
-                return true;
+                if (!WalletBatch(GetDatabase()).WriteMasterKey(master_key_id, new_master_key)) {
+                    return util::Unexpected{WalletError{WalletErrorCode::GenericError, _("Error: Writing the new encryption key to the wallet database failed")}};
+                }
+                WalletLogPrintf("Wallet passphrase changed to an nDeriveIterations of %i\n", new_master_key.nDeriveIterations);
+                master_key = std::move(new_master_key);
+                return {};
             }
         }
     }
 
-    return false;
+    return UnlockPassphraseError(strOldWalletPassphrase);
 }
 
 void CWallet::SetLastBlockProcessedInMem(int block_height, uint256 block_hash)
@@ -836,37 +857,23 @@ bool CWallet::EncryptWallet(const SecureString& strWalletPassphrase)
 
     {
         LOCK2(m_relock_mutex, cs_wallet);
-        mapMasterKeys[++nMasterKeyMaxID] = master_key;
-        WalletBatch* encrypted_batch = new WalletBatch(GetDatabase());
-        if (!encrypted_batch->TxnBegin()) {
-            delete encrypted_batch;
-            encrypted_batch = nullptr;
+        const unsigned int new_master_key_id{nMasterKeyMaxID + 1};
+        if (!RunWithinTxn(GetDatabase(), /*process_desc=*/"wallet encryption", [&](WalletBatch& batch) {
+                if (!batch.WriteMasterKey(new_master_key_id, master_key)) {
+                    return false;
+                }
+                for (const auto& spk_man_pair : m_spk_managers) {
+                    if (!spk_man_pair.second->Encrypt(plain_master_key, &batch)) {
+                        return false;
+                    }
+                }
+                return true;
+            })) {
             return false;
         }
-        encrypted_batch->WriteMasterKey(nMasterKeyMaxID, master_key);
-
-        for (const auto& spk_man_pair : m_spk_managers) {
-            auto spk_man = spk_man_pair.second.get();
-            if (!spk_man->Encrypt(plain_master_key, encrypted_batch)) {
-                encrypted_batch->TxnAbort();
-                delete encrypted_batch;
-                encrypted_batch = nullptr;
-                // We now probably have half of our keys encrypted in memory, and half not...
-                // die and let the user reload the unencrypted wallet.
-                assert(false);
-            }
-        }
-
-        if (!encrypted_batch->TxnCommit()) {
-            delete encrypted_batch;
-            encrypted_batch = nullptr;
-            // We now have keys encrypted in memory, but not on disk...
-            // die to avoid confusion and let the user reload the unencrypted wallet.
-            assert(false);
-        }
 
-        delete encrypted_batch;
-        encrypted_batch = nullptr;
+        nMasterKeyMaxID = new_master_key_id;
+        mapMasterKeys[new_master_key_id] = std::move(master_key);
 
         Lock();
         if (!Unlock(strWalletPassphrase)) {
@@ -4226,16 +4233,8 @@ util::Result<MigrationResult> MigrateLegacyToDescriptor(std::shared_ptr<CWallet>
     bool success = false;
 
     // Unlock the wallet if needed
-    if (local_wallet->IsLocked() && !local_wallet->Unlock(passphrase)) {
-        if (passphrase.find('\0') == std::string::npos) {
-            return util::Error{Untranslated("Error: Wallet decryption failed, the wallet passphrase was not provided or was incorrect.")};
-        } else {
-            return util::Error{Untranslated("Error: Wallet decryption failed, the wallet passphrase entered was incorrect. "
-                                            "The passphrase contains a null character (ie - a zero byte). "
-                                            "If this passphrase was set with a version of this software prior to 25.0, "
-                                            "please try again with only the characters up to — but not including — "
-                                            "the first null character.")};
-        }
+    if (local_wallet->IsLocked()) {
+        if (auto unlocked{local_wallet->Unlock(passphrase)}; !unlocked) return util::Error{unlocked.error().message};
     }
 
     // Indicates whether the current wallet is empty after migration.

### src/wallet/wallet.h
@@ -589,8 +589,8 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati
     // Used to prevent deleting the passphrase from memory when it is still in use.
     RecursiveMutex m_relock_mutex;
 
-    bool Unlock(const SecureString& strWalletPassphrase);
-    bool ChangeWalletPassphrase(const SecureString& strOldWalletPassphrase, const SecureString& strNewWalletPassphrase);
+    util::Expected<void, WalletError> Unlock(const SecureString& strWalletPassphrase);
+    util::Expected<void, WalletError> ChangeWalletPassphrase(const SecureString& strOldWalletPassphrase, const SecureString& strNewWalletPassphrase);
     bool EncryptWallet(const SecureString& strWalletPassphrase);
 
     unsigned int ComputeTimeSmart(const CWalletTx& wtx, bool rescanning_old_block) const;

### src/wallet/walletdb.cpp
@@ -234,10 +234,17 @@ bool WalletBatch::WriteDescriptorKey(const uint256& desc_id, const CPubKey& pubk
 
 bool WalletBatch::WriteCryptedDescriptorKey(const uint256& desc_id, const CPubKey& pubkey, const std::vector<unsigned char>& secret)
 {
-    if (!WriteIC(std::make_pair(DBKeys::WALLETDESCRIPTORCKEY, std::make_pair(desc_id, pubkey)), secret, false)) {
+    const auto descriptor_key{std::make_pair(desc_id, pubkey)};
+    const auto plaintext_key{std::make_pair(DBKeys::WALLETDESCRIPTORKEY, descriptor_key)};
+    const auto encrypted_key{std::make_pair(DBKeys::WALLETDESCRIPTORCKEY, descriptor_key)};
+
+    // Keep the write and erase atomic even when the caller has not started a transaction
+    const bool own_txn{!HasActiveTxn()};
+    if (own_txn && !TxnBegin()) return false;
+    if (!WriteIC(encrypted_key, secret, /*fOverwrite=*/false) || !EraseIC(plaintext_key) || (own_txn && !TxnCommit())) {
+        if (own_txn) TxnAbort();
         return false;
     }
-    EraseIC(std::make_pair(DBKeys::WALLETDESCRIPTORKEY, std::make_pair(desc_id, pubkey)));
     return true;
 }
 
@@ -1320,10 +1327,10 @@ bool WalletBatch::TxnAbort()
     return res;
 }
 
-void WalletBatch::RegisterTxnListener(const DbTxnListener& l)
+void WalletBatch::RegisterTxnListener(DbTxnListener l)
 {
     assert(m_batch->HasActiveTxn());
-    m_txn_listeners.emplace_back(l);
+    m_txn_listeners.emplace_back(std::move(l));
 }
 
 std::unique_ptr<WalletDatabase> MakeDatabase(const fs::path& path, const DatabaseOptions& options, DatabaseStatus& status, bilingual_str& error)

### src/wallet/walletdb.h
@@ -295,7 +295,7 @@ class WalletBatch
     bool HasActiveTxn() { return m_batch->HasActiveTxn(); }
 
     //! Registers db txn callback functions
-    void RegisterTxnListener(const DbTxnListener& l);
+    void RegisterTxnListener(DbTxnListener l);
 
 private:
     std::unique_ptr<DatabaseBatch> m_batch;

### test/functional/wallet_encryption.py
@@ -98,7 +98,7 @@ def run_test(self):
             assert_raises_rpc_error(-14, "wallet passphrase entered was incorrect", self.nodes[0].walletpassphrase, passphrase_with_nulls.partition("\0")[0], 10)
             assert_raises_rpc_error(-14, "The wallet passphrase entered was incorrect", self.nodes[0].walletpassphrasechange, passphrase_with_nulls.partition("\0")[0], "abc")
             assert_raises_rpc_error(-14, "wallet passphrase entered is incorrect. It contains a null character (ie - a zero byte)", self.nodes[0].walletpassphrase, passphrase_with_nulls + "\0", 10)
-            assert_raises_rpc_error(-14, "The old wallet passphrase entered is incorrect. It contains a null character (ie - a zero byte)", self.nodes[0].walletpassphrasechange, passphrase_with_nulls + "\0", "abc")
+            assert_raises_rpc_error(-14, "The wallet passphrase entered is incorrect. It contains a null character (ie - a zero byte)", self.nodes[0].walletpassphrasechange, passphrase_with_nulls + "\0", "abc")
             with WalletUnlock(self.nodes[0], passphrase_with_nulls):
                 sig = self.nodes[0].signmessage(address, msg)
                 assert self.nodes[0].verifymessage(address, sig, msg)

### test/functional/wallet_migration.py
@@ -556,14 +556,14 @@ def test_encrypted(self):
         bals["mine"]["nonmempool"] = Decimal('0.0')
 
         # Use self.migrate_and_get_rpc to test this error to get everything copied over to the master node
-        assert_raises_rpc_error(-4, "Error: Wallet decryption failed, the wallet passphrase was not provided or was incorrect", self.migrate_and_get_rpc, "encrypted")
+        assert_raises_rpc_error(-4, "Error: The wallet passphrase was not provided", self.migrate_and_get_rpc, "encrypted")
 
         # Use the RPC directly on the master node for the rest of these checks
         self.master_node.bumpmocktime(1) # Prevents filename duplication on wallet backups which is a problem on Windows
-        assert_raises_rpc_error(-4, "Error: Wallet decryption failed, the wallet passphrase was not provided or was incorrect", self.master_node.migratewallet, "encrypted", "badpass")
+        assert_raises_rpc_error(-4, "Error: The wallet passphrase entered was incorrect.", self.master_node.migratewallet, "encrypted", "badpass")
 
         self.master_node.bumpmocktime(1) # Prevents filename duplication on wallet backups which is a problem on Windows
-        assert_raises_rpc_error(-4, "The passphrase contains a null character", self.master_node.migratewallet, "encrypted", "pass\0with\0null")
+        assert_raises_rpc_error(-4, "Error: The wallet passphrase entered is incorrect. It contains a null character (ie - a zero byte).", self.master_node.migratewallet, "encrypted", "pass\0with\0null")
 
         # Verify we can properly migrate the encrypted wallet
         self.master_node.bumpmocktime(1) # Prevents filename duplication on wallet backups which is a problem on Windows