docs: catastrophic failure pre-mortem and multi-chain feasibility analysis#30
Conversation
… analysis Comprehensive pre-mortem analysis covering: - Top 5 project killers (bond ratio, no timelock, arbitrator incentive, no cross-chain, governance ossification) - 83 findings across 8 categories (SC, EC, GV, OF, PK, AG, MC, LF) - Economic attack scenarios with code-verified parameters - Multi-chain feasibility (hub-and-spoke recommended) - Infinite lifecycle requirements - 4-phase remediation roadmap (pre-mainnet through governance) - Risk matrix with likelihood x impact scoring Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughAdds a comprehensive pre-mortem security analysis document for the IRSB protocol, detailing 83 findings across smart contracts, economic models, governance, cross-chain architecture, and lifecycle risks. Includes executive summary, attack scenarios, severity levels, and a phased remediation roadmap. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
Summary of ChangesHello @jeremylongshore, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a critical new documentation asset: a comprehensive pre-mortem analysis for the IRSB protocol. This document meticulously details 83 unique findings across various categories, including smart contract vulnerabilities, economic attack vectors, governance risks, and multi-chain challenges. It identifies five existential threats to the protocol and proposes a structured, multi-phase remediation roadmap to address these issues, ultimately aiming to enhance the protocol's security, economic stability, and long-term decentralization. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive pre-mortem analysis document. The document is exceptionally detailed and identifies numerous critical risks and architectural issues. My review focuses on ensuring the internal consistency of the document and its alignment with the provided codebase. I've identified a couple of potential inaccuracies in the findings that should be addressed to ensure the document's correctness.
| **PM-PK-004 | SDK GraphQL query parameter injection** | ||
| - **File:** protocol/sdk/src/api/walletApi.ts:181-226 | ||
| - **Issue:** GraphQL queries constructed with string interpolation of user-supplied parameters. No parameterization or input sanitization. | ||
| - **Impact:** GraphQL injection, data exfiltration from subgraph | ||
| - **Status:** NEW |
There was a problem hiding this comment.
The finding PM-PK-004 states that GraphQL queries are constructed with string interpolation, which could lead to injection vulnerabilities. On reviewing protocol/sdk/src/api/walletApi.ts, the implementation appears to use parameterized queries via the graphql-request library (e.g., this.client.request(QUERY, { variables })). This is a standard practice that prevents GraphQL injection. The assessment in this finding may be incorrect as the code does not seem to use string interpolation for queries.
| | Path | User | Challenger | Treasury | Arbitrator | | ||
| |------|------|-----------|----------|------------| | ||
| | V1 Deterministic (Hub:274-295) | 80% | 15% | 5% | 0% | | ||
| | V2 Arbitration (ODM:251-274) | 70% | 0% | 20% | 10% | |
There was a problem hiding this comment.
The "V2 Arbitration" row in this table states that the Challenger receives 0% of the slash. However, the referenced code in OptimisticDisputeModule.sol (lines 251-274, specifically line 262) shows that the userShare (70%) is paid to dispute.challenger. The table should likely show 70% for the Challenger to accurately reflect the code's behavior. This inaccuracy could affect the analysis of economic incentives.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@000-docs/039-AA-AUDT-pre-mortem-analysis.md`:
- Around line 917-928: Update the remediation/prioritization in the document to
mark PM-LF-003 as a high-priority roadmap item: explicitly add a prioritized
implementation path to introduce a protocol fee on receipt finalization (in
addition to current slash-based revenue referenced at Types.sol:141 and
OptimisticDisputeModule.sol:36), describe the fee mechanics and rollout options
(e.g., flat vs. percentage, opt-in subscription tier for premium features), and
move the fee proposal to top of the remediation list with clear owner/next-steps
and timeline to address the infinite-lifecycle risk.
🧹 Nitpick comments (2)
000-docs/039-AA-AUDT-pre-mortem-analysis.md (2)
24-167: Excellent executive summary with actionable threat analysis.The five project killers are well-articulated with clear attack narratives and impact assessments. The code evidence strengthens credibility significantly.
However, since this PR only adds documentation without contract changes, be aware that:
- Specific line number references (e.g., "SolverRegistry.sol:18") will become stale as contracts evolve
- Claims about "verified against deployed Solidity source code" (line 26) cannot be validated within this PR's scope
💡 Recommendation: Make references more resilient to code changes
Consider supplementing exact line numbers with function/constant names that are more stable:
-**Code Evidence:** `MINIMUM_BOND = 0.1 ether` (SolverRegistry.sol:18) +**Code Evidence:** `MINIMUM_BOND = 0.1 ether` (SolverRegistry.sol, constant declaration in contract)This maintains specificity while reducing maintenance burden when line numbers shift.
Based on learnings, the timelock issue (Killer 2) correctly identifies that multisig governance improvement is pending, and the cross-chain analysis (Killer 4) aligns with IRSB's stated goal of becoming a cross-protocol standard.
839-852: Add language identifier to fenced code block for proper rendering.The ASCII diagram illustrating the hub-and-spoke architecture is helpful, but the fenced code block should specify a language identifier for proper syntax highlighting and rendering across different markdown viewers.
✨ Suggested improvement
-``` +```text ┌──────────────────────┐ │ L1 Canonical Hub │This ensures consistent rendering and satisfies markdown linters.
|
|
||
| **Current State:** Protocol has no revenue mechanism. Treasury receives 5% of v1 slashes (Types.sol:141) and 20% of v2 slashes (OptimisticDisputeModule.sol:36). This only generates revenue when disputes occur and solvers are slashed. | ||
|
|
||
| **Problem:** A healthy protocol has few disputes. If IRSB works well, disputes are rare, and treasury revenue approaches zero. The protocol cannot fund its own maintenance, upgrades, or multi-chain expansion. | ||
|
|
||
| **Risk:** HIGH for infinite lifecycle | ||
|
|
||
| **Remediation:** | ||
| - Protocol fee on receipt finalization (not just slashes) | ||
| - Optional subscription tier for premium features (faster dispute resolution, priority arbitration) | ||
| - Grant/bounty funding from Ethereum Foundation, ERC-8004 ecosystem | ||
|
|
There was a problem hiding this comment.
Critical insight: revenue model creates perverse incentive paradox.
PM-LF-003 identifies a fundamental economic tension:
"A healthy protocol has few disputes. If IRSB works well, disputes are rare, and treasury revenue approaches zero."
This is a profound observation that connects protocol health to financial sustainability. The current model (treasury receives percentage of slashes) creates a paradox where successful operation undermines funding for maintenance and development.
The suggested remediation (protocol fee on receipt finalization, not just slashes) would align economic incentives correctly. This finding deserves prioritization in the roadmap.
The economic sustainability gap poses a threat to infinite lifecycle viability, as protocols without self-sustaining revenue eventually depend on external funding that may not persist.
🤖 Prompt for AI Agents
In `@000-docs/039-AA-AUDT-pre-mortem-analysis.md` around lines 917 - 928, Update
the remediation/prioritization in the document to mark PM-LF-003 as a
high-priority roadmap item: explicitly add a prioritized implementation path to
introduce a protocol fee on receipt finalization (in addition to current
slash-based revenue referenced at Types.sol:141 and
OptimisticDisputeModule.sol:36), describe the fee mechanics and rollout options
(e.g., flat vs. percentage, opt-in subscription tier for premium features), and
move the fee proposal to top of the remediation list with clear owner/next-steps
and timeline to address the infinite-lifecycle risk.
1 participant
Summary
Key Findings
4 CRITICAL-zone findings (risk score >= 15):
Findings breakdown:
Deliverable
Single document:
protocol/000-docs/039-AA-AUDT-pre-mortem-analysis.md(1,258 lines)8 parts:
Test plan
protocol/000-docs/with proper naming convention (039-AA-AUDT)🤖 Generated with Claude Code
Summary by CodeRabbit