Skip to content
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

refactor: add interfaces #377

Merged
merged 3 commits into from
Aug 28, 2023
Merged

refactor: add interfaces #377

merged 3 commits into from
Aug 28, 2023

Conversation

horsefacts
Copy link
Collaborator

@horsefacts horsefacts commented Aug 28, 2023

Motivation

Minor, but this should make it easier to build on top of the various registries. Allows for it to be an easy reference for the interfaces and can be pulled into other projects to use, rather than needing to redefine it per project.

Change Summary

Simply adding interfaces. Main change is around where structs are located.

There are two options, not sure which one you prefer:

  1. Structs within the interface
  2. Structs in a *Structs.sol file and imported into the interface and implementation.

Merge Checklist


PR-Codex overview

This PR focuses on adding interface contracts and updating existing contracts to implement these interfaces.

Detailed summary

  • Added IStorageRegistry interface to StorageRegistry.sol
  • Added IIdRegistry interface to IdRegistry.sol
  • Added IKeyRegistry interface to KeyRegistry.sol
  • Added IBundler interface to Bundler.sol
  • Updated imports and function signatures to use the new interfaces

The following files were skipped due to too many changes: src/KeyRegistry.sol, test/KeyRegistry/KeyRegistry.t.sol, src/interfaces/IIdRegistry.sol, src/interfaces/IKeyRegistry.sol, src/interfaces/IStorageRegistry.sol, test/Bundler/Bundler.t.sol

✨ Ask PR-Codex anything about this PR by commenting with /codex {your question}

@horsefacts horsefacts added the feat New feature request label Aug 28, 2023
@horsefacts horsefacts mentioned this pull request Aug 28, 2023
4 tasks
Copy link
Member

@sds sds left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, but will let @varunsrin make the final call on whether this gets merged.

src/interfaces/IBundler.sol Show resolved Hide resolved
src/interfaces/IBundler.sol Show resolved Hide resolved
src/interfaces/IIdRegistry.sol Show resolved Hide resolved
src/interfaces/IIdRegistry.sol Show resolved Hide resolved
@github-actions
Copy link

Coverage after merging kartik/interfaces into main will be

98.65%

Coverage Report
FileStmtsBranchesFuncsLinesUncovered Lines
src
   Bundler.sol100%100%100%100%
   FnameResolver.sol100%100%100%100%
   IdRegistry.sol100%100%100%100%
   KeyRegistry.sol100%100%100%100%
   RecoveryProxy.sol100%100%100%100%
   StorageRegistry.sol100%100%100%100%
src/lib
   EIP712.sol50%100%50%50%19
   Signatures.sol100%100%100%100%
   TransferHelper.sol0%0%0%0%12, 17, 20, 20, 20
   TrustedCaller.sol100%100%100%100%
src/validators
   SignedKeyRequestValidator.sol100%100%100%100%

@horsefacts horsefacts merged commit 3e8e925 into main Aug 28, 2023
2 checks passed
@horsefacts horsefacts deleted the kartik/interfaces branch August 28, 2023 22:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat New feature request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants