Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Pre-generate runtime metadata, remove from runtime binary? #10057

Open
ascjones opened this issue Oct 19, 2021 · 13 comments
Open

Pre-generate runtime metadata, remove from runtime binary? #10057

ascjones opened this issue Oct 19, 2021 · 13 comments
Labels
Z2-medium Can be fixed by a coder with good Rust knowledge but little knowledge of the codebase.

Comments

@ascjones
Copy link
Contributor

Following on from #8615, the size of the metadata has increased (because of all the type information). Here are some benchmarks I did during the PR:

#8370 (comment)
#8615 (comment)
#8615 (comment)

There was some discussion #8370 (comment) (/cc @tomusdrw, @apopiak, @bkchr, @dvdplm) about whether it would be worth pre-generating the metadata and storing it somewhere other than in the runtime binary. For the purposes of getting the PR done sooner rather than later, we decided we could handle the larger binary size in the short term.

One solution would be to generate the metadata during build time, which might involve:

  • only include metadata when building with std
  • compile with std and then invoke a generate_metadata function
  • encode and (optionally) compress the metadata
  • calculate the hash of the encoded metadata
  • store the hash of the metadata in the runtime wasm, and store the metadata itself somewhere else.

I'm not intimately familiar with the existing build process, but I believe this workflow is not possible for the existing dev chain runtime where the wasm bytes are embedded directly into a constant for the std build.

One alternative might be to include the metadata only in the std build and have the node only allow the state_getMetadata query when running a native runtime, and then do some caching there. Although I think I heard that native runtimes might be deprecated in the future.

So the open questions here are:

  1. Is it possible to pre-generate the metadata during the build process? (@bkchr)
  2. Where should we store the metadata once it has been pre-generated?

Linking to #10056, which can be solved by this.

@ascjones ascjones added the Z2-medium Can be fixed by a coder with good Rust knowledge but little knowledge of the codebase. label Oct 19, 2021
@dvdplm
Copy link
Contributor

dvdplm commented Oct 19, 2021

I did some tinkering today and it doesn't look like it's the polkadot node that is slow here. I ran 10 connections on 10 threads for 30min against a local, synced, polkadot node, querying state_getMetadata:

Running 30m test @ http://127.0.0.1:9933
  10 threads and 10 connections
  Thread Stats   Avg      Stdev     Max   +/- Stdev
    Latency     5.76ms    1.03ms 117.13ms   89.15%
    Req/Sec   174.34     13.71   250.00     64.19%
  3126835 requests in 30.00m, 1.54TB read
Requests/sec:   1737.02
Transfer/sec:      0.87GB

This is http and not ws (which is what polkadot.js uses), but it seems like the node is pretty snappy.

So I'm not convinced we necessarily need to intervene on the substrate node side here, unless we want to make changes to offload the js client; things like doing the decoding on the node side, strip out doc strings, compressing the payload (likely not helping though), or some other filtering of what portion of the metadata to return.

@tomusdrw
Copy link
Contributor

@dvdplm state_getMetadata is performing a RuntimeApi call and it's indeed quite fast, but could be even faster (i.e. generating less load on the node, maintaining similar response rates) if stored in some alternative locations (e.g. one state read, or offchain db read)

I believe that the main point here is to split the metadata from the runtime itself, to lower the overall runtime code size, which affects other things beyond raw performance of retrieving the metadata (runtime upgrade transaction size, PoV blocks, etc).

@bkchr
Copy link
Member

bkchr commented Oct 21, 2021

For me the main question is still, where to put the metadata? If this question is solved, the rest is easy. How complicated is it to fetch the metadata for example from ipfs @jacogr ?

@xlc
Copy link
Contributor

xlc commented Oct 21, 2021

Substrate node is possible to serve IPFS content right? So one possible solution is have the native client serving the metadata over IPFS.

@bkchr
Copy link
Member

bkchr commented Oct 21, 2021

And how does the client gets the metadata?

@xlc
Copy link
Contributor

xlc commented Oct 21, 2021

The runtime includes the metadata multihash, and client just fetch it on IPFS network and hopefully can be served by one of the nodes? Network operators can also pin the metadata to ensure the availability.

@bkchr
Copy link
Member

bkchr commented Oct 21, 2021

Not sure about the IPFS status in Substrate.

@dvdplm
Copy link
Contributor

dvdplm commented Oct 21, 2021

@dvdplm state_getMetadata is performing a RuntimeApi call and it's indeed quite fast, but could be even faster (i.e. generating less load on the node, maintaining similar response rates) if stored in some alternative locations (e.g. one state read, or offchain db read)

@tomusdrw No doubt there are optimizations to be made here. The point I was trying to make is that on the user side of things, in polkadot.js and sidecar, fetching metadata is ~1000 times slower than getting the raw metdata over the RPC.
So efforts spent making this fast will have next to no impact on the user experience.

That said there are other good reasons for working on this, as you point out.

@jacogr
Copy link
Contributor

jacogr commented Oct 21, 2021

@bkchr As long as it has a hash, it can make an ipfs retrieval. Worst-case, if available on the node, can also store it "elsewhere" and return via a different RPC call that gets it from "elsewhere". (Wherever that may be - file system, runtime, offchain DB, ipfs, etc.) The only issue is making sure it is available exactly at the point the upgrade is live, and available to all.

@dvdplm Indeed via the JS API, it is much, much slower. The bottleneck from a JS API perspective is not the node, well, we had some issues with historic metadata at some point, but for latest, it certainly is never an issue. (Also the JS side is even slower if you are fetching a non-recent version of the metadata since there are conversion steps in-between). On my side I don't focus on performance at all, the prime directive with the time I have available is"getting it right".

@bkchr
Copy link
Member

bkchr commented Oct 21, 2021

@bkchr As long as it has a hash, it can make an ipfs retrieval. Worst-case, if available on the node, can also store it "elsewhere" and return via a different RPC call that gets it from "elsewhere". (Wherever that may be - file system, runtime, offchain DB, ipfs, etc.) The only issue is making sure it is available exactly at the point the upgrade is live, and available to all.

Okay. Then I would propose that we do the following:

  1. Change the metadata trait to:
	#[version(2)]
	pub trait Metadata {
		/// Returns the metadata of a runtime.
		#[changed_in(2)]
		fn metadata() -> MetadataOrHash;
		/// Returns the metadata of a runtime.
		fn metadata() -> MetadataOrHash;
	}

	enum MetadataOrHash {
		Metadata(OpaqueMetadata),
		Hash(Hash),
	}
  1. Introduce a feature for runtime that is not enabled by default, like the disable-logging feature. We could call it metadata-hash or whatever. When this feature is enabled the runtime api will return the hash. This will be added to the release build, so that this feature is enabled when we are building the on-chain wasm. Otherwise the endpoint returns the opaque metadata. This is especially useful for local testings where you just want to have it working and not want to require the user to somehow get the metadata etc.

@tomaka
Copy link
Contributor

tomaka commented Jan 12, 2022

I'm extremely opposed to adding the metadata on IPFS.

The IPFS protocol is extremely complicated, and you can only access it in three ways:

  • Using the Go node from Protocol Labs
  • Using an HTTP IPFS gateway
  • Creating our own implementation of IPFS

The first solution is problematic. Users that want to access a Substrate chain would have to install two binaries: the Substrate node client and an IPFS client. The IPFS node is also not embeddable in web pages, which makes it a no-go for substrate-connect.

The second solution goes against the idea of decentralization. If the gateway goes down or is blocked, IPFS becomes inaccessible.

The third solution would add thousands of hours of development and create its own problems, and is generally not realistic.

@olanod
Copy link
Contributor

olanod commented Jan 12, 2022

I thought there was already a lot of progress in Rust implementations of IPFS, would it really be thousands of hours?
The second option is not awful as long as developers don't have to hard-code a specific gateway, substrate-connect could do something like the IPFS browser extension and help resolve ipfs:///ipns:// protocols with preference for a locally deployed node. Maybe the extension can also encourage people to install a native version of the light node that will be IPFS capable in the future?
I think IPFS is the best bet to persistence and retrieval of arbitrarily sized blobs of data in decentralized set-ups, metadata fits well there imho. It might not be quite there yet ready for primetime but I think we can start paving the way to eventually get the "dreamed set-up". We have a concept runtime called Valor that mounts plugins as HTTP APIs as our bet for progressive decentralization which is basically acknowledging that decentralization is not perfect yet but we will totally get there without breaking people's code, it starts as centralized APIs api-sidecar style but can eventually run in the browser mocking a server with service workers, the "Blockchain API" plugin being a wrapper of our client library Sube that also starts with a less ideal HTTP backend but can transparently be replaceable by a smoldot based one in the future for example. ... All this to say that we don't need to make it perfect the first try but still chose the right tool for the job from the beginning.

@tomaka
Copy link
Contributor

tomaka commented Jan 12, 2022

It might not be quite there yet ready for primetime

We're talking about replacing something simple that works well and without any issue with something super complicated and not ready for prime time. This is not a good trade off.

As a side topic, I also want to point out that Substrate chains already have a DHT with the exact same algorithm as IPFS does. It's used to store the authority discovery records, but storing other things on it is trivial. All the functions are there and are robust.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Z2-medium Can be fixed by a coder with good Rust knowledge but little knowledge of the codebase.
Projects
None yet
Development

No branches or pull requests

8 participants