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

Migration example #352

Merged
merged 6 commits into from
Mar 7, 2023
Merged

Migration example #352

merged 6 commits into from
Mar 7, 2023

Conversation

volovyks
Copy link
Collaborator

@volovyks volovyks commented Mar 3, 2023

It appeared that additional parameter does not play well with the current design.
I've added new @Migrate decorator that is similar to @initialize, but does not read or checks the state.

@volovyks volovyks requested a review from ailisp as a code owner March 3, 2023 14:33
@volovyks volovyks linked an issue Mar 3, 2023 that may be closed by this pull request
migrateState(): Vector<NewMessageFormat> {
assert(this.messages.toArray().length == 0, "Contract state should not be deserialized in @migrate");
// retrieve the current state from the contract
let raw_vector = JSON.parse(near.storageRead("STATE")).messages;
Copy link
Member

@ailisp ailisp Mar 6, 2023

Choose a reason for hiding this comment

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

Instead of reading from raw state, migrateState can takes an argument and @migrate({}) does deserialization for it. e.g.:

    migrateState({messages}: {messages: Vector<OldMessageFormat>}): Vector<NewMessageFormat> {
        let old_messages = messages;
        // same as your implementation below

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Why should we pass old messages?
I think that the whole idea behind migration is to rewrite the data in new format. It means:

  • read the old data
  • reformat it
  • save new data to state

Copy link
Member

Choose a reason for hiding this comment

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

Your approach works and looks intuitive! I was thinking a syntax sugar to read the old data to avoid JSON.parse(near.storageRead("STATE")) pattern. User can write:

@Migrate
migrateState(oldState: OldStateFormat): NewStateFormat {
    let newState: NewStateFormat = some_reformat_steps(oldState);
    return newState;
}

@Migrate would know how to load (deserialize) oldState given OldStateFormat. @Migrate would also serialize reformat state using newStateFormat.

A possible @Migrates's deserialize:

let oldRaw = near.storageRead("STATE")
let old = OldStateFormat.deserialize ? OldStateFormat.deserialize(oldRaw) 
  : OldStateFormat.reconstruct ? OldStateFormat.reconstruct(JSON.parse(oldRaw))
  : JSON.parse(oldRaw)

Copy link
Member

Choose a reason for hiding this comment

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

^ is i think nice to have in API but it's up to your preference. The PR is considered done!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Since we do not have any NEP - or complex approaches are questionable. Let's get back to it if necessary.

Copy link
Member

@ailisp ailisp left a comment

Choose a reason for hiding this comment

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

Nice work! The implementation and example is solid. An experienced developer can figure out how to make migration for his contract. I leave a bit improvement to enhance DevX.

Copy link
Member

@ailisp ailisp left a comment

Choose a reason for hiding this comment

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

Thank you! Very helpful pattern and very good implementation

@volovyks volovyks merged commit 755e2fd into develop Mar 7, 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

Successfully merging this pull request may close these issues.

Contract state migration pattern
2 participants