Refund leases that have not started. #206
Conversation
|
This comment no longer applies due to code changes. |
eskimor
changed the title
eskimor
force-pushed
the
rk-migrate-lazy-leases
branch
from
cf419de to
099e179
Compare
| if lease.iter().any(Option::is_none) { | ||
| return None | ||
| if let Err(err) = slots::Pallet::<Runtime>::clear_all_leases( |
There was a problem hiding this comment.
If they currently have an active lease which ends in say three months, then also won an auction which gives them a lease starting in three months, then this will kick them off their current lease as well as the one that has not yet started. Surely the intended functionality is that they are refunded for only the lease that has not yet started.
Or can they only swap at the point their current lease ends, i.e. the second lease that has not started is always under a separate para_id?
There was a problem hiding this comment.
In your particular scenario there would be no Nones .. .you have a lease for the next three months and one an auction for the ones afterwards ... all good.
In general, what happens here is you either get your lease to coretime or not. If not, you get all your leases refunded - even an ongoing one as in fact you just lost it. So this looks correct to me.
There was a problem hiding this comment.
Sorry, bad example. Slightly different to get a None in there but keeping my question the same:
If they currently have an active lease which ends three months, then also won an auction which gives them a lease starting in four and half months, they have a None in there, they'll be refunded their future lease but also booted off their current lease three months earlier than they expected.
Maybe that's a bit contrived, but I don't think a current lease should not be migrated, even if it is refunded, just because they have a gap before the lease they have won starts.
Maybe this edge case doesn't exist in reality, in which case all good.
There was a problem hiding this comment.
But in that case they already have a hole and thus downtime. I would assume if that edge case even exists that some more downtime won't kill them.
Not saying that this is ideal, but:
(1) I would really assume that this is an edge case that will very likely not even exist. And if so, a solution might be similar to whatever we are going to do about swaps.
(2) We could trim the leases to the first None, but then we would have another problem: The leases after the Nones would keep their token locked, despite the fact that we would have ended their lease on coretime.
There was a problem hiding this comment.
Missed this the other day, that sounds sensible to me
Co-authored-by: joe petrowski <[email protected]>
|
/merge |
|
Enabled Available commands
For more information see the documentation |
Head branch was pushed to by a user without write access
|
/merge |
|
Enabled Available commands
For more information see the documentation |
Failed to update PR ❌There was an error while trying to keep this PR You may have conflicts More info in the logs 📋 |
Failed to update PR ❌There was an error while trying to keep this PR You may have conflicts More info in the logs 📋 |
fellowship-merge-bot
bot
merged commit e42821d
into
polkadot-fellows:main
Needed for a smooth migration to Coretime.