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

LIT-2841 - Remove ipfs-http-client and create encryption APIs for composition with any storage layer #427

Merged
merged 19 commits into from
Apr 18, 2024

Conversation

MaximusHaximus
Copy link
Contributor

@MaximusHaximus MaximusHaximus commented Apr 15, 2024

Description

Overview

This PR removes a problematic dependency that also happened to be very large code and dependency-wise - ipfs-http-client. We were using this in a single place in our SDK as part of a 'nice-to-have' function. Unfortunately, the package has issues with bundling in multiple environments and brings a very large # of dependencies into our bundle that a lot of our users never actually needed.

The API for the method that used this dependency was also problematic; it was hard-coded to use Infura credentials and endpoints, and Infura has disabled the ability to create IPFS-enabled API keys since November of 2023. Rather than re-work the API to allow arbitrary providers, authentication, etc, we decided that it would be more flexible for consumers, and better for the size of our bundles and compatibility in multiple environments, to remove the dependency and instead provide some functions that are un-opinionated about where the encrypted data is going or coming from.

Anyone who would like to encrypt to/from IPFS can simply call these new methods and then use the client of their choice to put the data in IPFS.

Even better, anyone who wants to encrypt/decrypt somewhere other than IPFS, but without compressing the data using our existing APIs that encrypt into ZIP files with metadata stored inside the ZIP archive can use these same methods to work with any other arbitrary data store they want to use.

Before this change, the only code that would wrap up the encrypted data with the metadata required to easily decrypt it later was the 'encryptToIpfs()' code which stored the encrypted data and metadata in the same JSON blob, always on IPFS, or encryptFileAndZipWithMetadata() code which stored the metadata as a separate file from the encrypted data in the ZIP archive and only worked with files (not bare strings).

Changes:

  • Removed encryptToIpfs() and decryptFromIpfs() methods in encryption, which were also exported from lit-node-client
  • Added encryptToJson() and decryptFromJson() in encryption, and export those methods from lit-node-client
    • These methods produce and consume JSON blobs in the same format as the previous encryptToIpfs() and decryptFromIpfs() methods produced and consumed.

Fixes:

  • Resolved an incorrectly applied polyfill for fetch, where we were mutating global fetch even if it was previously defined.
    • We now use cross-fetch to handle this polyfill, which is friendly to environments where it is already defined -- and was already part of our dependency tree due to other dependencies we already rely on.
    • Removed node-fetch direct dependency now that cross-fetch is being used to polyfill.
  • Removed ipfs-htttp-client dependency, which was causing bundling errors in mixed environments (NextJS)

Internal changes:

  • Removed build tooling hooks for post-build mutations that were we were not running and no longer used
  • Cleaned up some duplication in types touching access control conditions
  • Fixed a bunch of eslint/tslint errors in the files where changes were made

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

  • Added tests for encrypting/decrypting from JSON strings using both file input and string input
  • Ran entire E2E suite

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

@MaximusHaximus MaximusHaximus marked this pull request as ready for review April 15, 2024 16:58
@joshLong145 joshLong145 self-requested a review April 15, 2024 21:23
@MaximusHaximus MaximusHaximus requested a review from jtary April 16, 2024 13:07
joshLong145
joshLong145 previously approved these changes Apr 16, 2024
@MaximusHaximus
Copy link
Contributor Author

@joshLong145 I went ahead and made the connect() logging in the contracts SDK less spammy like we talked about -- the contents of abi were just arrays of numbers, so I create a map of contract name->address that looks like this:

resolved contract addresses for:  cayenne {
  StakingBalances: '0x095251de2aD2A78aDe96F2a11F7feAA7CF93e6B5',
  Staking: '0x5bFa704aF947b3b0f966e4248DED7bfa6edeF952',
  Multisender: '0xD4e3D27d21D6D6d596b6524610C486F8A9c70958',
  LITToken: '0x53695556f8a1a064EdFf91767f15652BbfaFaD04',
  PubkeyRouter: '0x4B5E97F2D811520e031A8F924e698B329ad83E29',
  PKPNFT: '0x58582b93d978F30b4c4E812A16a7b31C035A69f7',
  RateLimitNFT: '0x19593CbBC56Ddd339Fde26278A544a25166C2388',
  PKPHelper: '0xF02b6D6b0970DB3810963300a6Ad38D8429c4cdb',
  PKPPermissions: '0xD01c9C30f8F6fa443721629775e1CC7DD9c9e209',
  PKPNFTMetadata: '0xeD46dDcbFF662ad89b0987E0DFE2949901498Da6',
  Allowlist: '0xfc7Bebd150b36921549595A776D7723fBC4Bb2D9'
}

joshLong145
joshLong145 previously approved these changes Apr 16, 2024
joshLong145
joshLong145 previously approved these changes Apr 16, 2024
Ansonhkg
Ansonhkg previously approved these changes Apr 17, 2024
Copy link
Collaborator

@Ansonhkg Ansonhkg left a comment

Choose a reason for hiding this comment

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

lgtm!

@joshLong145 joshLong145 self-requested a review April 18, 2024 13:25
joshLong145
joshLong145 previously approved these changes Apr 18, 2024
Copy link

@joshLong145 joshLong145 left a comment

Choose a reason for hiding this comment

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

Approving as the polyfill has been tested.

…s-sdk`

- Remove `@ts-nocheck` pragma
- Fix two test cases that were being called without `cert` as an argument (error was hidden by `@ts-nocheck` pragma)
- Fix unnecessary TextEncoder/TextDecoder injection -- this is done by our jest config
- Use `cross-fetch` to ponyfill fetch
- Remove unused imports
…ss-fetch` and remove soon-to-be-unused `ipfs-http-client` and `ipfs-unixfs-importer` deps
…fetch (it's done in `lit-node-client-nodejs`, which this wraps!)
… with `cross-fetch/polyfill` usage. Squelch ESLint errors in global hackery.
…ncrypt/decrypt with to/from Json encrypt/decrypt

- Removed `...IpfsProps` and `...IpfsPayload` interfaces
- Defined `...JsonProps` and `...JsonPayload` interfaces
- Used `MultipleAccessControlConditions` interface in a bunch of places where we had duplicated the entire interface, comments and all, in-line in other interfaces.
- Used `EncryptRequestBase` as the basis for the new Json interfaces instead of duplicating the contents of that interface in-line
- Clarified `sessionSigs` from being an `any` type in a few interfaces, to being a `SessionSigsMap` type.
…Json, and remove now-unused ipfs validator

- Also eslint --fixed the file, and replaced an `any` type on `AuthMethodValidator` with `AuthMethod` type.
- Removed unused `decryptToZip` validator that used an `any` type; that method actually just uses the `decrypt` validator anyway
… from ipfs methods with `encryptoToJson` and `decryptToJson` methods
…Ipfs` and `decryptFromIpfs` with our new to/from JSON methods
… for `LitContracts`

- No longer logs huge arrays of numbers that represent the ABIs
- ESLint --fix'd the file
…properly based on `parsedJsonData.dataType` arg 🎉
…h `string` and `file` be passed to `EncryptToJsonValidator`
…ecrypting from JSON strings including metadata
@Ansonhkg Ansonhkg self-requested a review April 18, 2024 19:51
Copy link
Collaborator

@Ansonhkg Ansonhkg left a comment

Choose a reason for hiding this comment

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

lgtm

@Ansonhkg Ansonhkg merged commit 22293a2 into master Apr 18, 2024
4 checks passed
@Ansonhkg Ansonhkg deleted the LIT-2841 branch April 18, 2024 19:52
@Ansonhkg Ansonhkg mentioned this pull request May 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants