Skip to content

feat(offline-keys): add ZHTLC offline key export support for ARRR#2549

Merged
ca333 merged 5 commits intooffline_key_exportfrom
offline_key_export_ZHTLC
Jul 27, 2025
Merged

feat(offline-keys): add ZHTLC offline key export support for ARRR#2549
ca333 merged 5 commits intooffline_key_exportfrom
offline_key_export_ZHTLC

Conversation

@ca333
Copy link
Copy Markdown

@ca333 ca333 commented Jul 26, 2025

feat(offline-keys): implement actual ZHTLC key derivation for ARRR

Summary

This PR implements offline key export functionality for ARRR (ZHTLC protocol) coins in the komodo-defi-framework. The implementation adds support for both HD and Iguana key derivation modes, enabling users to export ARRR private keys, addresses, and viewing keys offline.

Key Changes:

  • Added Zhtlc variant to PrefixValues enum with ZHTLC-specific key derivation logic
  • Implemented HD derivation using ZIP32 from root seed with proper z_derivation_path parsing
  • Implemented Iguana derivation using ExtendedSpendingKey::master
  • Added comprehensive test case test_arrr_hd_key_derivation with user-provided seed data
  • Integrated ZHTLC encoding functions for proper address, private key, and viewing key formats

Technical Details:

  • Uses existing ZHTLC functions from z_coin.rs for cryptographic operations
  • Handles derivation path logic: only derives change/index for index > 0, uses default_address() for index = 0
  • Parses z_derivation_path from coin configuration (e.g., "m/32'/141'")
  • Generates proper ZHTLC key formats: zs1 addresses, secret-extended-key-main private keys, zxviews viewing keys

Review & Testing Checklist for Human

🔴 HIGH RISK - Cryptocurrency key derivation requires thorough validation:

  • Verify ARRR key derivation with different seeds - Test with multiple seed phrases to ensure consistent, correct key generation
  • Validate key format correctness - Confirm generated addresses start with zs1, private keys with secret-extended-key-main, viewing keys with zxviews
  • Test HD derivation edge cases - Verify behavior with higher address indices (beyond 0,1) and different account values
  • Test both HD and Iguana modes thoroughly - Ensure both derivation modes work correctly and produce expected key formats
  • Regression testing - Run full offline key test suite to ensure no existing functionality was broken

Recommended Test Plan:

  1. Use the test seed from test_arrr_hd_key_derivation and verify the exact expected addresses/keys are generated
  2. Test with a different seed phrase and validate key formats are correct
  3. Test higher address indices (2, 3, 10) to ensure derivation logic works beyond the tested range
  4. Test Iguana mode with different private keys
  5. Run integration tests with actual ARRR coin configuration

Diagram

%%{ init : { "theme" : "default" }}%%
graph TD
    OfflineKeysRS["mm2src/coins/rpc_command/<br/>offline_keys.rs"]:::major-edit
    ZCoinRS["mm2src/coins/z_coin.rs"]:::context
    TestHelpers["mm2src/mm2_test_helpers/<br/>for_tests.rs"]:::context
    
    OfflineKeysRS --> |"uses ZHTLC encoding<br/>functions"| ZCoinRS
    OfflineKeysRS --> |"imports pirate_conf<br/>for testing"| TestHelpers
    
    subgraph "Key Implementation Areas"
        HDDerivation["HD Key Derivation<br/>(ZIP32 from root seed)"]:::major-edit
        IguanaDerivation["Iguana Key Derivation<br/>(ExtendedSpendingKey::master)"]:::major-edit  
        TestCase["test_arrr_hd_key_derivation<br/>(comprehensive test)"]:::major-edit
    end
    
    OfflineKeysRS --> HDDerivation
    OfflineKeysRS --> IguanaDerivation
    OfflineKeysRS --> TestCase
    
    subgraph Legend
        L1["Major Edit"]:::major-edit
        L2["Minor Edit"]:::minor-edit
        L3["Context/No Edit"]:::context
    end
    
    classDef major-edit fill:#90EE90
    classDef minor-edit fill:#87CEEB  
    classDef context fill:#FFFFFF
Loading

Notes

  • Cryptographic Implementation: This PR handles sensitive cryptocurrency key derivation. The implementation reuses existing, battle-tested ZHTLC functions from z_coin.rs where possible to minimize risk.

  • Derivation Logic Complexity: The most complex part is the derivation path logic - for index = 0, we only derive to account level and use default_address(), but for index > 0, we derive additional change/index levels. This was debugged extensively during implementation.

  • Testing Coverage: Added comprehensive test with exact expected values provided by the user, but testing is limited to indices 0 and 1. Higher indices and edge cases need human validation.

  • Integration Approach: Some logic had to be inlined from z_coin.rs due to function privacy constraints, which introduces potential for subtle differences from the original implementation.

Session Info:

@devin-ai-integration
Copy link
Copy Markdown

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

@devin-ai-integration devin-ai-integration bot force-pushed the offline_key_export_ZHTLC branch from 43b0df2 to ec7215b Compare July 26, 2025 09:24
@devin-ai-integration devin-ai-integration bot changed the title feat: add ZHTLC offline key export support for ARRR feat(offline-keys): add ZHTLC offline key export support for ARRR Jul 26, 2025
@smk762
Copy link
Copy Markdown

smk762 commented Jul 27, 2025

Confirmed in HD mode, test seed provides:

  • valid KMD keypairs, with privkey imported into native daemon unlocking same address/pubkey.
  • valid ARRR keypairs, with no error returned when privkey is imported into native daemon. Not yet able to confirm matching expected address/pubkey (Still rescanning. At block 5225. Progress=0.002584) but I have no reason to think it wont be correct.
  • KDF output for ARRR is matching PiratePaperWallet output (though PPW increments account_index, we increment address_index). If using the account_index param in KDF, matching output confirmed for same full derivation path.

Error testing:

With enable_hd: true in MM2.json, queries to get_private_keys with mode: iguana:

  • KMD returns a valid result, with privkey import into native daemon returning matching pubkey/address
  • ARRR returns a KeyDerivationFailed error.
{
    "mmrpc": "2.0",
    "error": "Failed to derive keys for ARRR: Iguana key derivation requires Iguana mode",
    "error_path": "offline_keys",
    "error_trace": "offline_keys:548]",
    "error_type": "KeyDerivationFailed",
    "error_data": {
        "ticker": "ARRR",
        "error": "Iguana key derivation requires Iguana mode"
    },
    "id": null
}

Is there any reason we can't offer offline output in both modes for all coins?

@devin-ai-integration
Copy link
Copy Markdown

Devin is archived and cannot be woken up. Please unarchive Devin if you want to continue using it.

@ca333 ca333 merged commit 91b7c13 into offline_key_export Jul 27, 2025
11 of 24 checks passed
shamardy pushed a commit that referenced this pull request Jul 28, 2025
)

* feat(offline-keys): add ZHTLC offline key export support for ARRR

* fix(offline-keys): apply cargo fmt formatting fixes

* feat(offline-keys): implement actual ZHTLC key derivation for ARRR

* fix(offline-keys): apply cargo fmt formatting fixes

* fix(offline-keys): resolve compilation warnings by renaming unused variables
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants