Contract Migration #137

mrLSD · 2021-06-10T22:46:32Z

Universal migration mechanism for storage state.
Possible actions:

    // Add field with Value
    Add,
    // Rename key field
    RenameKey,
    // Update value for key field
    UpdateValue,
    // Rename key and update value
    UpdateKeyValue,
    // Remove key field
    Remove,
    // Remove only value for key field
    RemoveValue,

The migration function can be invoked only by the contract owner (for security reasons).

* Add tests for state check after selfdestruct * Aurora runner tracks storage usage to avoid underflow when storage is released in future transactions (#85) * Implement generational storage (#87) * Base precompile code between connectors (#73) * Base precompile code between connectors * Handle errors and validate input * Use proper result * Document `aurora encode-address` usage. * Cache cargo artifacts between CI runs. (#92) * Address comments from audit. (#86) * Validate register length in `read_input_arr20()` * Only read register length in `Engine::get_code_size` * Add `read_input_borsh()` * Ensure `method.args.len() == args_decoded.len()` * Ensure register size is 8 in `read_u64` * Use constant to specify the register ID used in `read_input()` * Reduce size of `cargo cache` in CI. (#95) * Define a `Wei` newtype for balances. (#96) * Fix evm-bully builds after recent refactoring. (#100) * Refactor `Engine::get_state` to return a `Result`. (#99) * Ensure that `Cargo.lock` in the repo is valid. (#101) * Remove unneeded nightly feature. (#102) * Implement BC generational storage. * fix address input * remove note * put key on the end of the storage key * remove pub from methods * Dispatch precompiles on the full address. (#107) * Support state migration on upgrade. (#103) * Implement the ETH connector. (#59) * Move when to call `set_generation` * Fix arg Co-authored-by: Marcelo Fornet <mfornet94@gmail.com> Co-authored-by: Arto Bendiken <arto@aurora.dev> Co-authored-by: Michael Birch <michael@near.org> Co-authored-by: Aleksey Kladov <aleksey.kladov@gmail.com> Co-authored-by: Evgeny Ukhanov <mrlsd@ya.ru> * Fix layout of the key * Fix all tests (don't wipe the storage all the time) * Use correct generation in writing storage * Remove unnecessary references Co-authored-by: Michael Birch <michael@near.org> Co-authored-by: Joshua J. Bouw <dev@joshuajbouw.com> Co-authored-by: Arto Bendiken <arto@aurora.dev> Co-authored-by: Aleksey Kladov <aleksey.kladov@gmail.com> Co-authored-by: Evgeny Ukhanov <mrlsd@ya.ru> Co-authored-by: Michael Birch <michael.birch@aurora.dev>

sept-en · 2021-06-10T22:49:37Z

Nice, thanks for working on this!
Please, make all I/O-related operations in src/lib.rs from the very beginning.
This way it will be easier to separate from business logic and we try to follow this pattern everywhere.

src/migration.rs

src/lib.rs

Move I/O from business logic Co-authored-by: Kirill <septengineering@pm.me>

Co-authored-by: Kirill <septengineering@pm.me>

mfornet · 2021-06-11T09:15:13Z

Haven't looked into the PR yet, but have few questions regarding the options:

/// Rename key field
RenameKey,
// Rename key and update value
UpdateKeyValue,

What do you mean by rename key? Borsh serialisation doesn't save the name of the fields on the storage. Or you meant something like. Moving value stored at key_1 to key_2 (and deleting key_1)?

// Remove only value for key field
RemoveValue,

What does it do, replace this value with empty sequence of bytes?

I don't know if I'm missing a conversation from some place. But can you provide here high level details (or even technical if necessary) about how are we planning to execute one migration using this changes, before jumping into the implementation.

mrLSD · 2021-06-11T12:48:54Z

Haven't looked into the PR yet, but have few questions regarding the options:
/// Rename key field
RenameKey,
// Rename key and update value
UpdateKeyValue,
What do you mean by rename key? Borsh serialisation doesn't save the name of the fields on the storage. Or you meant something like. Moving value stored at key_1 to key_2 (and deleting key_1)?
It doesn't matter. We can set any kind of key. So we can rename storage key (and don't touch value) and some other useful cases.

// Remove only value for key field
RemoveValue,
What does it do, replace this value with empty sequence of bytes?
yes

I don't know if I'm missing a conversation from some place. But can you provide here high level details (or even technical if necessary) about how are we planning to execute one migration using this changes, before jumping into the implementation.

@sept-en asks me to implement a migration mechanism for changing the storage state.

That logic currently covering all possible cases. Parameters set via json.

mrLSD · 2021-06-11T12:50:09Z

@birchmd Fill free for suggestions and ideas.
The short story is: @sept-en asks me about how to do the migration.
I proposed an idea and started implementation.

birchmd · 2021-06-11T17:23:13Z

This PR feels more like a design proposal and so I think it should be discussed separately from any actual implementation. Hopefully if we all agree on a design first then we can avoid churn on code itself which will save us all time in the long run.

I have enabled GitHub discussions for the Engine repo and created a section called Design where we can talk about designs for new features/functionality.

The discussion for this migration functionality is here: #140
Please feel free to add more details there (e.g. @sept-en @mrLSD if this is something you are pushing for please comment with some pros for having a migration DSL, as opposed to migrations in code itself).

As a general note I think we should be spending more time on design than we currently are. Spending some time on design up-front can hopefully prevent sudden pivots like we have seen recently (e.g. don't burn NEP-141 tokens in connector, generational storage, tracking promise creation in exit precompiles). I know that each of those examples were in response to problems that arose with the implementation, and that a design phase will not always catch all such problems, but at least it gives us a better chance of doing so.

mfornet · 2021-06-13T06:56:07Z

@sept-en asks me about how to do the migration. I proposed an idea and started implementation.

@mrLSD Where did you propose this idea, I can't find it anywhere, and the description of the PR doesn't help to it.

That logic currently covering all possible cases. Parameters set via json.

You didn't address the rest of my comments. So you are covering cases that are not described properly, and it is not straight forward to understand how they are going to be used.

Let's continue this conversation in #140 (thanks @birchmd for starting this discussion).

joshuajbouw · 2021-07-07T06:51:51Z

This PR is stale, whats the status @mrLSD ?

mrLSD · 2021-07-07T08:31:02Z

This PR is stale, whats the status @mrLSD ?

Paused. See #140

birchmd · 2021-10-04T14:22:14Z

Closing this as it is now vastly out of date with current develop branch.

birchmd and others added 11 commits June 1, 2021 19:04

Remove the dependency on Lunarity. (#115)

ebb2edf

Fix Clippy complaint with +nightly. (#117)

418a473

Add an evm-bully=yes build to CI. (#121)

0f11b37

Fix evm-bully builds. (#130)

544ad92

Handle transaction gas limit properly. (#123)

aaeec76

Merge branch 'master' into develop

a7d2b19

Add an is_used_proof interface. (#120)

d8f042d

Simplify the sdk::read_u64 return type. (#118)

b76c7a3

Merge branch 'master' into develop

9c81e52

Basic migration types & data

fc949d1

mrLSD requested a review from artob as a code owner June 10, 2021 22:46

mrLSD self-assigned this Jun 10, 2021

mrLSD added the C-enhancement Category: New feature or request label Jun 10, 2021

mrLSD requested a review from sept-en June 10, 2021 22:46

mrLSD marked this pull request as draft June 10, 2021 22:47

sept-en suggested changes Jun 10, 2021

View reviewed changes

src/migration.rs Outdated Show resolved Hide resolved

src/lib.rs Outdated Show resolved Hide resolved

mrLSD and others added 2 commits June 11, 2021 02:00

Update src/migration.rs

17971f5

Move I/O from business logic Co-authored-by: Kirill <septengineering@pm.me>

Update src/lib.rs

bcc4d66

Co-authored-by: Kirill <septengineering@pm.me>

mrLSD added 4 commits June 11, 2021 12:17

Migration parse json and test

54f2d57

Added tests, added parsers for Migration JSON

3aff7c7

Fix Migration JSON parser

5bb6c53

Added migration Add & test_migation_add

b721da9

Adde RenameKey & tests

8311b82

sept-en mentioned this pull request Jul 7, 2021

JSON: bugs fixes, add unit tests. #141

Merged

mrLSD mentioned this pull request Jul 28, 2021

NEP-141 compliance correctness #202

Merged

joshuajbouw force-pushed the develop branch 2 times, most recently from 216e375 to 9d09598 Compare August 13, 2021 16:25

joshuajbouw force-pushed the develop branch from 21bf309 to 422844a Compare September 13, 2021 09:11

birchmd force-pushed the develop branch from 7b04bd2 to 4ddad35 Compare September 24, 2021 16:51

birchmd closed this Oct 4, 2021

joshuajbouw deleted the feat/migration branch July 11, 2022 11:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Contract Migration #137

Contract Migration #137

mrLSD commented Jun 10, 2021

sept-en commented Jun 10, 2021

mfornet commented Jun 11, 2021

mrLSD commented Jun 11, 2021

mrLSD commented Jun 11, 2021

birchmd commented Jun 11, 2021 •

edited

mfornet commented Jun 13, 2021

joshuajbouw commented Jul 7, 2021

mrLSD commented Jul 7, 2021

birchmd commented Oct 4, 2021

Contract Migration #137

Contract Migration #137

Conversation

mrLSD commented Jun 10, 2021

sept-en commented Jun 10, 2021

mfornet commented Jun 11, 2021

mrLSD commented Jun 11, 2021

mrLSD commented Jun 11, 2021

birchmd commented Jun 11, 2021 • edited

mfornet commented Jun 13, 2021

joshuajbouw commented Jul 7, 2021

mrLSD commented Jul 7, 2021

birchmd commented Oct 4, 2021

birchmd commented Jun 11, 2021 •

edited