paritytech / substrate

Substrate: The platform for blockchain innovators
Apache License 2.0
8.39k stars 2.65k forks source link

asset-conversion pallet: persist a pool account id #14784

Closed muharem closed 1 year ago

muharem commented 1 year ago

Is there an existing issue?

Experiencing problems? Have you tried our Stack Exchange first?

Motivation

A pool account id is computed every time we need it. The computation involves a decoding which in theory might change over time (with some major updates for example). I can not reason from the code base why should the account id should be compute every time we need it.

Solution

Compute a pool account id on a pool creation and store it in the Pools storage as PoolInfo.

muharem commented 1 year ago

@jsidorenko @gilescope

gilescope commented 1 year ago

I like this idea as it also makes the data more easily discoverable for people/apps walking the state.

bkchr commented 1 year ago

The computation involves a decoding which in theory might change over time (with some major updates for example).

This argument could also be used for every type that may changes and thus the decoding could fail in the future.

I can not reason from the code base why should the account id should be compute every time we need it.

We are doing this everywhere were we use an account id. The question here would be on what is faster, a storage read or doing the computation.

muharem commented 1 year ago

@bkchr agree. and accessing the storage wont be faster. my benchmarks showed pool id generation is matter of nano seconds. but may be worth having this additional docs https://github.com/paritytech/substrate/pull/14811