feat: Add more storage options to Lens Client #1394
Conversation
jonathangus
changed the title
|
@imthatcarlos wdyt? |
|
good stuff @jonathangus - i was hoping someone would take care of this 😅 |
|
Have tested Pinata and Arweave but would love someone confirming Storj still works as expected. Was not able to get API keys |
|
@jonathangus I've looked thru the storj provider changes and it should be fine. Plus, I think the vast majority of Lens eco uses Arweave anyways. LGTM |
|
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 eslint
packages/client-lens/src/actions.tsOops! Something went wrong! :( ESLint: 9.18.0 ESLint couldn't find an eslint.config.(js|mjs|cjs) file. From ESLint v9.0.0, the default configuration file is now eslint.config.js. https://eslint.org/docs/latest/use/configure/migration-guide If you still have problems after following the migration guide, please stop by 📝 WalkthroughWalkthroughThe pull request introduces comprehensive enhancements to the Lens Protocol client, focusing on expanding storage provider capabilities and configuration options. The changes include adding new environment variables, implementing a flexible storage provider interface, and creating provider-specific classes for Arweave, Pinata, and Storj. The modifications enable more dynamic and configurable media storage and publication processes within the Lens Protocol integration. Changes
Possibly related PRs
Suggested labels
Finishing Touches
🪧 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 (
|
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (6)
packages/client-lens/src/providers/AreweaveProvider.ts (3)
23-28: Consider making 'ARWEAVE_JWK' mandatoryWhile a warning is logged when
ARWEAVE_JWKis missing, consider throwing an error to prevent runtime issues since the JWK is essential for transactions.
38-69: Handle potential network errors in 'uploadFile'Consider implementing retry logic or additional error handling for network failures during transaction posting to enhance reliability.
71-103: Enhance 'uploadJson' with error handlingSimilar to
uploadFile, adding retries or handling network exceptions inuploadJsonwill improve robustness.packages/client-lens/src/providers/PinataProvider.ts (1)
23-27: Make 'PINATA_JWT' a required settingSince
PINATA_JWTis crucial for authentication, consider throwing an error if it's not provided to prevent unexpected failures.packages/client-lens/src/providers/StorageProvider.ts (1)
14-23: Consider using a more specific type for uploadJson.The
Record<string, any>type is quite permissive. Consider defining a stricter interface for the JSON structure if possible.packages/client-lens/src/providers/StorjProvider.ts (1)
22-26: Fix typo in warning message.There's a typo in the warning message: "envornment" should be "environment".
- "To use Storj ipfs pinning service you need to set STORJ_API_USERNAME or STORJ_API_PASSWORD in envornment variables. Get your keys at https://storj.io" + "To use Storj ipfs pinning service you need to set STORJ_API_USERNAME or STORJ_API_PASSWORD in environment variables. Get your keys at https://storj.io"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (11)
.env.example(2 hunks)packages/client-lens/package.json(2 hunks)packages/client-lens/src/actions.ts(1 hunks)packages/client-lens/src/index.ts(2 hunks)packages/client-lens/src/interactions.ts(3 hunks)packages/client-lens/src/post.ts(3 hunks)packages/client-lens/src/providers/AreweaveProvider.ts(1 hunks)packages/client-lens/src/providers/PinataProvider.ts(1 hunks)packages/client-lens/src/providers/StorageProvider.ts(1 hunks)packages/client-lens/src/providers/StorjProvider.ts(5 hunks)packages/client-lens/src/utils.ts(2 hunks)
🔇 Additional comments (20)
packages/client-lens/src/index.ts (5)
6-12: Imports updated for new storage providersThe added imports for
StorageProvider,StorageProviderEnum,StorjProvider,PinataProvider, andArweaveProvidercorrectly incorporate the new storage options.
20-20: Property 'storage' replaces 'ipfs'Updating the property from
ipfstostorageenhances flexibility by generalizing the storage mechanism.
46-46: Initialize storage provider dynamicallyAssigning
this.storage = this.getStorageProvider();correctly initializes the storage provider based on configuration.
65-94: Implement 'getStorageProvider' methodThe
getStorageProvider()method efficiently selects the storage provider. Ensure all providers are correctly mapped instorageProviderMap.
97-99: Conditionally initialize storage providerChecking
if (this.storage.initialize)before callinginitialize()ensures compatibility with providers that may not require initialization.packages/client-lens/src/providers/AreweaveProvider.ts (1)
30-36: Robust error handling for JWK parsingProperly catching and logging errors during JWK parsing aids in debugging invalid key formats.
packages/client-lens/src/providers/PinataProvider.ts (3)
29-33: Optional 'PINATA_GATEWAY_URL' with fallbackInforming the user about the benefits of setting
PINATA_GATEWAY_URLis helpful. Ensure the default gateway functions effectively if not specified.
46-77: Ensure large file upload support in 'uploadFile'The
uploadFilemethod is correctly implemented. Verify that the configuration handles large files appropriately.
105-111: Correctly resolve content URLsThe
resolveUrlmethod constructs the access URL based onPINATA_GATEWAY_URL, defaulting to Pinata's gateway if not set.packages/client-lens/src/providers/StorageProvider.ts (2)
3-6: LGTM! Clean interface design for upload responses.The
UploadResponseinterface clearly defines the expected structure with both CID and URL.
8-12: LGTM! Well-defined enum for storage providers.The enum values match the expected provider names in lowercase.
packages/client-lens/src/actions.ts (1)
31-41: LGTM! Robust error handling for storage uploads.Good addition of try-catch block with informative warning message including the provider context.
packages/client-lens/src/utils.ts (1)
68-84: LGTM! Excellent type safety improvements.The function now properly handles different image types with proper TypeScript types instead of
any.packages/client-lens/src/providers/StorjProvider.ts (2)
97-98: Verify the implications of using Infinity for content length limits.Setting both
maxContentLengthandmaxBodyLengthtoInfinitycould potentially lead to memory issues with large files.Consider implementing reasonable limits or documenting the maximum supported file size.
Line range hint
51-79: LGTM! Clean implementation of uploadJson.Good type handling for both string and object inputs, with proper buffer conversion.
packages/client-lens/src/post.ts (2)
14-14: LGTM! Storage provider abstraction looks good.The change from
StorjProvidertoStorageProviderinterface enables flexible storage options while maintaining backward compatibility.Also applies to: 24-24
108-108: LGTM! Storage parameter update is consistent.The
sendPublicationcall correctly uses the new storage parameter.packages/client-lens/src/interactions.ts (1)
26-26: LGTM! Storage provider changes are consistent with post.ts.The implementation maintains consistency with the changes in post.ts.
Also applies to: 35-35
packages/client-lens/package.json (1)
25-25: Verify arweave package version compatibility.Let's ensure version 1.15.5 is the latest stable version and check for any security advisories.
✅ Verification successful
✅ arweave@1.15.5 is verified
Latest stable version with no known security vulnerabilities.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check arweave package version and security advisories # Check npm for latest versions curl -s https://registry.npmjs.org/arweave | jq '.["dist-tags"].latest' # Check for security advisories gh api graphql -f query=' { securityVulnerabilities(first: 5, ecosystem: NPM, package: "arweave") { nodes { advisory { summary severity publishedAt } vulnerableVersionRange firstPatchedVersion { identifier } } } }'Length of output: 423
.env.example (1)
361-364: LGTM! Environment variables are well documented.The configuration includes:
- Storage provider selection with Storj as default
- Provider-specific credentials
- Clear documentation for each variable
Also applies to: 366-372, 531-531
| async uploadJson( | ||
| json: Record<string, any> | string | ||
| ): Promise<UploadResponse> { | ||
| const data = typeof json === "string" ? JSON.parse(json) : json; | ||
|
|
||
| const { data: result } = await this.client.post( | ||
| "/pinning/pinJSONToIPFS", | ||
| { | ||
| pinataOptions: { | ||
| cidVersion: 1, | ||
| }, | ||
| pinataMetadata: { | ||
| name: "content.json", | ||
| }, | ||
| pinataContent: data, | ||
| } | ||
| ); | ||
|
|
||
| const url = this.resolveUrl(result.IpfsHash); | ||
|
|
||
| return { | ||
| cid: data.IpfsHash, | ||
| url, | ||
| }; | ||
| } |
There was a problem hiding this comment.
Fix variable reference when returning 'cid'
In uploadJson, cid: data.IpfsHash should be cid: result.IpfsHash since data refers to the input, not the response.
Apply this diff to correct the error:
- cid: data.IpfsHash,
+ cid: result.IpfsHash,📝 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.
| async uploadJson( | |
| json: Record<string, any> | string | |
| ): Promise<UploadResponse> { | |
| const data = typeof json === "string" ? JSON.parse(json) : json; | |
| const { data: result } = await this.client.post( | |
| "/pinning/pinJSONToIPFS", | |
| { | |
| pinataOptions: { | |
| cidVersion: 1, | |
| }, | |
| pinataMetadata: { | |
| name: "content.json", | |
| }, | |
| pinataContent: data, | |
| } | |
| ); | |
| const url = this.resolveUrl(result.IpfsHash); | |
| return { | |
| cid: data.IpfsHash, | |
| url, | |
| }; | |
| } | |
| async uploadJson( | |
| json: Record<string, any> | string | |
| ): Promise<UploadResponse> { | |
| const data = typeof json === "string" ? JSON.parse(json) : json; | |
| const { data: result } = await this.client.post( | |
| "/pinning/pinJSONToIPFS", | |
| { | |
| pinataOptions: { | |
| cidVersion: 1, | |
| }, | |
| pinataMetadata: { | |
| name: "content.json", | |
| }, | |
| pinataContent: data, | |
| } | |
| ); | |
| const url = this.resolveUrl(result.IpfsHash); | |
| return { | |
| cid: result.IpfsHash, | |
| url, | |
| }; | |
| } |
.env.example
Outdated
| # Lens Protocol Configuration | ||
| LENS_PROFILE_ID= # Your Lens Protocol profile ID (format: 0x1234) | ||
| LENS_DRY_RUN= # Set to 'true' to simulate posts without actually publishing | ||
| LENS_POLL_INTERVAL= # Interval in seconds to check for new interactions (default: 60) | ||
| LENS_STORAGE_PROVIDER= # Storage provider for media: 'arweave', 'pinata', or 'storj'. Default "storj" |
There was a problem hiding this comment.
Remove duplicate Lens Protocol configuration.
These variables are duplicates of the ones defined above.
-# Lens Protocol Configuration
-LENS_PROFILE_ID= # Your Lens Protocol profile ID (format: 0x1234)
-LENS_DRY_RUN= # Set to 'true' to simulate posts without actually publishing
-LENS_POLL_INTERVAL= # Interval in seconds to check for new interactions (default: 60)
-LENS_STORAGE_PROVIDER= # Storage provider for media: 'arweave', 'pinata', or 'storj'. Default "storj"
Risks
Low/Medium
Background
What does this PR do?
Enable more storage options in Lens Client. With this PR we now support:
Fix: Improve
getProfilePictureUrito use correct types instead ofanyWhat kind of change is this?
Improvements
Why are we doing this? Any context or related work?
Wanted to use Lens Client and posts where stored on Storj that required a beta account so wanted to introduce more options.
Fix TODO for
packages/client-lens/src/actions.tsin #1223Documentation changes needed?
Testing
Where should a reviewer start?
Create an agent with Lens Client enabled
Detailed testing steps
Change the environment variables for each storage provider
Storj:
Arweave:
Pinata:
Discord username
@0xheavydev
Summary by CodeRabbit
Configuration
New Features
Dependencies
Improvements