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

[🎯 Target v2.17.1+] Filebase Integration as Custom IPFS Provider and Future-proofing IPFS Expansion (no branch exists) #3147

Conversation

mattsmithies
Copy link
Contributor

Description:

This PR enhances the Guardian project by introducing the Filebase IPFS provider and establishing a testing suite for IPFS configurations. It facilitates the addition of new IPFS providers without requiring a full rebuild of the Guardian system and outlines a streamlined process for future expansions. Key updates include:

  • Integration of the Filebase IPFS provider, leveraging the IPFS Pinning Service API for a simplified REST API experience.
  • Adoption of a test-driven development approach to quickly verify IPFS configurations.
  • Provision of guidelines for adding and testing custom IPFS providers.

Updated Readme Snippets:

Setting up IPFS Filebase Bucket:

Configure the Filebase IPFS provider in ./configs/.env.develop.guardian.system with your generated Firebase Bucket Token for IPFS_STORAGE_API_KEY and set IPFS_PROVIDER to "filebase". For more, refer to Filebase's IPFS Pinning Service API documentation.

Implementing and Testing Custom IPFS Providers:

Our workflow is designed to easily integrate new IPFS providers. Set up your environment variables, run yarn test:ipfs in the "worker-service" directory for validation, and extend the "IpfsProvider" enum with your provider's logic. We recommend an interface-based approach for enhanced maintainability.

Notes for reviewer:

Please review the updated documentation and README for a comprehensive guide on integrating the new IPFS provider. This update primarily addresses a minor version transition from v2.17.0 to v2.17.1, particularly focusing on changes in token authentication mechanisms from v2.18.0 onwards. The code should forward-compatible up to v2.20.1+, with minor modification.

However, this PR stops short of implementing the scaling recommendations for IPFS providers. It instead proposes a framework and best practices for future integrations, with a focus on:

  • Transitioning to an interface-driven architecture for IPFS clients to standardize functionality and streamline the integration of new providers.
  • Reevaluating the incorporation of NATs Channel instances in v2.20.1, as its relevance within the IPFS client is debatable. A more modular approach, along with employing hash maps for linking IpfsProvider enums to respective interfaces, would result in a cleaner and more maintainable codebase.

As the Guardian project continues to evolve with the addition of more IPFS clients, the proposed methodology aims to offer a scalable and time-efficient integration solution.

Checklist

  • Documented (Code comments, README, etc.)
  • Tested (unit, integration, etc.)

@simvalery simvalery changed the base branch from main to develop January 29, 2024 13:35
@simvalery
Copy link
Collaborator

Please fix conflicts and we will merge this PR

@mattsmithies
Copy link
Contributor Author

@simvalery, I want to provide some context which you might not be fully aware of, as this was discussed extensively with the broader team on Slack:

The pull request I submitted specifically targets the 2.17.0 codebase, not the recent 2.21.x series. This is the primary reason for the code conflicts you're seeing. The intention here is to create a patch update moving from 2.17.0 to 2.17.1, but this code can also be beneficial for the 2.21.+ releases, particularly for resolving issues related to IPFS.

Unfortunately, there isn’t a direct branch for 2.17.0, and since I don’t have direct push access to the Guardian repository, this approach was my only viable option. This strategy was previously discussed and agreed upon with the team, especially given the significant changes that occurred from 2.17.0 to 2.18.0. I believe this decision was made in consideration of the broader picture and the project's continuity.

Thank you for taking the time to read this and for your understanding. Looking forward to collaborating further to resolve this matter.

@simvalery simvalery changed the base branch from develop to hotfix/2.17.1 February 13, 2024 10:03
@simvalery simvalery merged commit aea2674 into hashgraph:hotfix/2.17.1 Feb 13, 2024
0 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants