From 6d0ffe2f71552f9db4b2b4392e19007827250a9c Mon Sep 17 00:00:00 2001 From: cdesouza-chromium Date: Fri, 1 Nov 2024 17:47:35 +0000 Subject: [PATCH] [DanglingPtr][wallet] Prefer non-nullable states pt.1 (#26332) This change touches several parts of wallets where we are passing pointers around that are expected to be in a valid state, and remain so for the duration of the lifetime of the object receiving them. Historically, chromium had a guideline that required passing non-owning references as raw pointers. This has changed for a while now, and there are even supporting types like `raw_ref` for it. Raw pointers should be used for cases where flexibility is required about the lifetime of what is being pointed at. However, we should avoid what is in fact an anti-pattern, of passing pointers for the lifetime of an object, and then CHECKing the pointer for its validity. This change also removes certain uses of `std::unique_ptr` that suffers of the same issues. Resolves https://github.com/brave/brave-browser/issues/42016 --- .../eth_pending_tx_tracker_unittest.cc | 2 +- .../account_discovery_manager_unittest.cc | 2 +- .../browser/bitcoin/bitcoin_block_tracker.cc | 2 +- .../browser/bitcoin/bitcoin_block_tracker.h | 5 +- .../bitcoin/bitcoin_block_tracker_unittest.cc | 4 +- .../bitcoin/bitcoin_discover_account_task.cc | 11 +- .../bitcoin/bitcoin_discover_account_task.h | 17 +-- .../bitcoin_fetch_raw_transactions_task.cc | 2 +- .../bitcoin_fetch_raw_transactions_task.h | 6 +- .../browser/bitcoin/bitcoin_rpc.cc | 18 +-- .../browser/bitcoin/bitcoin_rpc.h | 8 +- .../browser/bitcoin/bitcoin_rpc_unittest.cc | 2 +- .../browser/bitcoin/bitcoin_test_utils.cc | 4 +- .../browser/bitcoin/bitcoin_test_utils.h | 2 +- .../browser/bitcoin/bitcoin_tx_manager.cc | 53 ++++---- .../browser/bitcoin/bitcoin_tx_manager.h | 20 ++- .../bitcoin/bitcoin_tx_manager_unittest.cc | 4 +- .../bitcoin/bitcoin_tx_state_manager.cc | 7 +- .../bitcoin/bitcoin_tx_state_manager.h | 7 +- .../bitcoin_tx_state_manager_unittest.cc | 2 +- .../browser/bitcoin/bitcoin_wallet_service.cc | 47 ++++---- .../browser/bitcoin/bitcoin_wallet_service.h | 16 ++- .../bitcoin_wallet_service_unittest.cc | 2 +- .../browser/brave_wallet_service.cc | 5 +- .../browser/eth_nonce_tracker_unittest.cc | 3 +- .../brave_wallet/browser/eth_tx_manager.cc | 97 +++++++-------- .../brave_wallet/browser/eth_tx_manager.h | 17 +-- .../browser/eth_tx_manager_unittest.cc | 28 ++--- .../browser/eth_tx_state_manager.cc | 7 +- .../browser/eth_tx_state_manager.h | 7 +- .../browser/eth_tx_state_manager_unittest.cc | 2 +- .../browser/fil_nonce_tracker_unittest.cc | 3 +- .../brave_wallet/browser/fil_tx_manager.cc | 70 +++++------ .../brave_wallet/browser/fil_tx_manager.h | 16 +-- .../browser/fil_tx_manager_unittest.cc | 5 +- .../browser/fil_tx_state_manager.cc | 7 +- .../browser/fil_tx_state_manager.h | 7 +- .../browser/fil_tx_state_manager_unittest.cc | 2 +- .../browser/keyring_service_unittest.cc | 21 ++-- .../brave_wallet/browser/scoped_txs_update.cc | 2 +- .../brave_wallet/browser/scoped_txs_update.h | 6 +- .../browser/solana_provider_impl.h | 1 + .../brave_wallet/browser/solana_tx_manager.cc | 91 +++++++------- .../brave_wallet/browser/solana_tx_manager.h | 15 +-- .../browser/solana_tx_manager_unittest.cc | 39 +++--- .../browser/solana_tx_state_manager.cc | 7 +- .../browser/solana_tx_state_manager.h | 7 +- .../solana_tx_state_manager_unittest.cc | 2 +- components/brave_wallet/browser/test_utils.cc | 2 +- components/brave_wallet/browser/tx_manager.cc | 11 +- components/brave_wallet/browser/tx_manager.h | 31 +++-- components/brave_wallet/browser/tx_service.cc | 26 ++-- components/brave_wallet/browser/tx_service.h | 2 +- .../brave_wallet/browser/tx_state_manager.cc | 16 +-- .../brave_wallet/browser/tx_state_manager.h | 14 +-- .../browser/tx_state_manager_unittest.cc | 2 +- .../browser/zcash/zcash_block_tracker.cc | 2 +- .../browser/zcash/zcash_block_tracker.h | 5 +- .../zcash_create_shield_transaction_task.cc | 2 +- .../zcash_create_shield_transaction_task.h | 5 +- ...ash_create_transparent_transaction_task.cc | 2 +- ...cash_create_transparent_transaction_task.h | 5 +- .../browser/zcash/zcash_orchard_storage.cc | 114 +++++++++--------- .../browser/zcash/zcash_orchard_storage.h | 5 +- .../zcash/zcash_resolve_balance_task.cc | 2 +- .../zcash/zcash_resolve_balance_task.h | 5 +- .../browser/zcash/zcash_tx_manager.cc | 51 ++++---- .../browser/zcash/zcash_tx_manager.h | 15 +-- .../browser/zcash/zcash_tx_state_manager.cc | 7 +- .../browser/zcash/zcash_tx_state_manager.h | 7 +- .../browser/zcash/zcash_wallet_service.cc | 8 +- 71 files changed, 491 insertions(+), 558 deletions(-) diff --git a/browser/brave_wallet/eth_pending_tx_tracker_unittest.cc b/browser/brave_wallet/eth_pending_tx_tracker_unittest.cc index 925f81428c5e..b2eceac1245c 100644 --- a/browser/brave_wallet/eth_pending_tx_tracker_unittest.cc +++ b/browser/brave_wallet/eth_pending_tx_tracker_unittest.cc @@ -59,7 +59,7 @@ class EthPendingTxTrackerUnitTest : public testing::Test { account_resolver_delegate_ = std::make_unique(); tx_state_manager_ = std::make_unique( - GetPrefs(), delegate_.get(), account_resolver_delegate_.get()); + *delegate_, *account_resolver_delegate_); eth_account_id_ = account_resolver_delegate_->RegisterAccount( MakeAccountId(mojom::CoinType::ETH, mojom::KeyringId::kDefault, diff --git a/components/brave_wallet/browser/account_discovery_manager_unittest.cc b/components/brave_wallet/browser/account_discovery_manager_unittest.cc index 7d594aebf210..146b6c5e588c 100644 --- a/components/brave_wallet/browser/account_discovery_manager_unittest.cc +++ b/components/brave_wallet/browser/account_discovery_manager_unittest.cc @@ -45,7 +45,7 @@ class AccountDiscoveryManagerUnitTest : public testing::Test { std::make_unique(nullptr, &prefs_, &local_state_); bitcoin_test_rpc_server_ = std::make_unique(); bitcoin_wallet_service_ = std::make_unique( - keyring_service_.get(), &prefs_, network_manager_.get(), + *keyring_service_, *network_manager_, bitcoin_test_rpc_server_->GetURLLoaderFactory()); bitcoin_wallet_service_->SetArrangeTransactionsForTesting(true); diff --git a/components/brave_wallet/browser/bitcoin/bitcoin_block_tracker.cc b/components/brave_wallet/browser/bitcoin/bitcoin_block_tracker.cc index 6056986d653b..10ddc3f2c52d 100644 --- a/components/brave_wallet/browser/bitcoin/bitcoin_block_tracker.cc +++ b/components/brave_wallet/browser/bitcoin/bitcoin_block_tracker.cc @@ -15,7 +15,7 @@ namespace brave_wallet { -BitcoinBlockTracker::BitcoinBlockTracker(bitcoin_rpc::BitcoinRpc* bitcoin_rpc) +BitcoinBlockTracker::BitcoinBlockTracker(bitcoin_rpc::BitcoinRpc& bitcoin_rpc) : bitcoin_rpc_(bitcoin_rpc) {} BitcoinBlockTracker::~BitcoinBlockTracker() = default; diff --git a/components/brave_wallet/browser/bitcoin/bitcoin_block_tracker.h b/components/brave_wallet/browser/bitcoin/bitcoin_block_tracker.h index 80003fa9dd32..b57ec38e06a3 100644 --- a/components/brave_wallet/browser/bitcoin/bitcoin_block_tracker.h +++ b/components/brave_wallet/browser/bitcoin/bitcoin_block_tracker.h @@ -10,6 +10,7 @@ #include #include +#include "base/memory/raw_ref.h" #include "base/memory/weak_ptr.h" #include "base/observer_list.h" #include "base/time/time.h" @@ -24,7 +25,7 @@ class BitcoinRpc; class BitcoinBlockTracker : public BlockTracker { public: - explicit BitcoinBlockTracker(bitcoin_rpc::BitcoinRpc* bitcoin_rpc); + explicit BitcoinBlockTracker(bitcoin_rpc::BitcoinRpc& bitcoin_rpc); ~BitcoinBlockTracker() override; BitcoinBlockTracker(const BitcoinBlockTracker&) = delete; BitcoinBlockTracker operator=(const BitcoinBlockTracker&) = delete; @@ -50,7 +51,7 @@ class BitcoinBlockTracker : public BlockTracker { std::map latest_height_map_; base::ObserverList observers_; - raw_ptr bitcoin_rpc_ = nullptr; + const raw_ref bitcoin_rpc_; base::WeakPtrFactory weak_ptr_factory_{this}; }; diff --git a/components/brave_wallet/browser/bitcoin/bitcoin_block_tracker_unittest.cc b/components/brave_wallet/browser/bitcoin/bitcoin_block_tracker_unittest.cc index 9df5e4a0834d..2d4e0526e996 100644 --- a/components/brave_wallet/browser/bitcoin/bitcoin_block_tracker_unittest.cc +++ b/components/brave_wallet/browser/bitcoin/bitcoin_block_tracker_unittest.cc @@ -60,8 +60,8 @@ class BitcoinBlockTrackerUnitTest : public testing::Test { brave_wallet::RegisterProfilePrefs(prefs_.registry()); network_manager_ = std::make_unique(&prefs_); bitcoin_rpc_ = std::make_unique( - network_manager_.get(), shared_url_loader_factory_); - tracker_ = std::make_unique(bitcoin_rpc_.get()); + *network_manager_, shared_url_loader_factory_); + tracker_ = std::make_unique(*bitcoin_rpc_); } std::string GetResponseString() const { diff --git a/components/brave_wallet/browser/bitcoin/bitcoin_discover_account_task.cc b/components/brave_wallet/browser/bitcoin/bitcoin_discover_account_task.cc index 00a22f120428..c114ce80655f 100644 --- a/components/brave_wallet/browser/bitcoin/bitcoin_discover_account_task.cc +++ b/components/brave_wallet/browser/bitcoin/bitcoin_discover_account_task.cc @@ -64,7 +64,7 @@ bool DiscoveredBitcoinAccount::operator==( } DiscoverAccountTaskBase::DiscoverAccountTaskBase( - BitcoinWalletService* bitcoin_wallet_service, + BitcoinWalletService& bitcoin_wallet_service, const std::string& network_id) : bitcoin_wallet_service_(bitcoin_wallet_service), network_id_(network_id), @@ -211,7 +211,7 @@ void DiscoverAccountTaskBase::OnGetAddressStats( } DiscoverWalletAccountTask::DiscoverWalletAccountTask( - BitcoinWalletService* bitcoin_wallet_service, + BitcoinWalletService& bitcoin_wallet_service, mojom::KeyringId keyring_id, uint32_t account_index) : DiscoverAccountTaskBase(bitcoin_wallet_service, @@ -224,12 +224,13 @@ DiscoverWalletAccountTask::~DiscoverWalletAccountTask() = default; mojom::BitcoinAddressPtr DiscoverWalletAccountTask::GetAddressById( const mojom::BitcoinKeyIdPtr& key_id) { - return bitcoin_wallet_service_->keyring_service() - ->GetBitcoinAccountDiscoveryAddress(keyring_id_, account_index_, key_id); + return bitcoin_wallet_service() + .keyring_service() + .GetBitcoinAccountDiscoveryAddress(keyring_id_, account_index_, key_id); } DiscoverExtendedKeyAccountTask::DiscoverExtendedKeyAccountTask( - BitcoinWalletService* bitcoin_wallet_service, + BitcoinWalletService& bitcoin_wallet_service, const std::string& network_id, const std::string& extended_key) : DiscoverAccountTaskBase(bitcoin_wallet_service, network_id), diff --git a/components/brave_wallet/browser/bitcoin/bitcoin_discover_account_task.h b/components/brave_wallet/browser/bitcoin/bitcoin_discover_account_task.h index 8055c361cdca..492b5d8e6d0f 100644 --- a/components/brave_wallet/browser/bitcoin/bitcoin_discover_account_task.h +++ b/components/brave_wallet/browser/bitcoin/bitcoin_discover_account_task.h @@ -8,7 +8,7 @@ #include -#include "base/memory/raw_ptr.h" +#include "base/memory/raw_ref.h" #include "base/memory/weak_ptr.h" #include "base/types/expected.h" #include "brave/components/brave_wallet/browser/bitcoin/bitcoin_rpc.h" @@ -17,7 +17,6 @@ namespace brave_wallet { class BitcoinWalletService; -class KeyringService; class HDKey; struct DiscoveredBitcoinAccount { @@ -37,7 +36,7 @@ class DiscoverAccountTaskBase { using DiscoverAccountCallback = base::OnceCallback)>; - DiscoverAccountTaskBase(BitcoinWalletService* bitcoin_wallet_service, + DiscoverAccountTaskBase(BitcoinWalletService& bitcoin_wallet_service, const std::string& network_id); virtual ~DiscoverAccountTaskBase(); @@ -64,8 +63,12 @@ class DiscoverAccountTaskBase { mojom::BitcoinAddressPtr address, base::expected stats); - raw_ptr bitcoin_wallet_service_; - raw_ptr keyring_service_; + BitcoinWalletService& bitcoin_wallet_service() { + return *bitcoin_wallet_service_; + } + + private: + const raw_ref bitcoin_wallet_service_; std::string network_id_; uint32_t active_requests_ = 0; @@ -82,7 +85,7 @@ class DiscoverAccountTaskBase { class DiscoverWalletAccountTask : public DiscoverAccountTaskBase { public: - DiscoverWalletAccountTask(BitcoinWalletService* bitcoin_wallet_service, + DiscoverWalletAccountTask(BitcoinWalletService& bitcoin_wallet_service, mojom::KeyringId keyring_id, uint32_t account_index); ~DiscoverWalletAccountTask() override; @@ -96,7 +99,7 @@ class DiscoverWalletAccountTask : public DiscoverAccountTaskBase { class DiscoverExtendedKeyAccountTask : public DiscoverAccountTaskBase { public: - DiscoverExtendedKeyAccountTask(BitcoinWalletService* bitcoin_wallet_service, + DiscoverExtendedKeyAccountTask(BitcoinWalletService& bitcoin_wallet_service, const std::string& network_id, const std::string& extended_key); ~DiscoverExtendedKeyAccountTask() override; diff --git a/components/brave_wallet/browser/bitcoin/bitcoin_fetch_raw_transactions_task.cc b/components/brave_wallet/browser/bitcoin/bitcoin_fetch_raw_transactions_task.cc index 55ba6acc8763..496ec43d38aa 100644 --- a/components/brave_wallet/browser/bitcoin/bitcoin_fetch_raw_transactions_task.cc +++ b/components/brave_wallet/browser/bitcoin/bitcoin_fetch_raw_transactions_task.cc @@ -23,7 +23,7 @@ namespace brave_wallet { FetchRawTransactionsTask::FetchRawTransactionsTask( - BitcoinWalletService* bitcoin_wallet_service, + BitcoinWalletService& bitcoin_wallet_service, const std::string& network_id, const std::vector& txids) : bitcoin_wallet_service_(bitcoin_wallet_service), diff --git a/components/brave_wallet/browser/bitcoin/bitcoin_fetch_raw_transactions_task.h b/components/brave_wallet/browser/bitcoin/bitcoin_fetch_raw_transactions_task.h index 372598e8a701..c0ba7e6b1ee0 100644 --- a/components/brave_wallet/browser/bitcoin/bitcoin_fetch_raw_transactions_task.h +++ b/components/brave_wallet/browser/bitcoin/bitcoin_fetch_raw_transactions_task.h @@ -11,7 +11,7 @@ #include #include -#include "base/memory/raw_ptr.h" +#include "base/memory/raw_ref.h" #include "base/memory/weak_ptr.h" #include "base/types/expected.h" #include "brave/components/brave_wallet/common/brave_wallet.mojom.h" @@ -26,7 +26,7 @@ class FetchRawTransactionsTask { using FetchRawTransactionsTaskCallback = base::OnceCallback>, std::string>)>; - FetchRawTransactionsTask(BitcoinWalletService* bitcoin_wallet_service, + FetchRawTransactionsTask(BitcoinWalletService& bitcoin_wallet_service, const std::string& network_id, const std::vector& txids); virtual ~FetchRawTransactionsTask(); @@ -44,7 +44,7 @@ class FetchRawTransactionsTask { SHA256HashArray txid, base::expected, std::string> raw_tx); - raw_ptr bitcoin_wallet_service_; + const raw_ref bitcoin_wallet_service_; std::string network_id_; std::vector txids_; bool requests_queued_ = false; diff --git a/components/brave_wallet/browser/bitcoin/bitcoin_rpc.cc b/components/brave_wallet/browser/bitcoin/bitcoin_rpc.cc index 63f10731e88f..9d4a2188057d 100644 --- a/components/brave_wallet/browser/bitcoin/bitcoin_rpc.cc +++ b/components/brave_wallet/browser/bitcoin/bitcoin_rpc.cc @@ -227,11 +227,11 @@ struct EndpointQueue { }; BitcoinRpc::BitcoinRpc( - NetworkManager* network_manager, + NetworkManager& network_manager, scoped_refptr url_loader_factory) : network_manager_(network_manager), - api_request_helper_(new APIRequestHelper(GetNetworkTrafficAnnotationTag(), - url_loader_factory)) {} + api_request_helper_(GetNetworkTrafficAnnotationTag(), + url_loader_factory) {} BitcoinRpc::~BitcoinRpc() = default; @@ -461,10 +461,10 @@ void BitcoinRpc::PostTransaction(const std::string& chain_id, weak_ptr_factory_.GetWeakPtr(), std::move(callback)); auto conversion_callback = base::BindOnce(&ConvertPlainStringToJsonArray); - api_request_helper_->Request( - net::HttpRequestHeaders::kPostMethod, request_url, payload, "", - std::move(internal_callback), {}, {.auto_retry_on_network_change = true}, - std::move(conversion_callback)); + api_request_helper_.Request(net::HttpRequestHeaders::kPostMethod, request_url, + payload, "", std::move(internal_callback), {}, + {.auto_retry_on_network_change = true}, + std::move(conversion_callback)); } void BitcoinRpc::OnPostTransaction(PostTransactionCallback callback, @@ -537,7 +537,7 @@ void BitcoinRpc::MaybeStartQueuedRequest(const GURL& endpoint_host) { endpoint.requests_queue.pop_front(); endpoint.active_requests++; - api_request_helper_->Request( + api_request_helper_.Request( net::HttpRequestHeaders::kGetMethod, request.request_url, "", "", base::BindOnce(&BitcoinRpc::OnRequestInternalDone, weak_ptr_factory_.GetWeakPtr(), endpoint_host, @@ -548,7 +548,7 @@ void BitcoinRpc::MaybeStartQueuedRequest(const GURL& endpoint_host) { void BitcoinRpc::SetUrlLoaderFactoryForTesting( scoped_refptr url_loader_factory) { - api_request_helper_->SetUrlLoaderFactoryForTesting( // IN-TEST + api_request_helper_.SetUrlLoaderFactoryForTesting( // IN-TEST std::move(url_loader_factory)); } diff --git a/components/brave_wallet/browser/bitcoin/bitcoin_rpc.h b/components/brave_wallet/browser/bitcoin/bitcoin_rpc.h index 911453e1314b..bc6a0f5107c4 100644 --- a/components/brave_wallet/browser/bitcoin/bitcoin_rpc.h +++ b/components/brave_wallet/browser/bitcoin/bitcoin_rpc.h @@ -8,11 +8,11 @@ #include #include -#include #include #include #include +#include "base/memory/raw_ref.h" #include "base/memory/weak_ptr.h" #include "base/types/expected.h" #include "brave/components/api_request_helper/api_request_helper.h" @@ -30,7 +30,7 @@ struct EndpointQueue; class BitcoinRpc { public: - BitcoinRpc(NetworkManager* network_manager, + BitcoinRpc(NetworkManager& network_manager, scoped_refptr url_loader_factory); ~BitcoinRpc(); @@ -107,11 +107,11 @@ class BitcoinRpc { GURL GetNetworkURL(const std::string& chain_id); - const raw_ptr network_manager_; + const raw_ref network_manager_; scoped_refptr url_loader_factory_; // Uses hostname as key. Tracks request throttling(if required) per host. std::map endpoints_; - std::unique_ptr api_request_helper_; + APIRequestHelper api_request_helper_; base::WeakPtrFactory weak_ptr_factory_{this}; }; diff --git a/components/brave_wallet/browser/bitcoin/bitcoin_rpc_unittest.cc b/components/brave_wallet/browser/bitcoin/bitcoin_rpc_unittest.cc index 667189691b37..6ad2ef281627 100644 --- a/components/brave_wallet/browser/bitcoin/bitcoin_rpc_unittest.cc +++ b/components/brave_wallet/browser/bitcoin/bitcoin_rpc_unittest.cc @@ -59,7 +59,7 @@ class BitcoinRpcUnitTest : public testing::Test { brave_wallet::RegisterProfilePrefs(prefs_.registry()); network_manager_ = std::make_unique(&prefs_); bitcoin_rpc_ = std::make_unique( - network_manager_.get(), shared_url_loader_factory_); + *network_manager_, shared_url_loader_factory_); mainnet_rpc_url_ = network_manager_ diff --git a/components/brave_wallet/browser/bitcoin/bitcoin_test_utils.cc b/components/brave_wallet/browser/bitcoin/bitcoin_test_utils.cc index 6dee5c51eab8..863dd2822223 100644 --- a/components/brave_wallet/browser/bitcoin/bitcoin_test_utils.cc +++ b/components/brave_wallet/browser/bitcoin/bitcoin_test_utils.cc @@ -119,9 +119,9 @@ BitcoinTestRpcServer::BitcoinTestRpcServer() } BitcoinTestRpcServer::BitcoinTestRpcServer( - BitcoinWalletService* bitcoin_wallet_service) + BitcoinWalletService& bitcoin_wallet_service) : BitcoinTestRpcServer() { - bitcoin_wallet_service->SetUrlLoaderFactoryForTesting(GetURLLoaderFactory()); + bitcoin_wallet_service.SetUrlLoaderFactoryForTesting(GetURLLoaderFactory()); } BitcoinTestRpcServer::~BitcoinTestRpcServer() = default; diff --git a/components/brave_wallet/browser/bitcoin/bitcoin_test_utils.h b/components/brave_wallet/browser/bitcoin/bitcoin_test_utils.h index 9fbbed52b93b..2d63d1fc9e77 100644 --- a/components/brave_wallet/browser/bitcoin/bitcoin_test_utils.h +++ b/components/brave_wallet/browser/bitcoin/bitcoin_test_utils.h @@ -60,7 +60,7 @@ inline constexpr char kBtcTestnetHardwareAccount1[] = class BitcoinTestRpcServer { public: BitcoinTestRpcServer(); - explicit BitcoinTestRpcServer(BitcoinWalletService* bitcoin_wallet_service); + explicit BitcoinTestRpcServer(BitcoinWalletService& bitcoin_wallet_service); ~BitcoinTestRpcServer(); static bitcoin_rpc::AddressStats EmptyAddressStats( diff --git a/components/brave_wallet/browser/bitcoin/bitcoin_tx_manager.cc b/components/brave_wallet/browser/bitcoin/bitcoin_tx_manager.cc index 0a8ccbd47063..aa3578b06c0e 100644 --- a/components/brave_wallet/browser/bitcoin/bitcoin_tx_manager.cc +++ b/components/brave_wallet/browser/bitcoin/bitcoin_tx_manager.cc @@ -25,37 +25,34 @@ namespace brave_wallet { BitcoinTxManager::BitcoinTxManager( - TxService* tx_service, - BitcoinWalletService* bitcoin_wallet_service, - KeyringService* keyring_service, - PrefService* prefs, - TxStorageDelegate* delegate, - AccountResolverDelegate* account_resolver_delegate) + TxService& tx_service, + BitcoinWalletService& bitcoin_wallet_service, + KeyringService& keyring_service, + TxStorageDelegate& delegate, + AccountResolverDelegate& account_resolver_delegate) : TxManager( - std::make_unique(prefs, - delegate, + std::make_unique(delegate, account_resolver_delegate), std::make_unique( - &bitcoin_wallet_service->bitcoin_rpc()), + bitcoin_wallet_service.bitcoin_rpc()), tx_service, - keyring_service, - prefs), + keyring_service), bitcoin_wallet_service_(bitcoin_wallet_service) { - block_tracker_observation_.Observe(GetBitcoinBlockTracker()); + block_tracker_observation_.Observe(&GetBitcoinBlockTracker()); } BitcoinTxManager::~BitcoinTxManager() = default; std::unique_ptr BitcoinTxManager::GetTxForTesting( const std::string& tx_meta_id) { - return GetBitcoinTxStateManager()->GetBitcoinTx(tx_meta_id); + return GetBitcoinTxStateManager().GetBitcoinTx(tx_meta_id); } void BitcoinTxManager::GetBtcHardwareTransactionSignData( const std::string& tx_meta_id, GetBtcHardwareTransactionSignDataCallback callback) { std::unique_ptr meta = - GetBitcoinTxStateManager()->GetBitcoinTx(tx_meta_id); + GetBitcoinTxStateManager().GetBitcoinTx(tx_meta_id); if (!meta || !meta->tx()) { std::move(callback).Run(nullptr); return; @@ -72,14 +69,14 @@ void BitcoinTxManager::ProcessBtcHardwareSignature( ProcessBtcHardwareSignatureCallback callback) { CHECK(hw_signature); std::unique_ptr meta = - GetBitcoinTxStateManager()->GetBitcoinTx(tx_meta_id); + GetBitcoinTxStateManager().GetBitcoinTx(tx_meta_id); if (!meta) { std::move(callback).Run(false); return; } meta->set_status(mojom::TransactionStatus::Approved); - if (!tx_state_manager_->AddOrUpdateTx(*meta)) { + if (!tx_state_manager().AddOrUpdateTx(*meta)) { std::move(callback).Run(false); return; } @@ -147,7 +144,7 @@ void BitcoinTxManager::ContinueAddUnapprovedTransaction( meta.set_status(mojom::TransactionStatus::Unapproved); meta.set_chain_id(chain_id); - if (!tx_state_manager_->AddOrUpdateTx(meta)) { + if (!tx_state_manager().AddOrUpdateTx(meta)) { std::move(callback).Run(false, "", WalletInternalErrorMessage()); return; } @@ -157,7 +154,7 @@ void BitcoinTxManager::ContinueAddUnapprovedTransaction( void BitcoinTxManager::ApproveTransaction(const std::string& tx_meta_id, ApproveTransactionCallback callback) { std::unique_ptr meta = - GetBitcoinTxStateManager()->GetBitcoinTx(tx_meta_id); + GetBitcoinTxStateManager().GetBitcoinTx(tx_meta_id); if (!meta) { DCHECK(false) << "Transaction should be found"; std::move(callback).Run( @@ -169,7 +166,7 @@ void BitcoinTxManager::ApproveTransaction(const std::string& tx_meta_id, } meta->set_status(mojom::TransactionStatus::Approved); - if (!tx_state_manager_->AddOrUpdateTx(*meta)) { + if (!tx_state_manager().AddOrUpdateTx(*meta)) { std::move(callback).Run(false, mojom::ProviderErrorUnion::NewBitcoinProviderError( mojom::BitcoinProviderError::kInternalError), @@ -191,7 +188,7 @@ void BitcoinTxManager::ContinueApproveTransaction( BitcoinTransaction transaction, std::string error) { std::unique_ptr meta = - GetBitcoinTxStateManager()->GetBitcoinTx(tx_meta_id); + GetBitcoinTxStateManager().GetBitcoinTx(tx_meta_id); if (!meta) { DCHECK(false) << "Transaction should be found"; std::move(callback).Run( @@ -212,7 +209,7 @@ void BitcoinTxManager::ContinueApproveTransaction( meta->set_status(mojom::TransactionStatus::Error); } - if (!tx_state_manager_->AddOrUpdateTx(*meta)) { + if (!tx_state_manager().AddOrUpdateTx(*meta)) { std::move(callback).Run(false, mojom::ProviderErrorUnion::NewBitcoinProviderError( mojom::BitcoinProviderError::kInternalError), @@ -243,12 +240,12 @@ void BitcoinTxManager::RetryTransaction(const std::string& tx_meta_id, NOTIMPLEMENTED() << "Bitcoin transaction retry is not supported"; } -BitcoinTxStateManager* BitcoinTxManager::GetBitcoinTxStateManager() { - return static_cast(tx_state_manager_.get()); +BitcoinTxStateManager& BitcoinTxManager::GetBitcoinTxStateManager() { + return static_cast(tx_state_manager()); } -BitcoinBlockTracker* BitcoinTxManager::GetBitcoinBlockTracker() { - return static_cast(block_tracker_.get()); +BitcoinBlockTracker& BitcoinTxManager::GetBitcoinBlockTracker() { + return static_cast(block_tracker()); } mojom::CoinType BitcoinTxManager::GetCoinType() const { @@ -257,7 +254,7 @@ mojom::CoinType BitcoinTxManager::GetCoinType() const { void BitcoinTxManager::UpdatePendingTransactions( const std::optional& chain_id) { - auto pending_transactions = tx_state_manager_->GetTransactionsByStatus( + auto pending_transactions = tx_state_manager().GetTransactionsByStatus( chain_id, mojom::TransactionStatus::Submitted, std::nullopt); std::set pending_chain_ids; for (const auto& pending_transaction : pending_transactions) { @@ -278,7 +275,7 @@ void BitcoinTxManager::OnGetTransactionStatus( return; } std::unique_ptr meta = - GetBitcoinTxStateManager()->GetBitcoinTx(tx_meta_id); + GetBitcoinTxStateManager().GetBitcoinTx(tx_meta_id); if (!meta) { return; } @@ -287,7 +284,7 @@ void BitcoinTxManager::OnGetTransactionStatus( mojom::TransactionStatus status = mojom::TransactionStatus::Confirmed; meta->set_status(status); meta->set_confirmed_time(base::Time::Now()); - tx_state_manager_->AddOrUpdateTx(*meta); + tx_state_manager().AddOrUpdateTx(*meta); } } diff --git a/components/brave_wallet/browser/bitcoin/bitcoin_tx_manager.h b/components/brave_wallet/browser/bitcoin/bitcoin_tx_manager.h index 0ff710c70ed4..98c637332072 100644 --- a/components/brave_wallet/browser/bitcoin/bitcoin_tx_manager.h +++ b/components/brave_wallet/browser/bitcoin/bitcoin_tx_manager.h @@ -10,14 +10,13 @@ #include #include +#include "base/memory/raw_ref.h" #include "base/scoped_observation.h" #include "brave/components/brave_wallet/browser/bitcoin/bitcoin_block_tracker.h" #include "brave/components/brave_wallet/browser/bitcoin/bitcoin_tx_meta.h" #include "brave/components/brave_wallet/browser/bitcoin/bitcoin_wallet_service.h" #include "brave/components/brave_wallet/browser/tx_manager.h" -class PrefService; - namespace brave_wallet { class AccountResolverDelegate; @@ -34,12 +33,11 @@ class BitcoinTxManager : public TxManager, using ProcessBtcHardwareSignatureCallback = mojom::BtcTxManagerProxy::ProcessBtcHardwareSignatureCallback; - BitcoinTxManager(TxService* tx_service, - BitcoinWalletService* bitcoin_wallet_service, - KeyringService* keyring_service, - PrefService* prefs, - TxStorageDelegate* delegate, - AccountResolverDelegate* account_resolver_delegate); + BitcoinTxManager(TxService& tx_service, + BitcoinWalletService& bitcoin_wallet_service, + KeyringService& keyring_service, + TxStorageDelegate& delegate, + AccountResolverDelegate& account_resolver_delegate); ~BitcoinTxManager() override; BitcoinTxManager(const BitcoinTxManager&) = delete; BitcoinTxManager& operator=(const BitcoinTxManager&) = delete; @@ -60,8 +58,8 @@ class BitcoinTxManager : public TxManager, FRIEND_TEST_ALL_PREFIXES(BitcoinTxManagerUnitTest, SubmitTransactionError); FRIEND_TEST_ALL_PREFIXES(BitcoinTxManagerUnitTest, SubmitTransactionError); - BitcoinTxStateManager* GetBitcoinTxStateManager(); - BitcoinBlockTracker* GetBitcoinBlockTracker(); + BitcoinTxStateManager& GetBitcoinTxStateManager(); + BitcoinBlockTracker& GetBitcoinBlockTracker(); // BitcoinBlockTracker::Observer void OnLatestHeightUpdated(const std::string& chain_id, @@ -106,7 +104,7 @@ class BitcoinTxManager : public TxManager, void OnGetTransactionStatus(const std::string& tx_meta_id, base::expected confirm_status); - raw_ptr bitcoin_wallet_service_ = nullptr; + const raw_ref bitcoin_wallet_service_; base::ScopedObservation block_tracker_observation_{this}; base::WeakPtrFactory weak_factory_{this}; diff --git a/components/brave_wallet/browser/bitcoin/bitcoin_tx_manager_unittest.cc b/components/brave_wallet/browser/bitcoin/bitcoin_tx_manager_unittest.cc index 2854e8cec2a0..0d5694454e35 100644 --- a/components/brave_wallet/browser/bitcoin/bitcoin_tx_manager_unittest.cc +++ b/components/brave_wallet/browser/bitcoin/bitcoin_tx_manager_unittest.cc @@ -62,14 +62,14 @@ class BitcoinTxManagerUnitTest : public testing::Test { bitcoin_test_rpc_server_ = std::make_unique(); bitcoin_wallet_service_ = std::make_unique( - keyring_service_.get(), &prefs_, network_manager_.get(), + *keyring_service_, *network_manager_, bitcoin_test_rpc_server_->GetURLLoaderFactory()); bitcoin_wallet_service_->SetArrangeTransactionsForTesting(true); ASSERT_TRUE(temp_dir_.CreateUniqueTempDir()); tx_service_ = std::make_unique( json_rpc_service_.get(), bitcoin_wallet_service_.get(), nullptr, - keyring_service_.get(), &prefs_, temp_dir_.GetPath(), + *keyring_service_, &prefs_, temp_dir_.GetPath(), base::SequencedTaskRunner::GetCurrentDefault()); WaitForTxStorageDelegateInitialized(tx_service_->GetDelegateForTesting()); diff --git a/components/brave_wallet/browser/bitcoin/bitcoin_tx_state_manager.cc b/components/brave_wallet/browser/bitcoin/bitcoin_tx_state_manager.cc index 5abbd82e4047..264e3ece0570 100644 --- a/components/brave_wallet/browser/bitcoin/bitcoin_tx_state_manager.cc +++ b/components/brave_wallet/browser/bitcoin/bitcoin_tx_state_manager.cc @@ -17,10 +17,9 @@ namespace brave_wallet { BitcoinTxStateManager::BitcoinTxStateManager( - PrefService* prefs, - TxStorageDelegate* delegate, - AccountResolverDelegate* account_resolver_delegate) - : TxStateManager(prefs, delegate, account_resolver_delegate) {} + TxStorageDelegate& delegate, + AccountResolverDelegate& account_resolver_delegate) + : TxStateManager(delegate, account_resolver_delegate) {} BitcoinTxStateManager::~BitcoinTxStateManager() = default; diff --git a/components/brave_wallet/browser/bitcoin/bitcoin_tx_state_manager.h b/components/brave_wallet/browser/bitcoin/bitcoin_tx_state_manager.h index 790a38219bda..e9736eae09f4 100644 --- a/components/brave_wallet/browser/bitcoin/bitcoin_tx_state_manager.h +++ b/components/brave_wallet/browser/bitcoin/bitcoin_tx_state_manager.h @@ -11,8 +11,6 @@ #include "brave/components/brave_wallet/browser/tx_state_manager.h" -class PrefService; - namespace base { class Value; } // namespace base @@ -25,9 +23,8 @@ class TxStorageDelegate; class BitcoinTxStateManager : public TxStateManager { public: - BitcoinTxStateManager(PrefService* prefs, - TxStorageDelegate* delegate, - AccountResolverDelegate* account_resolver_delegate); + BitcoinTxStateManager(TxStorageDelegate& delegate, + AccountResolverDelegate& account_resolver_delegate); ~BitcoinTxStateManager() override; BitcoinTxStateManager(const BitcoinTxStateManager&) = delete; BitcoinTxStateManager operator=(const BitcoinTxStateManager&) = delete; diff --git a/components/brave_wallet/browser/bitcoin/bitcoin_tx_state_manager_unittest.cc b/components/brave_wallet/browser/bitcoin/bitcoin_tx_state_manager_unittest.cc index 2d1b9dda8563..f5cf32a3b919 100644 --- a/components/brave_wallet/browser/bitcoin/bitcoin_tx_state_manager_unittest.cc +++ b/components/brave_wallet/browser/bitcoin/bitcoin_tx_state_manager_unittest.cc @@ -39,7 +39,7 @@ class BitcoinTxStateManagerUnitTest : public testing::Test { account_resolver_delegate_ = std::make_unique(); bitcoin_tx_state_manager_ = std::make_unique( - GetPrefs(), delegate_.get(), account_resolver_delegate_.get()); + *delegate_, *account_resolver_delegate_); } PrefService* GetPrefs() { return &prefs_; } diff --git a/components/brave_wallet/browser/bitcoin/bitcoin_wallet_service.cc b/components/brave_wallet/browser/bitcoin/bitcoin_wallet_service.cc index dc1657f5158d..f46c02771fdc 100644 --- a/components/brave_wallet/browser/bitcoin/bitcoin_wallet_service.cc +++ b/components/brave_wallet/browser/bitcoin/bitcoin_wallet_service.cc @@ -672,13 +672,11 @@ void DiscoverNextUnusedAddressTask::ScheduleWorkOnTask() { mojom::BitcoinAddressPtr DiscoverNextUnusedAddressTask::GetNextAddress( const mojom::BitcoinAddressPtr& address) { - auto* keyring_service = bitcoin_wallet_service_->keyring_service(); - CHECK(keyring_service); - auto next_key_id = current_address_->key_id.Clone(); next_key_id->index++; - return keyring_service->GetBitcoinAddress(account_id_, next_key_id); + return bitcoin_wallet_service_->keyring_service().GetBitcoinAddress( + account_id_, next_key_id); } void DiscoverNextUnusedAddressTask::WorkOnTask() { @@ -745,14 +743,11 @@ void DiscoverNextUnusedAddressTask::OnGetAddressStats( } BitcoinWalletService::BitcoinWalletService( - KeyringService* keyring_service, - PrefService* prefs, - NetworkManager* network_manager, + KeyringService& keyring_service, + NetworkManager& network_manager, scoped_refptr url_loader_factory) : keyring_service_(keyring_service), - bitcoin_rpc_( - std::make_unique(network_manager, - url_loader_factory)) { + bitcoin_rpc_(network_manager, url_loader_factory) { keyring_service_->AddObserver( keyring_service_observer_receiver_.BindNewPipeAndPassRemote()); } @@ -772,7 +767,7 @@ void BitcoinWalletService::GetBalance(mojom::AccountIdPtr account_id, GetBalanceCallback callback) { CHECK(IsBitcoinAccount(*account_id)); - auto addresses = keyring_service()->GetBitcoinAddresses(account_id); + auto addresses = keyring_service().GetBitcoinAddresses(account_id); if (!addresses) { std::move(callback).Run(nullptr, InternalErrorString()); return; @@ -789,7 +784,7 @@ void BitcoinWalletService::GetExtendedKeyAccountBalance( const std::string& extended_key, GetExtendedKeyAccountBalanceCallback callback) { CHECK(IsBitcoinNetwork(chain_id)); - auto task = std::make_unique(this, chain_id, + auto task = std::make_unique(*this, chain_id, extended_key); task->set_callback(base::BindOnce( @@ -822,7 +817,7 @@ void BitcoinWalletService::GetBitcoinAccountInfo( mojom::BitcoinAccountInfoPtr BitcoinWalletService::GetBitcoinAccountInfoSync( const mojom::AccountIdPtr& account_id) { - return keyring_service()->GetBitcoinAccountInfo(account_id); + return keyring_service().GetBitcoinAccountInfo(account_id); } void BitcoinWalletService::RunDiscovery(mojom::AccountIdPtr account_id, @@ -841,7 +836,7 @@ void BitcoinWalletService::AccountsAdded( // For new bitcoin account search for transacted and/or funded addresses. if (IsBitcoinKeyring(account->account_id->keyring_id)) { auto task = std::make_unique( - this, account->account_id->keyring_id, + *this, account->account_id->keyring_id, account->account_id->account_index); task->set_callback( @@ -892,7 +887,7 @@ void BitcoinWalletService::UpdateNextUnusedAddressForAccount( std::optional next_change_index = !address->key_id->change ? std::optional() : address->key_id->index; - keyring_service()->UpdateNextUnusedAddressForBitcoinAccount( + keyring_service().UpdateNextUnusedAddressForBitcoinAccount( account_id, next_receive_index, next_change_index); } @@ -900,7 +895,7 @@ void BitcoinWalletService::GetUtxos(mojom::AccountIdPtr account_id, GetUtxosCallback callback) { CHECK(IsBitcoinAccount(*account_id)); - auto addresses = keyring_service()->GetBitcoinAddresses(account_id); + auto addresses = keyring_service().GetBitcoinAddresses(account_id); if (!addresses) { NOTREACHED_IN_MIGRATION(); std::move(callback).Run(base::unexpected(InternalErrorString())); @@ -952,7 +947,7 @@ void BitcoinWalletService::SignAndPostTransaction( auto serialized_transaction = BitcoinSerializer::SerializeSignedTransaction(bitcoin_transaction); - bitcoin_rpc_->PostTransaction( + bitcoin_rpc_.PostTransaction( GetNetworkForBitcoinAccount(account_id), serialized_transaction, base::BindOnce(&BitcoinWalletService::OnPostTransaction, weak_ptr_factory_.GetWeakPtr(), @@ -975,7 +970,7 @@ void BitcoinWalletService::PostHwSignedTransaction( auto serialized_transaction = BitcoinSerializer::SerializeSignedTransaction(bitcoin_transaction); - bitcoin_rpc_->PostTransaction( + bitcoin_rpc_.PostTransaction( GetNetworkForBitcoinAccount(account_id), serialized_transaction, base::BindOnce(&BitcoinWalletService::OnPostTransaction, weak_ptr_factory_.GetWeakPtr(), @@ -998,7 +993,7 @@ void BitcoinWalletService::GetTransactionStatus( const std::string& chain_id, const std::string& txid, GetTransactionStatusCallback callback) { - bitcoin_rpc_->GetTransaction( + bitcoin_rpc_.GetTransaction( chain_id, txid, base::BindOnce(&BitcoinWalletService::OnGetTransaction, weak_ptr_factory_.GetWeakPtr(), txid, @@ -1027,7 +1022,7 @@ void BitcoinWalletService::FetchRawTransactions( const std::vector& txids, FetchRawTransactionsCallback callback) { auto task = - std::make_unique(this, network_id, txids); + std::make_unique(*this, network_id, txids); task->set_callback(base::BindOnce( &BitcoinWalletService::OnFetchRawTransactionsDone, @@ -1053,7 +1048,7 @@ void BitcoinWalletService::DiscoverNextUnusedAddress( DiscoverNextUnusedAddressCallback callback) { CHECK(IsBitcoinAccount(*account_id)); - auto account_info = keyring_service()->GetBitcoinAccountInfo(account_id); + auto account_info = keyring_service().GetBitcoinAccountInfo(account_id); if (!account_info) { return std::move(callback).Run(base::unexpected(InternalErrorString())); } @@ -1069,7 +1064,7 @@ void BitcoinWalletService::DiscoverWalletAccount( mojom::KeyringId keyring_id, uint32_t account_index, DiscoverWalletAccountCallback callback) { - auto task = std::make_unique(this, keyring_id, + auto task = std::make_unique(*this, keyring_id, account_index); task->set_callback(base::BindOnce( @@ -1131,7 +1126,7 @@ BitcoinWalletService::GetBtcHardwareTransactionSignData( bool BitcoinWalletService::SignTransactionInternal( BitcoinTransaction& tx, const mojom::AccountIdPtr& account_id) { - auto addresses = keyring_service()->GetBitcoinAddresses(account_id); + auto addresses = keyring_service().GetBitcoinAddresses(account_id); if (!addresses || addresses->empty()) { return false; } @@ -1155,14 +1150,14 @@ bool BitcoinWalletService::SignTransactionInternal( } auto& key_id = address_map.at(input.utxo_address); - auto signature = keyring_service()->SignMessageByBitcoinKeyring( + auto signature = keyring_service().SignMessageByBitcoinKeyring( account_id, key_id, *hash); if (!signature) { return false; } signature->push_back(tx.sighash_type()); - auto pubkey = keyring_service()->GetBitcoinPubkey(account_id, key_id); + auto pubkey = keyring_service().GetBitcoinPubkey(account_id, key_id); if (!pubkey) { return false; } @@ -1190,7 +1185,7 @@ bool BitcoinWalletService::ApplyHwSignatureInternal( void BitcoinWalletService::SetUrlLoaderFactoryForTesting( scoped_refptr url_loader_factory) { - bitcoin_rpc_->SetUrlLoaderFactoryForTesting( // IN-TEST + bitcoin_rpc_.SetUrlLoaderFactoryForTesting( // IN-TEST std::move(url_loader_factory)); } diff --git a/components/brave_wallet/browser/bitcoin/bitcoin_wallet_service.h b/components/brave_wallet/browser/bitcoin/bitcoin_wallet_service.h index e8609474d80c..b3c271a6389e 100644 --- a/components/brave_wallet/browser/bitcoin/bitcoin_wallet_service.h +++ b/components/brave_wallet/browser/bitcoin/bitcoin_wallet_service.h @@ -12,6 +12,7 @@ #include #include +#include "base/memory/raw_ref.h" #include "base/memory/weak_ptr.h" #include "base/types/expected.h" #include "brave/components/api_request_helper/api_request_helper.h" @@ -22,8 +23,6 @@ #include "brave/components/brave_wallet/common/brave_wallet.mojom.h" #include "mojo/public/cpp/bindings/receiver_set.h" -class PrefService; - namespace brave_wallet { class CreateTransactionTask; class DiscoverNextUnusedAddressTask; @@ -35,9 +34,8 @@ class BitcoinWalletService : public mojom::BitcoinWalletService, public KeyringServiceObserverBase { public: BitcoinWalletService( - KeyringService* keyring_service, - PrefService* prefs, - NetworkManager* network_manager, + KeyringService& keyring_service, + NetworkManager& network_manager, scoped_refptr url_loader_factory); ~BitcoinWalletService() override; @@ -118,8 +116,8 @@ class BitcoinWalletService : public mojom::BitcoinWalletService, const BitcoinTransaction& tx, const mojom::AccountIdPtr& account_id); - bitcoin_rpc::BitcoinRpc& bitcoin_rpc() { return *bitcoin_rpc_; } - KeyringService* keyring_service() { return keyring_service_; } + bitcoin_rpc::BitcoinRpc& bitcoin_rpc() { return bitcoin_rpc_; } + KeyringService& keyring_service() { return *keyring_service_; } void SetUrlLoaderFactoryForTesting( scoped_refptr url_loader_factory); @@ -171,7 +169,7 @@ class BitcoinWalletService : public mojom::BitcoinWalletService, const mojom::BitcoinSignature& hw_signature); void CreateTransactionTaskDone(CreateTransactionTask* task); - raw_ptr keyring_service_; + const raw_ref keyring_service_; scoped_refptr url_loader_factory_; std::list> create_transaction_tasks_; std::list> @@ -182,7 +180,7 @@ class BitcoinWalletService : public mojom::BitcoinWalletService, fetch_raw_transactions_tasks_; mojo::ReceiverSet receivers_; - std::unique_ptr bitcoin_rpc_; + bitcoin_rpc::BitcoinRpc bitcoin_rpc_; bool arrange_transactions_for_testing_ = false; mojo::Receiver keyring_service_observer_receiver_{this}; diff --git a/components/brave_wallet/browser/bitcoin/bitcoin_wallet_service_unittest.cc b/components/brave_wallet/browser/bitcoin/bitcoin_wallet_service_unittest.cc index 14c21d7a8a05..63694008800f 100644 --- a/components/brave_wallet/browser/bitcoin/bitcoin_wallet_service_unittest.cc +++ b/components/brave_wallet/browser/bitcoin/bitcoin_wallet_service_unittest.cc @@ -93,7 +93,7 @@ class BitcoinWalletServiceUnitTest : public testing::Test { std::make_unique(nullptr, &prefs_, &local_state_); bitcoin_test_rpc_server_ = std::make_unique(); bitcoin_wallet_service_ = std::make_unique( - keyring_service_.get(), &prefs_, network_manager_.get(), + *keyring_service_, *network_manager_, bitcoin_test_rpc_server_->GetURLLoaderFactory()); bitcoin_wallet_service_->SetArrangeTransactionsForTesting(true); diff --git a/components/brave_wallet/browser/brave_wallet_service.cc b/components/brave_wallet/browser/brave_wallet_service.cc index e5ee642a7bf5..9d11054d85bb 100644 --- a/components/brave_wallet/browser/brave_wallet_service.cc +++ b/components/brave_wallet/browser/brave_wallet_service.cc @@ -125,8 +125,7 @@ BraveWalletService::BraveWalletService( if (IsBitcoinEnabled()) { bitcoin_wallet_service_ = std::make_unique( - keyring_service(), profile_prefs, network_manager(), - url_loader_factory); + *keyring_service(), *network_manager(), url_loader_factory); } if (IsZCashEnabled()) { @@ -138,7 +137,7 @@ BraveWalletService::BraveWalletService( tx_service_ = std::make_unique( json_rpc_service(), GetBitcoinWalletService(), GetZcashWalletService(), - keyring_service(), profile_prefs, delegate_->GetWalletBaseDirectory(), + *keyring_service(), profile_prefs, delegate_->GetWalletBaseDirectory(), base::SequencedTaskRunner::GetCurrentDefault()); brave_wallet_p3a_ = std::make_unique( diff --git a/components/brave_wallet/browser/eth_nonce_tracker_unittest.cc b/components/brave_wallet/browser/eth_nonce_tracker_unittest.cc index 1a893aae3470..7db57d604323 100644 --- a/components/brave_wallet/browser/eth_nonce_tracker_unittest.cc +++ b/components/brave_wallet/browser/eth_nonce_tracker_unittest.cc @@ -122,8 +122,7 @@ TEST_F(EthNonceTrackerUnitTest, GetNonce) { GetTxStorageDelegateForTest(GetPrefs(), factory); auto account_resolver_delegate = std::make_unique(); - EthTxStateManager tx_state_manager(GetPrefs(), delegate.get(), - account_resolver_delegate.get()); + EthTxStateManager tx_state_manager(*delegate, *account_resolver_delegate); EthNonceTracker nonce_tracker(&tx_state_manager, json_rpc_service()); SetTransactionCount(2); diff --git a/components/brave_wallet/browser/eth_tx_manager.cc b/components/brave_wallet/browser/eth_tx_manager.cc index 561cf41360a4..ee43cc70fae2 100644 --- a/components/brave_wallet/browser/eth_tx_manager.cc +++ b/components/brave_wallet/browser/eth_tx_manager.cc @@ -110,33 +110,28 @@ bool EthTxManager::ValidateTxData1559(const mojom::TxData1559Ptr& tx_data, return true; } -EthTxManager::EthTxManager(TxService* tx_service, +EthTxManager::EthTxManager(TxService& tx_service, JsonRpcService* json_rpc_service, - KeyringService* keyring_service, - PrefService* prefs, - TxStorageDelegate* delegate, - AccountResolverDelegate* account_resolver_delegate) - : TxManager(std::make_unique(prefs, - delegate, + KeyringService& keyring_service, + TxStorageDelegate& delegate, + AccountResolverDelegate& account_resolver_delegate) + : TxManager(std::make_unique(delegate, account_resolver_delegate), std::make_unique(json_rpc_service), tx_service, - keyring_service, - prefs), - nonce_tracker_(std::make_unique(GetEthTxStateManager(), + keyring_service), + nonce_tracker_(std::make_unique(&GetEthTxStateManager(), json_rpc_service)), pending_tx_tracker_( - std::make_unique(GetEthTxStateManager(), + std::make_unique(&GetEthTxStateManager(), json_rpc_service, nonce_tracker_.get())), - prefs_(prefs), - json_rpc_service_(json_rpc_service), - account_resolver_delegate_(account_resolver_delegate) { - GetEthBlockTracker()->AddObserver(this); + json_rpc_service_(json_rpc_service) { + GetEthBlockTracker().AddObserver(this); } EthTxManager::~EthTxManager() { - GetEthBlockTracker()->RemoveObserver(this); + GetEthBlockTracker().RemoveObserver(this); } void EthTxManager::AddUnapprovedTransaction( @@ -314,7 +309,7 @@ void EthTxManager::ContinueAddUnapprovedTransaction( meta.set_status(mojom::TransactionStatus::Unapproved); meta.set_sign_only(sign_only); meta.set_chain_id(chain_id); - if (!tx_state_manager_->AddOrUpdateTx(meta)) { + if (!tx_state_manager().AddOrUpdateTx(meta)) { std::move(callback).Run( false, "", l10n_util::GetStringUTF8(IDS_WALLET_INTERNAL_ERROR)); return; @@ -416,8 +411,7 @@ void EthTxManager::OnGetGasOracleForUnapprovedTransaction( void EthTxManager::GetNonceForHardwareTransaction( const std::string& tx_meta_id, GetNonceForHardwareTransactionCallback callback) { - std::unique_ptr meta = - GetEthTxStateManager()->GetEthTx(tx_meta_id); + std::unique_ptr meta = GetEthTxStateManager().GetEthTx(tx_meta_id); if (!meta) { LOG(ERROR) << "No transaction found"; std::move(callback).Run(std::nullopt); @@ -441,8 +435,7 @@ void EthTxManager::GetNonceForHardwareTransaction( void EthTxManager::GetEthTransactionMessageToSign( const std::string& tx_meta_id, GetEthTransactionMessageToSignCallback callback) { - std::unique_ptr meta = - GetEthTxStateManager()->GetEthTx(tx_meta_id); + std::unique_ptr meta = GetEthTxStateManager().GetEthTx(tx_meta_id); if (!meta) { VLOG(1) << __FUNCTION__ << "No transaction found with id:" << tx_meta_id; std::move(callback).Run(std::nullopt); @@ -468,14 +461,14 @@ void EthTxManager::OnGetNextNonceForHardware( uint256_t nonce) { if (!success) { meta->set_status(mojom::TransactionStatus::Error); - tx_state_manager_->AddOrUpdateTx(*meta); + tx_state_manager().AddOrUpdateTx(*meta); VLOG(1) << __FUNCTION__ << "GetNextNonce failed for tx with meta:" << meta->id(); std::move(callback).Run(std::nullopt); return; } meta->tx()->set_nonce(nonce); - if (!tx_state_manager_->AddOrUpdateTx(*meta)) { + if (!tx_state_manager().AddOrUpdateTx(*meta)) { std::move(callback).Run(std::nullopt); return; } @@ -486,8 +479,7 @@ void EthTxManager::ProcessEthHardwareSignature( const std::string& tx_meta_id, mojom::EthereumSignatureVRSPtr hw_signature, ProcessEthHardwareSignatureCallback callback) { - std::unique_ptr meta = - GetEthTxStateManager()->GetEthTx(tx_meta_id); + std::unique_ptr meta = GetEthTxStateManager().GetEthTx(tx_meta_id); if (!meta) { VLOG(1) << __FUNCTION__ << "No transaction found with id" << tx_meta_id; std::move(callback).Run( @@ -501,7 +493,7 @@ void EthTxManager::ProcessEthHardwareSignature( << "Could not initialize a transaction with v,r,s for id:" << tx_meta_id; meta->set_status(mojom::TransactionStatus::Error); - tx_state_manager_->AddOrUpdateTx(*meta); + tx_state_manager().AddOrUpdateTx(*meta); std::move(callback).Run( false, mojom::ProviderError::kInternalError, l10n_util::GetStringUTF8( @@ -509,7 +501,7 @@ void EthTxManager::ProcessEthHardwareSignature( return; } meta->set_status(mojom::TransactionStatus::Approved); - if (!tx_state_manager_->AddOrUpdateTx(*meta)) { + if (!tx_state_manager().AddOrUpdateTx(*meta)) { std::move(callback).Run( false, mojom::ProviderError::kInternalError, l10n_util::GetStringUTF8(IDS_WALLET_INTERNAL_ERROR)); @@ -537,8 +529,7 @@ void EthTxManager::ContinueProcessHardwareSignature( void EthTxManager::ApproveTransaction(const std::string& tx_meta_id, ApproveTransactionCallback callback) { - std::unique_ptr meta = - GetEthTxStateManager()->GetEthTx(tx_meta_id); + std::unique_ptr meta = GetEthTxStateManager().GetEthTx(tx_meta_id); if (!meta) { LOG(ERROR) << "No transaction found"; std::move(callback).Run( @@ -569,7 +560,7 @@ void EthTxManager::OnGetNextNonce(std::unique_ptr meta, uint256_t nonce) { if (!success) { meta->set_status(mojom::TransactionStatus::Error); - tx_state_manager_->AddOrUpdateTx(*meta); + tx_state_manager().AddOrUpdateTx(*meta); LOG(ERROR) << "GetNextNonce failed"; std::move(callback).Run( false, @@ -592,7 +583,7 @@ void EthTxManager::OnGetNextNonce(std::unique_ptr meta, meta->tx()->set_nonce(nonce); - if (keyring_service_->IsLockedSync()) { + if (keyring_service().IsLockedSync()) { std::move(callback).Run( false, mojom::ProviderErrorUnion::NewProviderError( @@ -601,10 +592,10 @@ void EthTxManager::OnGetNextNonce(std::unique_ptr meta, return; } - keyring_service_->SignTransactionByDefaultKeyring(*meta->from(), meta->tx(), + keyring_service().SignTransactionByDefaultKeyring(*meta->from(), meta->tx(), chain_id); meta->set_status(mojom::TransactionStatus::Approved); - if (!tx_state_manager_->AddOrUpdateTx(*meta)) { + if (!tx_state_manager().AddOrUpdateTx(*meta)) { std::move(callback).Run( false, mojom::ProviderErrorUnion::NewProviderError( @@ -624,7 +615,7 @@ void EthTxManager::OnGetNextNonce(std::unique_ptr meta, if (meta->sign_only()) { meta->set_status(mojom::TransactionStatus::Signed); meta->set_tx_hash(meta->tx()->GetTransactionHash()); - if (!tx_state_manager_->AddOrUpdateTx(*meta)) { + if (!tx_state_manager().AddOrUpdateTx(*meta)) { std::move(callback).Run( false, mojom::ProviderErrorUnion::NewProviderError( @@ -661,7 +652,7 @@ void EthTxManager::OnPublishTransaction(const std::string& chain_id, const std::string& tx_hash, mojom::ProviderError error, const std::string& error_message) { - std::unique_ptr meta = tx_state_manager_->GetTx(tx_meta_id); + std::unique_ptr meta = tx_state_manager().GetTx(tx_meta_id); if (!meta) { DCHECK(false) << "Transaction should be found"; std::move(callback).Run( @@ -680,7 +671,7 @@ void EthTxManager::OnPublishTransaction(const std::string& chain_id, meta->set_status(mojom::TransactionStatus::Error); } - if (!tx_state_manager_->AddOrUpdateTx(*meta)) { + if (!tx_state_manager().AddOrUpdateTx(*meta)) { std::move(callback).Run( false, mojom::ProviderErrorUnion::NewProviderError( @@ -876,7 +867,7 @@ void EthTxManager::MakeERC1155TransferFromData( } void EthTxManager::NotifyUnapprovedTxUpdated(TxMeta* meta) { - tx_service_->OnUnapprovedTxUpdated(meta->ToTransactionInfo()); + tx_service().OnUnapprovedTxUpdated(meta->ToTransactionInfo()); } void EthTxManager::SetGasPriceAndLimitForUnapprovedTransaction( @@ -889,7 +880,7 @@ void EthTxManager::SetGasPriceAndLimitForUnapprovedTransaction( return; } - auto tx_meta = GetEthTxStateManager()->GetEthTx(tx_meta_id); + auto tx_meta = GetEthTxStateManager().GetEthTx(tx_meta_id); if (!tx_meta || tx_meta->status() != mojom::TransactionStatus::Unapproved) { std::move(callback).Run(false); return; @@ -908,7 +899,7 @@ void EthTxManager::SetGasPriceAndLimitForUnapprovedTransaction( } tx_meta->tx()->set_gas_limit(value); - if (!tx_state_manager_->AddOrUpdateTx(*tx_meta)) { + if (!tx_state_manager().AddOrUpdateTx(*tx_meta)) { std::move(callback).Run(false); return; } @@ -928,7 +919,7 @@ void EthTxManager::SetGasFeeAndLimitForUnapprovedTransaction( return; } - auto tx_meta = GetEthTxStateManager()->GetEthTx(tx_meta_id); + auto tx_meta = GetEthTxStateManager().GetEthTx(tx_meta_id); if (!tx_meta || tx_meta->status() != mojom::TransactionStatus::Unapproved || tx_meta->tx()->type() != 2 /* Eip1559 */) { std::move(callback).Run(false); @@ -956,7 +947,7 @@ void EthTxManager::SetGasFeeAndLimitForUnapprovedTransaction( } tx1559->set_gas_limit(value); - if (!tx_state_manager_->AddOrUpdateTx(*tx_meta)) { + if (!tx_state_manager().AddOrUpdateTx(*tx_meta)) { std::move(callback).Run(false); return; } @@ -968,14 +959,14 @@ void EthTxManager::SetDataForUnapprovedTransaction( const std::string& tx_meta_id, const std::vector& data, SetDataForUnapprovedTransactionCallback callback) { - auto tx_meta = GetEthTxStateManager()->GetEthTx(tx_meta_id); + auto tx_meta = GetEthTxStateManager().GetEthTx(tx_meta_id); if (!tx_meta || tx_meta->status() != mojom::TransactionStatus::Unapproved) { std::move(callback).Run(false); return; } tx_meta->tx()->set_data(data); - if (!tx_state_manager_->AddOrUpdateTx(*tx_meta)) { + if (!tx_state_manager().AddOrUpdateTx(*tx_meta)) { std::move(callback).Run(false); return; } @@ -987,7 +978,7 @@ void EthTxManager::SetNonceForUnapprovedTransaction( const std::string& tx_meta_id, const std::string& nonce, SetNonceForUnapprovedTransactionCallback callback) { - auto tx_meta = GetEthTxStateManager()->GetEthTx(tx_meta_id); + auto tx_meta = GetEthTxStateManager().GetEthTx(tx_meta_id); if (!tx_meta || tx_meta->status() != mojom::TransactionStatus::Unapproved) { std::move(callback).Run(false); return; @@ -1003,7 +994,7 @@ void EthTxManager::SetNonceForUnapprovedTransaction( } tx_meta->tx()->set_nonce(nonce_uint); } - if (!tx_state_manager_->AddOrUpdateTx(*tx_meta)) { + if (!tx_state_manager().AddOrUpdateTx(*tx_meta)) { std::move(callback).Run(false); return; } @@ -1013,7 +1004,7 @@ void EthTxManager::SetNonceForUnapprovedTransaction( std::unique_ptr EthTxManager::GetTxForTesting( const std::string& tx_meta_id) { - return GetEthTxStateManager()->GetEthTx(tx_meta_id); + return GetEthTxStateManager().GetEthTx(tx_meta_id); } void EthTxManager::OnNewBlock(const std::string& chain_id, @@ -1034,8 +1025,7 @@ void EthTxManager::SpeedupOrCancelTransaction( const std::string& tx_meta_id, bool cancel, SpeedupOrCancelTransactionCallback callback) { - std::unique_ptr meta = - GetEthTxStateManager()->GetEthTx(tx_meta_id); + std::unique_ptr meta = GetEthTxStateManager().GetEthTx(tx_meta_id); if (!meta || meta->status() != mojom::TransactionStatus::Submitted) { std::move(callback).Run( false, "", @@ -1175,8 +1165,7 @@ void EthTxManager::ContinueSpeedupOrCancel1559Transaction( void EthTxManager::RetryTransaction(const std::string& tx_meta_id, RetryTransactionCallback callback) { - std::unique_ptr meta = - GetEthTxStateManager()->GetEthTx(tx_meta_id); + std::unique_ptr meta = GetEthTxStateManager().GetEthTx(tx_meta_id); if (!meta || !meta->tx()) { std::move(callback).Run( @@ -1302,12 +1291,12 @@ void EthTxManager::Reset() { pending_tx_tracker_->Reset(); } -EthTxStateManager* EthTxManager::GetEthTxStateManager() { - return static_cast(tx_state_manager_.get()); +EthTxStateManager& EthTxManager::GetEthTxStateManager() { + return static_cast(tx_state_manager()); } -EthBlockTracker* EthTxManager::GetEthBlockTracker() { - return static_cast(block_tracker_.get()); +EthBlockTracker& EthTxManager::GetEthBlockTracker() { + return static_cast(block_tracker()); } } // namespace brave_wallet diff --git a/components/brave_wallet/browser/eth_tx_manager.h b/components/brave_wallet/browser/eth_tx_manager.h index 3a73bd5eb542..89572ae5e605 100644 --- a/components/brave_wallet/browser/eth_tx_manager.h +++ b/components/brave_wallet/browser/eth_tx_manager.h @@ -22,8 +22,6 @@ #include "brave/components/brave_wallet/common/brave_wallet.mojom.h" #include "brave/components/brave_wallet/common/fil_address.h" -class PrefService; - namespace brave_wallet { class EthTxManagerUnitTest; @@ -38,12 +36,11 @@ class EthTxManager : public TxManager, public EthBlockTracker::Observer { using AddUnapprovedEvmTransactionCallback = mojom::TxService::AddUnapprovedEvmTransactionCallback; - EthTxManager(TxService* tx_service, + EthTxManager(TxService& tx_service, JsonRpcService* json_rpc_service, - KeyringService* keyring_service, - PrefService* prefs, - TxStorageDelegate* delegate, - AccountResolverDelegate* account_resolver_delegate); + KeyringService& keyring_service, + TxStorageDelegate& delegate, + AccountResolverDelegate& account_resolver_delegate); ~EthTxManager() override; EthTxManager(const EthTxManager&) = delete; EthTxManager operator=(const EthTxManager&) = delete; @@ -291,14 +288,12 @@ class EthTxManager : public TxManager, public EthBlockTracker::Observer { uint256_t block_num) override {} void OnNewBlock(const std::string& chain_id, uint256_t block_num) override; - EthTxStateManager* GetEthTxStateManager(); - EthBlockTracker* GetEthBlockTracker(); + EthTxStateManager& GetEthTxStateManager(); + EthBlockTracker& GetEthBlockTracker(); std::unique_ptr nonce_tracker_; std::unique_ptr pending_tx_tracker_; - raw_ptr prefs_ = nullptr; raw_ptr json_rpc_service_ = nullptr; - raw_ptr account_resolver_delegate_ = nullptr; base::WeakPtrFactory weak_factory_{this}; }; diff --git a/components/brave_wallet/browser/eth_tx_manager_unittest.cc b/components/brave_wallet/browser/eth_tx_manager_unittest.cc index 17bf271ec153..429781fab619 100644 --- a/components/brave_wallet/browser/eth_tx_manager_unittest.cc +++ b/components/brave_wallet/browser/eth_tx_manager_unittest.cc @@ -260,7 +260,7 @@ class EthTxManagerUnitTest : public testing::Test { json_rpc_service_.get(), &profile_prefs_, &local_state_); ASSERT_TRUE(temp_dir_.CreateUniqueTempDir()); tx_service_ = std::make_unique( - json_rpc_service_.get(), nullptr, nullptr, keyring_service_.get(), + json_rpc_service_.get(), nullptr, nullptr, *keyring_service_, GetPrefs(), temp_dir_.GetPath(), base::SequencedTaskRunner::GetCurrentDefault()); WaitForTxStorageDelegateInitialized(tx_service_->GetDelegateForTesting()); @@ -331,7 +331,7 @@ class EthTxManagerUnitTest : public testing::Test { meta.set_id(orig_meta_id); meta.set_chain_id(chain_id); meta.set_status(status); - ASSERT_TRUE(eth_tx_manager()->tx_state_manager_->AddOrUpdateTx(meta)); + ASSERT_TRUE(eth_tx_manager()->tx_state_manager().AddOrUpdateTx(meta)); bool callback_called = false; eth_tx_manager()->SpeedupOrCancelTransaction( @@ -366,7 +366,7 @@ class EthTxManagerUnitTest : public testing::Test { meta.set_id(orig_meta_id); meta.set_chain_id(chain_id); meta.set_status(status); - ASSERT_TRUE(eth_tx_manager()->tx_state_manager_->AddOrUpdateTx(meta)); + ASSERT_TRUE(eth_tx_manager()->tx_state_manager().AddOrUpdateTx(meta)); bool callback_called = false; eth_tx_manager()->SpeedupOrCancelTransaction( @@ -1924,13 +1924,13 @@ TEST_F(EthTxManagerUnitTest, TestSubmittedToConfirmed) { meta.set_id("001"); meta.set_chain_id(mojom::kLocalhostChainId); meta.set_status(mojom::TransactionStatus::Submitted); - ASSERT_TRUE(eth_tx_manager()->tx_state_manager_->AddOrUpdateTx(meta)); + ASSERT_TRUE(eth_tx_manager()->tx_state_manager().AddOrUpdateTx(meta)); meta.set_id("002"); meta.set_chain_id(mojom::kMainnetChainId); meta.set_from(EthAccount(1)); meta.tx()->set_nonce(uint256_t(4)); meta.set_status(mojom::TransactionStatus::Submitted); - ASSERT_TRUE(eth_tx_manager()->tx_state_manager_->AddOrUpdateTx(meta)); + ASSERT_TRUE(eth_tx_manager()->tx_state_manager().AddOrUpdateTx(meta)); eth_tx_manager()->UpdatePendingTransactions(std::nullopt); @@ -1987,13 +1987,13 @@ TEST_F(EthTxManagerUnitTest, TestSubmittedToConfirmed) { meta.set_chain_id(mojom::kLocalhostChainId); meta.set_from(EthAccount(0)); meta.set_status(mojom::TransactionStatus::Submitted); - ASSERT_TRUE(eth_tx_manager()->tx_state_manager_->AddOrUpdateTx(meta)); + ASSERT_TRUE(eth_tx_manager()->tx_state_manager().AddOrUpdateTx(meta)); meta.set_id("002"); meta.set_chain_id(mojom::kMainnetChainId); meta.set_from(EthAccount(1)); meta.tx()->set_nonce(uint256_t(4)); meta.set_status(mojom::TransactionStatus::Submitted); - ASSERT_TRUE(eth_tx_manager()->tx_state_manager_->AddOrUpdateTx(meta)); + ASSERT_TRUE(eth_tx_manager()->tx_state_manager().AddOrUpdateTx(meta)); keyring_service_->Lock(); task_environment_.FastForwardBy( base::Seconds(kBlockTrackerDefaultTimeInSeconds + 1)); @@ -2221,7 +2221,7 @@ TEST_F(EthTxManagerUnitTest, RetryTransaction) { meta.set_id("001"); meta.set_chain_id(mojom::kLocalhostChainId); meta.set_status(mojom::TransactionStatus::Error); - ASSERT_TRUE(eth_tx_manager()->tx_state_manager_->AddOrUpdateTx(meta)); + ASSERT_TRUE(eth_tx_manager()->tx_state_manager().AddOrUpdateTx(meta)); bool callback_called = false; std::string tx_meta_id; @@ -2251,7 +2251,7 @@ TEST_F(EthTxManagerUnitTest, RetryTransaction) { meta.set_chain_id(mojom::kLocalhostChainId); meta.set_status(mojom::TransactionStatus::Error); meta.set_tx(std::make_unique(*tx1559)); - ASSERT_TRUE(eth_tx_manager()->tx_state_manager_->AddOrUpdateTx(meta)); + ASSERT_TRUE(eth_tx_manager()->tx_state_manager().AddOrUpdateTx(meta)); eth_tx_manager()->RetryTransaction( "002", base::BindOnce(&AddUnapprovedTransactionSuccessCallback, @@ -2404,10 +2404,10 @@ TEST_F(EthTxManagerUnitTest, MakeERC1155TransferFromData) { TEST_F(EthTxManagerUnitTest, Reset) { eth_tx_manager()->pending_chain_ids_.emplace(mojom::kLocalhostChainId); - eth_tx_manager()->block_tracker_->Start(mojom::kLocalhostChainId, + eth_tx_manager()->block_tracker().Start(mojom::kLocalhostChainId, base::Seconds(10)); EXPECT_TRUE( - eth_tx_manager()->block_tracker_->IsRunning(mojom::kLocalhostChainId)); + eth_tx_manager()->block_tracker().IsRunning(mojom::kLocalhostChainId)); EthTxMeta meta(from(), std::make_unique()); meta.set_id("001"); meta.set_chain_id(mojom::kLocalhostChainId); @@ -2417,16 +2417,16 @@ TEST_F(EthTxManagerUnitTest, Reset) { "0x016345785d8a0000", std::vector(), false, std::nullopt); auto tx = EthTransaction::FromTxData(tx_data, false); meta.set_tx(std::make_unique(*tx)); - ASSERT_TRUE(eth_tx_manager()->tx_state_manager_->AddOrUpdateTx(meta)); + ASSERT_TRUE(eth_tx_manager()->tx_state_manager().AddOrUpdateTx(meta)); EXPECT_EQ(tx_service_->GetDelegateForTesting()->GetTxs().size(), 1u); GetPrefs()->Set(kBraveWalletTransactions, ParseJson(R"({"ethereum":{}})")); tx_service_->Reset(); EXPECT_FALSE(GetPrefs()->HasPrefPath(kBraveWalletTransactions)); - EXPECT_TRUE(eth_tx_manager()->pending_chain_ids_.empty()); + EXPECT_TRUE(eth_tx_manager()->pending_chain_ids().empty()); EXPECT_FALSE( - eth_tx_manager()->block_tracker_->IsRunning(mojom::kLocalhostChainId)); + eth_tx_manager()->block_tracker().IsRunning(mojom::kLocalhostChainId)); // cache should be empty EXPECT_TRUE(tx_service_->GetDelegateForTesting()->GetTxs().empty()); // db should be empty diff --git a/components/brave_wallet/browser/eth_tx_state_manager.cc b/components/brave_wallet/browser/eth_tx_state_manager.cc index 218fdac74f7d..dd92c966a371 100644 --- a/components/brave_wallet/browser/eth_tx_state_manager.cc +++ b/components/brave_wallet/browser/eth_tx_state_manager.cc @@ -18,10 +18,9 @@ namespace brave_wallet { EthTxStateManager::EthTxStateManager( - PrefService* prefs, - TxStorageDelegate* delegate, - AccountResolverDelegate* account_resolver_delegate) - : TxStateManager(prefs, delegate, account_resolver_delegate) {} + TxStorageDelegate& delegate, + AccountResolverDelegate& account_resolver_delegate) + : TxStateManager(delegate, account_resolver_delegate) {} EthTxStateManager::~EthTxStateManager() = default; diff --git a/components/brave_wallet/browser/eth_tx_state_manager.h b/components/brave_wallet/browser/eth_tx_state_manager.h index ec075f31325b..a442ff160dbc 100644 --- a/components/brave_wallet/browser/eth_tx_state_manager.h +++ b/components/brave_wallet/browser/eth_tx_state_manager.h @@ -12,8 +12,6 @@ #include "brave/components/brave_wallet/browser/tx_state_manager.h" #include "brave/components/brave_wallet/common/brave_wallet.mojom.h" -class PrefService; - namespace base { class Value; } // namespace base @@ -26,9 +24,8 @@ class EthTxMeta; class EthTxStateManager : public TxStateManager { public: - EthTxStateManager(PrefService* prefs, - TxStorageDelegate* delegate, - AccountResolverDelegate* account_resolver_delegate); + EthTxStateManager(TxStorageDelegate& delegate, + AccountResolverDelegate& account_resolver_delegate); ~EthTxStateManager() override; EthTxStateManager(const EthTxStateManager&) = delete; EthTxStateManager operator=(const EthTxStateManager&) = delete; diff --git a/components/brave_wallet/browser/eth_tx_state_manager_unittest.cc b/components/brave_wallet/browser/eth_tx_state_manager_unittest.cc index 5d28485fd9df..f2d58bc91e9f 100644 --- a/components/brave_wallet/browser/eth_tx_state_manager_unittest.cc +++ b/components/brave_wallet/browser/eth_tx_state_manager_unittest.cc @@ -45,7 +45,7 @@ class EthTxStateManagerUnitTest : public testing::Test { account_resolver_delegate_ = std::make_unique(); eth_tx_state_manager_ = std::make_unique( - GetPrefs(), delegate_.get(), account_resolver_delegate_.get()); + *delegate_, *account_resolver_delegate_); } PrefService* GetPrefs() { return &prefs_; } diff --git a/components/brave_wallet/browser/fil_nonce_tracker_unittest.cc b/components/brave_wallet/browser/fil_nonce_tracker_unittest.cc index b0fcd5a89a64..55f913ddef80 100644 --- a/components/brave_wallet/browser/fil_nonce_tracker_unittest.cc +++ b/components/brave_wallet/browser/fil_nonce_tracker_unittest.cc @@ -114,8 +114,7 @@ TEST_F(FilNonceTrackerUnitTest, GetNonce) { GetTxStorageDelegateForTest(GetPrefs(), factory); auto account_resolver_delegate = std::make_unique(); - FilTxStateManager tx_state_manager(GetPrefs(), delegate.get(), - account_resolver_delegate.get()); + FilTxStateManager tx_state_manager(*delegate, *account_resolver_delegate); FilNonceTracker nonce_tracker(&tx_state_manager, json_rpc_service()); SetTransactionCount(2); diff --git a/components/brave_wallet/browser/fil_tx_manager.cc b/components/brave_wallet/browser/fil_tx_manager.cc index 7fc17e9cd268..c220f62bcd64 100644 --- a/components/brave_wallet/browser/fil_tx_manager.cc +++ b/components/brave_wallet/browser/fil_tx_manager.cc @@ -26,28 +26,24 @@ namespace brave_wallet { -FilTxManager::FilTxManager(TxService* tx_service, +FilTxManager::FilTxManager(TxService& tx_service, JsonRpcService* json_rpc_service, - KeyringService* keyring_service, - PrefService* prefs, - TxStorageDelegate* delegate, - AccountResolverDelegate* account_resolver_delegate) - : TxManager(std::make_unique(prefs, - delegate, + KeyringService& keyring_service, + TxStorageDelegate& delegate, + AccountResolverDelegate& account_resolver_delegate) + : TxManager(std::make_unique(delegate, account_resolver_delegate), std::make_unique(json_rpc_service), tx_service, - keyring_service, - prefs), - nonce_tracker_(std::make_unique(GetFilTxStateManager(), + keyring_service), + nonce_tracker_(std::make_unique(&GetFilTxStateManager(), json_rpc_service)), - json_rpc_service_(json_rpc_service), - account_resolver_delegate_(account_resolver_delegate) { - GetFilBlockTracker()->AddObserver(this); + json_rpc_service_(json_rpc_service) { + GetFilBlockTracker().AddObserver(this); } FilTxManager::~FilTxManager() { - GetFilBlockTracker()->RemoveObserver(this); + GetFilBlockTracker().RemoveObserver(this); } void FilTxManager::GetEstimatedGas(const std::string& chain_id, @@ -97,7 +93,7 @@ void FilTxManager::ContinueAddUnapprovedTransaction( meta.set_status(mojom::TransactionStatus::Unapproved); meta.set_chain_id(chain_id); - if (!tx_state_manager_->AddOrUpdateTx(meta)) { + if (!tx_state_manager().AddOrUpdateTx(meta)) { std::move(callback).Run( false, "", l10n_util::GetStringUTF8(IDS_WALLET_INTERNAL_ERROR)); return; @@ -145,8 +141,7 @@ void FilTxManager::AddUnapprovedTransaction( void FilTxManager::ApproveTransaction(const std::string& tx_meta_id, ApproveTransactionCallback callback) { - std::unique_ptr meta = - GetFilTxStateManager()->GetFilTx(tx_meta_id); + std::unique_ptr meta = GetFilTxStateManager().GetFilTx(tx_meta_id); if (!meta) { DCHECK(false) << "Transaction should be found"; std::move(callback).Run( @@ -176,7 +171,7 @@ void FilTxManager::OnGetNextNonce(std::unique_ptr meta, uint256_t nonce) { if (!success) { meta->set_status(mojom::TransactionStatus::Error); - tx_state_manager_->AddOrUpdateTx(*meta); + tx_state_manager().AddOrUpdateTx(*meta); LOG(ERROR) << "GetNextNonce failed"; std::move(callback).Run( false, @@ -185,7 +180,7 @@ void FilTxManager::OnGetNextNonce(std::unique_ptr meta, l10n_util::GetStringUTF8(IDS_WALLET_GET_NONCE_ERROR)); return; } - if (keyring_service_->IsLockedSync()) { + if (keyring_service().IsLockedSync()) { std::move(callback).Run( false, mojom::ProviderErrorUnion::NewFilecoinProviderError( @@ -199,7 +194,7 @@ void FilTxManager::OnGetNextNonce(std::unique_ptr meta, DCHECK(nonce <= static_cast(UINT64_MAX)); meta->tx()->set_nonce(static_cast(nonce)); meta->set_status(mojom::TransactionStatus::Approved); - if (!tx_state_manager_->AddOrUpdateTx(*meta)) { + if (!tx_state_manager().AddOrUpdateTx(*meta)) { std::move(callback).Run( false, mojom::ProviderErrorUnion::NewFilecoinProviderError( @@ -208,7 +203,7 @@ void FilTxManager::OnGetNextNonce(std::unique_ptr meta, return; } - auto signed_tx = keyring_service_->SignTransactionByFilecoinKeyring( + auto signed_tx = keyring_service().SignTransactionByFilecoinKeyring( *meta->from(), meta->tx()); if (!signed_tx) { std::move(callback).Run( @@ -231,7 +226,7 @@ void FilTxManager::OnSendFilecoinTransaction( const std::string& tx_cid, mojom::FilecoinProviderError error, const std::string& error_message) { - std::unique_ptr meta = tx_state_manager_->GetTx(tx_meta_id); + std::unique_ptr meta = tx_state_manager().GetTx(tx_meta_id); if (!meta) { NOTREACHED_IN_MIGRATION() << "Transaction should be found"; std::move(callback).Run( @@ -252,7 +247,7 @@ void FilTxManager::OnSendFilecoinTransaction( meta->set_status(mojom::TransactionStatus::Error); } - if (!tx_state_manager_->AddOrUpdateTx(*meta)) { + if (!tx_state_manager().AddOrUpdateTx(*meta)) { std::move(callback).Run( false, mojom::ProviderErrorUnion::NewFilecoinProviderError( @@ -285,8 +280,7 @@ void FilTxManager::RetryTransaction(const std::string& tx_meta_id, void FilTxManager::GetFilTransactionMessageToSign( const std::string& tx_meta_id, GetFilTransactionMessageToSignCallback callback) { - std::unique_ptr meta = - GetFilTxStateManager()->GetFilTx(tx_meta_id); + std::unique_ptr meta = GetFilTxStateManager().GetFilTx(tx_meta_id); if (!meta || !meta->tx()) { VLOG(1) << __FUNCTION__ << "No transaction found with id:" << tx_meta_id; std::move(callback).Run(std::nullopt); @@ -318,7 +312,7 @@ void FilTxManager::OnGetNextNonceForHardware( uint256_t nonce) { if (!success) { meta->set_status(mojom::TransactionStatus::Error); - tx_state_manager_->AddOrUpdateTx(*meta); + tx_state_manager().AddOrUpdateTx(*meta); std::move(callback).Run(std::nullopt); return; } @@ -327,7 +321,7 @@ void FilTxManager::OnGetNextNonceForHardware( DCHECK(nonce <= static_cast(UINT64_MAX)); meta->tx()->set_nonce(static_cast(nonce)); meta->set_status(mojom::TransactionStatus::Approved); - if (!tx_state_manager_->AddOrUpdateTx(*meta)) { + if (!tx_state_manager().AddOrUpdateTx(*meta)) { std::move(callback).Run(std::nullopt); return; } @@ -347,16 +341,16 @@ void FilTxManager::Reset() { std::unique_ptr FilTxManager::GetTxForTesting( const std::string& tx_meta_id) { - return GetFilTxStateManager()->GetFilTx(tx_meta_id); + return GetFilTxStateManager().GetFilTx(tx_meta_id); } -FilTxStateManager* FilTxManager::GetFilTxStateManager() { - return static_cast(tx_state_manager_.get()); +FilTxStateManager& FilTxManager::GetFilTxStateManager() { + return static_cast(tx_state_manager()); } void FilTxManager::UpdatePendingTransactions( const std::optional& chain_id) { - auto pending_transactions = tx_state_manager_->GetTransactionsByStatus( + auto pending_transactions = tx_state_manager().GetTransactionsByStatus( chain_id, mojom::TransactionStatus::Submitted, std::nullopt); std::set pending_chain_ids; for (const auto& pending_transaction : pending_transactions) { @@ -387,8 +381,7 @@ void FilTxManager::OnGetFilStateSearchMsgLimited( if (error != mojom::FilecoinProviderError::kSuccess) { return; } - std::unique_ptr meta = - GetFilTxStateManager()->GetFilTx(tx_meta_id); + std::unique_ptr meta = GetFilTxStateManager().GetFilTx(tx_meta_id); if (!meta) { return; } @@ -399,7 +392,7 @@ void FilTxManager::OnGetFilStateSearchMsgLimited( if (status == mojom::TransactionStatus::Confirmed) { meta->set_confirmed_time(base::Time::Now()); } - tx_state_manager_->AddOrUpdateTx(*meta); + tx_state_manager().AddOrUpdateTx(*meta); } void FilTxManager::OnLatestHeightUpdated(const std::string& chain_id, @@ -407,16 +400,15 @@ void FilTxManager::OnLatestHeightUpdated(const std::string& chain_id, UpdatePendingTransactions(chain_id); } -FilBlockTracker* FilTxManager::GetFilBlockTracker() { - return static_cast(block_tracker_.get()); +FilBlockTracker& FilTxManager::GetFilBlockTracker() { + return static_cast(block_tracker()); } void FilTxManager::ProcessFilHardwareSignature( const std::string& tx_meta_id, const mojom::FilecoinSignaturePtr& hw_signature, ProcessFilHardwareSignatureCallback callback) { - std::unique_ptr meta = - GetFilTxStateManager()->GetFilTx(tx_meta_id); + std::unique_ptr meta = GetFilTxStateManager().GetFilTx(tx_meta_id); if (!meta) { std::move(callback).Run( false, @@ -427,7 +419,7 @@ void FilTxManager::ProcessFilHardwareSignature( } meta->set_status(mojom::TransactionStatus::Approved); - if (!tx_state_manager_->AddOrUpdateTx(*meta)) { + if (!tx_state_manager().AddOrUpdateTx(*meta)) { std::move(callback).Run( false, mojom::ProviderErrorUnion::NewFilecoinProviderError( diff --git a/components/brave_wallet/browser/fil_tx_manager.h b/components/brave_wallet/browser/fil_tx_manager.h index dac05743d0de..876a73e8f3d6 100644 --- a/components/brave_wallet/browser/fil_tx_manager.h +++ b/components/brave_wallet/browser/fil_tx_manager.h @@ -16,8 +16,6 @@ #include "brave/components/brave_wallet/common/brave_wallet.mojom.h" #include "brave/components/brave_wallet/common/brave_wallet_types.h" -class PrefService; - namespace brave_wallet { class TxService; @@ -29,12 +27,11 @@ class FilTransaction; class FilTxManager : public TxManager, public FilBlockTracker::Observer { public: - FilTxManager(TxService* tx_service, + FilTxManager(TxService& tx_service, JsonRpcService* json_rpc_service, - KeyringService* keyring_service, - PrefService* prefs, - TxStorageDelegate* delegate, - AccountResolverDelegate* account_resolver_delegate); + KeyringService& keyring_service, + TxStorageDelegate& delegate, + AccountResolverDelegate& account_resolver_delegate); ~FilTxManager() override; FilTxManager(const FilTxManager&) = delete; FilTxManager operator=(const FilTxManager&) = delete; @@ -111,8 +108,8 @@ class FilTxManager : public TxManager, public FilBlockTracker::Observer { int64_t exit_code, mojom::FilecoinProviderError error, const std::string& error_message); - FilTxStateManager* GetFilTxStateManager(); - FilBlockTracker* GetFilBlockTracker(); + FilTxStateManager& GetFilTxStateManager(); + FilBlockTracker& GetFilBlockTracker(); // FilBlockTracker::Observer void OnLatestHeightUpdated(const std::string& chain_id, @@ -124,7 +121,6 @@ class FilTxManager : public TxManager, public FilBlockTracker::Observer { std::unique_ptr nonce_tracker_; raw_ptr json_rpc_service_ = nullptr; - raw_ptr account_resolver_delegate_ = nullptr; base::WeakPtrFactory weak_factory_{this}; }; diff --git a/components/brave_wallet/browser/fil_tx_manager_unittest.cc b/components/brave_wallet/browser/fil_tx_manager_unittest.cc index 21a6c5eb8776..dd89a7742275 100644 --- a/components/brave_wallet/browser/fil_tx_manager_unittest.cc +++ b/components/brave_wallet/browser/fil_tx_manager_unittest.cc @@ -83,9 +83,8 @@ class FilTxManagerUnitTest : public testing::Test { &prefs_, &local_state_); ASSERT_TRUE(temp_dir_.CreateUniqueTempDir()); tx_service_ = std::make_unique( - json_rpc_service_.get(), nullptr, nullptr, keyring_service_.get(), - &prefs_, temp_dir_.GetPath(), - base::SequencedTaskRunner::GetCurrentDefault()); + json_rpc_service_.get(), nullptr, nullptr, *keyring_service_, &prefs_, + temp_dir_.GetPath(), base::SequencedTaskRunner::GetCurrentDefault()); WaitForTxStorageDelegateInitialized(tx_service_->GetDelegateForTesting()); GetAccountUtils().CreateWallet(kMnemonicDivideCruise, kTestWalletPassword); diff --git a/components/brave_wallet/browser/fil_tx_state_manager.cc b/components/brave_wallet/browser/fil_tx_state_manager.cc index c19a4aaa2371..6d1b8374dfe9 100644 --- a/components/brave_wallet/browser/fil_tx_state_manager.cc +++ b/components/brave_wallet/browser/fil_tx_state_manager.cc @@ -15,10 +15,9 @@ namespace brave_wallet { FilTxStateManager::FilTxStateManager( - PrefService* prefs, - TxStorageDelegate* delegate, - AccountResolverDelegate* account_resolver_delegate) - : TxStateManager(prefs, delegate, account_resolver_delegate) {} + TxStorageDelegate& delegate, + AccountResolverDelegate& account_resolver_delegate) + : TxStateManager(delegate, account_resolver_delegate) {} FilTxStateManager::~FilTxStateManager() = default; diff --git a/components/brave_wallet/browser/fil_tx_state_manager.h b/components/brave_wallet/browser/fil_tx_state_manager.h index ca8ee2d77a9b..28e85c9c972e 100644 --- a/components/brave_wallet/browser/fil_tx_state_manager.h +++ b/components/brave_wallet/browser/fil_tx_state_manager.h @@ -11,8 +11,6 @@ #include "brave/components/brave_wallet/browser/tx_state_manager.h" -class PrefService; - namespace base { class Value; } // namespace base @@ -25,9 +23,8 @@ class FilTxMeta; class FilTxStateManager : public TxStateManager { public: - FilTxStateManager(PrefService* prefs, - TxStorageDelegate* delegate, - AccountResolverDelegate* account_resolver_delegate); + FilTxStateManager(TxStorageDelegate& delegate, + AccountResolverDelegate& account_resolver_delegate); ~FilTxStateManager() override; FilTxStateManager(const FilTxStateManager&) = delete; FilTxStateManager operator=(const FilTxStateManager&) = delete; diff --git a/components/brave_wallet/browser/fil_tx_state_manager_unittest.cc b/components/brave_wallet/browser/fil_tx_state_manager_unittest.cc index 96d0f9f898ef..980d50ce44bd 100644 --- a/components/brave_wallet/browser/fil_tx_state_manager_unittest.cc +++ b/components/brave_wallet/browser/fil_tx_state_manager_unittest.cc @@ -40,7 +40,7 @@ class FilTxStateManagerUnitTest : public testing::Test { account_resolver_delegate_ = std::make_unique(); fil_tx_state_manager_ = std::make_unique( - GetPrefs(), delegate_.get(), account_resolver_delegate_.get()); + *delegate_, *account_resolver_delegate_); } PrefService* GetPrefs() { return &prefs_; } diff --git a/components/brave_wallet/browser/keyring_service_unittest.cc b/components/brave_wallet/browser/keyring_service_unittest.cc index abf02c663e00..e316182a223c 100644 --- a/components/brave_wallet/browser/keyring_service_unittest.cc +++ b/components/brave_wallet/browser/keyring_service_unittest.cc @@ -3105,8 +3105,9 @@ TEST_F(KeyringServiceAccountDiscoveryUnitTest, AccountDiscovery) { BraveWalletService brave_wallet_service( shared_url_loader_factory(), TestBraveWalletServiceDelegate::Create(), GetPrefs(), GetLocalState()); + ASSERT_TRUE(brave_wallet_service.GetBitcoinWalletService()); BitcoinTestRpcServer bitcoin_test_rpc_server( - brave_wallet_service.GetBitcoinWalletService()); + *brave_wallet_service.GetBitcoinWalletService()); KeyringService& service = *brave_wallet_service.keyring_service(); @@ -3147,8 +3148,9 @@ TEST_F(KeyringServiceAccountDiscoveryUnitTest, SolAccountDiscovery) { shared_url_loader_factory(), TestBraveWalletServiceDelegate::Create(), GetPrefs(), GetLocalState()); KeyringService& service = *brave_wallet_service.keyring_service(); + ASSERT_TRUE(brave_wallet_service.GetBitcoinWalletService()); BitcoinTestRpcServer bitcoin_test_rpc_server( - brave_wallet_service.GetBitcoinWalletService()); + *brave_wallet_service.GetBitcoinWalletService()); NiceMock observer(service, task_environment_); @@ -3189,8 +3191,9 @@ TEST_F(KeyringServiceAccountDiscoveryUnitTest, FilAccountDiscovery) { shared_url_loader_factory(), TestBraveWalletServiceDelegate::Create(), GetPrefs(), GetLocalState()); KeyringService& service = *brave_wallet_service.keyring_service(); + ASSERT_TRUE(brave_wallet_service.GetBitcoinWalletService()); BitcoinTestRpcServer bitcoin_test_rpc_server( - brave_wallet_service.GetBitcoinWalletService()); + *brave_wallet_service.GetBitcoinWalletService()); NiceMock observer(service, task_environment_); @@ -3233,8 +3236,9 @@ TEST_F(KeyringServiceUnitTest, BitcoinDiscovery) { shared_url_loader_factory(), TestBraveWalletServiceDelegate::Create(), GetPrefs(), GetLocalState()); KeyringService& service = *brave_wallet_service.keyring_service(); + ASSERT_TRUE(brave_wallet_service.GetBitcoinWalletService()); BitcoinTestRpcServer bitcoin_test_rpc_server( - brave_wallet_service.GetBitcoinWalletService()); + *brave_wallet_service.GetBitcoinWalletService()); bitcoin_test_rpc_server.SetUpBitcoinRpc(std::nullopt, std::nullopt); BitcoinHDKeyring keyring_84(*MnemonicToSeed(kMnemonicAbandonAbandon), false); @@ -3311,8 +3315,9 @@ TEST_F(KeyringServiceAccountDiscoveryUnitTest, StopsOnError) { shared_url_loader_factory(), TestBraveWalletServiceDelegate::Create(), GetPrefs(), GetLocalState()); KeyringService& service = *brave_wallet_service.keyring_service(); + ASSERT_TRUE(brave_wallet_service.GetBitcoinWalletService()); BitcoinTestRpcServer bitcoin_test_rpc_server( - brave_wallet_service.GetBitcoinWalletService()); + *brave_wallet_service.GetBitcoinWalletService()); NiceMock observer(service, task_environment_); @@ -3353,8 +3358,9 @@ TEST_F(KeyringServiceAccountDiscoveryUnitTest, ManuallyAddAccount) { shared_url_loader_factory(), TestBraveWalletServiceDelegate::Create(), GetPrefs(), GetLocalState()); KeyringService& service = *brave_wallet_service.keyring_service(); + ASSERT_TRUE(brave_wallet_service.GetBitcoinWalletService()); BitcoinTestRpcServer bitcoin_test_rpc_server( - brave_wallet_service.GetBitcoinWalletService()); + *brave_wallet_service.GetBitcoinWalletService()); NiceMock observer(service, task_environment_); @@ -3417,8 +3423,9 @@ TEST_F(KeyringServiceAccountDiscoveryUnitTest, RestoreWalletTwice) { shared_url_loader_factory(), TestBraveWalletServiceDelegate::Create(), GetPrefs(), GetLocalState()); KeyringService& service = *brave_wallet_service.keyring_service(); + ASSERT_TRUE(brave_wallet_service.GetBitcoinWalletService()); BitcoinTestRpcServer bitcoin_test_rpc_server( - brave_wallet_service.GetBitcoinWalletService()); + *brave_wallet_service.GetBitcoinWalletService()); std::vector requested_addresses; bool first_restore = true; diff --git a/components/brave_wallet/browser/scoped_txs_update.cc b/components/brave_wallet/browser/scoped_txs_update.cc index 9034d2a03c08..0a05b6f023e6 100644 --- a/components/brave_wallet/browser/scoped_txs_update.cc +++ b/components/brave_wallet/browser/scoped_txs_update.cc @@ -9,7 +9,7 @@ namespace brave_wallet { -ScopedTxsUpdate::ScopedTxsUpdate(TxStorageDelegate* delegate) +ScopedTxsUpdate::ScopedTxsUpdate(TxStorageDelegate& delegate) : delegate_(delegate) {} ScopedTxsUpdate::~ScopedTxsUpdate() { diff --git a/components/brave_wallet/browser/scoped_txs_update.h b/components/brave_wallet/browser/scoped_txs_update.h index bdb1486af651..b7f761a77dc9 100644 --- a/components/brave_wallet/browser/scoped_txs_update.h +++ b/components/brave_wallet/browser/scoped_txs_update.h @@ -6,7 +6,7 @@ #ifndef BRAVE_COMPONENTS_BRAVE_WALLET_BROWSER_SCOPED_TXS_UPDATE_H_ #define BRAVE_COMPONENTS_BRAVE_WALLET_BROWSER_SCOPED_TXS_UPDATE_H_ -#include "base/memory/raw_ptr.h" +#include "base/memory/raw_ref.h" #include "base/values.h" namespace brave_wallet { @@ -15,7 +15,7 @@ class TxStorageDelegate; class ScopedTxsUpdate { public: - explicit ScopedTxsUpdate(TxStorageDelegate* delegate); + explicit ScopedTxsUpdate(TxStorageDelegate& delegate); ScopedTxsUpdate(const ScopedTxsUpdate&) = delete; ScopedTxsUpdate& operator=(const ScopedTxsUpdate&) = delete; virtual ~ScopedTxsUpdate(); @@ -27,7 +27,7 @@ class ScopedTxsUpdate { base::Value::Dict* operator->() { return &Get(); } private: - raw_ptr delegate_; + const raw_ref delegate_; }; } // namespace brave_wallet diff --git a/components/brave_wallet/browser/solana_provider_impl.h b/components/brave_wallet/browser/solana_provider_impl.h index f890a19325b0..5f0d41eb150b 100644 --- a/components/brave_wallet/browser/solana_provider_impl.h +++ b/components/brave_wallet/browser/solana_provider_impl.h @@ -14,6 +14,7 @@ #include "base/gtest_prod_util.h" #include "base/memory/raw_ptr.h" +#include "base/memory/raw_ref.h" #include "base/memory/weak_ptr.h" #include "base/scoped_observation.h" #include "brave/components/brave_wallet/browser/keyring_service_observer_base.h" diff --git a/components/brave_wallet/browser/solana_tx_manager.cc b/components/brave_wallet/browser/solana_tx_manager.cc index 24d5244de150..2ffdc2bd2434 100644 --- a/components/brave_wallet/browser/solana_tx_manager.cc +++ b/components/brave_wallet/browser/solana_tx_manager.cc @@ -183,27 +183,24 @@ void MergeGetTxFeeEstimationResponses( } // namespace SolanaTxManager::SolanaTxManager( - TxService* tx_service, + TxService& tx_service, JsonRpcService* json_rpc_service, - KeyringService* keyring_service, - PrefService* prefs, - TxStorageDelegate* delegate, - AccountResolverDelegate* account_resolver_delegate) + KeyringService& keyring_service, + TxStorageDelegate& delegate, + AccountResolverDelegate& account_resolver_delegate) : TxManager( - std::make_unique(prefs, - delegate, + std::make_unique(delegate, account_resolver_delegate), std::make_unique(json_rpc_service), tx_service, - keyring_service, - prefs), + keyring_service), json_rpc_service_(json_rpc_service), weak_ptr_factory_(this) { - GetSolanaBlockTracker()->AddObserver(this); + GetSolanaBlockTracker().AddObserver(this); } SolanaTxManager::~SolanaTxManager() { - GetSolanaBlockTracker()->RemoveObserver(this); + GetSolanaBlockTracker().RemoveObserver(this); } void SolanaTxManager::AddUnapprovedTransaction( @@ -265,7 +262,7 @@ void SolanaTxManager::ContinueAddUnapprovedTransaction( const std::string& error_message) { // Failed fetching base fee, add the transaction without fee estimation. if (!estimation || error != mojom::SolanaProviderError::kSuccess) { - if (!tx_state_manager_->AddOrUpdateTx(*meta)) { + if (!tx_state_manager().AddOrUpdateTx(*meta)) { std::move(callback).Run( false, "", l10n_util::GetStringUTF8(IDS_WALLET_INTERNAL_ERROR)); return; @@ -287,7 +284,7 @@ void SolanaTxManager::ContinueAddUnapprovedTransaction( meta->tx()->message()->AddPriorityFee(compute_units, fee_per_compute_unit); } - if (!tx_state_manager_->AddOrUpdateTx(*meta)) { + if (!tx_state_manager().AddOrUpdateTx(*meta)) { std::move(callback).Run( false, "", l10n_util::GetStringUTF8(IDS_WALLET_INTERNAL_ERROR)); return; @@ -299,7 +296,7 @@ void SolanaTxManager::ContinueAddUnapprovedTransaction( void SolanaTxManager::ApproveTransaction(const std::string& tx_meta_id, ApproveTransactionCallback callback) { std::unique_ptr meta = - GetSolanaTxStateManager()->GetSolanaTx(tx_meta_id); + GetSolanaTxStateManager().GetSolanaTx(tx_meta_id); if (!meta) { DCHECK(false) << "Transaction should be found"; std::move(callback).Run( @@ -313,7 +310,7 @@ void SolanaTxManager::ApproveTransaction(const std::string& tx_meta_id, const std::string blockhash = meta->tx()->message()->recent_blockhash(); auto chain_id = meta->chain_id(); if (blockhash.empty()) { - GetSolanaBlockTracker()->GetLatestBlockhash( + GetSolanaBlockTracker().GetLatestBlockhash( chain_id, base::BindOnce(&SolanaTxManager::OnGetLatestBlockhash, weak_ptr_factory_.GetWeakPtr(), std::move(meta), @@ -366,7 +363,7 @@ void SolanaTxManager::OnGetLatestBlockhash(std::unique_ptr meta, meta->tx()->message()->set_recent_blockhash(latest_blockhash); meta->tx()->message()->set_last_valid_block_height(last_valid_block_height); auto signed_transaction = meta->tx()->GetSignedTransactionBytes( - keyring_service_, meta->from(), nullptr); + &keyring_service(), meta->from(), nullptr); if (!signed_transaction) { std::move(callback).Run( false, @@ -377,7 +374,7 @@ void SolanaTxManager::OnGetLatestBlockhash(std::unique_ptr meta, } meta->tx()->set_wired_tx(base::Base64Encode(*signed_transaction)); - if (!tx_state_manager_->AddOrUpdateTx(*meta)) { + if (!tx_state_manager().AddOrUpdateTx(*meta)) { std::move(callback).Run( false, mojom::ProviderErrorUnion::NewSolanaProviderError( @@ -407,7 +404,7 @@ void SolanaTxManager::OnGetLatestBlockhashHardware( meta->tx()->message()->set_recent_blockhash(latest_blockhash); meta->tx()->message()->set_last_valid_block_height(last_valid_block_height); - if (!tx_state_manager_->AddOrUpdateTx(*meta)) { + if (!tx_state_manager().AddOrUpdateTx(*meta)) { std::move(callback).Run(std::nullopt); return; } @@ -427,7 +424,7 @@ void SolanaTxManager::OnSendSolanaTransaction( const std::string& tx_hash, mojom::SolanaProviderError error, const std::string& error_message) { - std::unique_ptr meta = tx_state_manager_->GetTx(tx_meta_id); + std::unique_ptr meta = tx_state_manager().GetTx(tx_meta_id); if (!meta) { DCHECK(false) << "Transaction should be found"; std::move(callback).Run( @@ -448,7 +445,7 @@ void SolanaTxManager::OnSendSolanaTransaction( meta->set_status(mojom::TransactionStatus::Error); } - if (!tx_state_manager_->AddOrUpdateTx(*meta)) { + if (!tx_state_manager().AddOrUpdateTx(*meta)) { std::move(callback).Run( false, mojom::ProviderErrorUnion::NewSolanaProviderError( @@ -468,20 +465,20 @@ void SolanaTxManager::OnSendSolanaTransaction( void SolanaTxManager::UpdatePendingTransactions( const std::optional& chain_id) { - std::set pending_chain_ids; + std::set ids; if (chain_id.has_value()) { - pending_chain_ids = pending_chain_ids_; - pending_chain_ids.emplace(*chain_id); + ids = pending_chain_ids(); + ids.emplace(*chain_id); json_rpc_service_->GetSolanaBlockHeight( *chain_id, base::BindOnce(&SolanaTxManager::OnGetBlockHeight, weak_ptr_factory_.GetWeakPtr(), *chain_id)); } else { - auto pending_transactions = tx_state_manager_->GetTransactionsByStatus( + auto pending_transactions = tx_state_manager().GetTransactionsByStatus( std::nullopt, mojom::TransactionStatus::Submitted, std::nullopt); for (const auto& pending_transaction : pending_transactions) { const auto& pending_chain_id = pending_transaction->chain_id(); // Skip already queried chain ids. - if (pending_chain_ids.contains(pending_chain_id)) { + if (ids.contains(pending_chain_id)) { continue; } @@ -489,10 +486,10 @@ void SolanaTxManager::UpdatePendingTransactions( pending_chain_id, base::BindOnce(&SolanaTxManager::OnGetBlockHeight, weak_ptr_factory_.GetWeakPtr(), pending_chain_id)); - pending_chain_ids.emplace(pending_chain_id); + ids.emplace(pending_chain_id); } } - CheckIfBlockTrackerShouldRun(pending_chain_ids); + CheckIfBlockTrackerShouldRun(ids); } void SolanaTxManager::OnGetBlockHeight(const std::string& chain_id, @@ -503,7 +500,7 @@ void SolanaTxManager::OnGetBlockHeight(const std::string& chain_id, return; } - auto pending_transactions = tx_state_manager_->GetTransactionsByStatus( + auto pending_transactions = tx_state_manager().GetTransactionsByStatus( chain_id, mojom::TransactionStatus::Submitted, std::nullopt); std::vector tx_meta_ids; std::vector tx_signatures; @@ -535,7 +532,7 @@ void SolanaTxManager::OnGetSignatureStatuses( for (size_t i = 0; i < tx_meta_ids.size(); i++) { std::unique_ptr meta = - GetSolanaTxStateManager()->GetSolanaTx(tx_meta_ids[i]); + GetSolanaTxStateManager().GetSolanaTx(tx_meta_ids[i]); if (!meta) { continue; } @@ -545,7 +542,7 @@ void SolanaTxManager::OnGetSignatureStatuses( if (base::Time::Now() >= meta->submitted_time() + base::Minutes(kSafeDropThresholdInMinutes)) { meta->set_status(mojom::TransactionStatus::Dropped); - tx_state_manager_->AddOrUpdateTx(*meta); + tx_state_manager().AddOrUpdateTx(*meta); continue; } @@ -562,7 +559,7 @@ void SolanaTxManager::OnGetSignatureStatuses( if (!signature_statuses[i]) { if (is_blockhash_expired) { meta->set_status(mojom::TransactionStatus::Dropped); - tx_state_manager_->AddOrUpdateTx(*meta); + tx_state_manager().AddOrUpdateTx(*meta); } else if (do_rebroadcast) { json_rpc_service_->SendSolanaTransaction( meta->chain_id(), meta->tx()->wired_tx(), @@ -574,7 +571,7 @@ void SolanaTxManager::OnGetSignatureStatuses( if (!signature_statuses[i]->err.empty()) { meta->set_signature_status(*signature_statuses[i]); meta->set_status(mojom::TransactionStatus::Error); - tx_state_manager_->AddOrUpdateTx(*meta); + tx_state_manager().AddOrUpdateTx(*meta); continue; } @@ -594,7 +591,7 @@ void SolanaTxManager::OnGetSignatureStatuses( meta->set_confirmed_time(base::Time::Now()); } - tx_state_manager_->AddOrUpdateTx(*meta); + tx_state_manager().AddOrUpdateTx(*meta); } } } @@ -609,7 +606,7 @@ void SolanaTxManager::SpeedupOrCancelTransaction( void SolanaTxManager::RetryTransaction(const std::string& tx_meta_id, RetryTransactionCallback callback) { std::unique_ptr meta = - GetSolanaTxStateManager()->GetSolanaTx(tx_meta_id); + GetSolanaTxStateManager().GetSolanaTx(tx_meta_id); if (!meta || !meta->tx()) { std::move(callback).Run( false, "", @@ -654,7 +651,7 @@ void SolanaTxManager::RetryTransaction(const std::string& tx_meta_id, meta->tx()->ClearRawSignatures(); - if (!tx_state_manager_->AddOrUpdateTx(*meta)) { + if (!tx_state_manager().AddOrUpdateTx(*meta)) { std::move(callback).Run( false, "", l10n_util::GetStringUTF8(IDS_WALLET_INTERNAL_ERROR)); return; @@ -667,7 +664,7 @@ void SolanaTxManager::GetSolTransactionMessageToSign( const std::string& tx_meta_id, GetSolTransactionMessageToSignCallback callback) { std::unique_ptr meta = - GetSolanaTxStateManager()->GetSolanaTx(tx_meta_id); + GetSolanaTxStateManager().GetSolanaTx(tx_meta_id); if (!meta || !meta->tx()) { VLOG(1) << __FUNCTION__ << "No transaction found with id:" << tx_meta_id; std::move(callback).Run(std::nullopt); @@ -677,7 +674,7 @@ void SolanaTxManager::GetSolTransactionMessageToSign( const std::string blockhash = meta->tx()->message()->recent_blockhash(); auto chain_id = meta->chain_id(); if (blockhash.empty()) { - GetSolanaBlockTracker()->GetLatestBlockhash( + GetSolanaBlockTracker().GetLatestBlockhash( chain_id, base::BindOnce(&SolanaTxManager::OnGetLatestBlockhashHardware, weak_ptr_factory_.GetWeakPtr(), std::move(meta), @@ -1151,7 +1148,7 @@ void SolanaTxManager::GetSolanaTxFeeEstimation( GetSolanaTxFeeEstimationCallback callback) { // Get the TxMeta. std::unique_ptr meta = - GetSolanaTxStateManager()->GetSolanaTx(tx_meta_id); + GetSolanaTxStateManager().GetSolanaTx(tx_meta_id); if (!meta) { std::move(callback).Run( {}, mojom::SolanaProviderError::kInternalError, @@ -1177,7 +1174,7 @@ void SolanaTxManager::GetSolanaTxFeeEstimationForMeta( GetSolanaTxFeeEstimationForMetaCallback callback) { if (meta->tx()->message()->recent_blockhash().empty()) { const std::string chain_id = meta->chain_id(); - GetSolanaBlockTracker()->GetLatestBlockhash( + GetSolanaBlockTracker().GetLatestBlockhash( chain_id, base::BindOnce(&SolanaTxManager::GetSolanaTxFeeEstimationWithBlockhash, weak_ptr_factory_.GetWeakPtr(), std::move(meta), @@ -1277,17 +1274,17 @@ void SolanaTxManager::OnLatestBlockhashUpdated( UpdatePendingTransactions(chain_id); } -SolanaTxStateManager* SolanaTxManager::GetSolanaTxStateManager() { - return static_cast(tx_state_manager_.get()); +SolanaTxStateManager& SolanaTxManager::GetSolanaTxStateManager() { + return static_cast(tx_state_manager()); } -SolanaBlockTracker* SolanaTxManager::GetSolanaBlockTracker() { - return static_cast(block_tracker_.get()); +SolanaBlockTracker& SolanaTxManager::GetSolanaBlockTracker() { + return static_cast(block_tracker()); } std::unique_ptr SolanaTxManager::GetTxForTesting( const std::string& tx_meta_id) { - return GetSolanaTxStateManager()->GetSolanaTx(tx_meta_id); + return GetSolanaTxStateManager().GetSolanaTx(tx_meta_id); } void SolanaTxManager::ProcessSolanaHardwareSignature( @@ -1295,7 +1292,7 @@ void SolanaTxManager::ProcessSolanaHardwareSignature( mojom::SolanaSignaturePtr hw_signature, ProcessSolanaHardwareSignatureCallback callback) { std::unique_ptr meta = - GetSolanaTxStateManager()->GetSolanaTx(tx_meta_id); + GetSolanaTxStateManager().GetSolanaTx(tx_meta_id); if (!meta) { std::move(callback).Run( false, @@ -1305,7 +1302,7 @@ void SolanaTxManager::ProcessSolanaHardwareSignature( return; } std::optional> transaction_bytes = - meta->tx()->GetSignedTransactionBytes(keyring_service_, meta->from(), + meta->tx()->GetSignedTransactionBytes(&keyring_service(), meta->from(), hw_signature); if (!transaction_bytes) { std::move(callback).Run( @@ -1319,7 +1316,7 @@ void SolanaTxManager::ProcessSolanaHardwareSignature( meta->set_status(mojom::TransactionStatus::Approved); meta->tx()->set_wired_tx(base::Base64Encode(*transaction_bytes)); - if (!tx_state_manager_->AddOrUpdateTx(*meta)) { + if (!tx_state_manager().AddOrUpdateTx(*meta)) { std::move(callback).Run( false, mojom::ProviderErrorUnion::NewSolanaProviderError( diff --git a/components/brave_wallet/browser/solana_tx_manager.h b/components/brave_wallet/browser/solana_tx_manager.h index d97b558f409b..4fe5c0430ade 100644 --- a/components/brave_wallet/browser/solana_tx_manager.h +++ b/components/brave_wallet/browser/solana_tx_manager.h @@ -19,8 +19,6 @@ #include "brave/components/brave_wallet/browser/tx_manager.h" #include "brave/components/brave_wallet/common/solana_address.h" -class PrefService; - namespace brave_wallet { class TxService; @@ -33,12 +31,11 @@ struct SolanaAccountInfo; class SolanaTxManager : public TxManager, public SolanaBlockTracker::Observer { public: - SolanaTxManager(TxService* tx_service, + SolanaTxManager(TxService& tx_service, JsonRpcService* json_rpc_service, - KeyringService* keyring_service, - PrefService* prefs, - TxStorageDelegate* delegate, - AccountResolverDelegate* account_resolver_delegate); + KeyringService& keyring_service, + TxStorageDelegate& delegate, + AccountResolverDelegate& account_resolver_delegate); ~SolanaTxManager() override; // TxManager @@ -250,8 +247,8 @@ class SolanaTxManager : public TxManager, public SolanaBlockTracker::Observer { const std::string& blockhash, uint64_t last_valid_block_height) override; - SolanaTxStateManager* GetSolanaTxStateManager(); - SolanaBlockTracker* GetSolanaBlockTracker(); + SolanaTxStateManager& GetSolanaTxStateManager(); + SolanaBlockTracker& GetSolanaBlockTracker(); raw_ptr json_rpc_service_ = nullptr; base::WeakPtrFactory weak_ptr_factory_; }; diff --git a/components/brave_wallet/browser/solana_tx_manager_unittest.cc b/components/brave_wallet/browser/solana_tx_manager_unittest.cc index d84bee5a770c..9d5a6b1959cf 100644 --- a/components/brave_wallet/browser/solana_tx_manager_unittest.cc +++ b/components/brave_wallet/browser/solana_tx_manager_unittest.cc @@ -125,9 +125,8 @@ class SolanaTxManagerUnitTest : public testing::Test { &prefs_, &local_state_); ASSERT_TRUE(temp_dir_.CreateUniqueTempDir()); tx_service_ = std::make_unique( - json_rpc_service_.get(), nullptr, nullptr, keyring_service_.get(), - &prefs_, temp_dir_.GetPath(), - base::SequencedTaskRunner::GetCurrentDefault()); + json_rpc_service_.get(), nullptr, nullptr, *keyring_service_, &prefs_, + temp_dir_.GetPath(), base::SequencedTaskRunner::GetCurrentDefault()); WaitForTxStorageDelegateInitialized(tx_service_->GetDelegateForTesting()); CreateWallet(); @@ -614,7 +613,7 @@ class SolanaTxManagerUnitTest : public testing::Test { const std::string& expected_blockhash = "") { SCOPED_TRACE(testing::Message() << location.ToString()); MockTxStateManagerObserver observer( - solana_tx_manager()->GetSolanaTxStateManager()); + &solana_tx_manager()->GetSolanaTxStateManager()); if (expected_result) { EXPECT_CALL(observer, OnNewUnapprovedTx(testing::_)).Times(1); } @@ -696,7 +695,7 @@ class SolanaTxManagerUnitTest : public testing::Test { TEST_F(SolanaTxManagerUnitTest, AddAndApproveTransaction) { // Stop the block tracker explicitly to make sure it will be started when we // submit our pending transactions. - solana_tx_manager()->GetSolanaBlockTracker()->Stop(); + solana_tx_manager()->GetSolanaBlockTracker().Stop(); const auto& from_account = sol_account(); std::string from_account_address = from_account->address; @@ -1411,7 +1410,7 @@ TEST_F(SolanaTxManagerUnitTest, DropTxWithInvalidBlockhash) { // Check two submitted tx. auto pending_transactions = - solana_tx_manager()->GetSolanaTxStateManager()->GetTransactionsByStatus( + solana_tx_manager()->GetSolanaTxStateManager().GetTransactionsByStatus( mojom::kSolanaMainnet, mojom::TransactionStatus::Submitted, std::nullopt); EXPECT_EQ(pending_transactions.size(), 2u); @@ -1440,14 +1439,14 @@ TEST_F(SolanaTxManagerUnitTest, DropTxWithInvalidBlockhash) { // Check blockhash1 should be dropped, blockhash2 stay as submitted. auto dropped_transactions = - solana_tx_manager()->GetSolanaTxStateManager()->GetTransactionsByStatus( + solana_tx_manager()->GetSolanaTxStateManager().GetTransactionsByStatus( mojom::kSolanaMainnet, mojom::TransactionStatus::Dropped, std::nullopt); ASSERT_EQ(dropped_transactions.size(), 1u); EXPECT_EQ(dropped_transactions[0]->id(), meta_id1); pending_transactions = - solana_tx_manager()->GetSolanaTxStateManager()->GetTransactionsByStatus( + solana_tx_manager()->GetSolanaTxStateManager().GetTransactionsByStatus( mojom::kSolanaMainnet, mojom::TransactionStatus::Submitted, std::nullopt); ASSERT_EQ(pending_transactions.size(), 1u); @@ -1487,7 +1486,7 @@ TEST_F(SolanaTxManagerUnitTest, DropTxWithInvalidBlockhash_DappBlockhash) { tx_meta2->tx()->message()->set_last_valid_block_height( last_valid_block_height2_ + kSolanaValidBlockHeightThreshold); ASSERT_TRUE( - solana_tx_manager()->GetSolanaTxStateManager()->AddOrUpdateTx(*tx_meta2)); + solana_tx_manager()->GetSolanaTxStateManager().AddOrUpdateTx(*tx_meta2)); WaitForUpdatePendingTransactions(); @@ -1506,7 +1505,7 @@ TEST_F(SolanaTxManagerUnitTest, DropTxWithInvalidBlockhash_DappBlockhash) { last_valid_block_height2_ + 150); EXPECT_EQ(tx_meta2->status(), mojom::TransactionStatus::Submitted); - ASSERT_TRUE(solana_tx_manager()->GetSolanaBlockTracker()->IsRunning( + ASSERT_TRUE(solana_tx_manager()->GetSolanaBlockTracker().IsRunning( mojom::kSolanaMainnet)); // Trigger block tracker to run, with block_height < last_valid_block_height. @@ -1523,7 +1522,7 @@ TEST_F(SolanaTxManagerUnitTest, DropTxWithInvalidBlockhash_DappBlockhash) { ASSERT_TRUE(tx_meta2); EXPECT_EQ(tx_meta2->status(), mojom::TransactionStatus::Submitted); - ASSERT_TRUE(solana_tx_manager()->GetSolanaBlockTracker()->IsRunning( + ASSERT_TRUE(solana_tx_manager()->GetSolanaBlockTracker().IsRunning( mojom::kSolanaMainnet)); // Fast forward to have block tracker run with the new interceptor, where @@ -1570,7 +1569,7 @@ TEST_F(SolanaTxManagerUnitTest, DropTxAfterSafeDropThreshold) { ASSERT_TRUE(tx); tx->set_submitted_time(base::Time::Now() - base::Minutes(30)); ASSERT_TRUE( - solana_tx_manager()->GetSolanaTxStateManager()->AddOrUpdateTx(*tx)); + solana_tx_manager()->GetSolanaTxStateManager().AddOrUpdateTx(*tx)); send_transaction_calls_ = 0; SetInterceptor(latest_blockhash2_, last_valid_block_height2_, tx_hash1_, "", @@ -1621,7 +1620,7 @@ TEST_F(SolanaTxManagerUnitTest, RetryTransaction) { vec.emplace_back(durable_nonce_meta->tx()->message()->instructions()[0]); durable_nonce_meta->tx()->message()->SetInstructionsForTesting(vec); - ASSERT_TRUE(solana_tx_manager()->GetSolanaTxStateManager()->AddOrUpdateTx( + ASSERT_TRUE(solana_tx_manager()->GetSolanaTxStateManager().AddOrUpdateTx( *durable_nonce_meta)); // Test retry transaction with invalid state. @@ -1637,9 +1636,9 @@ TEST_F(SolanaTxManagerUnitTest, RetryTransaction) { tx_meta1->set_status(mojom::TransactionStatus::Dropped); tx_meta2->set_status(mojom::TransactionStatus::Dropped); ASSERT_TRUE( - solana_tx_manager()->GetSolanaTxStateManager()->AddOrUpdateTx(*tx_meta1)); + solana_tx_manager()->GetSolanaTxStateManager().AddOrUpdateTx(*tx_meta1)); ASSERT_TRUE( - solana_tx_manager()->GetSolanaTxStateManager()->AddOrUpdateTx(*tx_meta2)); + solana_tx_manager()->GetSolanaTxStateManager().AddOrUpdateTx(*tx_meta2)); TestRetryTransaction(FROM_HERE, meta_id1, true, "", ""); TestRetryTransaction(FROM_HERE, meta_id2, true, "", nonce_account->address); @@ -1661,9 +1660,9 @@ TEST_F(SolanaTxManagerUnitTest, RetryTransaction) { tx_meta2->tx()->set_sign_tx_param(param.Clone()); ASSERT_TRUE( - solana_tx_manager()->GetSolanaTxStateManager()->AddOrUpdateTx(*tx_meta1)); + solana_tx_manager()->GetSolanaTxStateManager().AddOrUpdateTx(*tx_meta1)); ASSERT_TRUE( - solana_tx_manager()->GetSolanaTxStateManager()->AddOrUpdateTx(*tx_meta2)); + solana_tx_manager()->GetSolanaTxStateManager().AddOrUpdateTx(*tx_meta2)); TestRetryTransaction( FROM_HERE, meta_id1, false, l10n_util::GetStringUTF8(IDS_BRAVE_WALLET_TRANSACTION_NOT_RETRIABLE)); @@ -1677,7 +1676,7 @@ TEST_F(SolanaTxManagerUnitTest, RetryTransaction) { // Test retry transaction with invalid tx type. tx_meta1->tx()->set_tx_type(mojom::TransactionType::SolanaSwap); ASSERT_TRUE( - solana_tx_manager()->GetSolanaTxStateManager()->AddOrUpdateTx(*tx_meta1)); + solana_tx_manager()->GetSolanaTxStateManager().AddOrUpdateTx(*tx_meta1)); TestRetryTransaction( FROM_HERE, meta_id1, false, l10n_util::GetStringUTF8(IDS_BRAVE_WALLET_TRANSACTION_NOT_RETRIABLE)); @@ -1762,7 +1761,7 @@ TEST_F(SolanaTxManagerUnitTest, ProcessSolanaHardwareSignature) { meta->tx()->message()->set_recent_blockhash(latest_blockhash1_); meta->tx()->message()->set_last_valid_block_height(last_valid_block_height1_); ASSERT_TRUE( - solana_tx_manager()->GetSolanaTxStateManager()->AddOrUpdateTx(*meta)); + solana_tx_manager()->GetSolanaTxStateManager().AddOrUpdateTx(*meta)); // Valid blockhash and valid number of signers is valid TestProcessSolanaHardwareSignature(system_transfer_meta_id, signature_bytes, @@ -1900,7 +1899,7 @@ TEST_F(SolanaTxManagerUnitTest, GetSolanaTxFeeEstimation) { meta.set_chain_id(mojom::kSolanaMainnet); ASSERT_TRUE( - solana_tx_manager()->GetSolanaTxStateManager()->AddOrUpdateTx(meta)); + solana_tx_manager()->GetSolanaTxStateManager().AddOrUpdateTx(meta)); task_environment_.RunUntilIdle(); // Call fetch fee estimation with appropriate interceptors. Verify median and diff --git a/components/brave_wallet/browser/solana_tx_state_manager.cc b/components/brave_wallet/browser/solana_tx_state_manager.cc index 03b5e6318955..39e6ded625e5 100644 --- a/components/brave_wallet/browser/solana_tx_state_manager.cc +++ b/components/brave_wallet/browser/solana_tx_state_manager.cc @@ -15,10 +15,9 @@ namespace brave_wallet { SolanaTxStateManager::SolanaTxStateManager( - PrefService* prefs, - TxStorageDelegate* delegate, - AccountResolverDelegate* account_resolver_delegate) - : TxStateManager(prefs, delegate, account_resolver_delegate) {} + TxStorageDelegate& delegate, + AccountResolverDelegate& account_resolver_delegate) + : TxStateManager(delegate, account_resolver_delegate) {} SolanaTxStateManager::~SolanaTxStateManager() = default; diff --git a/components/brave_wallet/browser/solana_tx_state_manager.h b/components/brave_wallet/browser/solana_tx_state_manager.h index 3a636adee5d3..835f05145f50 100644 --- a/components/brave_wallet/browser/solana_tx_state_manager.h +++ b/components/brave_wallet/browser/solana_tx_state_manager.h @@ -11,8 +11,6 @@ #include "brave/components/brave_wallet/browser/tx_state_manager.h" -class PrefService; - namespace base { class Value; } // namespace base @@ -25,9 +23,8 @@ class SolanaTxMeta; class SolanaTxStateManager : public TxStateManager { public: - SolanaTxStateManager(PrefService* prefs, - TxStorageDelegate* delegate, - AccountResolverDelegate* account_resolver_delegate); + SolanaTxStateManager(TxStorageDelegate& delegate, + AccountResolverDelegate& account_resolver_delegate); ~SolanaTxStateManager() override; SolanaTxStateManager(const SolanaTxStateManager&) = delete; SolanaTxStateManager operator=(const SolanaTxStateManager&) = delete; diff --git a/components/brave_wallet/browser/solana_tx_state_manager_unittest.cc b/components/brave_wallet/browser/solana_tx_state_manager_unittest.cc index e5f1e01066e8..8748bc47f240 100644 --- a/components/brave_wallet/browser/solana_tx_state_manager_unittest.cc +++ b/components/brave_wallet/browser/solana_tx_state_manager_unittest.cc @@ -42,7 +42,7 @@ class SolanaTxStateManagerUnitTest : public testing::Test { account_resolver_delegate_ = std::make_unique(); solana_tx_state_manager_ = std::make_unique( - GetPrefs(), delegate_.get(), account_resolver_delegate_.get()); + *delegate_, *account_resolver_delegate_); } PrefService* GetPrefs() { return &prefs_; } diff --git a/components/brave_wallet/browser/test_utils.cc b/components/brave_wallet/browser/test_utils.cc index 4cd9efd69d1c..4403ab81e937 100644 --- a/components/brave_wallet/browser/test_utils.cc +++ b/components/brave_wallet/browser/test_utils.cc @@ -427,7 +427,7 @@ void WaitForTxStorageDelegateInitialized(TxStorageDelegate* delegate) { private: base::ScopedObservation observation_{this}; - raw_ref run_loop_; + const raw_ref run_loop_; } observer(delegate, run_loop); run_loop.Run(); } diff --git a/components/brave_wallet/browser/tx_manager.cc b/components/brave_wallet/browser/tx_manager.cc index 152ccf048d79..44f2a635d68d 100644 --- a/components/brave_wallet/browser/tx_manager.cc +++ b/components/brave_wallet/browser/tx_manager.cc @@ -23,17 +23,12 @@ namespace brave_wallet { TxManager::TxManager(std::unique_ptr tx_state_manager, std::unique_ptr block_tracker, - TxService* tx_service, - KeyringService* keyring_service, - PrefService* prefs) + TxService& tx_service, + KeyringService& keyring_service) : tx_state_manager_(std::move(tx_state_manager)), block_tracker_(std::move(block_tracker)), tx_service_(tx_service), - keyring_service_(keyring_service), - prefs_(prefs) { - DCHECK(tx_service_); - DCHECK(keyring_service_); - + keyring_service_(keyring_service) { tx_state_manager_->AddObserver(this); keyring_service_->AddObserver( keyring_observer_receiver_.BindNewPipeAndPassRemote()); diff --git a/components/brave_wallet/browser/tx_manager.h b/components/brave_wallet/browser/tx_manager.h index 7b5cd447da67..0ae58b736073 100644 --- a/components/brave_wallet/browser/tx_manager.h +++ b/components/brave_wallet/browser/tx_manager.h @@ -12,13 +12,13 @@ #include #include +#include "base/gtest_prod_util.h" +#include "base/memory/raw_ref.h" #include "brave/components/brave_wallet/browser/keyring_service_observer_base.h" #include "brave/components/brave_wallet/browser/tx_state_manager.h" #include "brave/components/brave_wallet/common/brave_wallet.mojom.h" #include "mojo/public/cpp/bindings/receiver.h" -class PrefService; - namespace brave_wallet { class BlockTracker; @@ -30,9 +30,8 @@ class TxManager : public TxStateManager::Observer, public: TxManager(std::unique_ptr tx_state_manager, std::unique_ptr block_tracker, - TxService* tx_service, - KeyringService* keyring_service, - PrefService* prefs); + TxService& tx_service, + KeyringService& keyring_service); ~TxManager() override; using AddUnapprovedTransactionCallback = @@ -74,14 +73,18 @@ class TxManager : public TxStateManager::Observer, virtual void UpdatePendingTransactions( const std::optional& chain_id) = 0; - std::unique_ptr tx_state_manager_; - std::unique_ptr block_tracker_; - raw_ptr tx_service_ = nullptr; - raw_ptr keyring_service_ = nullptr; - raw_ptr prefs_ = nullptr; - std::set pending_chain_ids_; + TxStateManager& tx_state_manager() { return *tx_state_manager_; } + const std::set& pending_chain_ids() const { + return pending_chain_ids_; + } + BlockTracker& block_tracker() { return *block_tracker_; } + KeyringService& keyring_service() { return *keyring_service_; } + TxService& tx_service() { return *tx_service_; } private: + friend class AssetDiscoveryManagerUnitTest; + FRIEND_TEST_ALL_PREFIXES(EthTxManagerUnitTest, Reset); + virtual mojom::CoinType GetCoinType() const = 0; // TxStateManager::Observer @@ -95,6 +98,12 @@ class TxManager : public TxStateManager::Observer, mojo::Receiver keyring_observer_receiver_{this}; + + std::unique_ptr tx_state_manager_; + std::unique_ptr block_tracker_; + const raw_ref tx_service_; + const raw_ref keyring_service_; + std::set pending_chain_ids_; }; } // namespace brave_wallet diff --git a/components/brave_wallet/browser/tx_service.cc b/components/brave_wallet/browser/tx_service.cc index 2fbd865d2ab1..59b02dbd7551 100644 --- a/components/brave_wallet/browser/tx_service.cc +++ b/components/brave_wallet/browser/tx_service.cc @@ -78,7 +78,7 @@ size_t CalculatePendingTxCount( TxService::TxService(JsonRpcService* json_rpc_service, BitcoinWalletService* bitcoin_wallet_service, ZCashWalletService* zcash_wallet_service, - KeyringService* keyring_service, + KeyringService& keyring_service, PrefService* prefs, const base::FilePath& wallet_base_directory, scoped_refptr ui_task_runner) @@ -88,25 +88,25 @@ TxService::TxService(JsonRpcService* json_rpc_service, delegate_ = std::make_unique(prefs, store_factory_, ui_task_runner); account_resolver_delegate_ = - std::make_unique(keyring_service); + std::make_unique(&keyring_service); tx_manager_map_[mojom::CoinType::ETH] = std::unique_ptr( - new EthTxManager(this, json_rpc_service, keyring_service, prefs, - delegate_.get(), account_resolver_delegate_.get())); + new EthTxManager(*this, json_rpc_service, keyring_service, *delegate_, + *account_resolver_delegate_)); tx_manager_map_[mojom::CoinType::SOL] = std::unique_ptr( - new SolanaTxManager(this, json_rpc_service, keyring_service, prefs, - delegate_.get(), account_resolver_delegate_.get())); + new SolanaTxManager(*this, json_rpc_service, keyring_service, *delegate_, + *account_resolver_delegate_)); tx_manager_map_[mojom::CoinType::FIL] = std::unique_ptr( - new FilTxManager(this, json_rpc_service, keyring_service, prefs, - delegate_.get(), account_resolver_delegate_.get())); + new FilTxManager(*this, json_rpc_service, keyring_service, *delegate_, + *account_resolver_delegate_)); if (IsBitcoinEnabled()) { if (!bitcoin_wallet_service) { CHECK_IS_TEST(); } else { tx_manager_map_[mojom::CoinType::BTC] = - std::make_unique( - this, bitcoin_wallet_service, keyring_service, prefs, - delegate_.get(), account_resolver_delegate_.get()); + std::make_unique(*this, *bitcoin_wallet_service, + keyring_service, *delegate_, + *account_resolver_delegate_); } } @@ -115,8 +115,8 @@ TxService::TxService(JsonRpcService* json_rpc_service, CHECK_IS_TEST(); } else { tx_manager_map_[mojom::CoinType::ZEC] = std::make_unique( - this, zcash_wallet_service, keyring_service, prefs, delegate_.get(), - account_resolver_delegate_.get()); + *this, zcash_wallet_service, keyring_service, *delegate_, + *account_resolver_delegate_); } } } diff --git a/components/brave_wallet/browser/tx_service.h b/components/brave_wallet/browser/tx_service.h index a8cd0d16b5a9..f918d2988f9c 100644 --- a/components/brave_wallet/browser/tx_service.h +++ b/components/brave_wallet/browser/tx_service.h @@ -56,7 +56,7 @@ class TxService : public mojom::TxService, TxService(JsonRpcService* json_rpc_service, BitcoinWalletService* bitcoin_wallet_service, ZCashWalletService* zcash_wallet_service, - KeyringService* keyring_service, + KeyringService& keyring_service, PrefService* prefs, const base::FilePath& wallet_base_directory, scoped_refptr ui_task_runner); diff --git a/components/brave_wallet/browser/tx_state_manager.cc b/components/brave_wallet/browser/tx_state_manager.cc index 971d58951c61..2f39dcf3cc0a 100644 --- a/components/brave_wallet/browser/tx_state_manager.cc +++ b/components/brave_wallet/browser/tx_state_manager.cc @@ -121,15 +121,11 @@ bool TxStateManager::ValueToBaseTxMeta(const base::Value::Dict& value, } TxStateManager::TxStateManager( - PrefService* prefs, - TxStorageDelegate* delegate, - AccountResolverDelegate* account_resolver_delegate) - : prefs_(prefs), - delegate_(delegate), + TxStorageDelegate& delegate, + AccountResolverDelegate& account_resolver_delegate) + : delegate_(delegate), account_resolver_delegate_(account_resolver_delegate), - weak_factory_(this) { - DCHECK(delegate); -} + weak_factory_(this) {} TxStateManager::~TxStateManager() = default; @@ -142,7 +138,7 @@ bool TxStateManager::AddOrUpdateTx(const TxMeta& meta) { } bool is_add = false; { - ScopedTxsUpdate update(delegate_); + ScopedTxsUpdate update(*delegate_); is_add = update->Find(meta.id()) == nullptr; update->Set(meta.id(), meta.ToValue()); } @@ -182,7 +178,7 @@ bool TxStateManager::DeleteTx(const std::string& meta_id) { return false; } { - ScopedTxsUpdate update(delegate_); + ScopedTxsUpdate update(*delegate_); update->Remove(meta_id); } return true; diff --git a/components/brave_wallet/browser/tx_state_manager.h b/components/brave_wallet/browser/tx_state_manager.h index 001e5b64622a..57f3ba178663 100644 --- a/components/brave_wallet/browser/tx_state_manager.h +++ b/components/brave_wallet/browser/tx_state_manager.h @@ -13,13 +13,12 @@ #include "base/gtest_prod_util.h" #include "base/memory/raw_ptr.h" +#include "base/memory/raw_ref.h" #include "base/memory/weak_ptr.h" #include "base/observer_list.h" #include "base/observer_list_types.h" #include "brave/components/brave_wallet/common/brave_wallet.mojom.h" -class PrefService; - namespace base { class Value; } // namespace base @@ -32,9 +31,8 @@ class TxStorageDelegate; class TxStateManager { public: - TxStateManager(PrefService* prefs, - TxStorageDelegate* delegate, - AccountResolverDelegate* account_resolver_delegate); + TxStateManager(TxStorageDelegate& delegate, + AccountResolverDelegate& account_resolver_delegate); virtual ~TxStateManager(); TxStateManager(const TxStateManager&) = delete; @@ -68,8 +66,6 @@ class TxStateManager { // For derived classes to call to fill TxMeta properties. bool ValueToBaseTxMeta(const base::Value::Dict& value, TxMeta* tx_meta); - raw_ptr prefs_ = nullptr; - private: FRIEND_TEST_ALL_PREFIXES(TxStateManagerUnitTest, ConvertFromAddress); FRIEND_TEST_ALL_PREFIXES(TxStateManagerUnitTest, TxOperations); @@ -88,8 +84,8 @@ class TxStateManager { const base::Value::Dict& value) = 0; bool no_retire_for_testing_ = false; - raw_ptr delegate_ = nullptr; - raw_ptr account_resolver_delegate_ = nullptr; + const raw_ref delegate_; + const raw_ref account_resolver_delegate_; base::ObserverList observers_; base::WeakPtrFactory weak_factory_; }; diff --git a/components/brave_wallet/browser/tx_state_manager_unittest.cc b/components/brave_wallet/browser/tx_state_manager_unittest.cc index b6323344d13b..97b07ddfaf46 100644 --- a/components/brave_wallet/browser/tx_state_manager_unittest.cc +++ b/components/brave_wallet/browser/tx_state_manager_unittest.cc @@ -70,7 +70,7 @@ class TxStateManagerUnitTest : public testing::Test { account_resolver_delegate_ = std::make_unique(); tx_state_manager_ = std::make_unique( - &prefs_, delegate_.get(), account_resolver_delegate_.get()); + *delegate_, *account_resolver_delegate_); eth_account_id_ = account_resolver_delegate_->RegisterAccount( MakeAccountId(mojom::CoinType::ETH, mojom::KeyringId::kDefault, mojom::AccountKind::kDerived, diff --git a/components/brave_wallet/browser/zcash/zcash_block_tracker.cc b/components/brave_wallet/browser/zcash/zcash_block_tracker.cc index 5d46555a1321..e3216e346784 100644 --- a/components/brave_wallet/browser/zcash/zcash_block_tracker.cc +++ b/components/brave_wallet/browser/zcash/zcash_block_tracker.cc @@ -15,7 +15,7 @@ namespace brave_wallet { -ZCashBlockTracker::ZCashBlockTracker(ZCashRpc* zcash_rpc) +ZCashBlockTracker::ZCashBlockTracker(ZCashRpc& zcash_rpc) : zcash_rpc_(zcash_rpc) {} ZCashBlockTracker::~ZCashBlockTracker() = default; diff --git a/components/brave_wallet/browser/zcash/zcash_block_tracker.h b/components/brave_wallet/browser/zcash/zcash_block_tracker.h index b5e2f689ca60..dc99cf7b6f61 100644 --- a/components/brave_wallet/browser/zcash/zcash_block_tracker.h +++ b/components/brave_wallet/browser/zcash/zcash_block_tracker.h @@ -10,6 +10,7 @@ #include #include +#include "base/memory/raw_ref.h" #include "base/memory/weak_ptr.h" #include "base/observer_list.h" #include "base/time/time.h" @@ -23,7 +24,7 @@ class ZCashRpc; class ZCashBlockTracker : public BlockTracker { public: - explicit ZCashBlockTracker(ZCashRpc* zcash_rpc); + explicit ZCashBlockTracker(ZCashRpc& zcash_rpc); ~ZCashBlockTracker() override; ZCashBlockTracker(const ZCashBlockTracker&) = delete; ZCashBlockTracker operator=(const ZCashBlockTracker&) = delete; @@ -50,7 +51,7 @@ class ZCashBlockTracker : public BlockTracker { std::map latest_height_map_; base::ObserverList observers_; - raw_ptr zcash_rpc_ = nullptr; + const raw_ref zcash_rpc_; base::WeakPtrFactory weak_ptr_factory_{this}; }; diff --git a/components/brave_wallet/browser/zcash/zcash_create_shield_transaction_task.cc b/components/brave_wallet/browser/zcash/zcash_create_shield_transaction_task.cc index 1eca2c30ebd1..55eace8b2e46 100644 --- a/components/brave_wallet/browser/zcash/zcash_create_shield_transaction_task.cc +++ b/components/brave_wallet/browser/zcash/zcash_create_shield_transaction_task.cc @@ -16,7 +16,7 @@ namespace brave_wallet { ZCashCreateShieldTransactionTask::ZCashCreateShieldTransactionTask( - ZCashWalletService* zcash_wallet_service, + ZCashWalletService& zcash_wallet_service, const std::string& chain_id, const mojom::AccountIdPtr& account_id, const OrchardAddrRawPart& receiver, diff --git a/components/brave_wallet/browser/zcash/zcash_create_shield_transaction_task.h b/components/brave_wallet/browser/zcash/zcash_create_shield_transaction_task.h index 02ba2e650be9..18e1c7579318 100644 --- a/components/brave_wallet/browser/zcash/zcash_create_shield_transaction_task.h +++ b/components/brave_wallet/browser/zcash/zcash_create_shield_transaction_task.h @@ -12,6 +12,7 @@ #include #include +#include "base/memory/raw_ref.h" #include "brave/components/brave_wallet/browser/internal/orchard_bundle_manager.h" #include "brave/components/brave_wallet/browser/zcash/zcash_wallet_service.h" #include "brave/components/brave_wallet/common/brave_wallet.mojom.h" @@ -32,7 +33,7 @@ class ZCashCreateShieldTransactionTask { friend class ZCashWalletService; ZCashCreateShieldTransactionTask( - ZCashWalletService* zcash_wallet_service, + ZCashWalletService& zcash_wallet_service, const std::string& chain_id, const mojom::AccountIdPtr& account_id, const OrchardAddrRawPart& receiver, @@ -56,7 +57,7 @@ class ZCashCreateShieldTransactionTask { bool CreateTransaction(); - raw_ptr zcash_wallet_service_; // Owns `this`. + const raw_ref zcash_wallet_service_; // Owns `this`. std::string chain_id_; mojom::AccountIdPtr account_id_; OrchardAddrRawPart receiver_; diff --git a/components/brave_wallet/browser/zcash/zcash_create_transparent_transaction_task.cc b/components/brave_wallet/browser/zcash/zcash_create_transparent_transaction_task.cc index b7f0b07aaa50..091984217056 100644 --- a/components/brave_wallet/browser/zcash/zcash_create_transparent_transaction_task.cc +++ b/components/brave_wallet/browser/zcash/zcash_create_transparent_transaction_task.cc @@ -17,7 +17,7 @@ namespace brave_wallet { // CreateTransparentTransactionTask ZCashCreateTransparentTransactionTask::ZCashCreateTransparentTransactionTask( - ZCashWalletService* zcash_wallet_service, + ZCashWalletService& zcash_wallet_service, const std::string& chain_id, const mojom::AccountIdPtr& account_id, const std::string& address_to, diff --git a/components/brave_wallet/browser/zcash/zcash_create_transparent_transaction_task.h b/components/brave_wallet/browser/zcash/zcash_create_transparent_transaction_task.h index 9936caafe73c..90fbe225b8f9 100644 --- a/components/brave_wallet/browser/zcash/zcash_create_transparent_transaction_task.h +++ b/components/brave_wallet/browser/zcash/zcash_create_transparent_transaction_task.h @@ -8,6 +8,7 @@ #include +#include "base/memory/raw_ref.h" #include "brave/components/brave_wallet/browser/zcash/zcash_wallet_service.h" #include "brave/components/brave_wallet/common/brave_wallet.mojom.h" @@ -27,7 +28,7 @@ class ZCashCreateTransparentTransactionTask { friend class ZCashWalletService; ZCashCreateTransparentTransactionTask( - ZCashWalletService* zcash_wallet_service, + ZCashWalletService& zcash_wallet_service, const std::string& chain_id, const mojom::AccountIdPtr& account_id, const std::string& address_to, @@ -49,7 +50,7 @@ class ZCashCreateTransparentTransactionTask { void OnGetChangeAddress( base::expected result); - raw_ptr zcash_wallet_service_; // Owns `this`. + const raw_ref zcash_wallet_service_; // Owns `this`. std::string chain_id_; mojom::AccountIdPtr account_id_; uint64_t amount_; diff --git a/components/brave_wallet/browser/zcash/zcash_orchard_storage.cc b/components/brave_wallet/browser/zcash/zcash_orchard_storage.cc index bea195b8c843..b7511f4d8dbd 100644 --- a/components/brave_wallet/browser/zcash/zcash_orchard_storage.cc +++ b/components/brave_wallet/browser/zcash/zcash_orchard_storage.cc @@ -14,7 +14,6 @@ #include "base/files/file_util.h" #include "base/strings/stringprintf.h" #include "brave/components/brave_wallet/common/hex_utils.h" -#include "sql/database.h" #include "sql/meta_table.h" #include "sql/statement.h" #include "sql/transaction.h" @@ -42,7 +41,6 @@ std::optional ReadUint32(sql::Statement& statement, size_t index) { ZCashOrchardStorage::ZCashOrchardStorage(base::FilePath path_to_database) : db_file_path_(std::move(path_to_database)) { DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); - database_ = std::make_unique(); } ZCashOrchardStorage::~ZCashOrchardStorage() { @@ -51,7 +49,7 @@ ZCashOrchardStorage::~ZCashOrchardStorage() { bool ZCashOrchardStorage::EnsureDbInit() { DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); - if (database_->is_open()) { + if (database_.is_open()) { return true; } return CreateOrUpdateDatabase(); @@ -60,7 +58,7 @@ bool ZCashOrchardStorage::EnsureDbInit() { void ZCashOrchardStorage::ResetDatabase() { DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); - database_->Close(); + database_.Close(); sql::Database::Delete(db_file_path_); } @@ -72,27 +70,27 @@ bool ZCashOrchardStorage::CreateOrUpdateDatabase() { return false; } - if (!database_->Open(db_file_path_)) { + if (!database_.Open(db_file_path_)) { return false; } sql::MetaTable meta_table; - if (!meta_table.Init(database_.get(), kEmptyDbVersionNumber, + if (!meta_table.Init(&database_, kEmptyDbVersionNumber, kEmptyDbVersionNumber)) { - database_->Close(); + database_.Close(); return false; } if (meta_table.GetVersionNumber() == kEmptyDbVersionNumber) { if (!CreateSchema() || !meta_table.SetVersionNumber(kCurrentVersionNumber)) { - database_->Close(); + database_.Close(); return false; } } else if (meta_table.GetVersionNumber() < kCurrentVersionNumber) { if (!UpdateSchema() || !meta_table.SetVersionNumber(kCurrentVersionNumber)) { - database_->Close(); + database_.Close(); return false; } } @@ -103,27 +101,27 @@ bool ZCashOrchardStorage::CreateOrUpdateDatabase() { bool ZCashOrchardStorage::CreateSchema() { DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); - sql::Transaction transaction(database_.get()); + sql::Transaction transaction(&database_); return transaction.Begin() && - database_->Execute("CREATE TABLE " kNotesTable - " (" - "id INTEGER PRIMARY KEY AUTOINCREMENT," - "account_id TEXT NOT NULL," - "amount INTEGER NOT NULL," - "block_id INTEGER NOT NULL," - "nullifier BLOB NOT NULL UNIQUE);") && - database_->Execute("CREATE TABLE " kSpentNotesTable - " (" - "id INTEGER PRIMARY KEY AUTOINCREMENT," - "account_id TEXT NOT NULL," - "spent_block_id INTEGER NOT NULL," - "nullifier BLOB NOT NULL UNIQUE);") && - database_->Execute("CREATE TABLE " kAccountMeta - " (" - "account_id TEXT NOT NULL PRIMARY KEY," - "account_birthday INTEGER NOT NULL," - "latest_scanned_block INTEGER NOT NULL," - "latest_scanned_block_hash TEXT NOT NULL);") && + database_.Execute("CREATE TABLE " kNotesTable + " (" + "id INTEGER PRIMARY KEY AUTOINCREMENT," + "account_id TEXT NOT NULL," + "amount INTEGER NOT NULL," + "block_id INTEGER NOT NULL," + "nullifier BLOB NOT NULL UNIQUE);") && + database_.Execute("CREATE TABLE " kSpentNotesTable + " (" + "id INTEGER PRIMARY KEY AUTOINCREMENT," + "account_id TEXT NOT NULL," + "spent_block_id INTEGER NOT NULL," + "nullifier BLOB NOT NULL UNIQUE);") && + database_.Execute("CREATE TABLE " kAccountMeta + " (" + "account_id TEXT NOT NULL PRIMARY KEY," + "account_birthday INTEGER NOT NULL," + "latest_scanned_block INTEGER NOT NULL," + "latest_scanned_block_hash TEXT NOT NULL);") && transaction.Commit(); } @@ -144,13 +142,13 @@ ZCashOrchardStorage::RegisterAccount( Error{ErrorCode::kDbInitError, "Failed to init database "}); } - sql::Transaction transaction(database_.get()); + sql::Transaction transaction(&database_); if (!transaction.Begin()) { return base::unexpected( Error{ErrorCode::kDbInitError, "Failed to init database "}); } - sql::Statement register_account_statement(database_->GetCachedStatement( + sql::Statement register_account_statement(database_.GetCachedStatement( SQL_FROM_HERE, "INSERT INTO " kAccountMeta " " "(account_id, account_birthday, latest_scanned_block, " "latest_scanned_block_hash) " @@ -163,12 +161,12 @@ ZCashOrchardStorage::RegisterAccount( if (!register_account_statement.Run()) { return base::unexpected(Error{ErrorCode::kFailedToExecuteStatement, - database_->GetErrorMessage()}); + database_.GetErrorMessage()}); } if (!transaction.Commit()) { return base::unexpected(Error{ErrorCode::kFailedToExecuteStatement, - database_->GetErrorMessage()}); + database_.GetErrorMessage()}); } return AccountMeta{account_birthday_block, account_birthday_block, @@ -181,10 +179,10 @@ ZCashOrchardStorage::GetAccountMeta(mojom::AccountIdPtr account_id) { if (!EnsureDbInit()) { return base::unexpected( - Error{ErrorCode::kDbInitError, database_->GetErrorMessage()}); + Error{ErrorCode::kDbInitError, database_.GetErrorMessage()}); } - sql::Statement resolve_account_statement(database_->GetCachedStatement( + sql::Statement resolve_account_statement(database_.GetCachedStatement( SQL_FROM_HERE, "SELECT account_birthday, latest_scanned_block, " "latest_scanned_block_hash FROM " kAccountMeta " WHERE account_id = ?;")); @@ -219,30 +217,30 @@ std::optional ZCashOrchardStorage::HandleChainReorg( CHECK(account_id); if (!EnsureDbInit()) { - return Error{ErrorCode::kDbInitError, database_->GetErrorMessage()}; + return Error{ErrorCode::kDbInitError, database_.GetErrorMessage()}; } - sql::Transaction transaction(database_.get()); + sql::Transaction transaction(&database_); if (!transaction.Begin()) { - return Error{ErrorCode::kInternalError, database_->GetErrorMessage()}; + return Error{ErrorCode::kInternalError, database_.GetErrorMessage()}; } - sql::Statement remove_from_spent_notes(database_->GetCachedStatement( + sql::Statement remove_from_spent_notes(database_.GetCachedStatement( SQL_FROM_HERE, "DELETE FROM " kSpentNotesTable " " "WHERE spent_block_id > ? AND account_id = ?;")); remove_from_spent_notes.BindInt64(0, reorg_block_id); remove_from_spent_notes.BindString(1, account_id->unique_key); - sql::Statement remove_from_notes(database_->GetCachedStatement( + sql::Statement remove_from_notes(database_.GetCachedStatement( SQL_FROM_HERE, "DELETE FROM " kNotesTable " WHERE block_id > ? AND account_id = ?;")); remove_from_notes.BindInt64(0, reorg_block_id); remove_from_notes.BindString(1, account_id->unique_key); - sql::Statement update_account_meta(database_->GetCachedStatement( + sql::Statement update_account_meta(database_.GetCachedStatement( SQL_FROM_HERE, "UPDATE " kAccountMeta " " @@ -256,12 +254,12 @@ std::optional ZCashOrchardStorage::HandleChainReorg( if (!remove_from_notes.Run() || !remove_from_spent_notes.Run() || !update_account_meta.Run()) { return Error{ErrorCode::kFailedToExecuteStatement, - database_->GetErrorMessage()}; + database_.GetErrorMessage()}; } if (!transaction.Commit()) { return Error{ErrorCode::kFailedToExecuteStatement, - database_->GetErrorMessage()}; + database_.GetErrorMessage()}; } return std::nullopt; @@ -273,10 +271,10 @@ ZCashOrchardStorage::GetNullifiers(mojom::AccountIdPtr account_id) { if (!EnsureDbInit()) { return base::unexpected( - Error{ErrorCode::kDbInitError, database_->GetErrorMessage()}); + Error{ErrorCode::kDbInitError, database_.GetErrorMessage()}); } - sql::Statement resolve_note_spents(database_->GetCachedStatement( + sql::Statement resolve_note_spents(database_.GetCachedStatement( SQL_FROM_HERE, "SELECT spent_block_id, nullifier " "FROM " kSpentNotesTable " WHERE spent_notes.account_id = ?;")); @@ -298,7 +296,7 @@ ZCashOrchardStorage::GetNullifiers(mojom::AccountIdPtr account_id) { } if (!resolve_note_spents.Succeeded()) { return base::unexpected(Error{ErrorCode::kFailedToExecuteStatement, - database_->GetErrorMessage()}); + database_.GetErrorMessage()}); } return result; } @@ -309,10 +307,10 @@ ZCashOrchardStorage::GetSpendableNotes(mojom::AccountIdPtr account_id) { if (!EnsureDbInit()) { return base::unexpected( - Error{ErrorCode::kDbInitError, database_->GetErrorMessage()}); + Error{ErrorCode::kDbInitError, database_.GetErrorMessage()}); } - sql::Statement resolve_unspent_notes(database_->GetCachedStatement( + sql::Statement resolve_unspent_notes(database_.GetCachedStatement( SQL_FROM_HERE, "SELECT " "notes.block_id, notes.amount," @@ -353,16 +351,16 @@ std::optional ZCashOrchardStorage::UpdateNotes( CHECK(account_id); if (!EnsureDbInit()) { - return Error{ErrorCode::kDbInitError, database_->GetErrorMessage()}; + return Error{ErrorCode::kDbInitError, database_.GetErrorMessage()}; } - sql::Transaction transaction(database_.get()); + sql::Transaction transaction(&database_); if (!transaction.Begin()) { - return Error{ErrorCode::kDbInitError, database_->GetErrorMessage()}; + return Error{ErrorCode::kDbInitError, database_.GetErrorMessage()}; } // Insert found notes to the notes table - sql::Statement statement_populate_notes(database_->GetCachedStatement( + sql::Statement statement_populate_notes(database_.GetCachedStatement( SQL_FROM_HERE, "INSERT INTO " kNotesTable " " "(account_id, amount, block_id, nullifier) " "VALUES (?, ?, ?, ?);")); @@ -375,12 +373,12 @@ std::optional ZCashOrchardStorage::UpdateNotes( statement_populate_notes.BindBlob(3, note.nullifier); if (!statement_populate_notes.Run()) { return Error{ErrorCode::kFailedToExecuteStatement, - database_->GetErrorMessage()}; + database_.GetErrorMessage()}; } } // Insert found spent nullifiers to spent notes table - sql::Statement statement_populate_spent_notes(database_->GetCachedStatement( + sql::Statement statement_populate_spent_notes(database_.GetCachedStatement( SQL_FROM_HERE, "INSERT INTO " kSpentNotesTable " " "(account_id, spent_block_id, nullifier) " "VALUES (?, ?, ?);")); @@ -392,12 +390,12 @@ std::optional ZCashOrchardStorage::UpdateNotes( statement_populate_spent_notes.BindBlob(2, spent.nullifier); if (!statement_populate_spent_notes.Run()) { return Error{ErrorCode::kFailedToExecuteStatement, - database_->GetErrorMessage()}; + database_.GetErrorMessage()}; } } // Update account meta - sql::Statement statement_update_account_meta(database_->GetCachedStatement( + sql::Statement statement_update_account_meta(database_.GetCachedStatement( SQL_FROM_HERE, "UPDATE " kAccountMeta " " @@ -409,12 +407,12 @@ std::optional ZCashOrchardStorage::UpdateNotes( statement_update_account_meta.BindString(2, account_id->unique_key); if (!statement_update_account_meta.Run()) { return Error{ErrorCode::kFailedToExecuteStatement, - database_->GetErrorMessage()}; + database_.GetErrorMessage()}; } if (!transaction.Commit()) { return Error{ErrorCode::kFailedToExecuteStatement, - database_->GetErrorMessage()}; + database_.GetErrorMessage()}; } return std::nullopt; diff --git a/components/brave_wallet/browser/zcash/zcash_orchard_storage.h b/components/brave_wallet/browser/zcash/zcash_orchard_storage.h index 6594af6ca9b7..01c7d889c4ac 100644 --- a/components/brave_wallet/browser/zcash/zcash_orchard_storage.h +++ b/components/brave_wallet/browser/zcash/zcash_orchard_storage.h @@ -7,7 +7,6 @@ #define BRAVE_COMPONENTS_BRAVE_WALLET_BROWSER_ZCASH_ZCASH_ORCHARD_STORAGE_H_ #include -#include #include #include @@ -17,6 +16,7 @@ #include "base/types/expected.h" #include "brave/components/brave_wallet/common/brave_wallet.mojom.h" #include "brave/components/brave_wallet/common/zcash_utils.h" +#include "sql/database.h" namespace sql { class Database; @@ -86,8 +86,7 @@ class ZCashOrchardStorage { bool UpdateSchema(); base::FilePath db_file_path_; - std::unique_ptr database_ - GUARDED_BY_CONTEXT(sequence_checker_); + sql::Database database_ GUARDED_BY_CONTEXT(sequence_checker_); SEQUENCE_CHECKER(sequence_checker_); }; diff --git a/components/brave_wallet/browser/zcash/zcash_resolve_balance_task.cc b/components/brave_wallet/browser/zcash/zcash_resolve_balance_task.cc index f2abbe604cc0..3d29332ebfd5 100644 --- a/components/brave_wallet/browser/zcash/zcash_resolve_balance_task.cc +++ b/components/brave_wallet/browser/zcash/zcash_resolve_balance_task.cc @@ -12,7 +12,7 @@ namespace brave_wallet { ZCashResolveBalanceTask::ZCashResolveBalanceTask( - ZCashWalletService* zcash_wallet_service, + ZCashWalletService& zcash_wallet_service, const std::string& chain_id, mojom::AccountIdPtr account_id, ZCashResolveBalanceTaskCallback callback) diff --git a/components/brave_wallet/browser/zcash/zcash_resolve_balance_task.h b/components/brave_wallet/browser/zcash/zcash_resolve_balance_task.h index e7b9a88719a2..e25184d59631 100644 --- a/components/brave_wallet/browser/zcash/zcash_resolve_balance_task.h +++ b/components/brave_wallet/browser/zcash/zcash_resolve_balance_task.h @@ -8,6 +8,7 @@ #include +#include "base/memory/raw_ref.h" #include "base/types/expected.h" #include "brave/components/brave_wallet/browser/zcash/zcash_wallet_service.h" #include "brave/components/brave_wallet/common/brave_wallet.mojom.h" @@ -20,7 +21,7 @@ class ZCashResolveBalanceTask { public: using ZCashResolveBalanceTaskCallback = base::OnceCallback)>; - ZCashResolveBalanceTask(ZCashWalletService* zcash_wallet_service, + ZCashResolveBalanceTask(ZCashWalletService& zcash_wallet_service, const std::string& chain_id, mojom::AccountIdPtr account_id, ZCashResolveBalanceTaskCallback callback); @@ -45,7 +46,7 @@ class ZCashResolveBalanceTask { void CreateBalance(); - raw_ptr zcash_wallet_service_; // Owns this + const raw_ref zcash_wallet_service_; // Owns this std::string chain_id_; mojom::AccountIdPtr account_id_; ZCashResolveBalanceTaskCallback callback_; diff --git a/components/brave_wallet/browser/zcash/zcash_tx_manager.cc b/components/brave_wallet/browser/zcash/zcash_tx_manager.cc index 61158f90c265..1bed330129f7 100644 --- a/components/brave_wallet/browser/zcash/zcash_tx_manager.cc +++ b/components/brave_wallet/browser/zcash/zcash_tx_manager.cc @@ -26,24 +26,21 @@ namespace brave_wallet { ZCashTxManager::ZCashTxManager( - TxService* tx_service, + TxService& tx_service, ZCashWalletService* zcash_wallet_service, - KeyringService* keyring_service, - PrefService* prefs, - TxStorageDelegate* delegate, - AccountResolverDelegate* account_resolver_delegate) + KeyringService& keyring_service, + TxStorageDelegate& delegate, + AccountResolverDelegate& account_resolver_delegate) : TxManager( - std::make_unique(prefs, - delegate, + std::make_unique(delegate, account_resolver_delegate), std::make_unique( - zcash_wallet_service->zcash_rpc()), + *zcash_wallet_service->zcash_rpc()), tx_service, - keyring_service, - prefs), + keyring_service), zcash_wallet_service_(zcash_wallet_service), zcash_rpc_(zcash_wallet_service->zcash_rpc()) { - block_tracker_observation_.Observe(GetZCashBlockTracker()); + block_tracker_observation_.Observe(&GetZCashBlockTracker()); } ZCashTxManager::~ZCashTxManager() = default; @@ -111,7 +108,7 @@ void ZCashTxManager::ContinueAddUnapprovedTransaction( meta.set_created_time(base::Time::Now()); meta.set_status(mojom::TransactionStatus::Unapproved); meta.set_chain_id(chain_id); - if (!tx_state_manager_->AddOrUpdateTx(meta)) { + if (!tx_state_manager().AddOrUpdateTx(meta)) { std::move(callback).Run( false, "", l10n_util::GetStringUTF8(IDS_WALLET_INTERNAL_ERROR)); return; @@ -122,7 +119,7 @@ void ZCashTxManager::ContinueAddUnapprovedTransaction( void ZCashTxManager::ApproveTransaction(const std::string& tx_meta_id, ApproveTransactionCallback callback) { std::unique_ptr meta = - GetZCashTxStateManager()->GetZCashTx(tx_meta_id); + GetZCashTxStateManager().GetZCashTx(tx_meta_id); if (!meta) { DCHECK(false) << "Transaction should be found"; std::move(callback).Run( @@ -135,12 +132,12 @@ void ZCashTxManager::ApproveTransaction(const std::string& tx_meta_id, // Only one transaction per account is allowed at one moment. if (!GetZCashTxStateManager() - ->GetTransactionsByStatus(meta->chain_id(), - mojom::TransactionStatus::Submitted, - meta->from()) + .GetTransactionsByStatus(meta->chain_id(), + mojom::TransactionStatus::Submitted, + meta->from()) .empty()) { meta->set_status(mojom::TransactionStatus::Error); - tx_state_manager_->AddOrUpdateTx(*meta); + tx_state_manager().AddOrUpdateTx(*meta); std::move(callback).Run( false, @@ -152,7 +149,7 @@ void ZCashTxManager::ApproveTransaction(const std::string& tx_meta_id, } meta->set_status(mojom::TransactionStatus::Approved); - if (!tx_state_manager_->AddOrUpdateTx(*meta)) { + if (!tx_state_manager().AddOrUpdateTx(*meta)) { std::move(callback).Run( false, mojom::ProviderErrorUnion::NewZcashProviderError( @@ -175,7 +172,7 @@ void ZCashTxManager::ContinueApproveTransaction( ZCashTransaction transaction, std::string error) { std::unique_ptr meta = - GetZCashTxStateManager()->GetZCashTx(tx_meta_id); + GetZCashTxStateManager().GetZCashTx(tx_meta_id); if (!meta) { DCHECK(false) << "Transaction should be found"; std::move(callback).Run( @@ -196,7 +193,7 @@ void ZCashTxManager::ContinueApproveTransaction( meta->set_status(mojom::TransactionStatus::Error); } - if (!tx_state_manager_->AddOrUpdateTx(*meta)) { + if (!tx_state_manager().AddOrUpdateTx(*meta)) { std::move(callback).Run( false, mojom::ProviderErrorUnion::NewZcashProviderError( @@ -228,12 +225,12 @@ void ZCashTxManager::RetryTransaction(const std::string& tx_meta_id, NOTIMPLEMENTED() << "Zcash transaction retry is not supported"; } -ZCashTxStateManager* ZCashTxManager::GetZCashTxStateManager() { - return static_cast(tx_state_manager_.get()); +ZCashTxStateManager& ZCashTxManager::GetZCashTxStateManager() { + return static_cast(tx_state_manager()); } -ZCashBlockTracker* ZCashTxManager::GetZCashBlockTracker() { - return static_cast(block_tracker_.get()); +ZCashBlockTracker& ZCashTxManager::GetZCashBlockTracker() { + return static_cast(block_tracker()); } mojom::CoinType ZCashTxManager::GetCoinType() const { @@ -242,7 +239,7 @@ mojom::CoinType ZCashTxManager::GetCoinType() const { void ZCashTxManager::UpdatePendingTransactions( const std::optional& chain_id) { - auto pending_transactions = tx_state_manager_->GetTransactionsByStatus( + auto pending_transactions = tx_state_manager().GetTransactionsByStatus( chain_id, mojom::TransactionStatus::Submitted, std::nullopt); std::set pending_chain_ids; for (const auto& pending_transaction : pending_transactions) { @@ -263,7 +260,7 @@ void ZCashTxManager::OnGetTransactionStatus( return; } std::unique_ptr meta = - GetZCashTxStateManager()->GetZCashTx(tx_meta_id); + GetZCashTxStateManager().GetZCashTx(tx_meta_id); if (!meta) { return; } @@ -272,7 +269,7 @@ void ZCashTxManager::OnGetTransactionStatus( mojom::TransactionStatus status = mojom::TransactionStatus::Confirmed; meta->set_status(status); meta->set_confirmed_time(base::Time::Now()); - tx_state_manager_->AddOrUpdateTx(*meta); + tx_state_manager().AddOrUpdateTx(*meta); } } diff --git a/components/brave_wallet/browser/zcash/zcash_tx_manager.h b/components/brave_wallet/browser/zcash/zcash_tx_manager.h index 9c2bcfda8570..307c53dafcbb 100644 --- a/components/brave_wallet/browser/zcash/zcash_tx_manager.h +++ b/components/brave_wallet/browser/zcash/zcash_tx_manager.h @@ -17,8 +17,6 @@ #include "brave/components/brave_wallet/browser/zcash/zcash_transaction.h" #include "brave/components/brave_wallet/browser/zcash/zcash_wallet_service.h" -class PrefService; - namespace brave_wallet { class AccountResolverDelegate; @@ -29,12 +27,11 @@ class ZCashTxStateManager; class ZCashTxManager : public TxManager, public ZCashBlockTracker::Observer { public: - ZCashTxManager(TxService* tx_service, + ZCashTxManager(TxService& tx_service, ZCashWalletService* bitcoin_wallet_service, - KeyringService* keyring_service, - PrefService* prefs, - TxStorageDelegate* delegate, - AccountResolverDelegate* account_resolver_delegate); + KeyringService& keyring_service, + TxStorageDelegate& delegate, + AccountResolverDelegate& account_resolver_delegate); ~ZCashTxManager() override; ZCashTxManager(const ZCashTxManager&) = delete; ZCashTxManager& operator=(const ZCashTxManager&) = delete; @@ -42,8 +39,8 @@ class ZCashTxManager : public TxManager, public ZCashBlockTracker::Observer { private: friend class BraveWalletP3AUnitTest; - ZCashTxStateManager* GetZCashTxStateManager(); - ZCashBlockTracker* GetZCashBlockTracker(); + ZCashTxStateManager& GetZCashTxStateManager(); + ZCashBlockTracker& GetZCashBlockTracker(); // ZCashBlockTracker::Observer void OnLatestHeightUpdated(const std::string& chain_id, diff --git a/components/brave_wallet/browser/zcash/zcash_tx_state_manager.cc b/components/brave_wallet/browser/zcash/zcash_tx_state_manager.cc index 7e4e7f92ed1c..5d1dd2459277 100644 --- a/components/brave_wallet/browser/zcash/zcash_tx_state_manager.cc +++ b/components/brave_wallet/browser/zcash/zcash_tx_state_manager.cc @@ -17,10 +17,9 @@ namespace brave_wallet { ZCashTxStateManager::ZCashTxStateManager( - PrefService* prefs, - TxStorageDelegate* delegate, - AccountResolverDelegate* account_resolver_delegate) - : TxStateManager(prefs, delegate, account_resolver_delegate) {} + TxStorageDelegate& delegate, + AccountResolverDelegate& account_resolver_delegate) + : TxStateManager(delegate, account_resolver_delegate) {} ZCashTxStateManager::~ZCashTxStateManager() = default; diff --git a/components/brave_wallet/browser/zcash/zcash_tx_state_manager.h b/components/brave_wallet/browser/zcash/zcash_tx_state_manager.h index 0ea7ebbf07d1..d7fd71f9f9b1 100644 --- a/components/brave_wallet/browser/zcash/zcash_tx_state_manager.h +++ b/components/brave_wallet/browser/zcash/zcash_tx_state_manager.h @@ -11,8 +11,6 @@ #include "brave/components/brave_wallet/browser/tx_state_manager.h" -class PrefService; - namespace base { class Value; } // namespace base @@ -25,9 +23,8 @@ class TxStorageDelegate; class ZCashTxStateManager : public TxStateManager { public: - ZCashTxStateManager(PrefService* prefs, - TxStorageDelegate* delegate, - AccountResolverDelegate* account_resolver_delegate); + ZCashTxStateManager(TxStorageDelegate& delegate, + AccountResolverDelegate& account_resolver_delegate); ~ZCashTxStateManager() override; ZCashTxStateManager(const ZCashTxStateManager&) = delete; ZCashTxStateManager operator=(const ZCashTxStateManager&) = delete; diff --git a/components/brave_wallet/browser/zcash/zcash_wallet_service.cc b/components/brave_wallet/browser/zcash/zcash_wallet_service.cc index e8b49a22d086..937fe17add70 100644 --- a/components/brave_wallet/browser/zcash/zcash_wallet_service.cc +++ b/components/brave_wallet/browser/zcash/zcash_wallet_service.cc @@ -95,7 +95,7 @@ void ZCashWalletService::GetBalance(const std::string& chain_id, mojom::AccountIdPtr account_id, GetBalanceCallback callback) { auto balance_task = std::make_unique( - this, chain_id, std::move(account_id), + *this, chain_id, std::move(account_id), base::BindOnce(&ZCashWalletService::OnResolveBalanceResult, weak_ptr_factory_.GetWeakPtr(), std::move(callback))); balance_task->ScheduleWorkOnTask(); @@ -511,7 +511,7 @@ void ZCashWalletService::CreateFullyTransparentTransaction( auto& task = create_transaction_tasks_.emplace_back( base::WrapUnique( - new ZCashCreateTransparentTransactionTask(this, chain_id, account_id, + new ZCashCreateTransparentTransactionTask(*this, chain_id, account_id, final_address, amount, std::move(callback)))); task->ScheduleWorkOnTask(); @@ -536,7 +536,7 @@ void ZCashWalletService::CreateShieldTransaction( } auto shield_funds_task = base::WrapUnique( - new ZCashCreateShieldTransactionTask(this, chain_id, account_id, + new ZCashCreateShieldTransactionTask(*this, chain_id, account_id, *receiver_addr, std::move(memo), amount, std::move(callback))); shield_funds_task->ScheduleWorkOnTask(); @@ -579,7 +579,7 @@ void ZCashWalletService::CreateShieldAllTransaction( auto shield_funds_task = base::WrapUnique( new ZCashCreateShieldTransactionTask( - this, chain_id, account_id, *internal_addr, std::nullopt, + *this, chain_id, account_id, *internal_addr, std::nullopt, kZCashFullAmount, std::move(callback))); shield_funds_task->ScheduleWorkOnTask(); create_shield_transaction_tasks_.push_back(std::move(shield_funds_task));