Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conflict during migration with data in the contract #49

Closed
olga24912 opened this issue May 11, 2023 · 3 comments
Closed

Conflict during migration with data in the contract #49

olga24912 opened this issue May 11, 2023 · 3 comments
Assignees
Milestone

Comments

@olga24912
Copy link
Collaborator

Do I understand correctly the migration mechanism:

  • First we create and init contract
  • Migrate the data

In that case, we already have some data in the contract. For example, we register the current_account_id and owner_id and change accounts_counter https://github.com/aurora-is-near/aurora-eth-connector/blob/master/eth-connector/src/lib.rs#L197

After migration, we don't delete the current_account_id and owner_id from the accounts list, but rewrite account_counter, as a result, it can become incorrect. https://github.com/aurora-is-near/aurora-eth-connector/blob/master/eth-connector/src/lib.rs#L625

Moreover, we don't check at the end of the migration that the data is consistent:

  • account_counter == self.ft.account.size();
  • ft.total_supply == sum(ft.accounts.amount)
  • ft.account_storage_usage == ???

And we don't migrate metadata

I propose checking before migration that all the data is empty and don't register anything in new function.

Or presenting more clear guarantee for the migration. For example, merge the data instead of rewriting it.

@mrLSD
Copy link
Collaborator

mrLSD commented May 15, 2023

I think it's one major mistake here. Data should be the same as migrated data from the Engine. It means:

  1. ft.total_supply == total_supply from migration
  2. ft.accounts != accounts count from migration. But all accounts from migration should exist in eth-connector
  3. ft.account_counter > self.ft.account.size();

But about account_counter field. May be good idea just recalculate it, yes.

Other data look consistent, and checked with method check_migration_correctness

@mrLSD mrLSD self-assigned this May 15, 2023
@mrLSD mrLSD added this to the v0.5.1 milestone May 15, 2023
@mrLSD
Copy link
Collaborator

mrLSD commented May 15, 2023

We shouldn't migrate metadata. It's constant data, and it's initialized with contract.

@mrLSD mrLSD modified the milestones: v0.5.1, v0.6.0 May 22, 2023
@mrLSD
Copy link
Collaborator

mrLSD commented May 24, 2023

Refactored at #56

@mrLSD mrLSD closed this as completed May 24, 2023
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

No branches or pull requests

2 participants