Skip to content

Conversation

jintukumardas
Copy link
Contributor

@jintukumardas jintukumardas commented Aug 5, 2025

PR Type

Enhancement


Description

  • Add detailed debug logs for transaction state transitions

  • Enhance logging in account retrieval and cache checks

  • Improve traceability for local and remote account handling

  • No functional changes, logging only


Changes walkthrough 📝

Relevant files
Enhancement
AccountSync.ts
Add debug log for transaction aging phase transition         

src/state-manager/AccountSync.ts

  • Added a debug log when a transaction moves to the aging phase.
  • Log includes transaction ID, timestamp, and account count.
  • +1/-0     
    TransactionQueue.ts
    Add debug log for transaction aging to processing transition

    src/state-manager/TransactionQueue.ts

  • Added a debug log when a transaction finishes aging and moves to
    processing.
  • Log includes transaction ID, timestamp, and aging duration.
  • +1/-0     
    index.ts
    Add verbose debug logs for account retrieval and cache checks

    src/state-manager/index.ts

  • Added debug logs for RI cache checks (hit/miss) in account retrieval.
  • Added logs for local vs remote account determination.
  • Improved traceability for account data source decisions.
  • +5/-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

    github-actions bot commented Aug 5, 2025

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

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

    Logging Noise

    The addition of multiple debug logs under the dapp_verbose flag may increase log verbosity significantly. Ensure this does not impact performance or overwhelm log storage in production environments.

      /* prettier-ignore */ if (logFlags.dapp_verbose) console.log(`[getLocalOrRemoteAccount] Checking RI cache - addr:${address}`)
      const riCacheResult = await this.app.getCachedRIAccountData([address])
      if (riCacheResult != null) {
        if (riCacheResult.length > 0) {
          /* prettier-ignore */ if (logFlags.dapp_verbose) console.log(`[getLocalOrRemoteAccount] RI cache HIT - addr:${address} data:${riCacheResult[0] != null}`)
          nestedCountersInstance.countEvent('stateManager', 'getLocalOrRemoteAccount: RI cache hit')
          if (logFlags.verbose) this.mainLogger.debug(`getLocalOrRemoteAccount: RI cache hit for ${address}`)
          wrappedAccount = riCacheResult[0] as ShardusTypes.WrappedDataFromQueue
          return wrappedAccount
        }
      } else {
        /* prettier-ignore */ if (logFlags.dapp_verbose) console.log(`[getLocalOrRemoteAccount] RI cache MISS - addr:${address}`)
        nestedCountersInstance.countEvent('stateManager', 'getLocalOrRemoteAccount: RI cache miss')
      }
    }
    
    let forceLocalGlobalLookup = false
    
    if (this.accountGlobals.isGlobalAccount(address) || isServiceMode()) {
      forceLocalGlobalLookup = true
    }
    
    //it seems backwards that isServiceMode would treat the account as always remote, as it has access to all data locally
    let accountIsRemote = isServiceMode() ? true : this.transactionQueue.isAccountRemote(address)
    
    // hack to say we have all the data
    if (!isServiceMode()) {
      if (this.currentCycleShardData.nodes.length <= this.currentCycleShardData.shardGlobals.consensusRadius) {
        accountIsRemote = false
      }
    }
    if (forceLocalGlobalLookup) {
      accountIsRemote = false
    }
    
    if (accountIsRemote) {
      /* prettier-ignore */ if (logFlags.dapp_verbose) console.log(`[getLocalOrRemoteAccount] Account is REMOTE - addr:${address} useRICache:${opts.useRICache} canThrow:${opts.canThrowException}`)
      let randomConsensusNode: P2PTypes.NodeListTypes.Node
      const preCheckLimit = 5
      for (let i = 0; i < preCheckLimit; i++) {
        randomConsensusNode = this.transactionQueue.getRandomConsensusNodeForAccount(address)
        if (randomConsensusNode == null) {
          nestedCountersInstance.countEvent('getLocalOrRemoteAccount', `precheck: no consensus node found`)
          throw new Error(`getLocalOrRemoteAccount: no consensus node found`)
        }
        // Node Precheck!.  this check our internal records to find a good node to talk to.
        // it is worth it to look through the list if needed.
        if (
          this.isNodeValidForInternalMessage(
            randomConsensusNode.id,
            'getLocalOrRemoteAccount',
            true,
            true,
            true,
            true
          ) === false
        ) {
          //we got to the end of our tries?
          if (i >= preCheckLimit - 1) {
            /* prettier-ignore */ if (logFlags.verbose || logFlags.getLocalOrRemote) this.getAccountFailDump(address, 'getLocalOrRemoteAccount: isNodeValidForInternalMessage failed, no retry')
            //return null   ....better to throw an error
            if (opts.canThrowException) {
              nestedCountersInstance.countEvent('getLocalOrRemoteAccount', `precheck: out of nodes to try`)
              throw new Error(`getLocalOrRemoteAccount: no consensus nodes worth asking`)
            } else return null
          }
        } else {
          break
        }
      }
    
      const message = { accountIds: [address] }
    
      let r: GetAccountDataWithQueueHintsResp
    
      // if (
      //   this.config.p2p.useBinarySerializedEndpoints &&
      //   this.config.p2p.getAccountDataWithQueueHintsBinary
      // ) {
      try {
        const serialized_res = await this.p2p.askBinary<
          GetAccountDataWithQueueHintsReqSerializable,
          GetAccountDataWithQueueHintsRespSerializable
        >(
          randomConsensusNode,
          InternalRouteEnum.binary_get_account_data_with_queue_hints,
          message,
          serializeGetAccountDataWithQueueHintsReq,
          deserializeGetAccountDataWithQueueHintsResp,
          {}
        )
        r = serialized_res as GetAccountDataWithQueueHintsResp
      } catch (er) {
        if (er instanceof ResponseError && logFlags.error) {
          this.mainLogger.error(
            `ASK FAIL getLocalOrRemoteAccount exception: ResponseError encountered. Code: ${er.Code}, AppCode: ${er.AppCode}, Message: ${er.Message}`
          )
        }
        if (logFlags.verbose || logFlags.getLocalOrRemote) this.mainLogger.error('askBinary', er)
        if (opts.canThrowException) {
          throw er
        } else {
          nestedCountersInstance.countEvent('getLocalOrRemoteAccount', `askBinary ex: ${er?.message}`)
        }
      }
      // } else {
      // r = await this.p2p.ask(randomConsensusNode, 'get_account_data_with_queue_hints', message)
      // }
    
      if (!r) {
        if (logFlags.error || logFlags.getLocalOrRemote)
          this.mainLogger.error('ASK FAIL getLocalOrRemoteAccount r === false')
        if (opts.canThrowException) throw new Error(`getLocalOrRemoteAccount: remote node had an exception`)
      }
    
      const result = r as GetAccountDataWithQueueHintsResp
      if (result != null && result.accountData != null && result.accountData.length > 0) {
        wrappedAccount = result.accountData[0]
        if (wrappedAccount == null) {
          if (logFlags.verbose || logFlags.getLocalOrRemote)
            this.getAccountFailDump(address, 'remote result.accountData[0] == null')
          nestedCountersInstance.countEvent('getLocalOrRemoteAccount', `remote result.accountData[0] == null`)
        }
        return wrappedAccount
      } else {
        //these cases probably should throw an error to, but dont wont to over prescribe the format yet
        //if the remote node has a major breakdown it should return false
        if (result == null) {
          /* prettier-ignore */ if (logFlags.verbose || logFlags.getLocalOrRemote) this.getAccountFailDump(address, 'remote request missing data: result == null')
          nestedCountersInstance.countEvent('getLocalOrRemoteAccount', `remote else.. result == null`)
        } else if (result.accountData == null) {
          /* prettier-ignore */ if (logFlags.verbose || logFlags.getLocalOrRemote) this.getAccountFailDump(address, 'remote request missing data: result.accountData == null ' + utils.stringifyReduce(result))
          nestedCountersInstance.countEvent('getLocalOrRemoteAccount', `remote else.. result.accountData == null`)
        } else if (result.accountData.length <= 0) {
          /* prettier-ignore */ if (logFlags.verbose || logFlags.getLocalOrRemote) this.getAccountFailDump(address, 'remote request missing data: result.accountData.length <= 0 ' + utils.stringifyReduce(result))
          nestedCountersInstance.countEvent('getLocalOrRemoteAccount', `remote else.. result.accountData.length <= 0 `)
        }
      }
    } else {
      // we are local!
      /* prettier-ignore */ if (logFlags.dapp_verbose) console.log(`[getLocalOrRemoteAccount] Account is LOCAL - addr:${address} useRICache:${opts.useRICache}`)

    Copy link

    github-actions bot commented Aug 5, 2025

    PR Code Suggestions ✨

    No code suggestions found for the PR.

    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.

    1 participant