Skip to content

Conversation

@BuilderFred
Copy link
Contributor

Implements a command-line wallet tool for managing RTC tokens. Supports wallet creation with BIP39 mnemonics, encrypted keystores, balance checks, and signed transfers.

Copy link
Owner

@Scottcjn Scottcjn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔧 Changes Requested - Wallet CLI

@BuilderFred Solid crypto implementation! A few fixes needed.

Critical Issues (Must Fix):

  1. NODE_URL Hardcoded

    # Change from:
    NODE_URL = "http://127.0.0.1:8099"
    # To:
    NODE_URL = os.environ.get("RUSTCHAIN_NODE_URL", "https://50.28.86.131")
  2. Missing requirements.txt

    mnemonic>=0.20
    PyNaCl>=1.5.0
    cryptography>=41.0
    click>=8.0
    requests>=2.31
    
  3. File Permissions

    • Add os.chmod(self.path, 0o600) after writing wallet keystore
  4. Duplicate arch_validation.py

    • Same issue as PR #16 - remove from root

Recommended (High Priority):

  1. Add --dry-run flag to send

    • Preview transaction without signing/sending
  2. Better Error Messages

    • Network timeout should say "Node unreachable at {url}"
    • Invalid address should show expected format

Crypto Review: ✅ APPROVED

  • BIP39 24-word seeds ✓
  • Ed25519 signing ✓
  • AES-256-GCM encryption ✓
  • PBKDF2 100k iterations ✓
  • No hardcoded keys ✓

Homework:

  • Add NODE_URL env override
  • Add requirements.txt
  • chmod 0600 on wallet files
  • Remove duplicate arch_validation.py
  • Add --dry-run flag
  • Improve error messages

The wallet crypto is solid - just needs the operational fixes above.

@Scottcjn
Copy link
Owner

Scottcjn commented Feb 3, 2026

@BuilderFred - Re-checked, fixes still missing:

❌ NODE_URL still hardcoded to 127.0.0.1:8099 - use os.environ.get('RUSTCHAIN_NODE_URL', 'https://50.28.86.131')
❌ requirements.txt not added
❌ chmod 0600 for keystore not added

Push fixes and ping when ready.

@Scottcjn
Copy link
Owner

Scottcjn commented Feb 3, 2026

👋 @BuilderFred — Following up on wallet CLI PR. Still need:

  1. ❌ Change NODE_URL to use os.environ.get('RUSTCHAIN_NODE_URL', 'https://50.28.86.131')
  2. ❌ Add requirements.txt
  3. ❌ Add chmod +x instruction in README

Quick fixes — ping me when updated!

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