-
Notifications
You must be signed in to change notification settings - Fork 50
[1/x] Pass an account as a parameter to note and tx script #798
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
base: next
Are you sure you want to change the base?
Changes from all commits
55340f8
eb50e48
001cc60
4d4a751
6d05f40
9e29530
32b2d64
88eaf94
e1f8ac1
9368f9c
3cdf7f8
35ccba4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a good reason to pass
accountby value, rather than by reference (either immutable or mutable, depending on the needs of the script)?One reason why I think that might be important, is that it would provide us enough information to be able to determine how a given script/note interacts with an account, as well as allow Rust to enforce at compile-time that you cannot call certain methods on the account because you don't have an appropriate reference type.
The bigger question I guess, is whether there is any value in non-mutable references as argument to a transaction script/note script. In theory, that would enforce read-only access - but I'm not clear on whether read-only scripts would be useful; and even if we had them, how other parts of our tooling would leverage that (i.e. depending on what type of reference your script entrypoint uses (e.g.
&vs&mut), we'd encode in the resulting package metadata that the script is read-only or can mutate an account. That could make it easier to notify users that they are about to perform an action with mutable access to an account - but I don't know the exact mechanism by which that would work.Anyhow, by passing the
accountparameter by-value, we don't even have the option of using reference mutability to encode that information, so it might be worth getting ahead of it a bit, by always passing a mutable reference for now (or if it's possible, we could look at the reference type of the method, and then have the proc macro machinery emit code to pass a reference of the appropriate type).What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I addressed this in #801 but forgot to mention it in this PR.
Also, I'm linking here for account methods
selfmutability issue: #802.