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

[fastx adapter] change entrypoint type signature rules to accept `&mu… #144

Merged
merged 1 commit into from
Jan 10, 2022

Conversation

sblackshear
Copy link
Collaborator

…t TxContext`

Previously, the adapter expected the last argument of an entrypoint to be an owned TxContext.
When playing with some example Move code, I realized that this makes entrypoint composition inconvenient. For example, say you have the following entrypoints:

public fun main1(ctx: TxContext) { ... }

public fun main2(ctx: TxContext) { ... }

If you want to create a third entrypoint that composes main1 and main2 like so

public fun main_compose(ctx: TxContext) {
  main1(ctx);
  main2(ctx) // error: ctx moved on previously line
}

, this will not work.

Switching the entrypoint signature to use &mut TxContext instead solves this problem without otherwise changing the programming experience.

…t TxContext`

Previously, the adapter expected the last argument of an entrypoint to be an owned `TxContext`.
When playing with some example Move code, I realized that this makes entrypoint composition inconvenient. For example, say you
have the following entrypoints:

```
public fun main1(ctx: TxContext) { ... }

public fun main2(ctx: TxContext) { ... }
```

If you want to create a third entrypoint that composes `main1` and `main2` like so

```
public fun main_compose(ctx: TxContext) {
  main1(ctx);
  main2(ctx) // error: ctx moved on previously line
}
```

, this will not work.

Switching the entrypoint signature to use `&mut TxContext` instead solves this problem without otherwise changing the programming experience.
Copy link
Contributor

@huitseeker huitseeker left a comment

Choose a reason for hiding this comment

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

The usual solution is method chaining :

let ctx2 = main1(ctx1);
let ctx3 = main2(ctx2);

Which has the advantage of making any mutation super explicit. Any reason why this wouldn't make sense here?

@sblackshear
Copy link
Collaborator Author

Any reason why this wouldn't make sense here?

Couple of reasons:

  • I don't want to require entrypoints to return a value. It will add boilerplate in both the type signature and in the body, and adds one more rule to the (already somewhat complex) set of entrypoint signature conditions
  • Adds some confusion about whether non-entrypoint type signatures should ask for a &mut TxContext or a TxContext + return it to the programmer. Basically, asking entrypoints to &mut TxContext forces programmers to do it in the non-FP way everywhere, which removes the confusion.

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.

Looks good -- I had some questions which I think I answered. Do check my understanding, and if not make a note in the comments to correct it.

// Input ref parameters we put in should be the same number we get out
debug_assert!(mutable_ref_objects.len() == mutable_ref_values.len());
// Input ref parameters we put in should be the same number we get out, plus one for the &mut TxContext
debug_assert!(mutable_ref_objects.len() + 1 == mutable_ref_values.len());
Copy link
Collaborator

Choose a reason for hiding this comment

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

Ok, just to check: this is not a mutable object that our system will persist right? It is internal to the move call?

// Input ref parameters we put in should be the same number we get out, plus one for the &mut TxContext
debug_assert!(mutable_ref_objects.len() + 1 == mutable_ref_values.len());
// discard the &mut TxContext arg
mutable_ref_values.pop().unwrap();
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think you just answered my question be poping and discarding. But then why return it at all?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah ok, because the move call returns it.

@sblackshear sblackshear merged commit 1f37fc1 into MystenLabs:main Jan 10, 2022
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.

3 participants