Skip to content

Refactor Telegram attestation verification to support 'validate' mode…#432

Merged
tcsenpai merged 2 commits intotg_identitiesfrom
fix-tg-identities-flow
Aug 20, 2025
Merged

Refactor Telegram attestation verification to support 'validate' mode…#432
tcsenpai merged 2 commits intotg_identitiesfrom
fix-tg-identities-flow

Conversation

@SergeyG-Solicy
Copy link
Contributor

@SergeyG-Solicy SergeyG-Solicy commented Aug 19, 2025

User description

Description:

  • Added handling for logic to fetch genesis blocks
  • Added modes 'attest' | 'validate' for verifyAttestation avoid of errors when after validation we mark Challenge as used and get error during broadcast validation.

PR Type

Enhancement


Description

  • Added 'validate' mode to Telegram attestation verification

  • Fixed genesis block parsing for bot authorization

  • Improved challenge format parsing and error handling

  • Enhanced transaction structure with required fields


Diagram Walkthrough

flowchart LR
  A["TelegramProofParser"] -- "calls with 'validate' mode" --> B["verifyAttestation"]
  B -- "parses genesis block" --> C["Bot Authorization"]
  B -- "validates challenge format" --> D["Challenge Verification"]
  D -- "allows reused challenges in validate mode" --> E["Signature Verification"]
  E -- "creates identity transaction" --> F["Transaction Output"]
Loading

File Walkthrough

Relevant files
Enhancement
telegram.ts
Switch to validate mode for attestation verification         

src/libs/abstraction/web2/telegram.ts

  • Modified verifyAttestation call to use 'validate' mode
  • Added comment explaining why validate mode is needed
+3/-1     
telegram.ts
Major refactor of Telegram verification logic                       

src/libs/identity/tools/telegram.ts

  • Added 'validate' mode parameter to verifyAttestation method
  • Fixed genesis block content parsing with proper JSON handling
  • Corrected challenge parsing format (6 parts instead of 5)
  • Enhanced transaction structure with required fields
  • Replaced SDK import with local interface definition
+68/-22 

Summary by CodeRabbit

  • New Features

    • Identity transactions now include sender info, transaction fees and pending/broadcast metadata.
    • Telegram proofs can optionally use transaction context for on‑chain validation and replay protection.
  • Bug Fixes

    • More robust handling of varying genesis data formats to preserve balances.
    • Fixed Telegram challenge parsing to prevent mismatches.
  • Improvements

    • Verification flow refined so challenges are only marked used during interactive attestations; non‑interactive validation proceeds without consuming challenges.

@qodo-code-review
Copy link
Contributor

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

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

Possible Issue

The updated challenge parser expects 6 parts and uses indices 3/4/5, but returns null immediately without checking array length in the old return path. Ensure all callsites providing challenges actually match "DEMOS_TG_BIND_

" to avoid false negatives.

private parseChallenge(challenge: string): {
    demosAddress: string
    timestamp: number
    nonce: string
} | null {
    const parts = challenge.split("_")
    // Expected format: DEMOS_TG_BIND_<demos_address>_<timestamp>_<nonce>
    if (parts.length !== 6 || parts[0] !== "DEMOS" || parts[1] !== "TG" || parts[2] !== "BIND") {
        return null
    }
    const demosAddress = parts[3]
    const timestamp = parseInt(parts[4])
    const nonce = parts[5]
    if (isNaN(timestamp)) {
        return null
    }

    return {
        demosAddress,
        timestamp,
        nonce,
    }
}
Type Safety

Casting fields like 'ed25519_signature', 'status' with 'as any' in the created Transaction may hide structural mismatches with the SDK's Transaction type. Verify the Transaction interface requires these fields and prefer proper enums/types to avoid runtime issues.

                payload: telegramPayload, // Telegram-specific payload
            },
        ],
        timestamp: Date.now(),
        nonce: 0, // Will be set by transaction processing
        gcr_edits: [], // Will be generated during transaction validation
    },
    signature: null, // User must sign this
    ed25519_signature: null as any,
    status: "pending" as any,
    blockNumber: 0,
}
Robustness

Genesis content parsing has multiple branches; if balances are missing or not in expected tuple format, map may throw or produce undefined. Add guards for 'genesisData.balances' being an array of [address, amount] and handle unexpected shapes gracefully.

// genesisBlock.content is a JSON object (typeorm column json), NOT a string.
// The original genesis data we need (balances) is embedded as string in content.extra.genesisData.
let genesisData: any
if (typeof genesisBlock.content === "string") {
    // (unlikely) stored as string JSON
    genesisData = JSON.parse(genesisBlock.content)
} else if (
    typeof genesisBlock.content === "object" &&
    genesisBlock.content?.extra?.genesisData
) {
    // extra.genesisData is a JSON string created in generateGenesisBlock
    try {
        genesisData = JSON.parse(genesisBlock.content.extra.genesisData)
    } catch (e) {
        log.error("Failed to parse embedded genesisData string:" + e)
        return []
    }
} else {
    // Fallback: maybe balances directly present
    genesisData = genesisBlock.content
}

// Extract addresses from balances array
this.authorizedBots = genesisData.balances?.map((balance: [string, string]) => 
    balance[0].toLowerCase(),

@qodo-code-review
Copy link
Contributor

qodo-code-review bot commented Aug 19, 2025

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
High-level
Validate-mode bypass risk

Allowing 'validate' mode to skip missing/used challenge checks opens replay risk
if an attacker replays an old attestation with a still-authorized bot key. Tie
validation to a unique transaction hash or embed the challenge hash in the
on-chain payload and require it to match a recorded attestation
timestamp/window, so cryptographic verification alone isn’t sufficient for
acceptance.

Examples:

src/libs/identity/tools/telegram.ts [180-183]
    async verifyAttestation(
        request: TelegramVerificationRequest,
        mode: 'attest' | 'validate' = 'attest',
    ): Promise<TelegramVerificationResponse> {
src/libs/identity/tools/telegram.ts [203-218]
            // 3. Fetch stored challenge (in-memory). For 'validate' we tolerate absence or reuse.
            const storedChallenge = this.challenges.get(
                `DEMOS_TG_BIND_${challengeData.demosAddress}_${challengeData.timestamp}_${challengeData.nonce}`,
            )
            if (!storedChallenge && mode === 'attest') {
                return {
                    success: false,
                    message: 'Challenge not found or expired',
                }
            }

 ... (clipped 6 lines)

Solution Walkthrough:

Before:

// In verifyAttestation
async verifyAttestation(request, mode = 'attest') {
  // ...
  // Challenge existence and 'used' status are only checked in 'attest' mode.
  // 'validate' mode skips these checks entirely.
  if (!storedChallenge && mode === 'attest') {
    return { success: false, message: 'Challenge not found' };
  }
  if (storedChallenge?.used && mode === 'attest') {
    return { success: false, message: 'Challenge already used' };
  }
  // ...
  // Signatures are verified, but the proof itself can be replayed in 'validate' mode.
}

After:

// In createIdentityTransaction
createIdentityTransaction(...) {
  const challengeString = ...; // Extract from signed_challenge
  const challengeHash = crypto.hash(challengeString);

  const transaction = {
    // ...
    content: {
      // ...
      data: [
        "identity",
        {
          context: "web2",
          payload: { ... },
          // Embed a unique identifier from the attestation
          attestation_id: challengeHash 
        },
      ],
    },
  };
  return transaction;
}
Suggestion importance[1-10]: 9

__

Why: This suggestion correctly identifies a critical replay attack vulnerability introduced by the 'validate' mode, which bypasses challenge reuse checks, and proposes a robust architectural solution.

High
Possible issue
Harden genesis balances parsing

Add defensive checks to ensure genesisData.balances is an array before mapping,
and handle malformed entries to prevent runtime exceptions. Log and return an
empty list if the structure is unexpected.

src/libs/identity/tools/telegram.ts [73-95]

 let genesisData: any
 if (typeof genesisBlock.content === "string") {
-    // (unlikely) stored as string JSON
     genesisData = JSON.parse(genesisBlock.content)
 } else if (
     typeof genesisBlock.content === "object" &&
     genesisBlock.content?.extra?.genesisData
 ) {
-    // extra.genesisData is a JSON string created in generateGenesisBlock
     try {
         genesisData = JSON.parse(genesisBlock.content.extra.genesisData)
     } catch (e) {
         log.error("Failed to parse embedded genesisData string:" + e)
         return []
     }
 } else {
-    // Fallback: maybe balances directly present
     genesisData = genesisBlock.content
 }
 
-// Extract addresses from balances array
-this.authorizedBots = genesisData.balances?.map((balance: [string, string]) => 
-    balance[0].toLowerCase(),
+const balances = Array.isArray(genesisData?.balances) ? genesisData.balances : null
+if (!balances) {
+    log.error("Genesis data missing or invalid balances array")
+    this.authorizedBots = []
+    this.lastGenesisCheck = Date.now()
+    return []
+}
+this.authorizedBots = balances
+    .filter((b: any) => Array.isArray(b) && typeof b[0] === 'string')
+    .map((balance: [string, any]) => balance[0].toLowerCase())

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 7

__

Why: The suggestion improves robustness by adding validation for the genesisData.balances structure before mapping over it, preventing potential runtime errors from malformed data.

Medium
Validate challenge input presence

Guard against undefined or empty request.signed_challenge before splitting to
avoid runtime errors. Explicitly validate that a non-empty string is provided
and bail out early with a clear message.

src/libs/identity/tools/telegram.ts [195]

-const challengeData = this.parseChallenge(request.signed_challenge.split(":")[0] || request.signed_challenge)
+if (!request.signed_challenge || typeof request.signed_challenge !== 'string') {
+    return {
+        success: false,
+        message: 'Missing or invalid signed_challenge',
+    }
+}
+const rawChallenge = request.signed_challenge.split(':')[0]
+const challengeData = this.parseChallenge(rawChallenge)

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 6

__

Why: The suggestion correctly identifies a potential runtime error if request.signed_challenge is null or undefined and proposes a robust guard clause, improving error handling.

Low
General
Validate challenge fields rigorously

Add basic validation for demosAddress and nonce to avoid accepting malformed
challenges that could bypass checks. Ensure demosAddress is a non-empty hex-like
string and nonce is a non-empty string of expected length/charset.

src/libs/identity/tools/telegram.ts [146-168]

 private parseChallenge(challenge: string): {
     demosAddress: string
     timestamp: number
     nonce: string
 } | null {
     const parts = challenge.split("_")
     // Expected format: DEMOS_TG_BIND_<demos_address>_<timestamp>_<nonce>
     if (parts.length !== 6 || parts[0] !== "DEMOS" || parts[1] !== "TG" || parts[2] !== "BIND") {
         return null
     }
     const demosAddress = parts[3]
     const timestamp = parseInt(parts[4])
     const nonce = parts[5]
     if (isNaN(timestamp)) {
         return null
     }
+    // Basic sanity checks
+    if (typeof demosAddress !== 'string' || demosAddress.length === 0) {
+        return null
+    }
+    if (typeof nonce !== 'string' || nonce.length < 8) {
+        return null
+    }
 
     return {
         demosAddress,
         timestamp,
         nonce,
     }
 }

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 6

__

Why: The suggestion correctly identifies that the demosAddress and nonce fields from the parsed challenge lack validation, and adding these checks improves the robustness of the challenge parsing logic.

Low
  • Update

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Aug 19, 2025

Walkthrough

Telegram verification and transaction handling were extended: web2 parser and abstraction now accept an optional Transaction and invoke verification in 'validate' mode; identity tools add a 'validate' mode, stronger parsing, transaction-aware challenge hashing, and updated transaction/payload shapes and signatures.

Changes

Cohort / File(s) Change Summary
Web2 parser
src/libs/abstraction/web2/telegram.ts
TelegramProofParser.readData signature changed to readData(proofData: string, transaction?: Transaction); when transaction present, extracts challenge hash and calls verifyAttestation(attestationData, 'validate', transactionChallengeHash); comments added; error/return shape unchanged.
Abstraction entrypoint
src/libs/abstraction/index.ts
verifyWeb2Proof(payload, sender, transaction?) signature added optional Transaction; telegram path now forwards transaction into parser (payload.context === 'telegram' ? transaction : undefined).
Identity tools
src/libs/identity/tools/telegram.ts
`verifyAttestation(request, mode = 'attest'
Network routine (usage)
src/libs/network/routines/transactions/handleIdentityRequest.ts
When handling web2_identity_assign, now passes the transaction tx as the third argument to verifyWeb2Proof to enable Telegram transaction-context validation; no other control-flow changes.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant Client as Client App
  participant Parser as TelegramProofParser
  participant TG as Identity.Tools.Telegram
  participant Crypto as Crypto Verify
  participant Store as Challenge Store
  note over Client,Parser: Read/validation flow (non-interactive)
  Client->>Parser: readData(proofData, transaction)
  Parser->>TG: verifyAttestation(attestationData, "validate", txHash)
  TG->>Crypto: verify bot & user signatures (uses txHash for replay check)
  Crypto-->>TG: verification result
  TG-->>Parser: validation result (does not mark challenge used)
  Parser-->>Client: signature data / error
Loading
sequenceDiagram
  autonumber
  participant User as End User
  participant App as Client App
  participant TG as Identity.Tools.Telegram
  participant Store as Challenge Store
  participant Crypto as Crypto Verify
  note over User,App: Interactive attestation flow (mode: "attest")
  User->>App: submit attestation request
  App->>TG: verifyAttestation(request, "attest")
  TG->>Store: load stored challenge (must exist & unused)
  Store-->>TG: challenge record
  TG->>Crypto: verify bot & user signatures
  Crypto-->>TG: verification result
  alt valid
    TG->>Store: mark challenge used
    TG-->>App: success response
  else invalid
    TG-->>App: error response
  end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Poem

A rabbit scurried through the code and pen,
I tracked a nonce, then tossed it in my den.
"Attest" for the handshake, "Validate" for the read—
I stitched the hash to keep replay misdeed freed.
Hops of joy, a tidy change — carrot-approved commit! 🥕

Tip

🔌 Remote MCP (Model Context Protocol) integration is now available!

Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats.


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Free

💡 Knowledge Base configuration:

  • Jira integration is disabled
  • Linear integration is disabled

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between ecfb0a9 and 7986fc0.

📒 Files selected for processing (4)
  • src/libs/abstraction/index.ts (3 hunks)
  • src/libs/abstraction/web2/telegram.ts (3 hunks)
  • src/libs/identity/tools/telegram.ts (10 hunks)
  • src/libs/network/routines/transactions/handleIdentityRequest.ts (1 hunks)

Note

🎁 Summarized by CodeRabbit Free

Your organization is on the Free plan. CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please upgrade your subscription to CodeRabbit Pro by visiting https://app.coderabbit.ai/login.

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Join our Discord community for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@tcsenpai
Copy link
Contributor

High-level
Validate-mode bypass risk

I'd look at this from #432 (comment) and then it looks good @SergeyG-Solicy

@tcsenpai tcsenpai merged commit 641658e into tg_identities Aug 20, 2025
1 check passed
@tcsenpai tcsenpai deleted the fix-tg-identities-flow branch August 20, 2025 13:47
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.

2 participants