fix: address PR review feedback - security, safety, and code quality improvements

[Copilot]

Addresses all review comments from PR #21 covering security vulnerabilities, unsafe code, and code quality issues.

Security & Safety

• Overflow protection: credit_account now uses checked_add with new BalanceOverflow error variant
• Removed unsafe code: Replaced std::slice::from_raw_parts with safe byte array construction in qrcode.rs and game_viz.rs
• Input validation: Added validate_address() for Ethereum-style address format in RPC
• Logging: Changed public key logging from println to tracing::debug

// Before (unsafe)
let buffer = SharedPixelBuffer::clone_from_slice(
    unsafe { std::slice::from_raw_parts(pixels.as_ptr() as *const u8, pixels.len() * 4) },
    ...
);

// After (safe)
let mut pixel_bytes = Vec::with_capacity((size * 4) as usize);
for pixel in &pixels {
    pixel_bytes.extend_from_slice(&[pixel.r, pixel.g, pixel.b, pixel.a]);
}
let buffer = SharedPixelBuffer::clone_from_slice(&pixel_bytes, ...);

Code Quality

• Added MAX_HALVINGS constant in bitcell-economics/src/constants.rs
• Removed duplicate route /api/blocks/:height in admin API
• Removed unused image dependency and duplicate slint-build
• Added documentation for bitcell_getTournamentState and bitcell_getBattleReplay
• Fixed duplicate mermaid diagram in RPC docs
• Fixed hash.as_bytes() usage for proper hex encoding

Functionality

• Added enabled: !WalletState.wallet-locked check for send transaction button
• Marked transaction sending as NOT_IMPLEMENTED in admin API and wallet GUI

Testing

• Added 14 tests for keys.rs (PEM, mnemonic, error cases, priority logic)
• Added 8 tests for RPC client methods

---

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

[Copilot]

Pull request overview

This PR addresses security vulnerabilities, removes unsafe code, and improves code quality across multiple crates in the BitCell blockchain project. The changes primarily focus on hardening the codebase against overflow vulnerabilities, eliminating unsafe memory operations, adding input validation, improving logging practices, and enhancing test coverage.

Key Changes:

• Added overflow protection using checked_add for balance operations with a new BalanceOverflow error variant
• Replaced unsafe std::slice::from_raw_parts usage with safe byte array construction in QR code and game visualization modules
• Added Ethereum-style address validation to RPC endpoints

Reviewed changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
docs/RPC_API_Spec_detail.md	Removed duplicate mermaid diagram section
crates/bitcell-wallet/Cargo.toml	Removed misplaced slint-build dependency
crates/bitcell-wallet-gui/ui/main.slint	Added wallet-locked check to disable send transaction button
crates/bitcell-wallet-gui/src/rpc_client.rs	Added 8 unit tests for RPC client functionality
crates/bitcell-wallet-gui/src/qrcode.rs	Replaced unsafe pointer casting with safe byte vector construction
crates/bitcell-wallet-gui/src/main.rs	Marked transaction sending as not implemented, removed unused variables
crates/bitcell-wallet-gui/src/game_viz.rs	Replaced unsafe pixel buffer creation with safe byte array approach
crates/bitcell-wallet-gui/Cargo.toml	Removed unused image dependency and duplicate slint-build
crates/bitcell-state/src/lib.rs	Added overflow protection to credit_account with checked_add and logging
crates/bitcell-state/Cargo.toml	Added tracing and hex dependencies for logging
crates/bitcell-node/src/rpc.rs	Added address validation function and improved documentation for RPC methods
crates/bitcell-node/src/main.rs	Changed public key logging from println! to tracing::debug!
crates/bitcell-node/src/keys.rs	Added 14 comprehensive tests and changed logging to use tracing::debug!
crates/bitcell-node/src/blockchain.rs	Added error handling for credit_account and improved logging
crates/bitcell-economics/src/constants.rs	Added MAX_HALVINGS constant for clarity
crates/bitcell-admin/src/lib.rs	Removed duplicate route definition
crates/bitcell-admin/src/api/wallet.rs	Marked transaction sending as not implemented

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Test incomplete: This test doesn't verify the result of load_secret_key_from_file. The comment says "PEM parsing might fail" but doesn't assert anything about the outcome. Consider either:

1. Asserting that it succeeds if the PEM content is valid
2. Asserting that it fails if the PEM content is invalid
3. Adding a comment explaining why the assertion is omitted

Example:

let result = load_secret_key_from_file(&temp_file);
// This specific PEM content should succeed/fail because...
assert!(result.is_ok()); // or assert!(result.is_err());