search

move-language / move

Apache License 2.0
2.26k stars 689 forks source link

[natives] add support for checking the existence of a resource #852

Closed davidiw closed 1 year ago

davidiw commented 1 year ago

Extend native support for verifying a resource exists.

verified here: https://github.com/aptos-labs/aptos-core/pull/6416/commits

Motivation

(Write your motivation for proposed changes here.)

Have you read the Contributing Guidelines on pull requests?

(Write your answer here.)

Test Plan

(Share your test plan here. If you changed code, please provide us with clear instructions for verifying that your changes work.)

davidiw commented 1 year ago

The goal of the native context is to wrap a dyn so that it can be used as an impl. Are you asking that I panic if any calls are made besides the load_resource? Or are you asking that I replace our get_resource with a more trivial exists_at?

wrwg commented 1 year ago

The goal of the native context is to wrap a dyn so that it can be used as an impl. Are you asking that I panic if any calls are made besides the load_resource? Or are you asking that I replace our get_resource with a more trivial exists_at?

Only exists_at has a clean semantics. get_resource works around borrow rules, meaning it can get a value which is not the actual, since while a value is mutated, it may not be updated in storage until the mutation ended. That is why this functionality is protected by acquires in Move. get_module bypasses the VM loader which is the source of truth for modules. Specifically, the module doesn't exists for the VM right after it is upgraded. For the objects you only need exists_at, and that is the only thing we should implement here.

wrwg commented 1 year ago

Ah, and can you please fix this cryptic PR description?