Refactor SyncState for zcash tests

Author: supermassive

components/brave_wallet/browser/brave_wallet_service.cc

@@ -211,8 +211,13 @@ BraveWalletService::BraveWalletService(
 
   if (IsZCashEnabled()) {
     zcash_wallet_service_ = std::make_unique<ZCashWalletService>(
-        delegate_->GetWalletBaseDirectory().AppendASCII(kZCashDataFolderName),
-        *keyring_service(), network_manager(), url_loader_factory);
+        *keyring_service(),
+        std::make_unique<ZCashRpc>(network_manager(), url_loader_factory));
+    zcash_wallet_service_->SetupSyncState(
+        OrchardSyncState::CreateSyncStateSequence(),
+        OrchardSyncState::CreateSyncState(
+            delegate_->GetWalletBaseDirectory().AppendASCII(
+                kZCashDataFolderName)));
   }
 
   if (IsCardanoEnabled()) {

components/brave_wallet/browser/internal/orchard_storage/orchard_storage.cc

@@ -15,6 +15,7 @@
 #include "base/check.h"
 #include "base/containers/to_vector.h"
 #include "base/files/file_util.h"
+#include "base/sequence_checker.h"
 #include "base/threading/scoped_blocking_call.h"
 #include "base/types/expected_macros.h"
 #include "sql/meta_table.h"
@@ -157,7 +158,10 @@ OrchardStorage::AccountMeta& OrchardStorage::AccountMeta::operator=(
 
 OrchardStorage::OrchardStorage(const base::FilePath& path_to_database)
     : db_file_path_(path_to_database),
-      database_(sql::Database::Tag("OrchardStorageDatabase")) {}
+      database_(sql::Database::Tag("OrchardStorageDatabase")) {
+  // Instantiated on default sequence. Operates on dedicated sequence.
+  DETACH_FROM_SEQUENCE(sequence_checker_);
+}
 
 OrchardStorage::~OrchardStorage() {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);

components/brave_wallet/browser/internal/orchard_storage/orchard_storage_unittest.cc

@@ -26,9 +26,8 @@ class OrchardStorageTest : public testing::Test {
 
 void OrchardStorageTest::SetUp() {
   ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
-  base::FilePath db_path(
-      temp_dir_.GetPath().Append(FILE_PATH_LITERAL("orchard.db")));
-  orchard_storage_ = std::make_unique<OrchardStorage>(db_path);
+  orchard_storage_ = std::make_unique<OrchardStorage>(
+      temp_dir_.GetPath().AppendASCII("orchard.db"));
   orchard_storage_->EnsureDbInit();
 }
 

components/brave_wallet/browser/internal/orchard_sync_state.cc

@@ -11,12 +11,28 @@
 #include "base/check.h"
 #include "base/check_is_test.h"
 #include "base/containers/extend.h"
+#include "base/task/thread_pool.h"
 #include "base/types/expected_macros.h"
-#include "brave/components/brave_wallet/browser/internal/orchard_storage/orchard_shard_tree_types.h"
 #include "brave/components/brave_wallet/common/zcash_utils.h"
 
 namespace brave_wallet {
 
+namespace {
+inline constexpr char kOrchardDatabaseName[] = "orchard.db";
+}
+
+// static
+scoped_refptr<base::SequencedTaskRunner>
+OrchardSyncState::CreateSyncStateSequence() {
+  return base::ThreadPool::CreateSequencedTaskRunner({base::MayBlock()});
+}
+
+// static
+std::unique_ptr<OrchardSyncState> OrchardSyncState::CreateSyncState(
+    const base::FilePath& zcash_data_path) {
+  return std::make_unique<OrchardSyncState>(zcash_data_path);
+}
+
 OrchardSyncState::SpendableNotesBundle::SpendableNotesBundle() = default;
 OrchardSyncState::SpendableNotesBundle::~SpendableNotesBundle() = default;
 OrchardSyncState::SpendableNotesBundle::SpendableNotesBundle(
@@ -25,8 +41,13 @@ OrchardSyncState::SpendableNotesBundle&
 OrchardSyncState::SpendableNotesBundle::operator=(
     OrchardSyncState::SpendableNotesBundle&&) = default;
 
-OrchardSyncState::OrchardSyncState(const base::FilePath& path_to_database)
-    : storage_(OrchardStorage(path_to_database)) {}
+OrchardSyncState::OrchardSyncState(
+    const base::FilePath& path_to_database_folder)
+    : storage_(OrchardStorage(
+          path_to_database_folder.AppendASCII(kOrchardDatabaseName))) {
+  // Instantiated on default sequence. Operates on dedicated sequence.
+  DETACH_FROM_SEQUENCE(sequence_checker_);
+}
 
 OrchardSyncState::~OrchardSyncState() {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);

components/brave_wallet/browser/internal/orchard_sync_state.h

@@ -12,6 +12,7 @@
 #include <vector>
 
 #include "base/sequence_checker.h"
+#include "base/threading/sequence_bound.h"
 #include "brave/components/brave_wallet/browser/internal/orchard_block_scanner.h"
 #include "brave/components/brave_wallet/browser/internal/orchard_storage/orchard_shard_tree_types.h"
 #include "brave/components/brave_wallet/browser/internal/orchard_storage/orchard_storage.h"
@@ -29,6 +30,15 @@ inline constexpr uint32_t kZCashPublicAddressMinConfirmations = 10u;
 // commitment tree, which is used to sign notes for spending.
 class OrchardSyncState {
  public:
+  using SequenceBound = base::SequenceBound<std::unique_ptr<OrchardSyncState>>;
+
+  // Creates a new sequence for an instance of OrchardSyncState to be bound to.
+  static scoped_refptr<base::SequencedTaskRunner> CreateSyncStateSequence();
+
+  // Creates a new sync state with the given data path.
+  static std::unique_ptr<OrchardSyncState> CreateSyncState(
+      const base::FilePath& zcash_data_path);
+
   struct SpendableNotesBundle {
     SpendableNotesBundle();
     SpendableNotesBundle(const SpendableNotesBundle&) = delete;
@@ -41,7 +51,7 @@ class OrchardSyncState {
     std::optional<uint32_t> anchor_block_id;
   };
 
-  explicit OrchardSyncState(const base::FilePath& path_to_database);
+  explicit OrchardSyncState(const base::FilePath& path_to_database_folder);
   virtual ~OrchardSyncState();
 
   base::expected<OrchardStorage::Result, OrchardStorage::Error> RegisterAccount(

components/brave_wallet/browser/internal/orchard_sync_state_unittest.cc

@@ -60,9 +60,7 @@ void OrchardSyncStateTest::SetUp() {
                                         mojom::KeyringId::kZCashMainnet,
                                         mojom::AccountKind::kDerived, 0);
   ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
-  base::FilePath db_path(
-      temp_dir_.GetPath().Append(FILE_PATH_LITERAL("orchard.db")));
-  sync_state_ = std::make_unique<OrchardSyncState>(db_path);
+  sync_state_ = std::make_unique<OrchardSyncState>(temp_dir_.GetPath());
   sync_state_->OverrideShardTreeForTesting(
       account_id_, orchard::CreateShardTreeForTesting(
                        sync_state_->orchard_storage(), account_id_));

components/brave_wallet/browser/zcash/zcash_action_context.cc

@@ -12,7 +12,7 @@ namespace brave_wallet {
 ZCashActionContext::ZCashActionContext(
     ZCashRpc& zcash_rpc,
     const std::optional<OrchardAddrRawPart>& account_internal_addr,
-    base::SequenceBound<OrchardSyncState>& sync_state,
+    OrchardSyncState::SequenceBound& sync_state,
     const mojom::AccountIdPtr& account_id)
     : zcash_rpc(zcash_rpc),
       account_internal_addr(account_internal_addr),

components/brave_wallet/browser/zcash/zcash_action_context.h

@@ -9,27 +9,26 @@
 #include <string>
 
 #include "base/memory/raw_ref.h"
-#include "base/threading/sequence_bound.h"
+#include "brave/components/brave_wallet/browser/internal/orchard_sync_state.h"
 #include "brave/components/brave_wallet/common/brave_wallet.mojom.h"
 #include "brave/components/brave_wallet/common/buildflags/buildflags.h"
 #include "brave/components/brave_wallet/common/zcash_utils.h"
 
 namespace brave_wallet {
 
-class OrchardSyncState;
 class ZCashRpc;
 
 // Basic context required by most orchard-related operations.
 struct ZCashActionContext {
   ZCashActionContext(
       ZCashRpc& zcash_rpc,
       const std::optional<OrchardAddrRawPart>& account_internal_addr,
-      base::SequenceBound<OrchardSyncState>& sync_state,
+      OrchardSyncState::SequenceBound& sync_state,
       const mojom::AccountIdPtr& account_id);
   ~ZCashActionContext();
   raw_ref<ZCashRpc> zcash_rpc;
   std::optional<OrchardAddrRawPart> account_internal_addr;
-  raw_ref<base::SequenceBound<OrchardSyncState>> sync_state;
+  raw_ref<OrchardSyncState::SequenceBound> sync_state;
   ZCashActionContext(ZCashActionContext&) = delete;
   ZCashActionContext& operator=(ZCashActionContext&) = delete;
   ZCashActionContext& operator=(ZCashActionContext&&);

components/brave_wallet/browser/zcash/zcash_auto_sync_manager_unittest.cc

@@ -11,11 +11,10 @@
 #include "base/files/scoped_temp_dir.h"
 #include "base/test/task_environment.h"
 #include "brave/components/brave_wallet/browser/pref_names.h"
-#include "brave/components/brave_wallet/browser/zcash/zcash_action_context.h"
+#include "brave/components/brave_wallet/browser/zcash/zcash_test_utils.h"
 #include "brave/components/brave_wallet/browser/zcash/zcash_wallet_service.h"
 #include "brave/components/brave_wallet/common/brave_wallet.mojom.h"
 #include "brave/components/brave_wallet/common/common_utils.h"
-#include "brave/components/brave_wallet/common/zcash_utils.h"
 #include "components/sync_preferences/testing_pref_service_syncable.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -26,14 +25,9 @@ using testing::_;
 
 namespace {
 
-class MockZCashWalletService : public ZCashWalletService {
+class MockZCashWalletService : public TestingZCashWalletService {
  public:
-  MockZCashWalletService(base::FilePath zcash_data_path,
-                         KeyringService& keyring_service,
-                         std::unique_ptr<ZCashRpc> zcash_rpc)
-      : ZCashWalletService(zcash_data_path,
-                           keyring_service,
-                           std::move(zcash_rpc)) {}
+  using TestingZCashWalletService::TestingZCashWalletService;
 
   MOCK_METHOD2(GetChainTipStatus,
                void(mojom::AccountIdPtr account_id,
@@ -54,28 +48,24 @@ class ZCashAutoSyncManagerTest : public testing::Test {
 
   void SetUp() override {
     ASSERT_TRUE(temp_directory_.CreateUniqueTempDir());
-    base::FilePath db_path(
-        temp_directory_.GetPath().Append(FILE_PATH_LITERAL("orchard.db")));
-    zcash_rpc_ = std::make_unique<ZCashRpc>(nullptr, nullptr);
     brave_wallet::RegisterProfilePrefs(prefs_.registry());
     brave_wallet::RegisterLocalStatePrefs(local_state_.registry());
     keyring_service_ =
         std::make_unique<KeyringService>(nullptr, &prefs_, &local_state_);
     mock_zcash_wallet_service_ = std::make_unique<MockZCashWalletService>(
-        db_path, *keyring_service_,
-        std::make_unique<ZCashRpc>(nullptr, nullptr));
+        *keyring_service_, std::make_unique<ZCashRpc>(nullptr, nullptr));
+    mock_zcash_wallet_service_->SetupSyncState(
+        OrchardSyncState::CreateSyncStateSequence(),
+        OrchardSyncState::CreateSyncState(temp_directory_.GetPath()));
+
     account_id_ = MakeIndexBasedAccountId(mojom::CoinType::ZEC,
                                           mojom::KeyringId::kZCashMainnet,
                                           mojom::AccountKind::kDerived, 0);
     zcash_auto_sync_manager_ = std::make_unique<ZCashAutoSyncManager>(
         *mock_zcash_wallet_service_,
-        ZCashActionContext(*zcash_rpc_, OrchardAddrRawPart(),
-                           mock_zcash_wallet_service_->sync_state(),
-                           account_id_));
+        mock_zcash_wallet_service_->CreateActionContext(account_id_));
   }
 
-  void TearDown() override { sync_state_.SynchronouslyResetForTest(); }
-
   base::test::TaskEnvironment& task_environment() { return task_environment_; }
 
   MockZCashWalletService& mock_zcash_wallet_service() {
@@ -87,16 +77,14 @@ class ZCashAutoSyncManagerTest : public testing::Test {
   }
 
  private:
+  base::test::TaskEnvironment task_environment_;
   base::ScopedTempDir temp_directory_;
   sync_preferences::TestingPrefServiceSyncable prefs_;
   sync_preferences::TestingPrefServiceSyncable local_state_;
   mojom::AccountIdPtr account_id_;
-  base::SequenceBound<OrchardSyncState> sync_state_;
   std::unique_ptr<KeyringService> keyring_service_;
-  std::unique_ptr<ZCashRpc> zcash_rpc_;
   std::unique_ptr<MockZCashWalletService> mock_zcash_wallet_service_;
   std::unique_ptr<ZCashAutoSyncManager> zcash_auto_sync_manager_;
-  base::test::TaskEnvironment task_environment_;
 };
 
 TEST_F(ZCashAutoSyncManagerTest, InitialSync) {

components/brave_wallet/browser/zcash/zcash_blocks_batch_scan_task.cc

@@ -14,7 +14,7 @@
 #include "base/check_op.h"
 #include "base/containers/extend.h"
 #include "base/strings/string_number_conversions.h"
-#include "brave/components/brave_wallet/browser/zcash/zcash_wallet_service.h"
+#include "brave/components/brave_wallet/browser/zcash/zcash_rpc.h"
 
 namespace brave_wallet {