feat(move-compiler): Add authenticator attribute #8951
Conversation
iota-ci
added
sc-platform
This change doesn't handle yet how serialization/deserialization of move-binary-format is managed or some other testing utilities that haven't been handled yet.
TheMrAI
force-pushed
the
vm-lang/aa-auth/8860-add-authenticator-attribute
branch
from
07db89a to
fd3deb5
Compare
|
The latest updates on your projects. Learn more about Vercel for GitHub. 6 Skipped Deployments
|
TheMrAI
changed the title
Some type related documentation was added some time ago and it had the format of rustdoc. CI didn't report on it as it executes the tests using nextest as: "cargo nextest run -p move-binary-format" But if you ran: "cargo test -p move-binary-format" they would all fail. They have now been disabled, by being marked as text. Marking them as "not code" from rust's perspective.
|
|
||
| fn functions( | ||
| context: &mut Context, | ||
| reporter: &DiagnosticReporter, |
There was a problem hiding this comment.
I guess we have 2 options here: 1. passing down reporter or 2. propagating up the error (changing the return of functions and function to Result). Have you thought about this? Is there a preferred way?
Just asking, not suggesting, both solutions seems equivalent at first glance.
There was a problem hiding this comment.
Given that adding a diagnostic message triggers a build failure in the end, it seemed cleaner to pass that down, instead of propagating the error back up.
In the propagation case one also has to handle the error itself and how they want to represent it.
There was a problem hiding this comment.
Not sure if move-compiler/tests/move_2024 is the appropriate path because it is not really a feature of Move 2024, is there an alternative?
Related to this, do we need to modify other feature gates to enable this new attribute safely?
There was a problem hiding this comment.
I am happy to move the tests to any position. Truth be told it was selected as an educated guess.
A similar path exists in reverse to confuse the enemy: external-crates/move/crates/move-compiler/tests/iota_mode/move_2024, but the equivalent path will be removed from sui at some point.
We can make a separate folder. Move it to external-crates/move/crates/move-compiler/tests/move_check/to_bytecode, not sure why some tests are where they are.
There was a problem hiding this comment.
| Related to this, do we need to modify other feature gates to enable this new attribute safely?
Probably, have to manage versioning for sure (if we put info into CompiledModule).
There was a problem hiding this comment.
Yepp, we can gate attributes by this mechanism: https://github.com/iotaledger/iota/blob/vm-lang/aa-auth/8860-add-authenticator-attribute/external-crates/move/crates/move-compiler/src/expansion/translate.rs#L1157
external-crates/move/crates/move-binary-format/src/deserializer.rs
Outdated
Show resolved
Hide resolved
| /// In case the function has been marked by "authenticator(version = x)" | ||
| /// attribute, this field will contain the specified version. | ||
| /// A value of None indicates that the function was not marked as an | ||
| /// authenticator. | ||
| pub authenticator_version: Option<u8>, |
There was a problem hiding this comment.
@TheMrAI @miker83z, I am leaving this comment just to discuss the idea.
What do you think if we created a struct to represent the authenticator info, for example:
pub struct AuthenticatorInfo {
pub version: u8,
...
}It will make it possible to conveniently extend the authenticator declaration with additional info:
// `description` can be shown in the explorer, for example.
#[authenticator(version = 2, description = b"User friendly description", ...)]Probably you have ideas about what we can add at the moment?
|
As discussed offline we abandoned for now trying to integrate attribute information into What we gain with this approach is that we can have minimal modifications to the compiler and thus avoid all potential danger. Don't have to design a method to be able to store attributes on chain, handle versions etc... What we loose is that the solution is and will have its limitations as this additional will temporarily be stored (for now, later we mitigated with PackageMetadata). Only set in: Run quick test |
4 participants
Description of change
To support defining authenticator functions for the IOTA abstract account workflow the user must be able to
specify in Move code which functions should be considered "authenticators".
Moreover in the future there may be more than one version of authenticators so a version number must be supported as well.
This PR provides three formats to define a move authenticator with version.
As a Named attribute:
In this case the version number is inferred to be 1.
As an Assigned attribute:
Where the version number is assigned explicitly.
And as a Parametrized attribute:
In this format the user must explicitly spell out the version number and assign a value to it. While the format is verbose, it may become a preferred variant as it could potentially be extended with additional features like:
Implementation notes
Attributes are a bit tricky to work with as they do not survive by the end of the compilation cycle.
They have a basic framework for being added, but apart from that, their processing is wholly up to the developer.
The authenticator attribute must be put into the final
CompilerModulestructure to be stored on-chain.For these reasons the processing of the attribute was pushed to the final stage of compilation called:
to_bytecode.This way we didn't have to add any additional data to function definitions or other structures for other interim compiler stages.
It is a bit odd however, that the compiler is basically done, by the time we start processing if the attribute even contains the appropriate type, but it is how it is.
Links to any relevant issues
fixes #8860
How the change has been tested