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

Update WNFS & UnixFS compat layer #536

Merged
merged 8 commits into from
Aug 18, 2023
Merged

Update WNFS & UnixFS compat layer #536

merged 8 commits into from
Aug 18, 2023

Conversation

icidasset
Copy link
Contributor

@icidasset icidasset commented Jul 24, 2023

Updates wnfs to the latest version: Doesn't use name filters anymore, AccessKey instead of PrivateRef and fixes a bug we had with older access keys.

This PR also adds the UnixFS compatibility layer. Any time a mutation is made in the public file system, the change is reflect in the UnixFS node. This was added so that public content can be rendered on the IPFS gateway (eg. apps or images)

Other changes:

  • Renamed capsuleRef to capsuleKey
  • Now uses a single Uint8Array instead of an object with multiple.
  • No more need for custom encoding/decoding of the "access key" 🎉
  • Split up event types to avoid circular imports.
  • Make appPath function applicable to both public and private paths.

Closes #546 and #547

@icidasset icidasset force-pushed the icidasset/update-wnfs branch 2 times, most recently from b5488fc to c559cc3 Compare August 11, 2023 16:17
@icidasset icidasset changed the base branch from next to icidasset/fission-account August 11, 2023 16:18
Base automatically changed from icidasset/fission-account to next August 16, 2023 10:39
@icidasset icidasset marked this pull request as ready for review August 17, 2023 17:07
@icidasset icidasset changed the title Update WNFS Update WNFS & UnixFS compat layer Aug 17, 2023
Copy link
Contributor

@matheus23 matheus23 left a comment

Choose a reason for hiding this comment

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

unixfs.ts looks like what I'd expect :) 👍

Keep in mind, we'll be doing a release of rs-wnfs in like, just an hour or so. That'll make your tests run faster.

Also, have you tried using rs-wnfs' native .cp function in the case where you're copying files or directories within the private partition? It should be a little faster than doing more round-trips from JS to Wasm.
And after the new rs-wnfs release, there will also be a public filesystem cp function.

src/events/authority.ts Show resolved Hide resolved
Comment on lines +40 to +52
export async function createPrivateForest(): Promise<PrivateForest> {
const rng = makeRngInterface()
const rsaKey = await Crypto.rsa.generateKey("sign")
const rsaMod = await webcrypto.subtle
.exportKey("jwk", rsaKey.publicKey)
.then(a => {
if (a.n) return a.n
else throw new Error("Expected public RSA key to have `n` property")
})
.then(n => Uint8Arrays.fromString(n, "base64url"))

return new PrivateForest(rng, rsaMod)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

Copy link
Member

@avivash avivash left a comment

Choose a reason for hiding this comment

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

I'm not fully up-to-speed on the requirements of the UnixFS compatability layer, but I think everything else looks good! Left a few notes

* Events interface.
*
* Subscribe to events using `on` and unsubscribe using `off`.
* There's also `once`, `onAny`, `offAny`, `anyEvent` and `events`.
Copy link
Member

Choose a reason for hiding this comment

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

Love this!

* @group Querying
*/
permalink(dataRoot: CID, path: Path.Distinctive<Path.Partitioned<Path.Partition>>): string {
if (this.#dependencies.depot.permalink) {
Copy link
Member

Choose a reason for hiding this comment

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

note: when i started integrating the ts-odd next branch into collector this week I had to add this babel plugin to handle # methods:

@babel/plugin-transform-private-methods

This may just be a quirk of react-native(I haven't tried the latest updates in a svelte app yet), but perhaps it's something we want to highlight in the readme?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah good point, maybe it’s because it’s a relatively new js feature (2021), ecmascript 2022 I think. The readme still needs to be rewritten but yeah we should definitely mention that. Can you change the compile target to es2022 in react native?

Copy link
Member

Choose a reason for hiding this comment

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

looks like I can! It's strange, the app extends the expo config, which specifies a target of ESNext, which seems like it should have that functionality, but specifying ES2022 explicitly seems to fix it 👌🏼

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice 👍 Yeah, esnext is confusing because it depends on when they last updated the pointer (could even be es2020 🤷). I'll mention in the readme that the SDK requires es2022.

}

////////
// ㊙️ //
Copy link
Member

Choose a reason for hiding this comment

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

I still find the emoji section headers a bit confusing 🙈 after some searching, it looks like this means secret? ㊙️ Should we just add the word Secret to these sections? May be clearer for outside contributors 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah right I promised you that emoji guide, I'll do that in this PR. I like that the un-exported items are grouped together in a section that is color coded by a red emoji 😅 (and conveniently it's the sign for "secret" yeah). I can also append the word secret after the emoji? 🤷

Copy link
Member

Choose a reason for hiding this comment

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

haha no worries, yeah I think just including the word Secret next to the emoji would be helpful, rather than having an emoji guide to refer to. I'm open to anything though. Whatever the most clear solution is for outside developers to tell what the section refers to 👍🏼

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh wait I did already add this, it's in src/README.md. It doesn't have an index table though, I'll add that.

@icidasset
Copy link
Contributor Author

Thanks @matheus23! I was waiting for the new release for the cp stuff, I'll add that tomorrow now that the release is out 👍

@icidasset icidasset merged commit ab8bbd3 into next Aug 18, 2023
@icidasset icidasset deleted the icidasset/update-wnfs branch August 18, 2023 13:43
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.

3 participants