Skip to content

fix(escrow): implement explicit legacy version and migration checks - #1249

Open
onyekachi66 wants to merge 1 commit into
Liquifact:mainfrom
onyekachi66:fix/issue-legacy-migration
Open

fix(escrow): implement explicit legacy version and migration checks#1249
onyekachi66 wants to merge 1 commit into
Liquifact:mainfrom
onyekachi66:fix/issue-legacy-migration

Conversation

@onyekachi66

Copy link
Copy Markdown
Contributor

Closes #1216

Approach

This PR addresses the correctness and security gap where older storage lacking a version marker could corrupt live escrow state if its layout was incorrectly guessed. We introduce an explicit LEGACY_VERSION (5) and a typed error EscrowError::AmbiguousLegacyStorage (93). The migrate function now explicitly detects the absence of a version marker and verifies if the storage matches the known legacy layout (by checking for the existence of DataKey::FundingToken and DataKey::Treasury). If it doesn't match the known legacy layout, the ambiguous storage is rejected with AmbiguousLegacyStorage.

Test Evidence

We have added focused unit and integration coverage for the new behavior in escrow/src/tests/migration_errors.rs, covering every requested edge case:

  • known legacy layout: test_known_legacy_layout_migration
  • missing marker: test_ambiguous_legacy_storage_missing_marker
  • unknown marker: test_unknown_marker
  • migration repeated: test_migration_repeated
  • partial migration: test_partial_migration

All existing tests pass with no regressions, and the new checks maintain atomic, bounded state changes with no dead code.

Security Notes

  • Storage compatibility is strictly preserved.
  • We rely on has(&DataKey::FundingToken) and has(&DataKey::Treasury) as the heuristic to identify a fully initialized V5 known legacy layout. If those keys are absent on a markerless instance, we reject it as ambiguous rather than guessing its layout, preventing corruption.
  • The AmbiguousLegacyStorage error is structured, stable, and does not leak internal details.
  • The existing auth and replay boundaries in migrate remain intact and precede the new version marker logic, ensuring safe concurrent invocation and replay resistance.

@drips-wave

drips-wave Bot commented Aug 30, 2026

Copy link
Copy Markdown

@onyekachi66 Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits.

You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀

Learn more about application limits

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.

Add a migration guard for missing version markers

1 participant