feat: add blockchain integrity fields to ResolutionProofToken#753
feat: add blockchain integrity fields to ResolutionProofToken#753RohanExploit wants to merge 1 commit into
Conversation
Added `integrity_hash` and `previous_integrity_hash` to the `ResolutionProofToken` model to maintain standard blockchain-style integrity chaining across core entities. Implemented a `token_last_hash_cache` in `backend/cache.py` to prevent redundant database queries and securely generate hashes using HMAC-SHA256 in `backend/resolution_proof_service.py` without degrading performance.
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
✅ Deploy Preview for fixmybharat canceled.
|
🙏 Thank you for your contribution, @RohanExploit!PR Details:
Quality Checklist:
Review Process:
Note: The maintainers will monitor code quality and ensure the overall project flow isn't broken. |
📝 WalkthroughWalkthroughResolution proof tokens now include a tamper-evident integrity hash chain. The ChangesToken Integrity Hashing
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Adds blockchain-style integrity chaining to ResolutionProofToken by introducing integrity_hash and previous_integrity_hash, and using a small in-process cache to avoid repeated “last hash” DB lookups during token generation.
Changes:
- Add
integrity_hash/previous_integrity_hashcolumns toResolutionProofToken. - Compute and persist an HMAC-SHA256 integrity hash during
generate_proof_token, chaining to the previous token’s hash. - Introduce
token_last_hash_cacheto reduce redundant DB queries for the prior hash.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| backend/resolution_proof_service.py | Computes chained integrity hashes for newly generated proof tokens and updates a last-hash cache. |
| backend/models.py | Adds integrity chaining fields to the ResolutionProofToken SQLAlchemy model. |
| backend/cache.py | Adds a dedicated token_last_hash_cache instance used by token generation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Generate Integrity Hash | ||
| prev_hash = token_last_hash_cache.get("last_hash") | ||
| if prev_hash is None: | ||
| # Cache miss: Fetch only the last hash from DB | ||
| last_record = db.query(ResolutionProofToken.integrity_hash).order_by(ResolutionProofToken.id.desc()).first() | ||
| prev_hash = last_record[0] if last_record and last_record[0] else "" | ||
| token_last_hash_cache.set(data=prev_hash, key="last_hash") | ||
|
|
| @@ -203,12 +219,16 @@ def generate_proof_token( | |||
| nonce=nonce, | |||
| token_signature=signature, | |||
| is_used=False, | |||
| integrity_hash=integrity_hash, | |||
| previous_integrity_hash=prev_hash, | |||
| ) | |||
| # Blockchain integrity fields | ||
| integrity_hash = Column(String, nullable=True) | ||
| previous_integrity_hash = Column(String, nullable=True, index=True) |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/resolution_proof_service.py (1)
193-230:⚠️ Potential issue | 🔴 Critical | 🏗️ Heavy liftSerialize token chain-head updates in the database.
This path can fork the token chain: two generators can read the same
prev_hashand commit different rows that both point to it. The in-memorytoken_last_hash_cachealso goes stale across multiple workers/instances, so a warm cache can keep chaining from an old head for up to an hour. Make the previous-hash lookup and head update authoritative in the database and perform them in the same transaction as the new token.One safe pattern
- prev_hash = token_last_hash_cache.get("last_hash") - if prev_hash is None: - last_record = db.query(ResolutionProofToken.integrity_hash).order_by(ResolutionProofToken.id.desc()).first() - prev_hash = last_record[0] if last_record and last_record[0] else "" - token_last_hash_cache.set(data=prev_hash, key="last_hash") + # Read the chain head under a DB lock (or advisory lock) so only one writer + # can advance the `resolution_proof_tokens` chain at a time. + prev_hash = locked_chain_head.last_hash or "" ... - db.commit() - token_last_hash_cache.set(data=integrity_hash, key="last_hash") + db.flush() + locked_chain_head.last_hash = integrity_hash + db.commit()🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/resolution_proof_service.py` around lines 193 - 230, The token chain can fork because prev_hash is read separately and the in-memory token_last_hash_cache is stale; fix by making the previous-hash read and the head update atomic in the DB transaction: inside the same DB session/transaction where you create the new ResolutionProofToken (the block that calls db.add(token), db.commit(), db.refresh(token) and token_last_hash_cache.set), first acquire and lock the canonical chain head row (either SELECT the last ResolutionProofToken row FOR UPDATE or use a dedicated singleton ChainHead model/table) and re-check its integrity_hash as prev_hash, then insert the new ResolutionProofToken with previous_integrity_hash set to that locked value and update the canonical head row to the new integrity_hash before committing; if the locked head changed, abort/retry so two generators cannot fork the chain, and only update token_last_hash_cache after a successful commit.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/models.py`:
- Around line 326-328: The migration for the resolution_proof_tokens table is
missing the integrity_hash and previous_integrity_hash columns (the model
defines them and generate_proof_token writes to them), so update the
resolution_proof_tokens migration block to add a nullable string column
integrity_hash and a nullable string column previous_integrity_hash and create
an index on previous_integrity_hash; ensure the column names and index match the
model fields (integrity_hash, previous_integrity_hash) so writes from
generate_proof_token succeed and the DB schema stays consistent with the
ResolutionProofToken model.
---
Outside diff comments:
In `@backend/resolution_proof_service.py`:
- Around line 193-230: The token chain can fork because prev_hash is read
separately and the in-memory token_last_hash_cache is stale; fix by making the
previous-hash read and the head update atomic in the DB transaction: inside the
same DB session/transaction where you create the new ResolutionProofToken (the
block that calls db.add(token), db.commit(), db.refresh(token) and
token_last_hash_cache.set), first acquire and lock the canonical chain head row
(either SELECT the last ResolutionProofToken row FOR UPDATE or use a dedicated
singleton ChainHead model/table) and re-check its integrity_hash as prev_hash,
then insert the new ResolutionProofToken with previous_integrity_hash set to
that locked value and update the canonical head row to the new integrity_hash
before committing; if the locked head changed, abort/retry so two generators
cannot fork the chain, and only update token_last_hash_cache after a successful
commit.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b201773e-8428-4411-b65e-aabe65c3e481
📒 Files selected for processing (3)
backend/cache.pybackend/models.pybackend/resolution_proof_service.py
| # Blockchain integrity fields | ||
| integrity_hash = Column(String, nullable=True) | ||
| previous_integrity_hash = Column(String, nullable=True, index=True) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C2 'resolution_proof_tokens|integrity_hash|previous_integrity_hash|create_index|add_column' .Repository: RohanExploit/VishwaGuru
Length of output: 45382
🏁 Script executed:
find . -type d -name alembic -o -name migrations -o -name migrate | head -20Repository: RohanExploit/VishwaGuru
Length of output: 49
🏁 Script executed:
ls -la backend/init_db.py && wc -l backend/init_db.pyRepository: RohanExploit/VishwaGuru
Length of output: 159
🏁 Script executed:
sed -n '260,280p' backend/init_db.pyRepository: RohanExploit/VishwaGuru
Length of output: 1262
Add migration for integrity_hash and previous_integrity_hash columns on resolution_proof_tokens table.
The model defines these columns and generate_proof_token immediately writes to them (lines 222–223 in resolution_proof_service.py), but init_db.py does not create them. Unlike other blockchain-chained models (ResolutionEvidence, EscalationAudit, EvidenceAuditLog, ClosureConfirmation), the resolution_proof_tokens migration block only handles nonce, valid_from, and valid_until. Add the missing column and index creation to lines 262–273 of init_db.py before deploying.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/models.py` around lines 326 - 328, The migration for the
resolution_proof_tokens table is missing the integrity_hash and
previous_integrity_hash columns (the model defines them and generate_proof_token
writes to them), so update the resolution_proof_tokens migration block to add a
nullable string column integrity_hash and a nullable string column
previous_integrity_hash and create an index on previous_integrity_hash; ensure
the column names and index match the model fields (integrity_hash,
previous_integrity_hash) so writes from generate_proof_token succeed and the DB
schema stays consistent with the ResolutionProofToken model.
There was a problem hiding this comment.
2 issues found across 3 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="backend/resolution_proof_service.py">
<violation number="1" location="backend/resolution_proof_service.py:193">
P1: Reading `previous_integrity_hash` from a local cache without DB-level serialization can fork the integrity chain under concurrent requests. This breaks linear blockchain-style integrity guarantees for `ResolutionProofToken`.</violation>
</file>
<file name="backend/models.py">
<violation number="1" location="backend/models.py:327">
P1: Missing database migration for the new `integrity_hash` and `previous_integrity_hash` columns (and the index on `previous_integrity_hash`). `init_db.py` handles migrations for `resolution_proof_tokens` but only covers `nonce`, `valid_from`, and `valid_until`. On existing deployments the ORM will try to read/write columns that don't exist, causing runtime errors. Add the corresponding `add_column` / `create_index` statements to the migration block in `init_db.py`.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| signature = ResolutionProofService._sign_payload(payload) | ||
|
|
||
| # Generate Integrity Hash | ||
| prev_hash = token_last_hash_cache.get("last_hash") |
There was a problem hiding this comment.
P1: Reading previous_integrity_hash from a local cache without DB-level serialization can fork the integrity chain under concurrent requests. This breaks linear blockchain-style integrity guarantees for ResolutionProofToken.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At backend/resolution_proof_service.py, line 193:
<comment>Reading `previous_integrity_hash` from a local cache without DB-level serialization can fork the integrity chain under concurrent requests. This breaks linear blockchain-style integrity guarantees for `ResolutionProofToken`.</comment>
<file context>
@@ -189,6 +189,22 @@ def generate_proof_token(
signature = ResolutionProofService._sign_payload(payload)
+ # Generate Integrity Hash
+ prev_hash = token_last_hash_cache.get("last_hash")
+ if prev_hash is None:
+ # Cache miss: Fetch only the last hash from DB
</file context>
| valid_until = Column(DateTime, nullable=True) | ||
|
|
||
| # Blockchain integrity fields | ||
| integrity_hash = Column(String, nullable=True) |
There was a problem hiding this comment.
P1: Missing database migration for the new integrity_hash and previous_integrity_hash columns (and the index on previous_integrity_hash). init_db.py handles migrations for resolution_proof_tokens but only covers nonce, valid_from, and valid_until. On existing deployments the ORM will try to read/write columns that don't exist, causing runtime errors. Add the corresponding add_column / create_index statements to the migration block in init_db.py.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At backend/models.py, line 327:
<comment>Missing database migration for the new `integrity_hash` and `previous_integrity_hash` columns (and the index on `previous_integrity_hash`). `init_db.py` handles migrations for `resolution_proof_tokens` but only covers `nonce`, `valid_from`, and `valid_until`. On existing deployments the ORM will try to read/write columns that don't exist, causing runtime errors. Add the corresponding `add_column` / `create_index` statements to the migration block in `init_db.py`.</comment>
<file context>
@@ -323,6 +323,10 @@ class ResolutionProofToken(Base):
valid_until = Column(DateTime, nullable=True)
+ # Blockchain integrity fields
+ integrity_hash = Column(String, nullable=True)
+ previous_integrity_hash = Column(String, nullable=True, index=True)
+
</file context>
Added
integrity_hashandprevious_integrity_hashto theResolutionProofTokenmodel to maintain standard blockchain-style integrity chaining across core entities. Implemented atoken_last_hash_cacheinbackend/cache.pyto prevent redundant database queries and securely generate hashes using HMAC-SHA256 inbackend/resolution_proof_service.pywithout degrading performance.PR created automatically by Jules for task 8330752688685032256 started by @RohanExploit
Summary by cubic
Adds blockchain-style integrity chaining to
ResolutionProofToken. Generates HMAC-SHA256 hashes and caches the last hash to avoid extra DB reads.New Features
integrity_hashandprevious_integrity_hash(indexed) toResolutionProofTokenfor chain linking.token_last_hash_cacheto store the last hash (TTL 1h, size 1) and reduce queries.secret_key.Migration
integrity_hashandprevious_integrity_hashto theResolutionProofTokentable.Written for commit 08318aa. Summary will update on new commits.
Summary by CodeRabbit
Release Notes