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

[WIP] Add XMTP Support #96

Closed
wants to merge 2 commits into from
Closed

Conversation

neekolas
Copy link
Contributor

@neekolas neekolas commented Feb 7, 2024

What changed? Why?

  • Added support for resolving POST requests coming from either XMTP clients OR Farcaster clients
  • Added support for verifying signatures from XMTP clients and determining the verifiedWalletAddress field. This is done without any network requests, since XMTP POST payloads can be verified as a self-contained payload.

Notes to reviewers

This will need to wait on a PR in @xmtp/frames-validator to make it support being imported from CommonJS modules. That should land tomorrow.

How has it been tested?

package.json Outdated
"packageManager": "yarn@4.0.2"
"packageManager": "yarn@4.0.2",
"dependencies": {
"@xmtp/frames-validator": "^0.2.0"
Copy link
Contributor

Choose a reason for hiding this comment

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

Where is the npm link for this package, I wasn't able to find it.

I am curios to see how big is it on Kb

@@ -724,6 +725,415 @@ __metadata:
languageName: unknown
linkType: soft

"@ethersproject/abi@npm:5.7.0, @ethersproject/abi@npm:^5.7.0":
Copy link
Contributor

Choose a reason for hiding this comment

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

These are quite a lot of dependencies is taking in.

@@ -0,0 +1,28 @@
import { FramePostPayload } from '@xmtp/frames-validator/dist/src/types';
import { FrameValidationResponse } from '../../core/types';
import { validateFramesPost } from '@xmtp/frames-validator';
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if this can be implemented independently.

I am a bit worry of how big is @xmtp/frames-validator dependency for OnchainKit.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Frames validator itself is very small, but it relies on xmtp-js. The good news is that we have someone already working on removing our dependency on ethers, which is the biggest dependency of that library.

Just for my own understanding, is there a plan to run onchainkit in browsers? Or is this going to remain purely for servers? If we want browser support, there are a few other changes I'd need to make to remove dependencies on node:crypto.

Copy link
Contributor

Choose a reason for hiding this comment

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

For context, any feature in OnchainKit has to meet three bars:

  • being able to run on Web client
  • being able to run on latest Backend Node.js
  • being able to run on React Native

Copy link
Contributor

Choose a reason for hiding this comment

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

As OnchainKit has to work out of the box for all Coinbase products using these three case scenarios.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK. That's very good to know. We can work with those requirements. RN is the most challenging, since it doesn't have a built in crypto module or wasm support, but we have a RN SDK so none of this is new to us.

I'll get back to you on a new timeline with those requirements in mind.

Copy link
Contributor

Choose a reason for hiding this comment

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

All the current functionality only makes sense on a server, and a lot of it is specifically designed around Next.js

Yeah that's fair. I think we did a lot of simplification with our Frames approach by using a third-party provider for validating messages.

Copy link
Contributor

Choose a reason for hiding this comment

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

Part of me is like, we should create a https://github.com/Zizzamia/a-frame-in-100-lines version that showcase how to use your library in backend for node.js and just call it the day.

After all OnchainKit is a client library, and it avoids the backend part as much as possible.

Copy link
Contributor

Choose a reason for hiding this comment

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

Or in some way, OnchainKit knows how to recognize if is XMTP, an that's it, if that's the case we advice to use your library to complete other parts.

Copy link
Contributor

Choose a reason for hiding this comment

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

You see what I mean, I think it's fair to find a way to make the two working well together, instead of trying to force OnchainKit to be something is not.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

These are good points. I'm going to take a step back and try again with a more abstract approach, where the validation logic and cryptographic libraries don't have to be embedded in onchainkit. That avoids the dependency issues and library bloat, plus it lets you potentially support more than just XMTP Frames.

@Zizzamia
Copy link
Contributor

Closing this, in favour of #116

@Zizzamia Zizzamia closed this Feb 13, 2024
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.

2 participants