Skip to content

Conversation

aterga
Copy link
Member

@aterga aterga commented Sep 1, 2025

Motivation

This PR continues the work towards migrating the lookup_application_with_origin map's keys from 8-byte prefixes of a full sha-256 sum.

< Previous PR |

Changes

  • Added a one-off call at the end of II's post_upgrade.

Tests

Integration

  • upgrade_and_rollback_with_realistic_data_migration - Upgrading the II canister succeeds, migrating 2,000 application-specific user accounts. This test passed locally in 319.35s; if that's not acceptable for the CI, we can reduce the number of accounts in this test to 10, and re-run it locally to gain confidence in the migration.

Unit

  • should_skip_migration_if_new_map_not_empty - When the new map already contains entries, the migration leaves both maps unchanged.
  • should_migrate_single_entry_successfully - Tests successful migration of one application from old hash map to new SHA-256 map, verifying correct return values and final state.
  • should_migrate_multiple_entries_successfully - Tests successful migration of multiple applications, ensuring all entries are properly transferred with correct mappings.
  • should_handle_missing_application_gracefully - Tests behavior when old map references non-existent applications, verifying missing apps are reported in return values and skipped during migration.
  • should_handle_empty_old_map - Tests migration behavior with empty maps, ensuring function returns empty collections and maps remain empty.
  • should_handle_hash_collisions_correctly - Tests collision detection where multiple applications exist in stable storage but only one is in the old map due to hash collision, verifying collision recovery mechanism.

@aterga aterga marked this pull request as ready for review September 1, 2025 18:21
@aterga aterga requested a review from lmuntaner September 1, 2025 18:22
Copy link
Collaborator

@lmuntaner lmuntaner left a comment

Choose a reason for hiding this comment

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

Looks great! Just a few questions before approving to better understand the changes.

Copy link
Collaborator

@lmuntaner lmuntaner left a comment

Choose a reason for hiding this comment

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

Thanks!

@aterga aterga requested a review from lmuntaner September 3, 2025 12:24
Copy link
Collaborator

@lmuntaner lmuntaner left a comment

Choose a reason for hiding this comment

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

Looking great! Thanks!

@aterga aterga enabled auto-merge September 3, 2025 15:59
@aterga aterga added this pull request to the merge queue Sep 3, 2025
Merged via the queue into main with commit 3edb0e2 Sep 3, 2025
76 of 77 checks passed
@aterga aterga deleted the arshavir/ID-351 branch September 3, 2025 16:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants