Smart account: canonical signers

[brozorec]

Fixes #639
Fixes #643
Fixes #644

PR Checklist

• Tests
• Documentation

Summary by CodeRabbit

• Improvements
  • Optimized signer validation to check the entire set in a single pass instead of per-candidate checks, improving performance.
  • Enhanced batch canonicalization for Ed25519 and WebAuthn signature verifiers for more efficient key processing.

[coderabbitai]

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: bfa88475-738a-4707-a9c0-ec40a7af47c0

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

Walkthrough

Added batch_canonicalize_key functions to Ed25519 and WebAuthn verifiers for efficient bulk key canonicalization. Refactored smart account duplicate signer detection from per-candidate checks to whole-set validation, updating all callers to use the new validate_no_canonical_duplicates function. Updated Verifier trait documentation for canonicalization requirements.

Changes

Cohort / File(s)	Summary
Verifier Batch Canonicalization packages/accounts/src/verifiers/ed25519.rs, packages/accounts/src/verifiers/webauthn.rs	Added new batch_canonicalize_key public functions that take a collection of keys and return canonicalized representations in a single operation, replacing need for per-key calls.
Verifier Trait Documentation packages/accounts/src/verifiers/mod.rs	Updated Verifier::verify trait documentation to clarify panic requirements: implementations MUST panic for invalid-length key data; additional format validation is optional based on resource cost justification.
Smart Account Duplicate Validation Refactoring packages/accounts/src/smart_account/storage.rs	Replaced per-candidate contains_canonical_duplicate with validate_no_canonical_duplicates that validates entire signer set atomically. Groups Signer::External entries by verifier, performs single batch canonicalization per verifier, and panics on collisions. Updated all callers: add_context_rule, add_signer, batch_add_signer.
Smart Account API Updates packages/accounts/src/smart_account/mod.rs	Removed re-export of contains_canonical_duplicate and added validate_no_canonical_duplicates to public API.
Smart Account Test Coverage packages/accounts/src/smart_account/test/context_rules.rs	Converted canonical-duplicate tests to use new validate_no_canonical_duplicates validation helper with panic expectations for duplicate scenarios.
Example Contract Updates examples/multisig-smart-account/ed25519-verifier/src/contract.rs, examples/multisig-smart-account/webauthn-verifier/src/contract.rs	Simplified batch_canonicalize_key implementations to delegate directly to corresponding verifier batch functions, removing manual iteration.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Possibly related PRs

• Smart accounts: key canonicalization, size limits and direct rule hints #581: Related changes to verifier canonicalization APIs and smart account duplicate-checking refactoring using the new batch validation approach.

Suggested labels

Small

Suggested reviewers

• ozgunozerk

Poem

🐰 A rabbit hops through keys so keen,
Batch-checking what once was seen,
Where duplicates could hide and play,
One call now banishes them away!
Efficiency blooms in the fray. 🌟

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'Smart account: canonical signers' is concise and clearly summarizes the main change—implementing canonical signer handling for smart accounts.
Description check	✅ Passed	The PR description references all three linked issues (#639, #643, #644) and indicates tests and documentation are completed, satisfying the template requirements.
Linked Issues check	✅ Passed	The PR implements batch_canonicalize_key functions [#644], optimizes signer duplication detection [#639], and updates canonicalize_key behavior to validate only key length not full validity [#643].
Out of Scope Changes check	✅ Passed	All changes are scoped to canonical signer handling, batch canonicalization, and related API updates directly addressing the linked issues.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch sa-canonical-signers

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.15%. Comparing base (a6d7329) to head (f42d7fe).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #657   +/-   ##
=======================================
  Coverage   96.15%   96.15%           
=======================================
  Files          59       59           
  Lines        6104     6110    +6     
=======================================
+ Hits         5869     5875    +6     
  Misses        235      235           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/accounts/src/smart_account/storage.rs`:
- Around line 556-561: The current logic skips verifier-side canonicalization
for singleton external signers and mandates batch_canonicalize_key for multi-key
cases, which allows malformed single key_data to be stored and breaks verifiers
that only implement canonicalize_key; change the flow around key_batch and
VerifierClient::new so every external key is always canonicalized: call
VerifierClient::batch_canonicalize_key when key_batch.len() > 1 as an
optimization but fall back to calling VerifierClient::canonicalize_key per key
(or call canonicalize_key for singletons) whenever batch_canonicalize_key is
unavailable or returns no result, validate against MAX_EXTERNAL_KEY_SIZE as
before, and add a regression test that stores a single short external key
(singleton) to ensure per-key canonicalization runs and rejects malformed
key_data.

🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: ebef629c-d543-45b7-9e55-0f4a92487265

📥 Commits

Reviewing files that changed from the base of the PR and between ff17d24 and e22cbd3.

📒 Files selected for processing (8)

• examples/multisig-smart-account/ed25519-verifier/src/contract.rs
• examples/multisig-smart-account/webauthn-verifier/src/contract.rs
• packages/accounts/src/smart_account/mod.rs
• packages/accounts/src/smart_account/storage.rs
• packages/accounts/src/smart_account/test/context_rules.rs
• packages/accounts/src/verifiers/ed25519.rs
• packages/accounts/src/verifiers/mod.rs
• packages/accounts/src/verifiers/webauthn.rs

[ozgunozerk]

instead of creating a mutable vector, pushing to it, and iterating through it for duplicate detection, we can utilize HashMap with () values (since we don't have HashSet in soroban_sdk).

I think, this should be more convenient in terms of both performance and the code readability and the intention behind it.