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

Metadata hash calculation #131

Open
sinzii opened this issue Jun 23, 2024 · 4 comments
Open

Metadata hash calculation #131

sinzii opened this issue Jun 23, 2024 · 4 comments
Labels

Comments

@sinzii
Copy link
Member

sinzii commented Jun 23, 2024

Detail: https://polkadot-fellows.github.io/RFCs/approved/0078-merkleized-metadata.html#metadata-digest

@josepot
Copy link

josepot commented Jul 6, 2024

FWIW @polkadot-api/merkleize-metadata already solves this problem, and it's currently being used by Talisman. Also, we have tested its performance with its WASM wrapper counterpart and it's actually a bit more performant, also its bundle-size is super small... Therefore, I think that it makes a lot of sense to use it both in dedot and Coong Wallet.

@sinzii
Copy link
Member Author

sinzii commented Jul 6, 2024

Hey @josepot, thanks a lot for the suggestion.

I think it makes sense to reuse the existing community works and not to duplicate the our efforts.

I've taken a look into @polkadot-api/merkleize-metadata, it's a really great work that you guys are doing there. But PAPI and Dedot are currently using different set of utilities & underlying libraries (like for scale-codec or hexToU8a conversion), so if Dedot were to ultilize this, it would result in the dapps into bundle duplicated source code for the same purpose (e.g: deshape & scale-ts would be bundled together inside the dapp), which is not ideal since it would unexpectedly increase the bundle size of the dapp. As a dapp developer, I would usually try to avoid this to help create an optimal dapp bundle size.

With that, I think we're still leaning toward having our own implementation. But we'll research and consider again when we start working on this.

@josepot
Copy link

josepot commented Jul 13, 2024

so if Dedot were to ultilize this, it would result in the dapps into bundle duplicated source code for the same purpose (e.g: deshape & scale-ts would be bundled together inside the dapp), which is not ideal since it would unexpectedly increase the bundle size of the dapp. As a dapp developer, I would usually try to avoid this to help create an optimal dapp bundle size.

Your concerns about the @polkadot-api/merkleize-metadata package size seem exaggerated. Let's break down its total size of 53.4Kb:

  • 20.09Kb comes from @noble/hashes (also used by your library)
  • 8.76Kb comes from @scure/base (also used by your library)
  • 8.29Kb is the implementation of the metadata merkleization, which you'd either have to implement or port from us, leaving little room for size reduction.
  • 3.76Kb comes from scale-ts, significantly less than deshape weight.
  • 11.55Kb comes from @substrate/bindings.

Given these numbers, it's clear that the issue is not as significant as you suggest. If the bundle size is truly a concern, please specify your target bundle size for this functionality. We can work to make it even smaller for you.

As demonstrated, the total added bundle size from this library is minimal. Do you really prefer to miss out on functionality to avoid adding less than 15Kb of uncompressed JS to Dedot's bundle size? This decision seems unreasonable without evidence that your own implementation would be more size-efficient.

Doesn't it make more sense to use our library initially, ensuring you have the functionality, and later, if you develop a more size-efficient solution, you can switch to your implementation?

@sinzii
Copy link
Member Author

sinzii commented Jul 17, 2024

Hey @josepot, thanks for the bundle-size analysis.

Yes agreed, for now dapps can using existing solutions provied by PAPI team or others. But we're still keeping the concerns about dapps having to bundle duplicate packages for the same purpose in mind and will revisit this later for room of optimization if needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants