-
Notifications
You must be signed in to change notification settings - Fork 58
feat: use SenderSignedData::input_objects instead of TransactionData::input_objects
#9304
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
feat: use SenderSignedData::input_objects instead of TransactionData::input_objects
#9304
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 6 Skipped Deployments
|
aa6978b to
e188f74
Compare
e188f74 to
6102764
Compare
…th/9278-check-tx-inputs-usage
miker83z
left a comment
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.
Please also write a comment in the indexer and replay crates where it would be needed to include the MoveAuthenticator inputs. This will be done later.
| } | ||
| } | ||
|
|
||
| fn read_objects_for_signing( |
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.
In this file, in the function fn index_tx there's:
cert.data()
.intent_message()
.value
.input_objects()?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.
I haven't touched the places related to indexing in this PR because, in terms of the indexer, transaction inputs are always associated with TransactionData, so if this PR works, it is probably better to refactor in a dedicated PR.
It is easy to find the places where |
64d2145 to
c99dc43
Compare
|
✅ Vercel Preview Deployment is ready! |
…ere are several shared objects with the same id but different initial versions
miker83z
left a comment
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.
Just some nit renamings
crates/iota-types/src/transaction.rs
Outdated
| /// example) can be duplicated in the transaction and authenticator | ||
| /// object lists, we load them independently to make it possible to | ||
| /// analyze the inputs in the transaction checkers. | ||
| pub fn collect_all_inputs_for_reading(&self) -> IotaResult<Vec<InputObjectKind>> { |
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.
| pub fn collect_all_inputs_for_reading(&self) -> IotaResult<Vec<InputObjectKind>> { | |
| pub fn collect_all_input_object_kind_for_reading(&self) -> IotaResult<Vec<InputObjectKind>> { |
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.
renamed in a51f8f4
crates/iota-types/src/transaction.rs
Outdated
| /// the object to authenticate. | ||
| /// 3. The object to authenticate from the sender `MoveAuthenticator`. | ||
| pub fn split_inputs_into_groups( | ||
| pub fn split_inputs_into_groups_for_reading( |
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.
| pub fn split_inputs_into_groups_for_reading( | |
| pub fn split_input_objects_into_groups( |
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.
Renamed in a51f8f4
But I left the _for_reading postfix at least for now. Since we have several input objects set, this postfix shows where the inputs should be used.
crates/iota-types/src/transaction.rs
Outdated
| /// Merges another SharedInputObject into self. | ||
| /// If there is a conflict in mutability, the resulting object will be | ||
| /// mutable. Errors if the id or initial_shared_version do not match. | ||
| pub fn union(&mut self, other: &SharedInputObject) -> UserInputResult<()> { |
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.
| pub fn union(&mut self, other: &SharedInputObject) -> UserInputResult<()> { | |
| pub fn left_union(&mut self, other: &SharedInputObject) -> UserInputResult<()> { |
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.
As an option we can consider merge for the function name.
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.
Renamed in a51f8f4
crates/iota-types/src/transaction.rs
Outdated
| /// For shared objects, if either is mutable, the result is mutable. | ||
| /// Fails if either is not a shared object, or if the IDs or initial | ||
| /// versions do not match. | ||
| pub fn union(&mut self, other: &InputObjectKind) -> UserInputResult<()> { |
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.
| pub fn union(&mut self, other: &InputObjectKind) -> UserInputResult<()> { | |
| pub fn left_union_with_checks(&mut self, other: &InputObjectKind) -> UserInputResult<()> { |
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.
Renamed in a51f8f4
kodemartin
left a comment
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.
lgtm for the changes in iota-analytics-indexer
330916d
into
vm-lang/aa-auth/8116-feature-branch
Description of change
Replace the function usage in several places, but there are some that still need to be checked:
Links to any relevant issues
fixes #9278