Fix runtime migrations check (#10570)

- On the scheduled run only download the snapshot and not try to run the
migration checks
- The job should fail when one of the migration checks was cancelled
(because of timeout for example)
- Increase the timeout to 120 min, because sometimes it is slower to
download the snapshot

Should be merged after:
#10566

---------

Co-authored-by: Evgeny Snitko <[email protected]>

Author: bkchr

.github/workflows/check-runtime-migration.yml

@@ -31,7 +31,7 @@ jobs:
     if: ${{ needs.preflight.outputs.changes_rust }}
     # We need to set this to rather long to allow the snapshot to be created, but the average time
     # should be much lower.
-    timeout-minutes: 60
+    timeout-minutes: 120
     needs: [preflight]
     container:
       image: ${{ needs.preflight.outputs.IMAGE }}
@@ -106,12 +106,14 @@ jobs:
           ./try-runtime create-snapshot --uri ${{ matrix.uri }} snapshot.raw
 
       - name: Build Runtime
+        if: ${{ github.event_name != 'schedule' }}
         id: required1
         run: |
           echo "---------- Building ${{ matrix.package }} runtime ----------"
           forklift cargo build --release --locked -p ${{ matrix.package }} --features try-runtime -q
 
       - name: Run Check
+        if: ${{ github.event_name != 'schedule' }}
         id: required2
         run: |
           echo "Running ${{ matrix.network }} runtime migration check"
@@ -124,7 +126,7 @@ jobs:
           sleep 5
 
       - name: Stop all workflows if failed
-        if: ${{ failure() && (steps.required1.conclusion == 'failure' || steps.required2.conclusion == 'failure') }}
+        if: ${{ failure() && github.event_name != 'schedule' && (steps.required1.conclusion == 'failure' || steps.required2.conclusion == 'failure') }}
         uses: ./.github/actions/workflow-stopper
         with:
           app-id: ${{ secrets.WORKFLOW_STOPPER_RUNNER_APP_ID }}
@@ -137,7 +139,7 @@ jobs:
     name: All runtime migrations passed
     # If any new job gets added, be sure to add it to this array
     needs: [check-runtime-migration]
-    if: always() && !cancelled()
+    if: always() && !cancelled() && github.event_name != 'schedule'
     steps:
       - run: |
           tee resultfile <<< '${{ toJSON(needs) }}'

substrate/frame/assets/src/lib.rs

@@ -2028,7 +2028,15 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 			);
 
 			let calculated_approvals = Approvals::<T, I>::iter_prefix((&asset_id,)).count() as u32;
-			ensure!(details.approvals == calculated_approvals, "Asset approvals count mismatch");
+
+			if details.approvals != calculated_approvals {
+				log::error!(
+					"Asset {asset_id:?} approvals count mismatch: calculated {calculated_approvals} vs expected {}",
+					details.approvals,
+				);
+
+				return Err("Asset approvals count mismatch".into())
+			}
 		}
 		Ok(())
 	}

substrate/frame/assets/src/tests.rs

@@ -355,11 +355,13 @@ fn transfer_approved_all_funds() {
 		Balances::make_free_balance_be(&1, 2);
 		assert_ok!(Assets::approve_transfer(RuntimeOrigin::signed(1), 0, 2, 50));
 		assert_eq!(Asset::<Test>::get(0).unwrap().approvals, 1);
+		assert!(Approvals::<Test>::contains_key((0, 1, 2)));
 		assert_eq!(Balances::reserved_balance(&1), 1);
 
 		// transfer the full amount, which should trigger auto-cleanup
 		assert_ok!(Assets::transfer_approved(RuntimeOrigin::signed(2), 0, 1, 3, 50));
 		assert_eq!(Asset::<Test>::get(0).unwrap().approvals, 0);
+		assert!(!Approvals::<Test>::contains_key((0, 1, 2)));
 		assert_eq!(Assets::balance(0, 1), 50);
 		assert_eq!(Assets::balance(0, 3), 50);
 		assert_eq!(Balances::reserved_balance(&1), 0);