pallet-contracts: Do not return an error on already applied upgrades

[bkchr]

Migrations in general should just skip the operation, if they were already applied. This is especially useful for CI checks that run in each pr to check that the current set of migrations are valid.

[pgherveou]

Migrations in general should just skip the operation, if they were already applied. This is especially useful for CI checks that run in each pr to check that the current set of migrations are valid.

agreed, let me update that to a warn log. I think the is_upgrade_supported might need to be adjusted as well. Double checking..

[bkchr]

agreed, let me update that to a warn log. I think the is_upgrade_supported might need to be adjusted as well. Double checking..

There is a warn in on_runtime_upgrade.

[paritytech-cicd-pr]

The CI pipeline was cancelled due to failure one of the required jobs.
Job name: test-linux-stable 3/3
Logs: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/4127294

[liamaharon]

already fixed #2079

[pgherveou]

@liamaharon
I feel that this fix here is a bit more accurate, if a migration step is missing we should panic in try_runtime not just log a warning, it means that the migration will for sure fails on_runtime_upgrade.

This should stay

ensure!(
	T::Migrations::is_upgrade_supported(on_chain_version, current_version),
	"Unsupported upgrade: VERSION_RANGE should be (on-chain storage version + 1, current storage version)"
);

the noop case though should just trigger a warning.
Maybe I am missing something, feel free to comment and re-close again

[liamaharon]

@pgherveou can that case be hit during normal execution (if the migration is left in the Executive tuple indefinitely) or does it only exec in an error path?