polkadot-fellows / runtimes

The various runtimes which make up the core subsystems of networks for which the Fellowship is represented.
GNU General Public License v3.0
135 stars 85 forks source link

Polkadot People Chain #319

Closed joepetrowski closed 3 months ago

joepetrowski commented 4 months ago

Polkadot People Chain

To verify prior to release: People Chain poke_deposit weight must be less than 2x Relay's. E.g.:

>>> weight_per_nano = 1_000
>>> relay_read = 20_499 * weight_per_nano
>>> relay_write = 83_471 * weight_per_nano
>>> para_read = 25_000 * weight_per_nano
>>> para_write = 100_000 * weight_per_nano
>>>
>>> def relay():
...     time = 145_781_000 + (3 * relay_read) + (3 * relay_write)
...     pov = 11_037
...     return (time, pov)
...
>>> def para():
...     time = 52_060_000 + (3 * para_read) + (3 * para_write)
...     pov = 6_723
...     return (time, pov)
...
>>>
>>> (relay_time, relay_pov) = relay()
>>> (para_time, para_pov) = para()
>>>
>>> (2 * relay_time) > para_time
True
>>> (2 * relay_pov) > para_pov
True
github-actions[bot] commented 4 months ago

Review required! Latest push from author must always be reviewed

bkontur commented 4 months ago

Looking ready, just a few small things below. people-polkadot is also missing from runtimes-matrix.json.

I also didn't check the weights as they seem to be unchanged from people-kusama. Will you run the benchmarks in this PR or elsewhere? Would be a good check to make sure the factor of two is sufficient between relay and para in case the ratio is very different on Polkadot

@joepetrowski @seadanda fresh weights and fixes are ready here: https://github.com/joepetrowski/runtimes/pull/2

joepetrowski commented 3 months ago

/merge

fellowship-merge-bot[bot] commented 3 months ago

Enabled auto-merge in Pull Request

Available commands - `/merge`: Enables auto-merge for Pull Request - `/merge cancel`: Cancels auto-merge for Pull Request - `/merge help`: Shows this menu For more information see the [documentation](https://github.com/paritytech/auto-merge-bot)
seadanda commented 3 months ago

I found there are too many duplicated code for a new system parachain. This is a code smell and should be addressed at some stage. We will have more system parachains and we need to reduce the overhead of making one before making more system parachains.

@xlc would be good to get your input on https://github.com/paritytech/polkadot-sdk/issues/4815 as a first step towards minimising the diff between system parachains