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

Some cleanups around input object checking #1097

Merged
merged 1 commit into from
Mar 29, 2022

Conversation

lxfind
Copy link
Contributor

@lxfind lxfind commented Mar 28, 2022

This PR cleans up some logic when checking input objects:

  1. Check gas object immutability early, instead of waiting till transaction execution phase to find out.
  2. Use full transfer eligibility on the transfer object, which would check a bunch of different things
  3. Make sure we don't use move package as object
  4. Rename SharedMoveObject and OwnedMoveObject in input object kind to make them more accurate.

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.

Overall this makes sense to me!

@@ -456,32 +456,37 @@ SuiError:
- object_id:
TYPENAME: ObjectID
4:
UnexpectedOwnerType: UNIT
MovePackageAsObject:
Copy link
Contributor

Choose a reason for hiding this comment

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

A lot of these are due to reordering of the error types,

  • this is bound to create a ton of churn as new enum options introduced early in the giant SuiError enum invalidate everything,
  • we do generate a single error type for everything.

Here's an example of consequence this churn can have.
=> do we want to open an issue to start breaking up the SuiError type in nested types have those grouped by area where they can be triggered?

@lxfind lxfind force-pushed the check-transfer-input-separate branch from 3be7cbc to 7dcc1d3 Compare March 29, 2022 02:19
@lxfind lxfind requested a review from oxade March 29, 2022 02:51
@lxfind lxfind merged commit d749531 into main Mar 29, 2022
@lxfind lxfind deleted the check-transfer-input-separate branch March 29, 2022 02:51
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.

2 participants