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

Improve and fix libs-credentials #569

Merged
merged 11 commits into from
Oct 9, 2024
Merged

Improve and fix libs-credentials #569

merged 11 commits into from
Oct 9, 2024

Conversation

waddaboo
Copy link
Collaborator

@waddaboo waddaboo commented Oct 1, 2024

Description

This PR fixes a bug where the queryGraph function does not return the request function as intended.

This PR also has a code refactor that introduces a breaking change at validate easAttestation.
Specifically, validate easAttestation context no longer takes in queryGraph but network and address instead.

Example (Old):

validateCredentials(
   {
     ...criteria
   },
   {
     queryGraph: () => queryGraph
   }
)

Example (New):

validateCredentials(
   {
     ...criteria
   },
   {
     network: EASNetworks.ETHEREUM,
     address: "0x"
   }
)

Related Issue

#568

Does this introduce a breaking change?

  • Yes
  • No

Please refer to the examples given above to migrate from the old easAttestation context to the new easAttestation context.

@waddaboo waddaboo requested a review from vplasencia October 1, 2024 11:00
Copy link

vercel bot commented Oct 1, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
bandada-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 9, 2024 1:12pm
bandada-website ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 9, 2024 1:12pm

@vplasencia
Copy link
Member

Thank you very much for the updates @waddaboo. I left some comments.

@waddaboo
Copy link
Collaborator Author

waddaboo commented Oct 9, 2024

Thanks for the review and catching the parts that I missed.

I have updated the code based on your comments and is now ready for review again :)

Copy link
Member

@vplasencia vplasencia left a comment

Choose a reason for hiding this comment

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

It looks great! Just left one comment. I think the PR is ready to be merged after the comment is resolved. Thank you very much.

@waddaboo
Copy link
Collaborator Author

waddaboo commented Oct 9, 2024

Done :)

@vplasencia
Copy link
Member

Thanks @waddaboo! Could you also update the readme file for the EAS credential?

@waddaboo
Copy link
Collaborator Author

waddaboo commented Oct 9, 2024

I do have another PR #572 that updates the readme file, or did I miss out something?

@vplasencia
Copy link
Member

Great! I didn't see that you also updated that PR. It looks great! I will merge both PRs and release a new version of the packages. Thank you very much @waddaboo

Feel free to update the API and Dashboard with the new changes.

@vplasencia vplasencia merged commit a59f0c4 into dev Oct 9, 2024
5 checks passed
@waddaboo waddaboo deleted the fix/libs/credentials branch October 10, 2024 07:49
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.

2 participants