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

feat: Get the EVM address for a given Farcaster User #114

Merged
merged 14 commits into from
Feb 13, 2024

Conversation

Sneh1999
Copy link
Contributor

@Sneh1999 Sneh1999 commented Feb 9, 2024

What changed? Why?

Ability to extract the EVM address for a given Farcaster user

It gives the client ability to either get the verified address or the custody address

Notes to reviewers

How has it been tested?
yarn test
yarn integration:test

@Sneh1999 Sneh1999 marked this pull request as ready for review February 9, 2024 22:15
@Sneh1999 Sneh1999 requested review from robpolak and Zizzamia February 9, 2024 22:15
package.json Show resolved Hide resolved
src/farcaster/types.ts Outdated Show resolved Hide resolved
@Zizzamia
Copy link
Contributor

@Sneh1999 can you share where in the Farcaster spec those are specified. Also how they are used.

In particular I am curios to understand from where the Request is coming from.

@Sneh1999
Copy link
Contributor Author

Sneh1999 commented Feb 10, 2024

@Sneh1999 can you share where in the Farcaster spec those are specified. Also how they are used.

In particular I am curios to understand from where the Request is coming from.

I found this about verifications in the protocol docs
and this about custody address

Custody address is used to check if a given message is valid or not. A FID is connected to a custody address.

Verified address on the other hand can be multiple like snehkoul.eth or zizzamia.eth. You can add as many verified addresses to your account as you want but you need to verify that you indeed hold the private keys for these addresses. This is done through EIP-712 signatures.

src/farcaster/types.ts Outdated Show resolved Hide resolved
src/farcaster/types.ts Outdated Show resolved Hide resolved
src/index.ts Outdated Show resolved Hide resolved
@Sneh1999 Sneh1999 requested a review from Zizzamia February 12, 2024 20:56
@Sneh1999 Sneh1999 changed the title chore: Get the EVM address for a given Farcaster User feat: Get the EVM address for a given Farcaster User Feb 12, 2024
fid: number,
options?: GetFarcasterUserAddressOptions,
): Promise<GetFarcasterUserAddressResponse | null> {
try {
Copy link
Contributor

Choose a reason for hiding this comment

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

@Sneh1999 side not for the next PR, large try/catch are a bad practice in Javascript, as each Try/Catch will hold on memory almost everything inside a try till the end of the execution. It's advice to have smaller try/catch.

options?: GetFarcasterUserAddressOptions,
): Promise<GetFarcasterUserAddressResponse | null> {
try {
const hasCustodyAddresses = options?.hasCustodyAddresses ?? true;
Copy link
Contributor

Choose a reason for hiding this comment

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

hasCustodyAddresses @Sneh1999 did you mentioned there can be only one address for custody?

Copy link
Contributor

Choose a reason for hiding this comment

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

So why not hasCustodyAddress as singular?

Copy link
Contributor

@Zizzamia Zizzamia left a comment

Choose a reason for hiding this comment

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

Niiiice!!!!

Great work @Sneh1999

@Zizzamia Zizzamia merged commit 926bc30 into main Feb 13, 2024
7 checks passed
@Zizzamia Zizzamia deleted the get-connected-address branch February 13, 2024 00:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants