-
Notifications
You must be signed in to change notification settings - Fork 21
fix(WebAuthValidator): add fixes #22
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR refactors WebAuthnValidator to streamline credential ID parameters, enforce sorted and unique IDs, and update tests accordingly.
- Removed the
requireUVparameter fromgenerateCredentialId,removeCredential, andhasCredentialmethods. - Introduced a
NotSortederror and added checks to ensure credential ID arrays are sorted and uniquified. - Bumped the Solidity compiler version to
^0.8.28and updated unit and integration tests to match signature changes.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| test/WebAuthnValidator/unit/concrete/WebAuthnValidator.t.sol | Updated test calls to remove requireUV, added sorting imports, and adjusted array ordering. |
| test/WebAuthnValidator/integration/WebAuthnValidator.t.sol | Aligned integration tests with new method signatures and data encoding. |
| src/WebAuthnValidator/WebAuthnValidator.sol | Removed requireUV usage, added NotSorted error, enforced sorted IDs, and bumped compiler. |
| foundry.toml | Updated compiler setting from 0.8.25 to 0.8.28. |
Comments suppressed due to low confidence (4)
src/WebAuthnValidator/WebAuthnValidator.sol:210
- The contract uses
isSorted()without importing or attaching theLibSortlibrary. Addimport { LibSort } from "solady/utils/LibSort.sol";andusing LibSort for bytes32[];at the top of the file.
require(credentialIds.isSorted(), NotSorted());
src/WebAuthnValidator/WebAuthnValidator.sol:497
- Same issue: switch to
if (!context.credentialIds.isSortedAndUniquified()) revert NotSorted();and ensureLibSortis imported and applied.
require(context.credentialIds.isSortedAndUniquified(), NotSorted());
test/WebAuthnValidator/unit/concrete/WebAuthnValidator.t.sol:13
- [nitpick] The
consoleimport is not used in this test. Remove it to clean up unnecessary dependencies.
import { console } from "forge-std/console.sol";
test/WebAuthnValidator/unit/concrete/WebAuthnValidator.t.sol:20
- [nitpick] The
LibSortlibrary is imported but not utilized here; either remove the import/using directive or apply itssort()method instead of manual swaps.
using LibSort for bytes32[];
No description provided.