-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Migrate parachain swaps to Coretime #3714
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
Changes from 23 commits
36a6368
37962d0
d60b31e
03aaad2
bd32142
4bd69ab
518a072
296521a
e0a1424
ff0d3d7
2f72d26
b2c5ddd
1953bc2
e419bb7
8986843
7f630c8
cc2efb7
a8f17f7
f5f6dce
113bbcf
89ef634
fce11f9
7351ea6
0bbcfd1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| title: Handle legacy lease swaps on coretime | ||
|
|
||
| doc: | ||
| - audience: Runtime Dev | ||
| description: | | ||
| When a `registar::swap` extrinsic is executed it swaps two leases on the relay chain but the | ||
| broker chain never knows about this swap. This change notifies the broker chain via a XCM | ||
| message for a swap so that it can update its state. | ||
| crates: | ||
| - name: pallet-broker |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -437,4 +437,22 @@ impl<T: Config> Pallet<T> { | |
| Self::deposit_event(Event::AllowedRenewalDropped { core, when }); | ||
| Ok(()) | ||
| } | ||
|
|
||
| pub(crate) fn do_swap_leases(id: TaskId, other: TaskId) -> DispatchResult { | ||
| let mut id_leases_count = 0; | ||
| let mut other_leases_count = 0; | ||
| Leases::<T>::mutate(|leases| { | ||
| leases.iter_mut().for_each(|lease| { | ||
| if lease.task == id { | ||
| lease.task = other; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: I don't think
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good point though!
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I thought there could be multiple leases and we do want to swap all of them. If this is the case, we'd better add another
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Like this -> f5f6dce The error code I use is not entirely correct but I don't see a point adding a new one just for a temp call.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No Basti has a point. The "correctness" of the swap has already been verified on the relay chain, we should be very lenient on the Coretime chain. In particular, there could be a race and the lease of one para expired by the time the message arrives at the coretime chain (admittedly super unlikely edge case, but still), we should still do the swap, although one of the leases does not exist. I mean that race in particular is really super niche, if a user updates that close he could miss the deadline himself already. Nevertheless, I would err on as little ensures as possible: The swap is confirmed, it actually has to be good.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, I came to the same conclusion and removed everything. Even if someone decides to use the extrinsic directly it can be called again and 'undo' the result. |
||
| id_leases_count += 1; | ||
| } else if lease.task == other { | ||
| lease.task = id; | ||
| other_leases_count += 1; | ||
| } | ||
| }) | ||
| }); | ||
|
|
||
| Ok(()) | ||
| } | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.