Skip to content

Fix #6102#6384

Merged
eskimor merged 8 commits into
masterfrom
rk-relax-assign-core-requirements
Nov 6, 2024
Merged

Fix #6102#6384
eskimor merged 8 commits into
masterfrom
rk-relax-assign-core-requirements

Conversation

@eskimor

@eskimor eskimor commented Nov 6, 2024

Copy link
Copy Markdown
Member

Relax requirements for assign_core so that it accepts updates for the last scheduled entry.

Fixes #6102

@eskimor

eskimor commented Nov 6, 2024

Copy link
Copy Markdown
Member Author

/cmd prdoc

@eskimor eskimor added T2-pallets This PR/Issue is related to a particular pallet. T8-polkadot This PR/Issue is related to/affects the Polkadot network. labels Nov 6, 2024
Comment thread polkadot/runtime/parachains/src/assigner_coretime/mod.rs Outdated
Comment thread polkadot/runtime/parachains/src/assigner_coretime/mod.rs Outdated
Comment thread polkadot/runtime/parachains/src/assigner_coretime/mod.rs Outdated
.try_fold(PartsOf57600::ZERO, |sum, parts| {
sum.checked_add(parts).ok_or(Error::<T>::OverScheduled)
})?;
ensure!(parts_sum.is_full(), Error::<T>::UnderScheduled);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if the schedule is split up, but we only receive one message in time? Will it break any assumptions if we don't have the full parts scheduled?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No. I checked, the code does not really care whether the schedule is full or not.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably worth a unit test in case this behaviour is changed in future.
Should be very quick to add - just an additional core that is not fully assigned in the test.

Comment thread polkadot/runtime/parachains/src/assigner_coretime/mod.rs
.try_fold(PartsOf57600::ZERO, |sum, parts| {
sum.checked_add(parts).ok_or(Error::<T>::OverScheduled)
})?;
ensure!(parts_sum.is_full(), Error::<T>::UnderScheduled);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably worth a unit test in case this behaviour is changed in future.
Should be very quick to add - just an additional core that is not fully assigned in the test.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

T2-pallets This PR/Issue is related to a particular pallet. T8-polkadot This PR/Issue is related to/affects the Polkadot network.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Relay chain coretime assigner does not support more assignments than fit in a single XCM message (currently 28)

6 participants