Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rent based PVF registration #1796

Open
eskimor opened this issue Oct 5, 2023 · 9 comments
Open

Rent based PVF registration #1796

eskimor opened this issue Oct 5, 2023 · 9 comments

Comments

@eskimor
Copy link
Member

eskimor commented Oct 5, 2023

We should make sure the deposit for registering a PVF is significant and reflects storage costs properly. It is better to make this smaller later, if need be than the other way round.

@eskimor
Copy link
Member Author

eskimor commented Oct 12, 2023

PVF deposit seems to be quite large already. In fact probably too large for on-demand. We should think of an alternative model in addition to the deposit one, where you have to pay "rent": A relatively small amount, but on a regular basis and if you don't pay the storage gets cleared. Pruning of unpaid storage can be triggered off-chain via an extrinsic.

@bkchr
Copy link
Member

bkchr commented Oct 15, 2023

Pruning of unpaid storage can be triggered off-chain via an extrinsic.

Whoever triggers this, should get the initial deposit.

@Szegoo
Copy link
Contributor

Szegoo commented Oct 19, 2023

I'd like to propose a solution for implementing a "rental system" without the need for any storage migrations.

The following describes the steps of my suggested solution:

  • Three configuration constants should be added to the paras_registrar pallet:
/// Defines how frequently the rent needs to be paid.
#[pallet::constant]
type RentDuration: Get<BlockNumberFor<Self>>;

/// The initial 'base' deposit of registering a parathread.
#[pallet::constant]
type RentalParaDeposit: Get<BalanceOf<Self>>;

/// The recurring rental cost as a percentage of the initial rental registration payment.
#[pallet::constant]
type RecurringRentCost: Get<Perbill>;
  • A new RentedParas storage map should be added that will map ParaId to RentInfo.
pub struct RentInfo<Balance, BlockNumber> {
	// Stores information about the last time the rent was paid.
	last_rent_payment: BlockNumber,
	// The amount that needs to be paid every `T::RentDuration` blocks.
	rent_cost: Balance,
}
  • The same way we have the register extrinsic we should introduce a new register_rental extrinsic that will charge the caller a smaller initial deposit compared to regular.
pub fn register_rental(
	origin: OriginFor<T>,
	id: ParaId,
	genesis_head: HeadData,
	validation_code: ValidationCode,
) -> DispatchResult {
	let who = ensure_signed(origin)?;

	Self::do_register(who, None, id, genesis_head, validation_code, true, ParaKind::Parathread)?;

	Ok(())
}

/* -- snip -- */
fn do_register(
	who: T::AccountId,
	deposit_override: Option<BalanceOf<T>>,
	id: ParaId,
	genesis_head: HeadData,
	validation_code: ValidationCode,
	ensure_reserved: bool,
	para_kind: ParaKind, // <- new `para_kind` parameter.
) -> DispatchResult {
	/* -- snip -- */
	
	let (genesis, deposit) = 
		Self::validate_onboarding_data(genesis_head, validation_code, para_kind)?;
	// ^^^ Based on the `para_kind` parameter it will figure out the proper deposit.

	/* -- snip -- */
}

We would also need to add other two extrinsics:

  • pay_rent - Allows the caller to pay the rent_info.rent_cost and updates the rent_info.last_rent_payment
  • prune_unpaid - Can be called by anyone. It will remove all information related to the on-demand chain, and as a reward, the caller will receive the initial deposit.

If no prior work has been done on this, and if this solution seems sensible, I would like to implement it.

@eskimor
Copy link
Member Author

eskimor commented Oct 19, 2023

Removing the PVF of a parachain that has opened channels and stuff might be problematic/not possible. What we could do, is have the hash of the PVF registered for a very small deposit. Then we can safely remove the actual PVF, because if the parachain wants to produce a block again, it can just re-register the PVF (with same hash) and produce the block. Not even pre-checking would be required, as long as the hash stays the same.

@Szegoo
Copy link
Contributor

Szegoo commented Oct 19, 2023

True, that can make things more trickier. I think storing the hash of the PVF is a good solution and it shouldn't be hard to implement.

Probably not necessary since this would increase complexity, but if we really want to minimize the cost of registering a PVF, the additional deposit for storing the hash could be accounted for when opening a channel.

@eskimor
Copy link
Member Author

eskimor commented Oct 19, 2023

If no prior work has been done on this, and if this solution seems sensible, I would like to implement it.

Awesome! At a glance the solution seems sensible. I have two more concerns though:

  1. Is it possible to end the rent willingly - and get the deposit back? Or does the owner always race with others? If the deposit is small enough and we communicate it as part of the rent (consider it lost). The latter should be fine.
  2. For the hash solution, there are also some things to consider:

It must never happen that the PVF exists in backing, but no longer in approval/disputes. This would be really bad. In general we need to handle the case that the actual PVF does not exist.

Both should be resolvable by a two-phase process:

  1. Rent is over - parachain moves into "hibernated" state. - Still exists, but is no longer allowed to author any blocks.
  2. After a full session has passed, we actually remove the PVF from storage.
    It should be cheap enough, to just check rent status before scheduling any para. Then we only need a transaction for (2).

@Szegoo
Copy link
Contributor

Szegoo commented Oct 19, 2023

  1. Is it possible to end the rent willingly - and get the deposit back? Or does the owner always race with others? If the deposit is small enough and we communicate it as part of the rent (consider it lost). The latter should be fine.

Yeah, It should be fairly simple to add a separate extrinsic to end the rent and claim the deposit back, but as you said there is probably no need for that.

Then we only need a transaction for (2).

I am not sure if I correctly understand what you are referring to here.

After a full session has passed, we actually remove the PVF from storage.

What would trigger this? I assume we don't want the scheduler to trigger this and not sure if putting this logic in a hook like on_initialize is ideal, but I might be wrong.

If both of these removal phases are triggered by users the prune_unpaid extrinsic would need to be called twice. Once to move the parachain to the "hibernated" state and once to actually remove it from state.
This would mean that for users calling prune_unpaid would only be worth it when the parachain is already in a "hibernated" state assuming that is when the 'reward' is received.
A solution to this might be to split the 'reward'. The caller that moves the parachain into a "hibernated" state would receive half of the deposit and whoever manages to call prune_unpaid the second time will get the other half.

@eskimor
Copy link
Member Author

eskimor commented Oct 20, 2023

What would trigger this? I assume we don't want the scheduler to trigger this and not sure if putting this logic in a hook like on_initialize is ideal, but I might be wrong.

The prune transaction would be invalid, if the block where the rent expired has not been a complete session ago. As sessions are time based and not block based it might make more sense to specify the rent in terms of sessions. Then this check would be super simple.

The hibernation state could be calculated lazily by accessing code - so no actual state change needs to occur. We just check whether the rent was paid for the current session, if not - we won't schedule that para.

Does this make sense?

@Szegoo
Copy link
Contributor

Szegoo commented Oct 20, 2023

@eskimor Yeah, this makes sense. Thanks.

@eskimor eskimor changed the title Increase PVF deposit Rent based PVF registration Mar 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Backlog
Status: Todo
Development

No branches or pull requests

3 participants