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

DIP16 Draft: Transaction Scripts #153

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

DIP16 Draft: Transaction Scripts #153

wants to merge 2 commits into from

Conversation

sblackshear
Copy link
Contributor

No description provided.

dips/dip-16.md Outdated Show resolved Hide resolved
Copy link
Contributor

@davidiw davidiw left a comment

Choose a reason for hiding this comment

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

overall seems pretty reasonable, I would love to see pointers to other docs, so that this DIP could be included in some developer guide or specification on an end-to-end process.

Not sure what the point of the changelog is?

dips/dip-16.md Outdated Show resolved Hide resolved
Both script functions and the single function in a transaction script bytecode file have the following restrictions. For a function` f<ability_params>(param_types): ret_types` :

* The `ret_types` list is empty (i.e., the function does not return a value)
* The `param_types` list begins with one or more `signer` types
Copy link
Contributor

Choose a reason for hiding this comment

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

can you point to signer types?

Comment on lines +54 to +56
module: ModuleId,
function: Identifier,
ty_args: Vec<TypeTag>,
Copy link
Contributor

Choose a reason for hiding this comment

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

are these defined anywhere? can we point to them?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

They are all defined in move-core-types. I'll add a note below.

pub struct Script {
code: Vec<u8>,
ty_args: Vec<TypeTag>,
args: Vec<TransactionArgument>,
Copy link
Contributor

Choose a reason for hiding this comment

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

can you point to TransactionArgument?

Co-authored-by: David Wolinsky <isaac.wolinsky@gmail.com>
@sblackshear
Copy link
Contributor Author

Not sure what the point of the changelog is?

This is (I think, but am not sure) how we agreed to document which parts of a DIP apply to which versions of Diem. Here, it's a bit awkward because the DIP didn't exist before Diem v1, but it is describing both v1 and v2 features. Definitely open to a different way of conveying this information if we can all standardize on it.

Copy link
Contributor

@davidiw davidiw left a comment

Choose a reason for hiding this comment

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

add links and let's land this?

### Changing script functions

* The name and type signature of an existing script function will never be changed.
* The set of error codes that may be returned by a script function is included in the [developer documentation](https://github.com/diem/diem/blob/main/language/diem-framework/transaction_scripts/doc/transaction_script_documentation.md) for each script. The error codes returned by a script function may grow or shrink, but the meaning of a given error code will remain fixed. E.g.:
Copy link
Contributor

Choose a reason for hiding this comment

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

update link

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