[DanglingPtr][wallet] Prefer non-nullable states pt.2

browser/brave_wallet/asset_discovery_manager_unittest.cc

@@ -9,6 +9,7 @@
 #include "base/test/bind.h"
 #include "base/test/scoped_feature_list.h"
 #include "base/time/time.h"
+#include "brave/components/api_request_helper/api_request_helper.h"
 #include "brave/components/brave_wallet/browser/brave_wallet_constants.h"
 #include "brave/components/brave_wallet/browser/brave_wallet_service.h"
 #include "brave/components/brave_wallet/browser/brave_wallet_service_delegate.h"
@@ -29,6 +30,7 @@
 #include "components/prefs/scoped_user_pref_update.h"
 #include "components/sync_preferences/testing_pref_service_syncable.h"
 #include "content/public/test/browser_task_environment.h"
+#include "net/traffic_annotation/network_traffic_annotation_test_helper.h"
 #include "services/data_decoder/public/cpp/test_support/in_process_data_decoder.h"
 #include "services/network/public/cpp/weak_wrapper_shared_url_loader_factory.h"
 #include "services/network/test/test_url_loader_factory.h"
@@ -123,11 +125,15 @@ class AssetDiscoveryManagerUnitTest : public testing::Test {
     simple_hash_client_ =
         std::make_unique<SimpleHashClient>(shared_url_loader_factory_);
     asset_discovery_manager_ = std::make_unique<AssetDiscoveryManager>(
-        shared_url_loader_factory_, wallet_service_.get(), json_rpc_service_,
-        keyring_service_, simple_hash_client_.get(), GetPrefs());
+        shared_url_loader_factory_, *wallet_service_, *json_rpc_service_,
+        *keyring_service_, *simple_hash_client_, GetPrefs());
     wallet_service_observer_ = std::make_unique<
         TestBraveWalletServiceObserverForAssetDiscoveryManager>();
     wallet_service_->AddObserver(wallet_service_observer_->GetReceiver());
+
+    api_request_helper_ =
+        std::make_unique<api_request_helper::APIRequestHelper>(
+            TRAFFIC_ANNOTATION_FOR_TESTS, shared_url_loader_factory_);
   }
 
   PrefService* GetPrefs() { return profile_->GetPrefs(); }
@@ -140,6 +146,7 @@ class AssetDiscoveryManagerUnitTest : public testing::Test {
   std::unique_ptr<TestBraveWalletServiceObserverForAssetDiscoveryManager>
       wallet_service_observer_;
   content::BrowserTaskEnvironment task_environment_;
+  std::unique_ptr<api_request_helper::APIRequestHelper> api_request_helper_;
   std::unique_ptr<ScopedTestingLocalState> local_state_;
   std::unique_ptr<TestingProfile> profile_;
   std::unique_ptr<BraveWalletService> wallet_service_;
@@ -275,8 +282,9 @@ TEST_F(AssetDiscoveryManagerUnitTest, DiscoverAssetsOnAllSupportedChains) {
   task_environment_.FastForwardBy(
       base::Minutes(kAssetDiscoveryMinutesPerRequest));
   std::queue<std::unique_ptr<AssetDiscoveryTask>> tasks;
-  tasks.push(std::make_unique<AssetDiscoveryTask>(nullptr, nullptr, nullptr,
-                                                  nullptr, nullptr));
+  tasks.push(std::make_unique<AssetDiscoveryTask>(
+      *api_request_helper_, *simple_hash_client_, *wallet_service_,
+      *json_rpc_service_, GetPrefs()));
   asset_discovery_manager_->SetQueueForTesting(std::move(tasks));
   TestDiscoverAssetsOnAllSupportedChains({}, true, true, {}, 1);
 

browser/brave_wallet/asset_discovery_task_unittest.cc

@@ -184,8 +184,8 @@ class AssetDiscoveryTaskUnitTest : public testing::Test {
     simple_hash_client_ =
         std::make_unique<SimpleHashClient>(shared_url_loader_factory_);
     asset_discovery_task_ = std::make_unique<AssetDiscoveryTask>(
-        api_request_helper_.get(), simple_hash_client_.get(),
-        wallet_service_.get(), json_rpc_service_, GetPrefs());
+        *api_request_helper_, *simple_hash_client_, *wallet_service_,
+        *json_rpc_service_, GetPrefs());
     wallet_service_observer_ =
         std::make_unique<TestBraveWalletServiceObserverForAssetDiscoveryTask>();
     wallet_service_->AddObserver(wallet_service_observer_->GetReceiver());

components/brave_wallet/browser/account_discovery_manager.cc

@@ -48,8 +48,8 @@ AccountDiscoveryManager::DiscoveryContext::DiscoveryContext(
 AccountDiscoveryManager::DiscoveryContext::~DiscoveryContext() = default;
 
 AccountDiscoveryManager::AccountDiscoveryManager(
-    JsonRpcService* rpc_service,
-    KeyringService* keyring_service,
+    JsonRpcService& rpc_service,
+    KeyringService& keyring_service,
     BitcoinWalletService* bitcoin_wallet_service)
     : json_rpc_service_(rpc_service),
       keyring_service_(keyring_service),
@@ -108,11 +108,6 @@ void AccountDiscoveryManager::AddDiscoveryAccount(
     return;
   }
 
-  if (!json_rpc_service_) {
-    CHECK_IS_TEST();
-    return;
-  }
-
   auto chain_id = context->chain_id;
   auto coin_type = context->coin_type;
   if (context->coin_type == mojom::CoinType::ETH) {

components/brave_wallet/browser/account_discovery_manager.h

@@ -27,8 +27,8 @@ struct DiscoveredBitcoinAccount;
 // transactions.
 class AccountDiscoveryManager {
  public:
-  AccountDiscoveryManager(JsonRpcService* rpc_service,
-                          KeyringService* keyring_service,
+  AccountDiscoveryManager(JsonRpcService& rpc_service,
+                          KeyringService& keyring_service,
                           BitcoinWalletService* bitcoin_wallet_service);
   ~AccountDiscoveryManager();
 
@@ -75,8 +75,8 @@ class AccountDiscoveryManager {
   void ProcessDiscoveryResult(std::unique_ptr<DiscoveryContext> context,
                               bool result);
 
-  raw_ptr<brave_wallet::JsonRpcService> json_rpc_service_;
-  raw_ptr<brave_wallet::KeyringService> keyring_service_;
+  raw_ref<brave_wallet::JsonRpcService> json_rpc_service_;
+  raw_ref<brave_wallet::KeyringService> keyring_service_;
   raw_ptr<brave_wallet::BitcoinWalletService> bitcoin_wallet_service_;
 
   base::WeakPtrFactory<AccountDiscoveryManager> weak_ptr_factory_{this};

components/brave_wallet/browser/account_discovery_manager_unittest.cc

@@ -24,6 +24,7 @@
 #include "components/sync_preferences/testing_pref_service_syncable.h"
 #include "services/data_decoder/public/cpp/test_support/in_process_data_decoder.h"
 #include "services/network/public/cpp/shared_url_loader_factory.h"
+#include "services/network/public/cpp/weak_wrapper_shared_url_loader_factory.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -32,7 +33,10 @@ namespace brave_wallet {
 class AccountDiscoveryManagerUnitTest : public testing::Test {
  public:
   AccountDiscoveryManagerUnitTest()
-      : task_environment_(base::test::TaskEnvironment::TimeSource::MOCK_TIME) {}
+      : task_environment_(base::test::TaskEnvironment::TimeSource::MOCK_TIME),
+        shared_url_loader_factory_(
+            base::MakeRefCounted<network::WeakWrapperSharedURLLoaderFactory>(
+                &url_loader_factory_)) {}
 
   ~AccountDiscoveryManagerUnitTest() override = default;
 
@@ -43,6 +47,8 @@ class AccountDiscoveryManagerUnitTest : public testing::Test {
     network_manager_ = std::make_unique<NetworkManager>(&prefs_);
     keyring_service_ =
         std::make_unique<KeyringService>(nullptr, &prefs_, &local_state_);
+    json_rpc_service_ = std::make_unique<JsonRpcService>(
+        shared_url_loader_factory_, network_manager_.get(), &prefs_, nullptr);
     bitcoin_test_rpc_server_ = std::make_unique<BitcoinTestRpcServer>();
     bitcoin_wallet_service_ = std::make_unique<BitcoinWalletService>(
         *keyring_service_, *network_manager_,
@@ -71,8 +77,11 @@ class AccountDiscoveryManagerUnitTest : public testing::Test {
   sync_preferences::TestingPrefServiceSyncable prefs_;
   sync_preferences::TestingPrefServiceSyncable local_state_;
 
+  network::TestURLLoaderFactory url_loader_factory_;
+  scoped_refptr<network::SharedURLLoaderFactory> shared_url_loader_factory_;
   std::unique_ptr<BitcoinTestRpcServer> bitcoin_test_rpc_server_;
   std::unique_ptr<NetworkManager> network_manager_;
+  std::unique_ptr<JsonRpcService> json_rpc_service_;
   std::unique_ptr<KeyringService> keyring_service_;
   std::unique_ptr<BitcoinWalletService> bitcoin_wallet_service_;
   std::unique_ptr<BitcoinHDKeyring> keyring_;
@@ -89,8 +98,8 @@ TEST_F(AccountDiscoveryManagerUnitTest, DiscoverBtcAccountCreatesNew) {
 
   EXPECT_EQ(0u, GetAccountUtils().AllBtcAccounts().size());
 
-  AccountDiscoveryManager discovery_manager(nullptr, keyring_service_.get(),
-                                            bitcoin_wallet_service_.get());
+  AccountDiscoveryManager discovery_manager(
+      *json_rpc_service_, *keyring_service_, bitcoin_wallet_service_.get());
   discovery_manager.StartDiscovery();
   task_environment_.RunUntilIdle();
 
@@ -130,8 +139,8 @@ TEST_F(AccountDiscoveryManagerUnitTest, DiscoverBtcAccountUpdatesExisting) {
             *keyring_service_->GetBitcoinAccountInfo(account_id)
                  ->next_change_address->key_id);
 
-  AccountDiscoveryManager discovery_manager(nullptr, keyring_service_.get(),
-                                            bitcoin_wallet_service_.get());
+  AccountDiscoveryManager discovery_manager(
+      *json_rpc_service_, *keyring_service_, bitcoin_wallet_service_.get());
   discovery_manager.StartDiscovery();
   task_environment_.RunUntilIdle();
 

components/brave_wallet/browser/account_resolver_delegate_impl.cc

@@ -8,7 +8,7 @@
 namespace brave_wallet {
 
 AccountResolverDelegateImpl::AccountResolverDelegateImpl(
-    KeyringService* keyring_service)
+    KeyringService& keyring_service)
     : keyring_service_(keyring_service) {}
 
 mojom::AccountIdPtr AccountResolverDelegateImpl::ResolveAccountId(

components/brave_wallet/browser/account_resolver_delegate_impl.h

@@ -15,15 +15,15 @@ namespace brave_wallet {
 
 class AccountResolverDelegateImpl : public AccountResolverDelegate {
  public:
-  explicit AccountResolverDelegateImpl(KeyringService* keyring_service);
+  explicit AccountResolverDelegateImpl(KeyringService& keyring_service);
 
   mojom::AccountIdPtr ResolveAccountId(
       const std::string* from_account_id,
       const std::string* from_address) override;
   bool ValidateAccountId(const mojom::AccountIdPtr& account_id) override;
 
  private:
-  raw_ptr<KeyringService> keyring_service_ = nullptr;
+  raw_ref<KeyringService> keyring_service_;
 };
 
 }  // namespace brave_wallet

components/brave_wallet/browser/account_resolver_delegate_impl_unittest.cc

@@ -49,7 +49,7 @@ class AccountResolverDelegateImplUnitTest : public testing::Test {
     GetAccountUtils().CreateWallet(kMnemonicDivideCruise, kTestWalletPassword);
 
     resolver_ =
-        std::make_unique<AccountResolverDelegateImpl>(keyring_service_.get());
+        std::make_unique<AccountResolverDelegateImpl>(*keyring_service_);
   }
 
   ~AccountResolverDelegateImplUnitTest() override = default;

components/brave_wallet/browser/asset_discovery_manager.cc

@@ -49,10 +49,10 @@ namespace brave_wallet {
 
 AssetDiscoveryManager::AssetDiscoveryManager(
     scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
-    BraveWalletService* wallet_service,
-    JsonRpcService* json_rpc_service,
-    KeyringService* keyring_service,
-    SimpleHashClient* simple_hash_client,
+    BraveWalletService& wallet_service,
+    JsonRpcService& json_rpc_service,
+    KeyringService& keyring_service,
+    SimpleHashClient& simple_hash_client,
     PrefService* prefs)
     : api_request_helper_(std::make_unique<APIRequestHelper>(
           GetAssetDiscoveryManagerNetworkTrafficAnnotationTag(),
@@ -161,8 +161,8 @@ void AssetDiscoveryManager::AddTask(
   auto non_fungible_supported_chains = GetNonFungibleSupportedChains();
 
   auto task = std::make_unique<AssetDiscoveryTask>(
-      api_request_helper_.get(), simple_hash_client_, wallet_service_,
-      json_rpc_service_, prefs_);
+      *api_request_helper_, *simple_hash_client_, *wallet_service_,
+      *json_rpc_service_, prefs_);
   auto callback = base::BindOnce(&AssetDiscoveryManager::FinishTask,
                                  weak_ptr_factory_.GetWeakPtr());
   auto* task_ptr = task.get();

components/brave_wallet/browser/asset_discovery_manager.h

@@ -31,18 +31,17 @@ class BraveWalletService;
 class JsonRpcService;
 class SimpleHashClient;
 class KeyringService;
-class NetworkManager;
 
 class AssetDiscoveryManager : public KeyringServiceObserverBase {
  public:
   using APIRequestHelper = api_request_helper::APIRequestHelper;
   using APIRequestResult = api_request_helper::APIRequestResult;
   AssetDiscoveryManager(
       scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
-      BraveWalletService* wallet_service,
-      JsonRpcService* json_rpc_service,
-      KeyringService* keyring_service,
-      SimpleHashClient* simple_hash_client,
+      BraveWalletService& wallet_service,
+      JsonRpcService& json_rpc_service,
+      KeyringService& keyring_service,
+      SimpleHashClient& simple_hash_client,
       PrefService* prefs);
 
   AssetDiscoveryManager(const AssetDiscoveryManager&) = delete;
@@ -85,11 +84,10 @@ class AssetDiscoveryManager : public KeyringServiceObserverBase {
 
   std::unique_ptr<APIRequestHelper> api_request_helper_;
   std::queue<std::unique_ptr<AssetDiscoveryTask>> queue_;
-  raw_ptr<BraveWalletService> wallet_service_;
-  raw_ptr<NetworkManager> network_manager_;
-  raw_ptr<JsonRpcService> json_rpc_service_;
-  raw_ptr<KeyringService> keyring_service_;
-  raw_ptr<SimpleHashClient> simple_hash_client_;
+  raw_ref<BraveWalletService> wallet_service_;
+  raw_ref<JsonRpcService> json_rpc_service_;
+  raw_ref<KeyringService> keyring_service_;
+  raw_ref<SimpleHashClient> simple_hash_client_;
   raw_ptr<PrefService> prefs_;
   mojo::Receiver<brave_wallet::mojom::KeyringServiceObserver>
       keyring_service_observer_receiver_{this};

components/brave_wallet/browser/asset_discovery_task.cc

@@ -29,10 +29,10 @@
 
 namespace brave_wallet {
 
-AssetDiscoveryTask::AssetDiscoveryTask(APIRequestHelper* api_request_helper,
-                                       SimpleHashClient* simple_hash_client,
-                                       BraveWalletService* wallet_service,
-                                       JsonRpcService* json_rpc_service,
+AssetDiscoveryTask::AssetDiscoveryTask(APIRequestHelper& api_request_helper,
+                                       SimpleHashClient& simple_hash_client,
+                                       BraveWalletService& wallet_service,
+                                       JsonRpcService& json_rpc_service,
                                        PrefService* prefs)
     : api_request_helper_(api_request_helper),
       simple_hash_client_(simple_hash_client),

components/brave_wallet/browser/asset_discovery_task.h

@@ -35,10 +35,10 @@ class AssetDiscoveryTask {
   using APIRequestHelper = api_request_helper::APIRequestHelper;
   using APIRequestResult = api_request_helper::APIRequestResult;
 
-  AssetDiscoveryTask(APIRequestHelper* api_request_helper,
-                     SimpleHashClient* simple_hash_client,
-                     BraveWalletService* wallet_service,
-                     JsonRpcService* json_rpc_service,
+  AssetDiscoveryTask(APIRequestHelper& api_request_helper,
+                     SimpleHashClient& simple_hash_client,
+                     BraveWalletService& wallet_service,
+                     JsonRpcService& json_rpc_service,
                      PrefService* prefs);
 
   AssetDiscoveryTask(const AssetDiscoveryTask&) = delete;
@@ -141,10 +141,10 @@ class AssetDiscoveryTask {
   static std::optional<SolanaAddress> DecodeMintAddress(
       const std::vector<uint8_t>& data);
 
-  raw_ptr<APIRequestHelper> api_request_helper_;
-  raw_ptr<SimpleHashClient> simple_hash_client_;
-  raw_ptr<BraveWalletService> wallet_service_;
-  raw_ptr<JsonRpcService> json_rpc_service_;
+  raw_ref<APIRequestHelper> api_request_helper_;
+  raw_ref<SimpleHashClient> simple_hash_client_;
+  raw_ref<BraveWalletService> wallet_service_;
+  raw_ref<JsonRpcService> json_rpc_service_;
   raw_ptr<PrefService> prefs_;
   base::WeakPtrFactory<AssetDiscoveryTask> weak_ptr_factory_;
 };

components/brave_wallet/browser/brave_wallet_service.cc

@@ -131,8 +131,7 @@ BraveWalletService::BraveWalletService(
   if (IsZCashEnabled()) {
     zcash_wallet_service_ = std::make_unique<ZCashWalletService>(
         delegate_->GetWalletBaseDirectory().AppendASCII(kZCashDataFolderName),
-        keyring_service(), profile_prefs, network_manager(),
-        url_loader_factory);
+        *keyring_service(), network_manager(), url_loader_factory);
   }
 
   tx_service_ = std::make_unique<TxService>(
@@ -145,8 +144,8 @@ BraveWalletService::BraveWalletService(
 
   simple_hash_client_ = std::make_unique<SimpleHashClient>(url_loader_factory);
   asset_discovery_manager_ = std::make_unique<AssetDiscoveryManager>(
-      url_loader_factory, this, json_rpc_service(), keyring_service(),
-      simple_hash_client_.get(), profile_prefs);
+      url_loader_factory, *this, *json_rpc_service(), *keyring_service(),
+      *simple_hash_client_, profile_prefs);
 
   delegate_->AddObserver(this);
 
@@ -1134,8 +1133,7 @@ void BraveWalletService::OnActiveOriginChanged(
 
 void BraveWalletService::WalletRestored() {
   account_discovery_manager_ = std::make_unique<AccountDiscoveryManager>(
-      json_rpc_service_.get(), keyring_service_.get(),
-      bitcoin_wallet_service_.get());
+      *json_rpc_service_, *keyring_service_, bitcoin_wallet_service_.get());
   account_discovery_manager_->StartDiscovery();
 }
 

components/brave_wallet/browser/ethereum_provider_impl.cc

@@ -1319,7 +1319,7 @@ mojom::AccountIdPtr EthereumProviderImpl::FindAuthenticatedAccountByAddress(
 
 mojom::AccountIdPtr EthereumProviderImpl::FindAccountByAddress(
     const std::string& address) {
-  AccountResolverDelegateImpl resolver(keyring_service_);
+  AccountResolverDelegateImpl resolver(*keyring_service_);
 
   auto account_id = resolver.ResolveAccountId(nullptr, &address);
   if (!account_id || account_id->coin != mojom::CoinType::ETH) {

components/brave_wallet/browser/tx_service.cc

@@ -88,7 +88,7 @@ TxService::TxService(JsonRpcService* json_rpc_service,
   delegate_ = std::make_unique<TxStorageDelegateImpl>(prefs, store_factory_,
                                                       ui_task_runner);
   account_resolver_delegate_ =
-      std::make_unique<AccountResolverDelegateImpl>(&keyring_service);
+      std::make_unique<AccountResolverDelegateImpl>(keyring_service);
 
   tx_manager_map_[mojom::CoinType::ETH] = std::unique_ptr<TxManager>(
       new EthTxManager(*this, json_rpc_service, keyring_service, *delegate_,
@@ -115,7 +115,7 @@ TxService::TxService(JsonRpcService* json_rpc_service,
       CHECK_IS_TEST();
     } else {
       tx_manager_map_[mojom::CoinType::ZEC] = std::make_unique<ZCashTxManager>(
-          *this, zcash_wallet_service, keyring_service, *delegate_,
+          *this, *zcash_wallet_service, keyring_service, *delegate_,
           *account_resolver_delegate_);
     }
   }

components/brave_wallet/browser/zcash/zcash_create_transparent_transaction_task.cc

@@ -54,7 +54,7 @@ void ZCashCreateTransparentTransactionTask::WorkOnTask() {
   }
 
   if (!chain_height_) {
-    zcash_wallet_service_->zcash_rpc()->GetLatestBlock(
+    zcash_wallet_service_->zcash_rpc().GetLatestBlock(
         chain_id_,
         base::BindOnce(&ZCashCreateTransparentTransactionTask::OnGetChainHeight,
                        weak_ptr_factory_.GetWeakPtr()));