Review fix #1

components/brave_wallet/browser/internal/orchard_block_scanner.cc

@@ -5,7 +5,8 @@
 
 #include "brave/components/brave_wallet/browser/internal/orchard_block_scanner.h"
 
-#include "brave/components/brave_wallet/browser/zcash/rust/orchard_decoded_blocks_bunde.h"
+#include "base/threading/thread_restrictions.h"
+#include "brave/components/brave_wallet/browser/zcash/rust/orchard_decoded_blocks_bundle.h"
 
 namespace brave_wallet {
 
@@ -33,8 +34,10 @@ OrchardBlockScanner::~OrchardBlockScanner() = default;
 
 base::expected<OrchardBlockScanner::Result, OrchardBlockScanner::ErrorCode>
 OrchardBlockScanner::ScanBlocks(
-    OrchardTreeState tree_state,
-    std::vector<zcash::mojom::CompactBlockPtr> blocks) {
+    const OrchardTreeState& tree_state,
+    const std::vector<zcash::mojom::CompactBlockPtr>& blocks) {
+  base::AssertLongCPUWorkAllowed();
+
   std::unique_ptr<orchard::OrchardDecodedBlocksBundle> result =
       decoder_->ScanBlocks(tree_state, blocks);
   if (!result) {

components/brave_wallet/browser/internal/orchard_block_scanner.h

@@ -14,7 +14,7 @@
 #include "base/types/expected.h"
 #include "brave/components/brave_wallet/browser/internal/orchard_block_scanner.h"
 #include "brave/components/brave_wallet/browser/zcash/rust/orchard_block_decoder.h"
-#include "brave/components/brave_wallet/browser/zcash/rust/orchard_decoded_blocks_bunde.h"
+#include "brave/components/brave_wallet/browser/zcash/rust/orchard_decoded_blocks_bundle.h"
 #include "brave/components/brave_wallet/common/zcash_utils.h"
 
 namespace brave_wallet {
@@ -28,7 +28,6 @@ class OrchardBlockScanner {
   struct Result {
     Result();
     Result(std::vector<OrchardNote> discovered_notes,
-
            std::vector<OrchardNoteSpend> spent_notes,
            std::unique_ptr<orchard::OrchardDecodedBlocksBundle> scanned_blocks);
     Result(const Result&) = delete;
@@ -37,10 +36,11 @@ class OrchardBlockScanner {
     Result& operator=(Result&&);
     ~Result();
 
-    // New notes have been discovered
+    // New notes have been discovered.
     std::vector<OrchardNote> discovered_notes;
-    // Nullifiers for the previously discovered notes
+    // Nullifiers for the previously discovered notes.
     std::vector<OrchardNoteSpend> found_spends;
+    // Decoded blocks bundle to be insterted in the shard tree.
     std::unique_ptr<orchard::OrchardDecodedBlocksBundle> scanned_blocks;
   };
 
@@ -50,8 +50,8 @@ class OrchardBlockScanner {
   // Scans blocks to find incoming notes related to fvk
   // Also checks whether existing notes were spent.
   virtual base::expected<Result, OrchardBlockScanner::ErrorCode> ScanBlocks(
-      OrchardTreeState tree_state,
-      std::vector<zcash::mojom::CompactBlockPtr> blocks);
+      const OrchardTreeState& tree_state,
+      const std::vector<zcash::mojom::CompactBlockPtr>& blocks);
 
   static Result CreateResultForTesting(
       const OrchardTreeState& tree_state,

components/brave_wallet/browser/internal/orchard_bundle_manager.cc

@@ -22,7 +22,7 @@ std::optional<size_t> OrchardBundleManager::random_seed_for_testing_ =
 // static
 std::unique_ptr<OrchardBundleManager> OrchardBundleManager::Create(
     base::span<const uint8_t> tree_state,
-    const std::vector<::brave_wallet::OrchardOutput>& orchard_outputs) {
+    const std::vector<OrchardOutput>& orchard_outputs) {
   if (orchard_outputs.empty()) {
     return nullptr;
   }

components/brave_wallet/browser/internal/orchard_bundle_manager.h

@@ -45,7 +45,7 @@ class OrchardBundleManager {
   // Returns in unauthorized state
   static std::unique_ptr<OrchardBundleManager> Create(
       base::span<const uint8_t> tree_state,
-      const std::vector<::brave_wallet::OrchardOutput>& orchard_outputs);
+      const std::vector<OrchardOutput>& orchard_outputs);
 
   static void OverrideRandomSeedForTesting(size_t seed) {
     random_seed_for_testing_ = seed;

components/brave_wallet/browser/internal/orchard_shard_tree_manager.cc

@@ -5,7 +5,8 @@
 
 #include "brave/components/brave_wallet/browser/internal/orchard_shard_tree_manager.h"
 
-#include "brave/components/brave_wallet/common/zcash_utils.h"
+#include "brave/components/brave_wallet/browser/zcash/rust/orchard_shard_tree.h"
+#include "brave/components/brave_wallet/common/orchard_shard_tree_delegate.h"
 
 namespace brave_wallet {
 
@@ -36,26 +37,29 @@ OrchardShardTreeManager::OrchardShardTreeManager(
   orchard_shard_tree_ = std::move(shard_tree);
 }
 
-OrchardShardTreeManager::~OrchardShardTreeManager() {}
+OrchardShardTreeManager::~OrchardShardTreeManager() = default;
 
 bool OrchardShardTreeManager::InsertCommitments(
-    OrchardBlockScanner::Result result) {
+    OrchardBlockScanner::Result&& result) {
   return orchard_shard_tree_->ApplyScanResults(
       std::move(result.scanned_blocks));
 }
 
 base::expected<std::vector<OrchardInput>, std::string>
-OrchardShardTreeManager::CalculateWitness(std::vector<OrchardInput> notes,
-                                          uint32_t checkpoint_position) {
+OrchardShardTreeManager::CalculateWitness(
+    const std::vector<OrchardInput>& notes,
+    uint32_t checkpoint_position) {
+  std::vector<OrchardInput> result;
   for (auto& input : notes) {
     auto witness = orchard_shard_tree_->CalculateWitness(
         input.note.orchard_commitment_tree_position, checkpoint_position);
     if (!witness.has_value()) {
       return base::unexpected(witness.error());
     }
-    input.witness = witness.value();
+    result.push_back(input);
+    result.back().witness = witness.value();
   }
-  return notes;
+  return base::ok(std::move(result));
 }
 
 bool OrchardShardTreeManager::Truncate(uint32_t checkpoint) {

components/brave_wallet/browser/internal/orchard_shard_tree_manager.h

@@ -13,27 +13,40 @@
 
 #include "base/types/expected.h"
 #include "brave/components/brave_wallet/browser/internal/orchard_block_scanner.h"
-#include "brave/components/brave_wallet/browser/zcash/rust/orchard_shard_tree.h"
-#include "brave/components/brave_wallet/common/zcash_utils.h"
 
 namespace brave_wallet {
 
+class OrchardShardTreeDelegate;
+namespace orchard {
+class OrchardShardTree;
+}  // namespace orchard
+
+// Presents Orchard commitment tree.
+// Provides methods for inserting leafs to the tree and
+// calculating witness information for specified leaf positions.
 class OrchardShardTreeManager {
  public:
-  OrchardShardTreeManager(
+  explicit OrchardShardTreeManager(
       std::unique_ptr<::brave_wallet::orchard::OrchardShardTree> shard_tree);
   ~OrchardShardTreeManager();
-  bool InsertCommitments(OrchardBlockScanner::Result commitments);
+  // Inserts leafs extracted from the provided scan result.
+  bool InsertCommitments(OrchardBlockScanner::Result&& commitments);
+  // Calculates witness(merkle path to the tree root) for the provided
+  // set of notes.
+  // Checkpoint is also provided as an achor(selected right-most border of the
+  // tree).
   base::expected<std::vector<OrchardInput>, std::string> CalculateWitness(
-      std::vector<OrchardInput> notes,
+      const std::vector<OrchardInput>& notes,
       uint32_t checkpoint_position);
+  // Truncates tree including specified checkpoint.
+  // Needed for chain reorg cases.
   bool Truncate(uint32_t checkpoint);
-  base::expected<bool, std::string> VerifyCheckpoint();
 
+  // Creates shard tree size of 32.
   static std::unique_ptr<OrchardShardTreeManager> Create(
       std::unique_ptr<OrchardShardTreeDelegate> delegate);
 
-  // Creates shard tree size of 8 for testing
+  // Creates shard tree size of 8 for testing.
   static std::unique_ptr<OrchardShardTreeManager> CreateForTesting(
       std::unique_ptr<OrchardShardTreeDelegate> delegate);