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
125 stars 72 forks source link

Adjust to new Broker pallet. #334

Closed eskimor closed 3 weeks ago

eskimor commented 1 month ago
muharem commented 1 month ago

@eskimor is it still draft or ready for review? also some required CIs are falling

eskimor commented 4 weeks ago

@muharem so far draft. The polkadot-sdk release this PR relies on does not exist yet.

eskimor commented 3 weeks ago

Test is fixed. For adjusting the base price, I was thinking about writing a migration, but we could equally well let the price controller do its thing: It should adjust downwards itself in the next cycle. Only "downside" would be that the minimum price stays the same for another cycle.

seadanda commented 3 weeks ago

If this enacts before rotate_sale and at least one core is sold on the open market we start at 230 KSM, decreasing with a step to a minimum of 2.3.

If no cores are sold it will be 2300 down to 23 and we'll lose a month of info about the new pricing model. Currently we're in the fixed price stage and sellout price is still None, so that's quite likely IMO.

If it enacts after rotate_sale then it will be adapted by the old price adapter relative to the number of cores sold. currently we've sold 7/15 (renewals) and we're at the fixed price, so the base price would be about 12.8 KSM. That means the price varies between 1280 and 12.8. But in this case the new renewals coming off a lease will have a high price IIUC.

After weighing that up I would say a migration for the base price makes sense, but only if we are sure we can enact and perform the runtime upgrade in time before the sale is rotated, otherwise I would say we schedule it for the interlude and add a migration for the renewal prices for those coming off their leases the next month who have high prices since 12 KSM isn't an awful minimum. WDYT?

eskimor commented 3 weeks ago

If this enacts before rotate_sale and at least one core is sold on the open market we start at 230 KSM, decreasing with a step to a minimum of 2.3.

Yep, this scenario would be fine.

But the same would be true if no core would be sold as we then take the current base price as target price.

That means the price varies between 1280 and 12.8. But in this case the new renewals coming off a lease will have a high price IIUC.

Indeed the renewal price would then be 128 KSM, which is a bit much. They could go down and wait for the market, but then losing their renewal guarantee.

So yeah, a migration adjusting the renewal price to the current price makes sense. We should also adjust periods - the fix price length can be much shorter.

seadanda commented 3 weeks ago

But the same would be true if no core would be sold as we then take the current base price as target price.

Ah great, knew I was missing something - this makes more sense. I was mixing up the target price and base/min/regular/fixed price (🫠).

So yeah, a migration adjusting the renewal price to the current price makes sense. We should also adjust periods - the fix price length can be much shorter.

I agree. However in general this is the minimum time that the secondary markets have to operate within, so we don't want to go too short on it. Maybe roughly 1:1 primary:secondary? Or what were you thinking? I suppose it could even be 2:1 if we expect the target price to be usually correct long term.

eskimor commented 3 weeks ago

Good point on secondary markets! I was thinking 2:1. 2 weeks leadin, 1 week fixed, but we can also leave it as is, because price wise we are actually fine:

If the migration hits before rotate sale - we already concluded that we are fine, if it hits after then the price for renewals is already determined by the old controller - so we are good as well. On the next sale the prices will be adjusted already.

So lease renewal price will be 12.8 not 128.

eskimor commented 3 weeks ago

@muharem ready for review.

joepetrowski commented 3 weeks ago

/merge

fellowship-merge-bot[bot] commented 3 weeks 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)