← Watch feed

Informational 18 Cryptographic libraries Committed Jun 4, 2026 · 18:52 UTC

ledger: lazy view key loading

XMR

Monero Project/Monero

03018306 View source ↗

Public commit record

What the developer wrote

Authored by jeffro256

78/100 · Adequate

ledger: lazy view key loading

Defer loading of private view key from device until first time it is needed.
Do not fail if this fails. This has two effects:

1. The prompt to export the view key is only needed once when creating a `cryptonote::account_base`
2. The call to `connect()` doesn't fail if the user decies to not export the viewkey, and thus usage of the device without exporting view keys is possible

This is a small convenience for Ledger users, but will be an even larger convenience for Ledger device testing

✓ Descriptive subject✓ Names a concrete action or component✓ Provides detailed explanatory context✓ Mentions testing or verification

The short version

What changed, and why it matters

This change makes Monero's Ledger hardware wallet integration ask for the private view key only when it is first needed, rather than immediately when connecting. If the user refuses, the software no longer fails and can continue in a slower mode where the Ledger does more cryptographic work. The change also clears the cached view key from memory when disconnecting. It is a usability improvement, not a fix for an active security flaw.

Recommended action

No urgent action required. Treat as a normal code review item: verify that `soft_request_view_key()` is only called in contexts that can tolerate the slower Ledger-only path, and that `disconnect()` scrubbing does not interfere with reconnection workflows.

Security signals we found

Private key material is now scrubbed on disconnect (`this->viewkey.scrub()`)

View-key export failure is no longer fatal, reducing a denial-of-service condition for Ledger users

Lazy loading reduces the number of times the user is prompted for key export

No new input validation, buffer handling, or cryptographic operations are added

Technical analysis

Evidence from the diff

The commit introduces soft_request_view_key(), which lazily calls get_secret_keys() once and swallows any exception. Previously connect() unconditionally requested the view key and would fail if the user declined. Now connect() succeeds regardless, and downstream code paths call soft_request_view_key() to decide whether to use a locally cached view key or fall back to Ledger-assisted operations. disconnect() now scrubs this->viewkey and resets the export flags. The diff shows no cryptographic weakness introduced; it is a workflow change.

Changed components

src/device/device_ledger.cppsrc/device/device_ledger.hppLedger hardware wallet integration in Monero wallet software

Inspect captured patch +38 / −11

diff --git a/src/device/device_ledger.cpp b/src/device/device_ledger.cpp
index f8631e4..a643cc2 100644
--- a/src/device/device_ledger.cpp
+++ b/src/device/device_ledger.cpp
@@ -303,6 +303,7 @@ namespace hw {
       this->reset_buffer();      
       this->mode = NONE;
       this->has_view_key = false;
+      this->requested_view_key = false;
       this->tx_in_progress = false;
       MDEBUG( "Device "<<this->id <<" Created");
     }
@@ -440,6 +441,25 @@ namespace hw {
       }
     }
 
+    bool device_ledger::soft_request_view_key() {
+      const std::lock_guard lock_dev(this->device_locker); // recursive is needed due to call to get_secret_keys()
+      if (this->has_view_key)
+        return true;
+      else if (this->requested_view_key)
+        return false;
+
+      this->requested_view_key = true;
+      try
+      {
+        crypto::secret_key v, s;
+        this->get_secret_keys(v, s);
+      }
+      catch (...)
+      { /* error message is logged in get_secret_keys()*/ }
+
+      return this->has_view_key;
+    }
+
     bool device_ledger::reset() {
       reset_buffer();
       int offset = set_command_header_noopt(INS_RESET);
@@ -540,9 +560,6 @@ namespace hw {
       cryptonote::account_public_address pubkey;
       this->get_public_address(pubkey);
       #endif
-      crypto::secret_key vkey;
-      crypto::secret_key skey;
-      this->get_secret_keys(vkey,skey);
 
       return true;
     }
@@ -553,6 +570,9 @@ namespace hw {
 
     bool device_ledger::disconnect() {
       hw_device.disconnect();
+      this->viewkey.scrub();
+      this->has_view_key = false;
+      this->requested_view_key = false;
       return true;
     }
 
@@ -688,7 +708,7 @@ namespace hw {
         #ifdef DEBUG_HWDEVICE
         const crypto::public_key pub_x = pub;
         crypto::key_derivation derivation_x;
-         if ((this->mode == TRANSACTION_PARSE) && has_view_key) {    
+         if ((this->mode == TRANSACTION_PARSE) && this->soft_request_view_key()) {
           derivation_x = derivation;
         } else {
           derivation_x = hw::ledger::decrypt(derivation);
@@ -703,7 +723,7 @@ namespace hw {
         log_hexbuffer("derive_subaddress_public_key: [[OUT]] derived_pub", derived_pub_x.data, 32);
         #endif
 
-      if ((this->mode == TRANSACTION_PARSE) && has_view_key) {     
+      if ((this->mode == TRANSACTION_PARSE) && this->soft_request_view_key()) {
         //If we are in TRANSACTION_PARSE, the given derivation has been retrieved unencrypted (without the help
         //of the device), so continue that way.
         MDEBUG( "derive_subaddress_public_key  : PARSE mode with known viewkey");     
@@ -739,7 +759,7 @@ namespace hw {
     }
 
     crypto::public_key device_ledger::get_subaddress_spend_public_key(const cryptonote::account_keys& keys, const cryptonote::subaddress_index &index) {
-        if (has_view_key) {
+        if (this->soft_request_view_key()) {
             cryptonote::account_keys keys_{keys};
             keys_.m_view_secret_key = this->viewkey;
             return this->controle_device->get_subaddress_spend_public_key(keys_, index);
@@ -796,7 +816,7 @@ namespace hw {
     }
 
     cryptonote::account_public_address device_ledger::get_subaddress(const cryptonote::account_keys& keys, const cryptonote::subaddress_index &index) {
-        if (has_view_key) {
+        if (this->soft_request_view_key()) {
             cryptonote::account_keys keys_{keys};
             keys_.m_view_secret_key = this->viewkey;
             return this->controle_device->get_subaddress(keys_, index);
@@ -1063,7 +1083,7 @@ namespace hw {
         log_hexbuffer("generate_key_derivation: [[OUT]] derivation", derivation_x.data, 32);
         #endif
 
-      if ((this->mode == TRANSACTION_PARSE)  && has_view_key) {
+      if ((this->mode == TRANSACTION_PARSE)  && this->soft_request_view_key()) {
         //A derivation is requested in PARSE mode and we have the view key,
         //so do that without the device and return the derivation unencrypted.
         MDEBUG( "generate_key_derivation  : PARSE mode with known viewkey");     
@@ -1091,7 +1111,7 @@ namespace hw {
       }
       #ifdef DEBUG_HWDEVICE
       crypto::key_derivation derivation_clear ;
-      if ((this->mode == TRANSACTION_PARSE)  && has_view_key) {
+      if ((this->mode == TRANSACTION_PARSE)  && this->soft_request_view_key()) {
         derivation_clear  = derivation;
       } else {
         derivation_clear  = hw::ledger::decrypt(derivation);
@@ -1315,7 +1335,7 @@ namespace hw {
     bool device_ledger::derive_view_tag(const crypto::key_derivation &derivation, const std::size_t output_index, crypto::view_tag &view_tag){
       #ifdef DEBUG_HWDEVICE
       crypto::key_derivation derivation_x;
-      if ((this->mode == TRANSACTION_PARSE) && has_view_key) {
+      if ((this->mode == TRANSACTION_PARSE) && this->soft_request_view_key()) {
         derivation_x = derivation;
       } else {
         derivation_x = hw::ledger::decrypt(derivation);
@@ -1328,7 +1348,7 @@ namespace hw {
       log_hexbuffer("derive_view_tag: [[OUT]] view_tag ", &view_tag_x.data, 1);
       #endif
 
-      if ((this->mode == TRANSACTION_PARSE) && has_view_key) {
+      if ((this->mode == TRANSACTION_PARSE) && this->soft_request_view_key()) {
         //If we are in TRANSACTION_PARSE, the given derivation has been retrieved unencrypted (without the help
         //of the device), so continue that way.
         MDEBUG( "derive_view_tag  : PARSE mode with known viewkey");
diff --git a/src/device/device_ledger.hpp b/src/device/device_ledger.hpp
index 2634454..1db6fbf 100644
--- a/src/device/device_ledger.hpp
+++ b/src/device/device_ledger.hpp
@@ -160,6 +160,12 @@ namespace hw {
         void send_simple(unsigned char ins, unsigned char p1 = 0x00);
         void send_secret(const unsigned char sec[32], int &offset);
         void receive_secret(unsigned char sec[32], int &offset);
+        /**
+         * @brief Request user export view privkey if not requested before, then return whether it has been exported
+         *
+         * Thread-safe. Result in this->viewkey iff this->has_view_key. I/O is performed iff !this->requested_view_key
+         */
+        bool soft_request_view_key();
 
         bool tx_in_progress;
 
@@ -174,6 +180,7 @@ namespace hw {
         // To speed up blockchain parsing the view key maybe handle here.
         crypto::secret_key viewkey = crypto::null_skey;
         bool has_view_key = false;
+        bool requested_view_key = false;
 
         device *controle_device;

Risk score

Why this scored 18/100

Our methodology →

Potential impact 2/30

Exploitability 1/25

Stealth signal 1/15

Affected reach 2/15

Confidence 8/10

Evidence quality 4/5

Human-validated context

Community notes

Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.

No validated notes yet.

The AI analysis stands alone for now. Submit a note if you can add evidence or important context.

What the developer wrote

What changed, and why it matters

Security signals we found

Evidence from the diff

Changed components

Why this scored 18/100

Community notes

Related watches

Merge pull request #11462

Merge pull request #11460

wallet2: trim stale transfer maps after output imports