autonomys / subspace

Subspace Network reference implementation
https://subspace.network
369 stars 242 forks source link

Merge domain config items bundle_slot_probability and target_bundles_per_block #2365

Open dariolina opened 8 months ago

dariolina commented 8 months ago

Currently, the target_bundles_per_block domain config item is not influencing anything about bundle production and is in conflict with the bundle_slot_probability item, which must be > 0 and ≤ 1. Particularly, given any bundle_slot_probability , target_bundles_per_block above 6 is not attainable. bundle_slot_probability is used as a multiplier in the VRF election to control bundle production of 1 or fewer bundles per slot.

jfrank-summit commented 7 months ago

@dariolina given the changes that will happen in #2413 is this issue still valid?

dariolina commented 7 months ago

@jfrank-summit Yes, I'd like to see both in. If this one lands first, then #2413 will need to be updated to new definition.

vedhavyas commented 3 months ago

Particularly, given any bundle_slot_probability , target_bundles_per_block above 6 is not attainable.

I agree the target_bundles_per_block is not useful and can be removed.

bundle_slot_probability should be renamed to target_bundles_per_slot and defined as a float with value >0 and possibly above 1 to allow for domains who want more bundles per slot. target_bundles_per_slot will be used where bundle_slot_probability is used rn and for bundle weight limits.

I'm confused with target_bundles_per_slot actually since this number can is not a fraction anymore, how does this play into proof of election. There should be a max bundle per domain since more bundles can lead to higher domain block weight With this change, We would need to think more on the max extrinsic weight I believe

dariolina commented 3 months ago

target_bundles_per_slot could still be a fraction and above 1, i.e. (3,2) and I believe it would still work fine with the election. But I'm also ok with keeping bundle_slot_probability <= 1 for now and we can later support higher if there is a usecase.

vedhavyas commented 3 months ago

But I'm also ok with keeping bundle_slot_probability <= 1 for now and we can later support higher if there is a usecase.

Okay works for me.

target_bundles_per_block

We cannot directly remove it right now for gemini-3h without migration. Do you prefer removing it right now or before deprecating gemini-3h ?

dariolina commented 3 months ago

I don't think we're going to have another network after gemini-3h, that'd be directly mainnet. But @NingLin-P said the migration would be blocked by #2712, so that would need to be resolved first. While we're changing the domain config, we should also change weight and size limits to be per bundle, as in corresponding spec changes https://github.com/subspace/protocol-specs/pull/22