Skip to content

Conversation

aniketdivekar
Copy link
Contributor

@aniketdivekar aniketdivekar commented Jul 9, 2025

PR Type

Bug fix, Enhancement


Description

  • Add debug logs for account hash mismatches and global account restoration

  • Automatically fix and update mismatched global account hashes in DB

  • Improve logging for account data and global account report API endpoints

  • Refine error and debug output for better traceability


Changes walkthrough 📝

Relevant files
Enhancement
update_network_account_listOfChanges.ts
Add debug logs for listOfChanges length during update       

scripts/update_network_account_listOfChanges.ts

  • Add logs to print listOfChanges length before and after update
  • Improve traceability of changes to network account
  • +2/-0     
    API.ts
    Add debug logs for account data and global account report APIs

    src/API.ts

  • Add debug logs for account data and global account report API
    responses
  • Refine error and debug output for easier debugging
  • Minor formatting fix in object construction
  • +9/-3     
    State.ts
    Refine error logging in cycle record comparison                   

    src/State.ts

  • Improve error logging in compareCycleRecordWithOtherArchivers
  • Add context to error output for easier debugging
  • +1/-1     
    Bug fix
    Collector.ts
    Add debug logs for mismatched account hashes in receipts 

    src/Data/Collector.ts

  • Add detailed debug log for mismatched account hashes
  • Improve error logging with more context for account hash mismatches
  • Minor formatting improvements for code clarity
  • +14/-3   
    GlobalAccount.ts
    Auto-fix and log global account hash mismatches on load   

    src/GlobalAccount.ts

  • Add logic to recalculate and auto-fix global account hash mismatches
  • Add warning log with detailed account info when mismatch is detected
  • Update account and DB if hash mismatch is found
  • +20/-0   

    Need help?
  • Type /help how to ... in the comments thread for any questions about PR-Agent usage.
  • Check out the documentation for more information.
  • Copy link
    Contributor

    github-actions bot commented Jul 9, 2025

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
    🏅 Score: 87
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Automatic Hash Correction

    The code now automatically updates mismatched global account hashes in the database. This is a significant change to data integrity logic and should be carefully reviewed to ensure it does not mask underlying data corruption or introduce unintended side effects.

    export const loadGlobalAccounts = async (): Promise<void> => {
      const sql = `SELECT * FROM accounts WHERE isGlobal=1`
      const values = []
      const accounts = await AccountDB.fetchAccountsBySqlQuery(sql, values)
      for (const account of accounts) {
        const recalculatedHash = accountSpecificHash(account.data)
    
        if (recalculatedHash !== account.hash) {
          Logger.mainLogger.warn(
            '[DEBUG RESTORE] loadGlobalAccounts() - recalculatedHash !== account.hash',
            'accountId:',
            account.accountId,
            'account.hash:',
            account.hash,
            'recalculatedHash:',
            recalculatedHash,
            '\naccount data:',
            StringUtils.safeStringify(account)
          )
          account.hash = recalculatedHash
          account.data.hash = recalculatedHash
          await AccountDB.updateAccount(account)
        }
    Enhanced Debug Logging

    Additional debug logs for account hash mismatches and account data are introduced. Ensure that these logs do not inadvertently leak sensitive information or overwhelm log storage in production environments.

      data: account.data,
      timestamp: account.data.timestamp,
      hash: account.hash,
      cycleNumber: cycle,
      isGlobal: account.isGlobal || false,
    }
    if (account.timestamp !== account.data['timestamp'])
      Logger.mainLogger.error('Mismatched account timestamp', txId, account.accountId)
    if (account.hash !== account.data['hash']) {
      Logger.mainLogger.error(
        '[DEBUG RESTORE] Mismatched account hash',
        txId,
        account.accountId,
        'account.hash:',
        account.hash,
        'account.data.hash:',
        account.data['hash']
      )
    }
    Error Logging Consistency

    The error logging in compareCycleRecordWithOtherArchivers is now more descriptive. Confirm that this does not expose sensitive internal error details and is consistent with the application's logging policy.

      console.error('compareCycleRecordWithOtherArchivers Error', error)
    })

    Signed-off-by: Mehdi Sabraoui <[email protected]>
    Signed-off-by: Mehdi Sabraoui <[email protected]>
    @mssabr01 mssabr01 force-pushed the dev-restore-global-acc-hash branch from 27b56f9 to 1a72e67 Compare July 10, 2025 21:49
    @shardeum shardeum deleted a comment from github-actions bot Jul 10, 2025
    @mssabr01
    Copy link
    Contributor

    do security review

    Copy link
    Contributor

    🔍 Security Review Report

    Click to expand security review details
    Fetching pull request(s) from github...
    https://github.com/shardeum/archiver/pull/279
    https://github.com/shardeum/archiver/pull/279
    Processing PR 1/1: https://github.com/shardeum/archiver/pull/279
    Code review...
    ┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓
    ┃                            Security Audit Report                             ┃
    ┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛
    
                                        Summary                                     
    
    Title: feat: add debug logs Description: This pull request introduces enhanced  
    debugging logs and a data integrity verification mechanism. The changes aim to  
    improve diagnostics, particularly for restore operations, and to ensure the     
    consistency of account data by verifying and correcting hashes upon loading.    
    
    The majority of changes involve adding or improving log messages to provide more
    context for debugging. A notable addition in src/GlobalAccount.ts is a          
    self-healing mechanism that recalculates and updates account hashes if they are 
    found to be inconsistent with the stored account data. This enhances the overall
    data integrity of the system.                                                   
    
    ────────────────────────────────────────────────────────────────────────────────
                                    List of Changes                                 
    
                         Change 1: Debugging Script Enhancement                     
    
     • Pull Request: https://github.com/shardeum/archiver/pull/279                  
     • File: scripts/update_network_account_listOfChanges.ts                        
     • Description: Added console.log statements to a utility script to display the 
       length of listOfChanges before and after modification. This change aids in   
       debugging the script's behavior.                                             
    
                          Change 2: API Logging and Formatting                      
    
     • Pull Request: https://github.com/shardeum/archiver/pull/279                  
     • File: src/API.ts                                                             
     • Description: Introduced more detailed debug logs for account data and global 
       account report endpoints. These logs are tagged with [DEBUG RESTORE] for     
       better context during debugging. A minor formatting change was also included.
    
                    Change 3: Data Collector Logging and Formatting                 
    
     • Pull Request: https://github.com/shardeum/archiver/pull/279                  
     • File: src/Data/Collector.ts                                                  
     • Description: Improved error logging for mismatched account hashes by         
       including the actual mismatched hash values, which is beneficial for         
       debugging. A minor code formatting change was also made.                     
    
                         Change 4: Data Integrity Verification                      
    
     • Pull Request: https://github.com/shardeum/archiver/pull/279                  
     • File: src/GlobalAccount.ts                                                   
     • Description: A data integrity check was added to the loadGlobalAccounts      
       function. This new logic recalculates the hash of each global account's data 
       upon loading and compares it with the stored hash. If an inconsistency is    
       detected, it logs a detailed warning and updates the database with the       
       correct hash, effectively acting as a self-healing mechanism.                
    
                           Change 5: State Management Logging                       
    
     • Pull Request: https://github.com/shardeum/archiver/pull/279                  
     • File: src/State.ts                                                           
     • Description: An error message within a .catch block was made more descriptive
       to provide clearer context when an error occurs during the comparison of     
       cycle records.                                                               
    
    ────────────────────────────────────────────────────────────────────────────────
                              Overall Security Assessment                           
    
    The pull request primarily focuses on improving logging and data integrity. The 
    changes are well-contained and do not introduce any identifiable security       
    vulnerabilities. The most significant change, the self-healing hash verification
    in src/GlobalAccount.ts, is a positive security and robustness enhancement. It  
    helps ensure data integrity by correcting inconsistencies, which is a good      
    defensive programming practice. Other changes are limited to improving          
    diagnostics and do not impact the security posture of the application.          
    
    ────────────────────────────────────────────────────────────────────────────────
                                   Security Findings                                
    
    No security vulnerabilities were found in this review.                          
    
    ────────────────────────────────────────────────────────────────────────────────
                                   Security Concerns                                
    
    There are no security concerns to report regarding these changes. The enhanced  
    logging is set at the debug and warn levels, which is appropriate and shouldn't 
    leak sensitive information in a properly configured production environment.     
    
    ────────────────────────────────────────────────────────────────────────────────
                                         Resume                                     
    
    The changes in this pull request have been reviewed and are considered safe.    
    They improve the application's debuggability and data integrity without         
    introducing any security risks. The pull request is approved from a security    
    standpoint.                                                                     
    
    NO MAJOR SECURITY CONCERNS FOUND                                                
    Done
    
    

    Generated by argus-agent security review

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    3 participants