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

[Move] Add shared mutable object support #722

Merged
merged 1 commit into from
Mar 10, 2022
Merged

Conversation

lxfind
Copy link
Contributor

@lxfind lxfind commented Mar 10, 2022

This PR adds support to use shared mutable object in Move.
Shared mutable object is not fully supported in Sui yet, as the sequencer is still being developed. However adding the support and library in Move will allow us to start writing interesting contracts using shared object.
This PR also writes a new version of TicTacToe that uses shared object, which provides an example for people to compare with the version that doesn't.

@666lcz
Copy link
Contributor

666lcz commented Mar 10, 2022

Sweet, V2 is 100+ lines less than V1!

@lxfind lxfind force-pushed the add-shared-object-in-move branch 2 times, most recently from e4f0e14 to 163d20b Compare March 10, 2022 05:59
@gdanezis
Copy link
Collaborator

Quick Q on the systems side @lxfind : do we allow shared objects to be deleted? If so it makes the safety / liveness checks more expensive / complex. If a shared object can never be deleted then life is simpler.

Copy link
Collaborator

@gdanezis gdanezis left a comment

Choose a reason for hiding this comment

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

Nice one! See question on delete & transfer!

@@ -48,6 +48,7 @@ pub fn all_natives(
),
("Transfer", "transfer_internal", transfer::transfer_internal),
("Transfer", "freeze_object", transfer::freeze_object),
("Transfer", "share_object", transfer::share_object),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we allow shared objects to be transferred?

@sblackshear
Copy link
Collaborator

Quick Q on the systems side @lxfind : do we allow shared objects to be deleted? If so it makes the safety / liveness checks more expensive / complex. If a shared object can never be deleted then life is simpler.

Do we allow shared objects to be transferred?

There's nothing in the current programmability model that prevents deletion, transfer, or wrapping of shared objects. If you have a shared T, you can use it as a transaction input and do anything you can do with a single-owner T.

However, we can make any or all of these illegal via:

  1. Runtime failures. We can abort inside the postprocessing logic adapter if we see a deletion, transfer, or wrap of an object ID that we know was a shared input to the tx.

This isn't the best UX because you might have deleted a shared object in the middle of a tx, then continued executing for some time.

  1. New rules on the structure of entrypoints. One idea is to ask entrypoints to take shared T's as input via a Shared<T> wrapper that does not support transfer, deletion, or wrapping. We could then expose functions for some (or none) of these operations if we want to support a subset of them. I like this a bit better because you prevent the bad behavior statically instead of via a dynamic check. But it will be a bit annoying for a programmer to need to dig inside of the Shared<T> every time they want to touch the shared object.

I'm sure there are several other options and am very curious to hear @lxfind's ideas. But perhaps the first thing to align on is which subset of {deletion, transfer, wrap} we want to support for shared objects.

// to take turns to place the marker, there won't be a singificant overhead in practice.
// As we can see, by using shared mutable object, the implementation is much
// simpler than the other implementation.
module Games::TicTacToeV2 {
Copy link
Collaborator

Choose a reason for hiding this comment

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

One thing I'm wondering about (and this is philosophical/for the future): if we want to support both "modes" of tic tac toe, what is the best way to engineer this?

  1. Fully separate modules [current approach]
  2. Separate modules that share a common library
  3. Single module with some _async functions that can only be called in single-owner mode, some _sync functions that can only be called in shared mode, and (hopefully) a large number of common functions

My instinct is that (2) is probably a cleaner approach than (1) (should lead to less code/test/Move prover spec duplication), but (3) may be a bridge too far. But at some point, we should try them all and see how it shakes out.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was somewhat surprising that these two approaches don't share much code at all. The primary reason is that the data storage model changed from storing objects as data into storing primitive data directly. So every function changed because of that. The only things that are still sharable are probably the constants.

@awelc
Copy link
Contributor

awelc commented Mar 10, 2022

It may be worth explaining in TestScenario.move that scenario's object pool contains both "per-address" objects as well as "shared objects" (only the former is implied at this point). The reason I think this could be useful is that it may not super-obvious that after Transfer::share_object any user in any transaction can get this object by calling TestScenario::remove_object.

sui_programmability/framework/sources/Transfer.move Outdated Show resolved Hide resolved
/// can access and mutate. This is irreversible, i.e. once an object
/// is shared, it will stay shared forever.
/// Shared mutable object is not yet fully supported in Sui, which is being
/// actively worked on and should be suppored very soon. This API is exposed
Copy link
Contributor

Choose a reason for hiding this comment

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

Are we going to have shared objects by GDC? The comment explaining the status is obviously there but if we are not I wonder if we should still postpone releasing tests for not-yet-fully-supported features to avoid confusion (e.g., people trying this with wallet CLI).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No we won't have shared objects support in Sui by GDC.
The idea is to be able to put more Move examples that do interesting things (many of them would need shared object support) by GDC though.

@lxfind
Copy link
Contributor Author

lxfind commented Mar 10, 2022

Re: do we allow shared objects to be transferred/deleted

At the moment shared object isn't allowed in Sui (it's only enabled in Move). So it's up to us to decide what we want to allow latter.
Intuitively, I think using a Shared wrapper can be cleaner.
Specifically, when we share an object, what we do is to actually create a new object, with type Shared, which wraps the old object and a non-droppable token in it. This separates the original object and the newly shared object, likely giving us more flexibility on deletion if we ever want to. It also signals a clear intention on which object will be shared in the entry point, which makes Move developers aware of its cost.
To make it easier to use, at entry, the Shared wrapper can be unpacked to the object and the token. The contract can then deal with the object directly in whatever way it wants, but at the end, it must call a native function to put back the object and the token into the Shared wrapper. This ensures that the object cannot disappear otherwise the token will be left alone. Not sure if this would be too confusing though.

@lxfind lxfind merged commit 87e6397 into main Mar 10, 2022
@lxfind lxfind deleted the add-shared-object-in-move branch March 10, 2022 18:52
mwtian pushed a commit that referenced this pull request Sep 12, 2022
See dtolnay/anyhow#250
The `backtrace` feature is unified against the various anyhow versions we use.
mwtian pushed a commit to mwtian/sui that referenced this pull request Sep 29, 2022
See dtolnay/anyhow#250
The `backtrace` feature is unified against the various anyhow versions we use.
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.

5 participants