Skip to content

Refactor ERC-1271 validation - #1484

Open
AdriGeorge wants to merge 1 commit into
mainfrom
feat/erc-1271-check-signature
Open

AdriGeorge wants to merge 1 commit into
mainfrom
feat/erc-1271-check-signature

Conversation

@AdriGeorge

@AdriGeorge AdriGeorge commented Sep 29, 2026 •

Copy link
Copy Markdown
Collaborator
  • fallback check all supported networks, not only the first one in config

Summary by CodeRabbit

  • Bug Fixes
    • Improved smart-account signature validation when no chain is specified: validation now checks configured networks instead of relying on a single network.
    • When a chain is specified, validation checks that chain. Missing network entries and validation errors on one network no longer prevent checks on other configured networks.

@coderabbitai

coderabbitai Bot commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: bb2d127c-968d-4ebd-aa02-63e62ea222df

📥 Commits

Reviewing files that changed from the base of the PR and between c974bd7 and 2bb7181.

📒 Files selected for processing (1)
  • src/components/core/utils/nonceHandler.ts

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

Smart-account signature validation now checks the specified chain or tries each configured network when no chain is specified. It skips missing network entries and continues after per-network ERC-1271 validation errors.

Changes

Signature validation

Layer / File(s) Summary
Network validation
src/components/core/utils/nonceHandler.ts
When chainId is set, validation checks only that chain. Otherwise, it tries each configured network, skips missing entries, and attempts both the custom hash and EIP-191 hash formats. Errors are logged per network and do not stop later attempts.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 2bb71

The validator can continue past networks that do not validate the signature, allowing a later configured network to validate it. No concrete merge-blocking risk was identified.

Architecture Summary

Architecture risk: 🔵 Low · up to 2bb71

The change affects 1 system.

Changed systems: src

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

  • observed — src (service) was modified; 1 changed file maps to changed impact.

Before / after behavior

  • observed — Modified behavior in src/components/core/utils/nonceHandler.ts: ERC-1271 validation now uses only the specified chain when chainId is set, or iterates over all configured networks otherwise; missing network entries are skipped. Each network tries both the custom hash and EIP-191 hash formats. Errors are logged per network and do not prevent trying subsequent networks. Previously, validation tried only the specified chain or the first configured network.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the main change: refactoring ERC-1271 validation to support fallback checks across configured networks.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 2
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
🛠️ Fix failing CI checks 💡
  • Commit to this branch
  • Create a new PR

Warning

Some tools did not complete. Review the errors below.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint install timed out. The project may have too many dependencies for the sandbox.


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.

❤️ Share

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

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant