[FH-2] Open Frames Implementation (#163) #202
Conversation
WalkthroughThe pull request introduces several changes across multiple files, primarily focusing on the restructuring of payload validation and type definitions. Key updates include renaming types from Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Outside diff range and nitpick comments (9)
templates/figma/handlers/click.ts (1)
Line range hint
1-24: Summary of changes and potential impactThe changes in this file appear to be part of a larger refactoring effort:
- The import source for
FramePayloadValidatedhas been updated.- The
bodyparameter type in theclickfunction has been changed toFramePayloadValidated.- The way the button index is accessed has been modified.
These changes improve type safety and consistency. However, they may have broader implications across the codebase.
Consider the following recommendations:
- Ensure that all files importing
FramePayloadValidatedhave been updated.- Verify that the new structure of
FramePayloadValidatedis consistently used across all relevant functions.- Update any documentation or comments that might reference the old structure.
- If this is part of a breaking change, ensure it's properly communicated to other developers and update the version number accordingly.
app/api/compose/[templateId]/route.ts (1)
Line range hint
1-89: Summary: Farcaster-specific validation refactoring looks good.The changes in this file are minimal and focused on updating the validation function to be Farcaster-specific. This refactoring improves the clarity and specificity of the code. The overall structure and logic of the file remain intact, which is good.
Consider the following architectural advice:
- Ensure that this change is part of a consistent pattern across the codebase, where Farcaster-specific logic is separated from general frame handling.
- If there are other frame types (e.g., for different platforms), consider creating a factory or strategy pattern for validation to make it easier to add new frame types in the future.
- Update any relevant documentation or comments to reflect these changes, especially if there are architectural diagrams or API specifications that mention the validation process.
app/(app)/f/[frameId]/[handler]/[[...query]]/route.ts (3)
166-166: Address the TODO comment regarding interaction loggingThere's a TODO comment indicating uncertainty about supporting interaction logging for non-Farcaster frames:
// TODO Do we want to support interaction logging for non-Farcaster frames?Clarify whether interaction logging should be supported for non-Farcaster frames. If so, consider implementing the necessary logic; if not, you might want to document this decision for future reference.
Would you like assistance in implementing interaction logging for non-Farcaster frames or creating a GitHub issue to track this task?
172-172: Replaceconsole.logwith a proper logging mechanismUsing
console.login production code can expose sensitive information and is generally not recommended. Consider using a logging library or framework that allows you to control log levels and outputs.Apply this diff to replace
console.log:- console.log(JSON.stringify(airstackPayloadValidated, null, 2)) + // Use a proper logging mechanism instead of console.log + logger.debug('Validated Airstack payload:', airstackPayloadValidated)Ensure that
loggeris appropriately configured in your application.
181-182: Validate optional fields before accessing propertiesWhen accessing optional properties like
inputTextandstate, ensure they are validated or handle cases where they might be undefined or null to prevent potential runtime errors.Consider adding checks or using optional chaining:
- inputText: airstackPayloadValidated.message.data.frameActionBody.inputText || undefined, - state: airstackPayloadValidated.message.data.frameActionBody.state || undefined, + inputText: airstackPayloadValidated.message.data.frameActionBody?.inputText, + state: airstackPayloadValidated.message.data.frameActionBody?.state,lib/validate.ts (4)
107-107: Remove unnecessaryconsole.logstatementThe
console.log(r.action)statement in thevalidatePayloadFarcasterfunction may expose sensitive information and clutter the console output in production environments. It's advisable to remove this statement or use a proper logging mechanism with appropriate log levels.Apply this diff to remove the console log:
@@ -107,1 +107,0 @@ - console.log(r.action)
20-21: UpdateinputTexttype definitionThere's a TODO comment indicating that
inputTextshould useNeynarValidatedFrameActionResponse['action']['input']['text']:// TODO inputText: NeynarValidatedFrameActionResponse['action']['input']['text'] inputText: string,Updating the type definition will ensure consistency with the validated response and improve type safety.
Would you like assistance in updating the type definition for
inputTextto reflect the correct type?
189-190: Implement user details retrieval for Lens protocolThe
userNameanduserIconfor the Lens protocol are currently placeholders with TODO comments:userName: userId, // TODO use lensclient? userIcon: undefined, // TODO use lensclient?Integrating
lensclientto fetch these details will enhance the completeness of the standardized payload.I can help integrate
lensclientto retrieve the user's name and icon. Would you like me to draft the implementation or open a GitHub issue to track this task?
209-210: Retrieve user details using XMTP clientThe
userNameanduserIconfields for XMTP are set to default values, with TODO comments suggesting the use of an XMTP client:userName: userId, // TODO use xmtp client? userIcon: undefined, // TODO use xmtp client?Fetching user details using the XMTP client would improve the payload's accuracy and usefulness.
Would you like assistance in integrating the XMTP client to obtain the user's name and icon?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (7)
- app/(app)/f/[frameId]/[handler]/[[...query]]/route.ts (2 hunks)
- app/api/compose/[templateId]/route.ts (2 hunks)
- lib/farcaster.ts (2 hunks)
- lib/serve.ts (1 hunks)
- lib/validate.ts (1 hunks)
- package.json (1 hunks)
- templates/figma/handlers/click.ts (2 hunks)
🧰 Additional context used
🔇 Additional comments (6)
templates/figma/handlers/click.ts (2)
17-17: Verify the correctness of the new button index access.The way the button index is accessed has changed from
body.tapped_button.indextobody.buttonIndex.toString(). This reflects a structural change in theFramePayloadValidatedtype.Let's verify the consistency of this change and its impact on the function's logic:
#!/bin/bash # Description: Check for consistency in button index access across the codebase # Test 1: Search for any remaining usages of the old button index access pattern echo "Checking for any remaining usages of body.tapped_button.index:" rg --type typescript "body\.tapped_button\.index" # Test 2: Verify the new button index access pattern is used consistently echo "Verifying the usage of body.buttonIndex:" rg --type typescript "body\.buttonIndex" # Test 3: Check if buttonIndex is consistently treated as a number echo "Checking if buttonIndex is consistently treated as a number:" rg --type typescript "body\.buttonIndex(\.toString\(\)|\.toFixed\(|\.toNumber\()"Please ensure that this change doesn't break any existing logic related to button handling in the function.
Line range hint
7-15: LGTM! Verify usage ofbodywithin the function.The type of the
bodyparameter has been updated toFramePayloadValidated, which aligns with the import change. This is a good practice for type safety.Let's verify that the usage of
bodywithin the function is consistent with this type change:#!/bin/bash # Description: Check the usage of 'body' within the click function # Test: Search for all usages of 'body' within the click function echo "Checking usages of 'body' within the click function:" ast-grep --lang typescript --pattern $'function click({ $$$ }: { body: FramePayloadValidated $$$ }) { $$$ $body$ $$$ }'app/api/compose/[templateId]/route.ts (2)
38-38: LGTM! Ensure the new validation function behaves as expected.The update to use
validatePayloadFarcasteris consistent with the import change and improves the specificity of the validation for Farcaster frames.To ensure the new validation function behaves as expected, please verify its implementation and run appropriate tests. You can use the following script to check the implementation:
#!/bin/bash # Description: Check the implementation of validatePayloadFarcaster # Test: Display the content of the file containing validatePayloadFarcaster fd --type f --full-path '.*farcaster.*' | xargs rg -A 10 'export.*function validatePayloadFarcaster'
3-3: LGTM! Verify the impact of this change across the codebase.The update to import
validatePayloadFarcasterfrom '@/lib/farcaster' looks good. This change appears to be part of a refactoring to make the validation more specific to Farcaster frames.To ensure this change doesn't introduce any issues, please run the following script to check for any remaining usage of the old
validatePayloadfunction:package.json (1)
67-67: Approved: frames.js dependency updated. Verify compatibility and review changelog.The update of
frames.jsfrom^0.11.0to^0.19.3is a significant version jump, which aligns with the PR objectives for "Open Frames Implementation". This change is approved, but please consider the following:
- Verify that this update is compatible with the rest of your codebase, especially any components or functions that directly use
frames.js.- Review the changelog for
frames.jsbetween versions 0.11.0 and 0.19.3 to be aware of any breaking changes or new features that might affect your implementation.To help verify the impact of this change, you can run the following script:
This script will help identify areas of the codebase that might be affected by the
frames.jsupdate and remind you to check the official changelog.lib/validate.ts (1)
139-140: Verify Farcaster timestamp conversion logicIn
standardizeFarcasterPayload, the timestamp is calculated using a custom epoch:const farcasterEpoch = new Date('2021-01-01T00:00:00Z').getTime(); const timestampInMs = farcasterEpoch + (Number(payload.timestamp) * 1000);Ensure that this conversion accurately represents the intended timestamp. Miscalculations could lead to incorrect time data, affecting time-sensitive features.
| import type { BuildFrameData } from '@/lib/farcaster' | ||
| import type { FramePayloadValidated } from '@/lib/validate' |
There was a problem hiding this comment.
💡 Codebase verification
Inconsistent import of FramePayloadValidated detected.
The import source for FramePayloadValidated has not been updated consistently across the codebase. Please update the following files to import from @/lib/validate:
templates/swap/handlers/more.tstemplates/swap/handlers/success.tstemplates/swap/handlers/estimate.tstemplates/swap/handlers/txData.tstemplates/quizlet/handlers/success.tstemplates/quizlet/handlers/review.tstemplates/quizlet/handlers/page.tstemplates/presentation/handlers/cover.tstemplates/quizlet/handlers/answer.tstemplates/presentation/handlers/slide.tstemplates/substack/handlers/page.tstemplates/presentation-v1/handlers/page.tstemplates/quizlet/handlers/results.tstemplates/luma/handlers/success.tstemplates/poll/handlers/vote.tstemplates/luma/handlers/register.tstemplates/form/handlers/success.tstemplates/form/handlers/field.tstemplates/fundraiser/handlers/about.tstemplates/fundraiser/handlers/confirmation.tstemplates/fundraiser/handlers/txData.tstemplates/form-v1/handlers/input.tstemplates/fundraiser/handlers/status.tstemplates/fundraiser/handlers/success.tstemplates/gated/handlers/page.tstemplates/beehiiv/handlers/page.tstemplates/cal/handlers/duration.tstemplates/cal/handlers/month.tstemplates/contract/handlers/confirm.tstemplates/figma/handlers/slide.tstemplates/cal/handlers/day.tstemplates/cal/handlers/confirm.tstemplates/cal/handlers/hour.tsapp/(app)/p/[frameId]/[handler]/[[...query]]/route.ts
🔗 Analysis chain
Verify the impact of the import change across the codebase.
The import source for FramePayloadValidated has been changed from '@/lib/farcaster' to '@/lib/validate'. This change suggests a restructuring of the project's modules.
Let's verify that this change has been consistently applied across the codebase:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining imports of FramePayloadValidated from @/lib/farcaster
# Test 1: Search for imports of FramePayloadValidated from @/lib/farcaster
echo "Checking for imports of FramePayloadValidated from @/lib/farcaster:"
rg --type typescript "import.*FramePayloadValidated.*from\s+['\"]@/lib/farcaster['\"]"
# Test 2: Verify the new import is used consistently
echo "Verifying the new import of FramePayloadValidated from @/lib/validate:"
rg --type typescript "import.*FramePayloadValidated.*from\s+['\"]@/lib/validate['\"]"
Length of output: 775
Script:
#!/bin/bash
# Description: Check for any remaining imports of FramePayloadValidated from @/lib/farcaster and verify the new import from @/lib/validate
# Test 1: Search for imports of FramePayloadValidated from @/lib/farcaster
echo "Checking for imports of FramePayloadValidated from '@/lib/farcaster':"
rg --type ts "import\s+.*FramePayloadValidated.*from\s+['\"]@/lib/farcaster['\"]"
# Test 2: Verify the new import is used consistently from @/lib/validate
echo "Verifying the new import of FramePayloadValidated from '@/lib/validate':"
rg --type ts "import\s+.*FramePayloadValidated.*from\s+['\"]@/lib/validate['\"]"
Length of output: 4644
| export type FarcasterFramePayload = FramesJSFrameActionPayload | ||
| export type FarcasterFramePayloadValidated = NeynarValidatedFrameActionResponse['action'] |
There was a problem hiding this comment.
💡 Codebase verification
Remaining references to old type names found
The verification script detected instances of FramePayload and FramePayloadValidated that were not updated to FarcasterFramePayload and FarcasterFramePayloadValidated:
lib/validate.tsapp/(app)/f/[frameId]/[handler]/[[...query]]/route.ts
Please update these occurrences to maintain consistency and avoid potential conflicts.
🔗 Analysis chain
Approve renaming of FramePayload and FramePayloadValidated types
The renaming of FramePayload to FarcasterFramePayload and FramePayloadValidated to FarcasterFramePayloadValidated improves code clarity and consistency. This change:
- Explicitly associates these types with Farcaster, reducing ambiguity.
- Aligns with the naming convention of other Farcaster-related types in the file.
- Potentially reduces naming conflicts with similar types from other libraries.
The underlying type definitions remain unchanged, ensuring no functional impact.
To ensure this change doesn't introduce any issues, please verify that all references to these types throughout the codebase have been updated accordingly. Run the following script to check for any remaining occurrences of the old type names:
If the script returns any results, those occurrences should be updated to use the new type names.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining occurrences of the old type names
# Test: Search for old type names. Expect: No results
rg --type typescript "type (FramePayload|FramePayloadValidated)\b"
Length of output: 103
Script:
#!/bin/bash
# Description: Check for any remaining occurrences of the old type names in .ts and .tsx files
# Test: Search for old type names. Expect: No results
rg "type\s+(FramePayload|FramePayloadValidated)\b" --glob "*.ts" --glob "*.tsx"
Length of output: 358
| import type { BuildFrameData, FarcasterFramePayload } from '@/lib/farcaster' | ||
| import { updateFrameStorage } from '@/lib/frame' | ||
| import { buildFramePage, validatePayload, validatePayloadAirstack } from '@/lib/serve' | ||
| import { buildFramePage } from '@/lib/serve' | ||
| import { isFarcasterFrameActionPayload, validatePayload, validatePayloadAirstack, type FramePayload } from '@/lib/validate' |
There was a problem hiding this comment.
Ensure consistent use of payload types and imports
There appears to be a mix of FramePayload and FarcasterFramePayload imports:
- Line 3 imports
FarcasterFramePayloadfrom'@/lib/farcaster'. - Line 6 imports
type FramePayloadfrom'@/lib/validate'.
This could lead to confusion or type mismatches when handling payloads. Consider standardizing the payload type across the module to ensure consistency.
Apply this diff to update the import and usage:
import { isFarcasterFrameActionPayload, validatePayload, validatePayloadAirstack, type FramePayload } from '@/lib/validate'
+import type { FarcasterFramePayload } from '@/lib/farcaster'
// Replace all instances of FramePayload with FarcasterFramePayload in this file
-const payload = (await request.json()) as FramePayload
+const payload = (await request.json()) as FarcasterFramePayloadCommittable suggestion was skipped due to low confidence.
| const airstackKey = frame.config?.airstackKey || process.env.AIRSTACK_API_KEY | ||
|
|
||
| const airstackPayloadValidated = await validatePayloadAirstack(payload as FarcasterFramePayload, airstackKey) | ||
|
|
There was a problem hiding this comment.
💡 Codebase verification
airstackKey Configuration Missing
The airstackKey is not set in any frame configurations, and the AIRSTACK_API_KEY environment variable is also missing. This will lead to runtime errors when airstackKey is undefined.
🔗 Analysis chain
Ensure airstackKey is properly configured
The variable airstackKey is obtained from frame.config?.airstackKey or environment variables. Ensure that this key is correctly configured and that missing or invalid keys are appropriately handled to prevent runtime errors.
Run the following script to check for the presence of airstackKey in configuration:
Ensure that frames have airstackKey configured or that the environment variable AIRSTACK_API_KEY is set.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that 'airstackKey' is set in frame configurations or environment variables.
# Test: Search for frames missing 'airstackKey' in configurations
jq '.config | select(.airstackKey == null)' frames/*.json
# Test: Check if 'AIRSTACK_API_KEY' is set in environment variables
if [[ -z "${AIRSTACK_API_KEY}" ]]; then
echo "AIRSTACK_API_KEY is not set in environment variables."
else
echo "AIRSTACK_API_KEY is set."
fi
Length of output: 264
| await client | ||
| .insert(interactionTable) | ||
| .values({ | ||
| frame: frame.id, | ||
| fid: airstackPayloadValidated.message.data.fid.toString(), | ||
| buttonIndex: | ||
| airstackPayloadValidated.message.data.frameActionBody.buttonIndex.toString(), | ||
| inputText: airstackPayloadValidated.message.data.frameActionBody.inputText || undefined, | ||
| state: airstackPayloadValidated.message.data.frameActionBody.state || undefined, | ||
| transactionHash: | ||
| airstackPayloadValidated.message.data.frameActionBody.transactionId || undefined, | ||
| castFid: airstackPayloadValidated.message.data.frameActionBody.castId.fid.toString(), | ||
| castHash: airstackPayloadValidated.message.data.frameActionBody.castId.hash, | ||
| createdAt: new Date(), | ||
| }) | ||
| .run() |
There was a problem hiding this comment.
Handle potential database insertion errors
While inserting data into the interactionTable, there is no error handling in case the database operation fails. Unhandled promise rejections could lead to application crashes or inconsistent state.
Wrap the database insertion in a try-catch block to handle potential errors:
+ try {
await client
.insert(interactionTable)
.values({
frame: frame.id,
fid: airstackPayloadValidated.message.data.fid.toString(),
buttonIndex:
airstackPayloadValidated.message.data.frameActionBody.buttonIndex.toString(),
inputText: airstackPayloadValidated.message.data.frameActionBody.inputText || undefined,
state: airstackPayloadValidated.message.data.frameActionBody.state || undefined,
transactionHash:
airstackPayloadValidated.message.data.frameActionBody.transactionId || undefined,
castFid: airstackPayloadValidated.message.data.frameActionBody.castId.fid.toString(),
castHash: airstackPayloadValidated.message.data.frameActionBody.castId.hash,
createdAt: new Date(),
})
.run()
+ } catch (e) {
+ // Handle the error appropriately
+ logger.error('Database insertion failed:', e)
+ }Ensure you have a logging mechanism (logger) configured to capture the error details.
Committable suggestion was skipped due to low confidence.
| metadata['of:version'] = 'vNext' | ||
|
|
||
| // Lens Protocol Open Frames version the handler supports | ||
| metadata['of:accepts:lens'] = '1.0.0' | ||
|
|
||
| // XMTP Open Frames version the handler supports | ||
| metadata['of:accepts:xmtp'] = 'vNext' |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Define version constants for maintainability
Currently, the version strings 'vNext' and '1.0.0' are hardcoded. Consider defining them as constants at the beginning of the file or in a configuration file. This improves maintainability and makes it easier to update versions in the future.
Apply this diff to define version constants:
+const OPEN_FRAMES_VERSION = 'vNext';
+const LENS_PROTOCOL_VERSION = '1.0.0';
+const XMTP_VERSION = 'vNext';
...
- metadata['of:version'] = 'vNext';
+ metadata['of:version'] = OPEN_FRAMES_VERSION;
- metadata['of:accepts:lens'] = '1.0.0';
+ metadata['of:accepts:lens'] = LENS_PROTOCOL_VERSION;
- metadata['of:accepts:xmtp'] = 'vNext';
+ metadata['of:accepts:xmtp'] = XMTP_VERSION;Committable suggestion was skipped due to low confidence.
| const openFrameEquivalentTags: Record<string, string> = { | ||
| 'fc:frame:state': 'of:state', | ||
| 'fc:frame:image': 'of:image', | ||
| 'fc:frame:image:aspect_ratio': 'of:image:aspect_ratio', | ||
| 'fc:frame:post_url': 'of:post_url', | ||
| 'fc:frame:input:text': 'of:input:text', | ||
| 'fc:frame:button:1': 'of:button:1', | ||
| 'fc:frame:button:1:action': 'of:button:1:action', | ||
| 'fc:frame:button:1:target': 'of:button:1:target', | ||
| 'fc:frame:button:1:post_url': 'of:button:1:post_url', | ||
| 'fc:frame:button:2': 'of:button:2', | ||
| 'fc:frame:button:2:action': 'of:button:2:action', | ||
| 'fc:frame:button:2:target': 'of:button:2:target', | ||
| 'fc:frame:button:2:post_url': 'of:button:2:post_url', | ||
| 'fc:frame:button:3': 'of:button:3', | ||
| 'fc:frame:button:3:action': 'of:button:3:action', | ||
| 'fc:frame:button:3:target': 'of:button:3:target', | ||
| 'fc:frame:button:3:post_url': 'of:button:3:post_url', | ||
| 'fc:frame:button:4': 'of:button:4', | ||
| 'fc:frame:button:4:action': 'of:button:4:action', | ||
| 'fc:frame:button:4:target': 'of:button:4:target', | ||
| 'fc:frame:button:4:post_url': 'of:button:4:post_url', |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Generate button tag mappings dynamically to reduce duplication
The mappings for button tags are hardcoded for buttons 1 to 4. To enhance scalability and reduce code duplication, consider generating these mappings dynamically. This approach simplifies the code and makes it easier to support more buttons in the future.
Here's how you can modify the code to generate button tag mappings dynamically:
const openFrameEquivalentTags: Record<string, string> = {
'fc:frame:state': 'of:state',
'fc:frame:image': 'of:image',
'fc:frame:image:aspect_ratio': 'of:image:aspect_ratio',
'fc:frame:post_url': 'of:post_url',
'fc:frame:input:text': 'of:input:text',
};
+// Dynamically generate button tag mappings
+const maxButtons = 4; // Adjust if needed
+for (let i = 1; i <= maxButtons; i++) {
+ openFrameEquivalentTags[`fc:frame:button:${i}`] = `of:button:${i}`;
+ openFrameEquivalentTags[`fc:frame:button:${i}:action`] = `of:button:${i}:action`;
+ openFrameEquivalentTags[`fc:frame:button:${i}:target`] = `of:button:${i}:target`;
+ openFrameEquivalentTags[`fc:frame:button:${i}:post_url`] = `of:button:${i}:post_url`;
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const openFrameEquivalentTags: Record<string, string> = { | |
| 'fc:frame:state': 'of:state', | |
| 'fc:frame:image': 'of:image', | |
| 'fc:frame:image:aspect_ratio': 'of:image:aspect_ratio', | |
| 'fc:frame:post_url': 'of:post_url', | |
| 'fc:frame:input:text': 'of:input:text', | |
| 'fc:frame:button:1': 'of:button:1', | |
| 'fc:frame:button:1:action': 'of:button:1:action', | |
| 'fc:frame:button:1:target': 'of:button:1:target', | |
| 'fc:frame:button:1:post_url': 'of:button:1:post_url', | |
| 'fc:frame:button:2': 'of:button:2', | |
| 'fc:frame:button:2:action': 'of:button:2:action', | |
| 'fc:frame:button:2:target': 'of:button:2:target', | |
| 'fc:frame:button:2:post_url': 'of:button:2:post_url', | |
| 'fc:frame:button:3': 'of:button:3', | |
| 'fc:frame:button:3:action': 'of:button:3:action', | |
| 'fc:frame:button:3:target': 'of:button:3:target', | |
| 'fc:frame:button:3:post_url': 'of:button:3:post_url', | |
| 'fc:frame:button:4': 'of:button:4', | |
| 'fc:frame:button:4:action': 'of:button:4:action', | |
| 'fc:frame:button:4:target': 'of:button:4:target', | |
| 'fc:frame:button:4:post_url': 'of:button:4:post_url', | |
| const openFrameEquivalentTags: Record<string, string> = { | |
| 'fc:frame:state': 'of:state', | |
| 'fc:frame:image': 'of:image', | |
| 'fc:frame:image:aspect_ratio': 'of:image:aspect_ratio', | |
| 'fc:frame:post_url': 'of:post_url', | |
| 'fc:frame:input:text': 'of:input:text', | |
| }; | |
| // Dynamically generate button tag mappings | |
| const maxButtons = 4; // Adjust if needed | |
| for (let i = 1; i <= maxButtons; i++) { | |
| openFrameEquivalentTags[`fc:frame:button:${i}`] = `of:button:${i}`; | |
| openFrameEquivalentTags[`fc:frame:button:${i}:action`] = `of:button:${i}:action`; | |
| openFrameEquivalentTags[`fc:frame:button:${i}:target`] = `of:button:${i}:target`; | |
| openFrameEquivalentTags[`fc:frame:button:${i}:post_url`] = `of:button:${i}:post_url`; | |
| } |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (2)
lib/validate.ts (2)
20-20: Address TODO comment for inputTextThere's a TODO comment regarding the
inputTextproperty. Consider implementing the correct type forinputTextusingNeynarValidatedFrameActionResponse['action']['input']['text']as suggested in the comment.
189-190: Address TODO comments for user information in Lens and XMTPThere are TODO comments regarding
userNameanduserIconin both the Lens and XMTP standardization functions. Consider implementing these fields using the respective protocol clients (Lens client and XMTP client) to provide more complete user information.Also applies to: 209-210
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- lib/validate.ts (1 hunks)
🧰 Additional context used
🔇 Additional comments (5)
lib/validate.ts (5)
1-53: LGTM: Import statements and type definitionsThe import statements and type definitions are well-structured and comprehensive. They cover all the necessary protocols (Farcaster, Lens, and XMTP) and provide a clear structure for the validated frame payload.
55-72: LGTM: validatePayload functionThe
validatePayloadfunction is well-structured and handles different protocols appropriately. It correctly delegates to protocol-specific validation functions and provides a clear error message for unsupported protocols.
80-110: LGTM: validatePayloadFarcaster functionThe
validatePayloadFarcasterfunction is well-implemented. It properly handles API communication, error cases, and validity checks. The use of an environment variable for the API key is a good security practice.
137-212: LGTM: Standardization functionsThe standardization functions for Farcaster, Lens, and XMTP are well-implemented. They correctly map protocol-specific payloads to the common
FramePayloadValidatedtype, ensuring consistency across different protocols. The additional protocol-specific information in the Farcaster function provides valuable context.
1-212: Overall assessment: Well-implemented multi-protocol frame validation systemThe
lib/validate.tsfile implements a comprehensive and well-structured system for validating and standardizing frame payloads across multiple protocols (Farcaster, Lens, and XMTP). The code is generally well-organized and follows good practices.Key strengths:
- Clear type definitions for frame payloads
- Protocol-specific validation functions
- Standardization functions to ensure consistency across protocols
Areas for improvement:
- Enhance Farcaster payload detection logic
- Add validity check for Airstack response
- Address TODO comments for user information in Lens and XMTP standardization
- Implement the correct type for
inputTextinFramePayloadValidatedAddressing these minor issues will further improve the robustness and completeness of the implementation.
| export function isFarcasterFrameActionPayload(framePayload: FramePayload): boolean { | ||
| return !framePayload.clientProtocol | ||
| } |
There was a problem hiding this comment.
Enhance Farcaster payload detection logic
The current implementation of isFarcasterFrameActionPayload assumes that any payload with an undefined clientProtocol is a Farcaster payload. This approach might lead to false positives if clientProtocol is undefined for other reasons. Consider adding additional checks or a more explicit identifier for Farcaster payloads to improve accuracy.
| export async function validatePayloadAirstack( | ||
| body: FarcasterFramePayload, | ||
| airstackKey: string | ||
| ): Promise<any> { | ||
| const options = { | ||
| method: 'POST', | ||
| headers: { | ||
| 'Content-Type': 'application/octet-stream', | ||
| 'x-airstack-hubs': airstackKey, | ||
| }, | ||
| body: new Uint8Array( | ||
| body.trustedData.messageBytes.match(/.{1,2}/g)!.map((byte) => Number.parseInt(byte, 16)) | ||
| ), | ||
| } | ||
|
|
||
| const r = await fetch('https://hubs.airstack.xyz/v1/validateMessage', options) | ||
| .then((response) => response.json()) | ||
| .catch((err) => { | ||
| console.error(err) | ||
| throw new Error('AIRSTACK_PAYLOAD_COULD_NOT_BE_VALIDATED') | ||
| }) | ||
|
|
||
| return r | ||
| } |
There was a problem hiding this comment.
Add validity check for Airstack response
The validatePayloadAirstack function successfully sends a request to the Airstack API and handles potential errors. However, unlike validatePayloadFarcaster, it doesn't check if the response indicates a valid payload. Consider adding a validity check to ensure the Airstack validation was successful before returning the response.
Example:
if (!r.valid) {
throw new Error('AIRSTACK_PAYLOAD_NOT_VALID');
}
# Conflicts: # package-lock.json
Closes #163
Sample post on hey.xyz / Lens
Link
Sample convo with XMTP
Frames.js debugger verification
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Bug Fixes
Chores
frames.jspackage to a newer version.Documentation