Skip to content

Convert mutable: bool to mutability: SharedObjectMutability. Backward compatible because of enum encoding. #23947

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Oct 16, 2025
Merged

Convert mutable: bool to mutability: SharedObjectMutability. Backward compatible because of enum encoding. #23947

merged 2 commits into from
Oct 16, 2025
+571 −313

Conversation

mystenmark
Copy link
Contributor

No description provided.

@mystenmark mystenmark requested review from a team and ronny-mysten as code owners October 15, 2025 23:06
@mystenmark mystenmark requested a review from tnowacki October 15, 2025 23:06
@mystenmark mystenmark temporarily deployed to sui-typescript-aws-kms-test-env October 15, 2025 23:06 — with GitHub Actions Inactive
@vercel Vercel
Copy link

vercel bot commented Oct 15, 2025 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
sui-docs Ready Ready Preview Comment Oct 16, 2025 11:10pm
2 Skipped Deployments
Project Deployment Preview Comments Updated (UTC)
multisig-toolkit Ignored Ignored Preview Oct 16, 2025 11:10pm
sui-kiosk Ignored Ignored Preview Oct 16, 2025 11:10pm

@mystenmark mystenmark force-pushed the mlogan-shared-object-mutability branch from ab9a829 to ca4f27c Compare October 15, 2025 23:43
@mystenmark mystenmark temporarily deployed to sui-typescript-aws-kms-test-env October 15, 2025 23:43 — with GitHub Actions Inactive
amnn
amnn reviewed Oct 16, 2025
View reviewed changes
crates/sui-indexer-alt-graphql/src/api/types/transaction_kind/programmable/inputs/mod.rs
Copy link
Contributor

Choose a reason for hiding this comment

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

Change looks good! Where we can we go to learn more about what the future schema needs to expose?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is pulled out of #23733, which adds SharedObjectMutability::NonExclusiveWrite, which allows transactions to perform non-conflicting dynamic field operations in parallel.

Copy link
Contributor

Choose a reason for hiding this comment

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

Okay, for GraphQL, we can work this migration by adding a new field -- mutability that returns an enum, and add a deprecation notice to the old mutable flag, before disappearing it after two more releases.

@mystenmark mystenmark force-pushed the mlogan-shared-object-mutability branch from ca4f27c to f591c1f Compare October 16, 2025 15:48
@mystenmark mystenmark temporarily deployed to sui-typescript-aws-kms-test-env October 16, 2025 15:48 — with GitHub Actions Inactive
tnowacki
tnowacki approved these changes Oct 16, 2025
View reviewed changes
Copy link
Contributor

@tnowacki tnowacki left a comment

Choose a reason for hiding this comment

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

Concerned about some possible breakages (looking at the yaml) But I imagine you are on top of it

crates/sui-types/src/transaction.rs
pub fn is_mutable(&self) -> bool {
match self {
SharedObjectMutability::Mutable => true,
SharedObjectMutability::Immutable => false,
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this something we want to support? What happens after the non-exclusive variant is added?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll handle that in that PR, but I didn't want to bring those sorts of details backward into this change

crates/sui-types/src/transaction.rs Outdated
Comment on lines 3981 to 3984

#[cfg(test)]
mod tests {
use super::*;
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there no existing unit test file where this should go?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

moved to a new test file

crates/sui-types/src/transaction.rs Outdated
Mutable,
}

impl std::fmt::Display for SharedObjectMutability {
Copy link
Contributor

Choose a reason for hiding this comment

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

Where is this used?

Copy link
Contributor

Choose a reason for hiding this comment

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

Mostly making sure this isn't going to fork if it is used somewhere stupid

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed, it was apparently unused

crates/sui-core/tests/staged/sui.yaml
TYPENAME: SequenceNumber
- mutable: BOOL
- mutability:
TYPENAME: SharedObjectMutability
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't fully know the results of this change and who it will break

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think there's anything that directly reads sui.yaml, if that's what you mean. For readers of BCS, the format is compatible. Am I missing your point?

@mystenmark mystenmark force-pushed the mlogan-shared-object-mutability branch from 1399c0b to ff80d4e Compare October 16, 2025 23:08
@mystenmark mystenmark temporarily deployed to sui-typescript-aws-kms-test-env October 16, 2025 23:08 — with GitHub Actions Inactive
@mystenmark mystenmark enabled auto-merge (squash) October 16, 2025 23:08
@mystenmark mystenmark merged commit 61dcfdb into main Oct 16, 2025
53 of 54 checks passed
@mystenmark mystenmark deleted the mlogan-shared-object-mutability branch October 16, 2025 23:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@amnn amnn amnn left review comments

@tnowacki tnowacki tnowacki approved these changes

@ronny-mysten ronny-mysten Awaiting requested review from ronny-mysten ronny-mysten is a code owner

@jessiemongeon1 jessiemongeon1 Awaiting requested review from jessiemongeon1 jessiemongeon1 is a code owner

@henryachen henryachen Awaiting requested review from henryachen henryachen is a code owner automatically assigned from MystenLabs/devx-indexers-and-rpcs

@emmazzz emmazzz Awaiting requested review from emmazzz emmazzz is a code owner automatically assigned from MystenLabs/devx-indexers-and-rpcs

@wlmyng wlmyng Awaiting requested review from wlmyng wlmyng is a code owner automatically assigned from MystenLabs/devx-indexers-and-rpcs

@tpham-mysten tpham-mysten Awaiting requested review from tpham-mysten tpham-mysten is a code owner automatically assigned from MystenLabs/devx-indexers-and-rpcs

@evan-wall-mysten evan-wall-mysten Awaiting requested review from evan-wall-mysten evan-wall-mysten is a code owner automatically assigned from MystenLabs/devx-indexers-and-rpcs

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@mystenmark @amnn @tnowacki