fix(genesis-tool): close 4 medium audit findings (#533–#536)

genesis-tool/src/execute.rs

@@ -32,8 +32,7 @@ fn deploy_bsc_style(byte_code_dir: &str, total_stake: U256) -> InMemoryDB {
         let hex_path = format!("{}/{}.hex", byte_code_dir, contract_name);
         let bytecode_hex = read_hex_from_file(&hex_path);
 
-        // For BSC style, we need to extract runtime bytecode from constructor bytecode
-        let runtime_bytecode = extract_runtime_bytecode(&bytecode_hex);
+        let runtime_bytecode = decode_runtime_bytecode(&bytecode_hex);
 
         // Set balance for Genesis contract (needs to fund validator stake pools)
         let balance = if contract_name == "Genesis" {
@@ -69,10 +68,20 @@ fn deploy_bsc_style(byte_code_dir: &str, total_stake: U256) -> InMemoryDB {
     db
 }
 
-/// Extract runtime bytecode from constructor bytecode
-/// This is a simplified implementation - the bytecode should already be runtime bytecode
-fn extract_runtime_bytecode(constructor_bytecode: &str) -> Vec<u8> {
-    let trimmed = constructor_bytecode.trim();
+/// Decode a `.hex` file's contents into runtime bytecode bytes.
+///
+/// The caller's contract is: the input MUST already be runtime bytecode
+/// (i.e. Forge `deployedBytecode.object`, as produced by
+/// `scripts/helpers/extract_bytecode.py`). Constructor bytecode here would
+/// be deployed verbatim and brick every call into the predeploy.
+///
+/// We do not try to detect constructor-vs-runtime from the first byte:
+/// both shapes start with `60 80 60 40` (the Solidity free-memory-pointer
+/// prologue), so any such heuristic is noise, not signal. Pipelines are
+/// responsible for selecting `deployedBytecode`; this function only
+/// validates that what it receives is non-empty, well-formed hex.
+fn decode_runtime_bytecode(hex_str: &str) -> Vec<u8> {
+    let trimmed = hex_str.trim();
     let bytes = hex::decode(trimmed).unwrap_or_else(|e| {
         panic!(
             "FATAL: Failed to decode hex bytecode: {}. Input (first 100 chars): {}",
@@ -86,18 +95,7 @@ fn extract_runtime_bytecode(constructor_bytecode: &str) -> Vec<u8> {
         panic!("FATAL: Decoded bytecode is empty — possible corrupted or empty hex file");
     }
 
-    // Simple heuristic: if the bytecode starts with typical constructor patterns,
-    // we need to extract the runtime part
-    if bytes.len() > 100 && (bytes[0] == 0x60 || bytes[0] == 0x61) {
-        // This looks like constructor bytecode
-        // For now, we'll use a simplified approach and return the original bytecode
-        // In a real implementation, we'd execute the constructor and extract the returned bytecode
-        warn!("   [!] Warning: Using constructor bytecode as runtime bytecode");
-        bytes
-    } else {
-        // This looks like runtime bytecode already
-        bytes
-    }
+    bytes
 }
 
 pub fn prepare_env(chain_id: u64, block_timestamp_secs: u64) -> Env {
@@ -214,7 +212,7 @@ pub fn genesis_generate(
     for (contract_name, contract_address) in CONTRACTS {
         let hex_path = format!("{}/{}.hex", byte_code_dir, contract_name);
         let bytecode_hex = read_hex_from_file(&hex_path);
-        let runtime_bytecode = extract_runtime_bytecode(&bytecode_hex);
+        let runtime_bytecode = decode_runtime_bytecode(&bytecode_hex);
 
         genesis_state.insert(
             contract_address,

genesis-tool/src/genesis.rs

@@ -507,21 +507,38 @@ pub fn convert_config_to_sol(config: &GenesisConfig) -> SolGenesisInitParams {
     };
 
     // Convert Validators
+    //
+    // Invariant: at genesis, a validator's voting power MUST equal its staked
+    // bond. `ValidatorManagement.initialize` deliberately bypasses
+    // `_validateRegistration` (no PoP / minimumBond check), so this tool is
+    // the only line of defense against an initial set whose consensus weight
+    // is decoupled from economic backing. Reject any config where the two
+    // fields disagree rather than silently shipping a divergent genesis.
     let validators: Vec<SolInitialValidator> = config
         .validators
         .iter()
-        .map(|v| SolInitialValidator {
-            operator: parse_address(&v.operator),
-            owner: parse_address(&v.owner),
-            staker: parse_address(&v.staker),
-            stakeAmount: parse_u256(&v.stake_amount),
-            moniker: v.moniker.clone(),
-            consensusPubkey: parse_hex_bytes(&v.consensus_pubkey).into(),
-            consensusPop: parse_hex_bytes(&v.consensus_pop).into(),
-            // BCS encode network addresses from human-readable format
-            networkAddresses: bcs_encode_string(&v.network_addresses).into(),
-            fullnodeAddresses: bcs_encode_string(&v.fullnode_addresses).into(),
-            votingPower: parse_u256(&v.voting_power),
+        .map(|v| {
+            let stake_amount = parse_u256(&v.stake_amount);
+            let voting_power = parse_u256(&v.voting_power);
+            assert!(
+                stake_amount == voting_power,
+                "validator {:?} (operator {}): votingPower ({}) must equal stakeAmount ({}) at genesis — \
+                 voting weight cannot be decoupled from staked bond",
+                v.moniker, v.operator, voting_power, stake_amount,
+            );
+            SolInitialValidator {
+                operator: parse_address(&v.operator),
+                owner: parse_address(&v.owner),
+                staker: parse_address(&v.staker),
+                stakeAmount: stake_amount,
+                moniker: v.moniker.clone(),
+                consensusPubkey: parse_hex_bytes(&v.consensus_pubkey).into(),
+                consensusPop: parse_hex_bytes(&v.consensus_pop).into(),
+                // BCS encode network addresses from human-readable format
+                networkAddresses: bcs_encode_string(&v.network_addresses).into(),
+                fullnodeAddresses: bcs_encode_string(&v.fullnode_addresses).into(),
+                votingPower: voting_power,
+            }
         })
         .collect();
 
@@ -620,23 +637,30 @@ pub fn call_get_active_validators() -> TxEnv {
     new_system_call_txn(VALIDATOR_MANAGER_ADDR, call_data.into())
 }
 
-pub fn print_active_validators_result(result: &ExecutionResult, config: &GenesisConfig) {
-    let _ = handle_execution_result(result, "getActiveValidators", |output_bytes| {
+pub fn print_active_validators_result(
+    result: &ExecutionResult,
+    config: &GenesisConfig,
+) -> Result<(), String> {
+    handle_execution_result(result, "getActiveValidators", |output_bytes| {
         let decoded =
             IValidatorManagement::getActiveValidatorsCall::abi_decode_returns(output_bytes, false)
-                .expect("Failed to decode getActiveValidators result");
+                .map_err(|e| format!("Failed to decode getActiveValidators result: {:?}", e))?;
 
         let validators = &decoded._0;
         info!("Active validators count: {}", validators.len());
 
-        // Validate against config
+        // A count mismatch means the on-chain initialize silently dropped
+        // (or duplicated) a validator versus the input config. Fail loudly
+        // so CI pipelines that gate releases on `genesis-tool generate`
+        // cannot ship a divergent artifact behind a buried error log.
         if validators.len() != config.validators.len() {
-            error!(
-                "❌ Validator count mismatch! Expected: {}, Actual: {}",
+            let msg = format!(
+                "validator count mismatch: config has {}, on-chain getActiveValidators returned {}",
                 config.validators.len(),
                 validators.len()
             );
-            return;
+            error!("❌ {}", msg);
+            return Err(msg);
         }
 
         for (i, validator) in validators.iter().enumerate() {
@@ -662,5 +686,6 @@ pub fn print_active_validators_result(result: &ExecutionResult, config: &Genesis
             "🎉 All {} validators initialized successfully!",
             validators.len()
         );
-    });
+        Ok(())
+    })
 }

genesis-tool/src/main.rs

@@ -177,7 +177,8 @@ async fn run_generate(byte_code_dir: &str, config_file: &str, output: &str) -> R
         db,
         bundle_state,
         &config,
-    );
+    )
+    .map_err(|e| anyhow::anyhow!("Genesis post-generation verification failed: {}", e))?;
 
     info!("Gravity Genesis Generate completed successfully");
     Ok(())

genesis-tool/src/post_genesis.rs

@@ -14,9 +14,15 @@ use crate::{
 ///
 /// This function provides a common structure for all print_* functions,
 /// reducing code duplication and making the codebase more maintainable.
+///
+/// The `success_handler` returns `Result<(), String>` so that semantic
+/// failures discovered during decoding (e.g. validator-count mismatch
+/// against the input config) propagate as `Err` rather than silently
+/// logging — otherwise `run_generate` would exit 0 on a broken artifact
+/// and ship it through CI.
 pub fn handle_execution_result<F>(result: &ExecutionResult, function_name: &str, success_handler: F) -> Result<(), String>
 where
-    F: FnOnce(&[u8]),
+    F: FnOnce(&[u8]) -> Result<(), String>,
 {
     match result {
         ExecutionResult::Success { output, .. } => {
@@ -33,8 +39,7 @@ where
                 info!("Raw output (truncated): 0x{}...", hex::encode(&output_bytes[..64]));
             }
 
-            success_handler(output_bytes);
-            Ok(())
+            success_handler(output_bytes)
         }
         ExecutionResult::Revert { output, .. } => {
             error!("{} call reverted", function_name);
@@ -88,23 +93,20 @@ fn verify_active_validators(db: impl DatabaseRef, bundle_state: BundleState, con
         "active validators",
         config.chain_id,
         resolve_block_timestamp(config),
-        |result| {
-            print_active_validators_result(result, config);
-            Ok(())
-        },
+        |result| print_active_validators_result(result, config),
     )
 }
 
 pub fn verify_result(
     db: InMemoryDB,
     bundle_state: BundleState,
     config: &GenesisConfig,
-) {
-    verify_active_validators(db.clone(), bundle_state.clone(), config)
-        .expect("Genesis verification: active validators check FAILED");
+) -> Result<(), String> {
+    verify_active_validators(db.clone(), bundle_state.clone(), config)?;
     // Add more verification steps as needed:
     // - verify_jwks()
     // - verify_epoch_config()
     // - verify_randomness_config()
     // etc.
+    Ok(())
 }

genesis-tool/src/verify.rs

@@ -108,6 +108,8 @@ pub fn verify_genesis_file(genesis_path: &str) -> Result<VerifyResult> {
             .balance
             .as_ref()
             .map(|b| parse_u256_hex(b))
+            .transpose()
+            .with_context(|| format!("Failed to parse balance for account {}", addr_str))?
             .unwrap_or(U256::ZERO);
 
         let nonce = entry.nonce.unwrap_or(0);
@@ -141,8 +143,15 @@ pub fn verify_genesis_file(genesis_path: &str) -> Result<VerifyResult> {
         // Insert storage
         if let Some(storage) = &entry.storage {
             for (key_str, value_str) in storage {
-                let key = parse_u256_hex(key_str);
-                let value = parse_u256_hex(value_str);
+                let key = parse_u256_hex(key_str).with_context(|| {
+                    format!("Failed to parse storage key for account {}", addr_str)
+                })?;
+                let value = parse_u256_hex(value_str).with_context(|| {
+                    format!(
+                        "Failed to parse storage value for account {} key {}",
+                        addr_str, key_str
+                    )
+                })?;
                 db.insert_account_storage(addr, key, value)
                     .expect("Failed to insert storage");
             }
@@ -347,12 +356,20 @@ fn process_execution_result(
     }
 }
 
-fn parse_u256_hex(s: &str) -> U256 {
-    let s = s.strip_prefix("0x").unwrap_or(s);
-    if s.is_empty() {
-        return U256::ZERO;
+/// Parse a 0x-prefixed (or bare) hex string into a U256.
+///
+/// Returns `Err` on malformed input rather than silently defaulting to zero.
+/// The `verify` subcommand is the independent release-gate check, so a
+/// trivially malformed balance/storage value (e.g. a decimal string) must
+/// surface as a verification failure — not a silent zero that lets a
+/// corrupted alloc pass review.
+fn parse_u256_hex(s: &str) -> Result<U256> {
+    let stripped = s.strip_prefix("0x").unwrap_or(s);
+    if stripped.is_empty() {
+        return Ok(U256::ZERO);
     }
-    U256::from_str_radix(s, 16).unwrap_or(U256::ZERO)
+    U256::from_str_radix(stripped, 16)
+        .map_err(|e| anyhow!("invalid hex U256 value {:?}: {}", s, e))
 }
 
 /// Print verification summary