Skip to content

Conversation

jordanjennings-mysten
Copy link
Contributor

@jordanjennings-mysten jordanjennings-mysten commented Dec 19, 2024

Description

support additive and deponly policies for upgrade errors

Test plan

snapshot testing


Release notes

Check each box that your changes affect. If none of the boxes relate to your changes, release notes aren't required.

For each box you select, include information after the relevant heading that describes the impact of your changes that a user might notice and any actions they must take to implement updates.

  • Protocol:
  • Nodes (Validators and Full nodes):
  • JSON-RPC:
  • GraphQL:
  • CLI:
  • Rust SDK:

Copy link

vercel bot commented Dec 19, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
sui-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 8, 2025 1:26am
3 Skipped Deployments
Name Status Preview Comments Updated (UTC)
multisig-toolkit ⬜️ Ignored (Inspect) Visit Preview Jan 8, 2025 1:26am
sui-kiosk ⬜️ Ignored (Inspect) Visit Preview Jan 8, 2025 1:26am
sui-typescript-docs ⬜️ Ignored (Inspect) Visit Preview Jan 8, 2025 1:26am

Copy link
Contributor

@amnn amnn left a comment

Choose a reason for hiding this comment

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

Some small questions here and there, but otherwise good to go!

impl InclusionCheckMode for CliInclusionCheckMode {
type Error = Vec<UpgradeCompatibilityModeError>;

fn module_name_mismatch(&mut self, _old_address: &Identifier, _new_address: &Identifier) {}
Copy link
Contributor

Choose a reason for hiding this comment

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

Is a module name mismatch even possible? I thought we performed this check by first lining up modules by their name. Should this case be removed from the interface?

Copy link
Contributor Author

@jordanjennings-mysten jordanjennings-mysten Jan 7, 2025

Choose a reason for hiding this comment

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

I think the move here is to bundle name and address check into a single module_id_mismatch as compatibility check does so at least we don't have an extra function just for the name check. I'll do it in another smaller PR since it affects code outside of the sui crate.

@jordanjennings-mysten jordanjennings-mysten temporarily deployed to sui-typescript-aws-kms-test-env January 8, 2025 01:25 — with GitHub Actions Inactive
@jordanjennings-mysten jordanjennings-mysten merged commit c378799 into MystenLabs:main Jan 8, 2025
48 checks passed
thibault-martinez pushed a commit to iotaledger/iota that referenced this pull request Apr 18, 2025
# Description of change

Port of MystenLabs/sui#20695

## Links to any relevant issues

Closes #5719

## Type of change

Choose a type of change, and delete any options that are not relevant.

- Bug fix (a non-breaking change which fixes an issue)
- Enhancement (a non-breaking change which adds functionality)
- Breaking change (fix or feature that would cause existing
functionality to not work as expected)
- Documentation Fix

## How the change has been tested

Describe the tests that you ran to verify your changes.

Make sure to provide instructions for the maintainer as well as any
relevant configurations.

## Change checklist

Tick the boxes that are relevant to your changes, and delete any items
that are not.

- [ ] I have followed the contribution guidelines for this project
- [ ] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have made corresponding changes to the documentation
- [ ] I have added tests that prove my fix is effective or that my
feature works
- [ ] I have checked that new and existing unit tests pass locally with
my changes

### Release Notes

<!--
Check each box that your changes affect. If none of the boxes relate to
your changes, release notes aren't required.

For each box you select, include information after the relevant heading
that describes the impact of your changes that a user might notice and
any actions they must take to implement updates.
-->

- [ ] Protocol:
- [ ] Nodes (Validators and Full nodes):
- [ ] Indexer:
- [ ] JSON-RPC:
- [ ] GraphQL:
- [ ] CLI:
- [ ] Rust SDK:
- [ ] REST API:
thibault-martinez pushed a commit to iotaledger/iota that referenced this pull request Apr 24, 2025
# Description of change

Port of MystenLabs/sui#20695

## Links to any relevant issues

Closes #5719

## Type of change

Choose a type of change, and delete any options that are not relevant.

- Bug fix (a non-breaking change which fixes an issue)
- Enhancement (a non-breaking change which adds functionality)
- Breaking change (fix or feature that would cause existing
functionality to not work as expected)
- Documentation Fix

## How the change has been tested

Describe the tests that you ran to verify your changes.

Make sure to provide instructions for the maintainer as well as any
relevant configurations.

## Change checklist

Tick the boxes that are relevant to your changes, and delete any items
that are not.

- [ ] I have followed the contribution guidelines for this project
- [ ] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have made corresponding changes to the documentation
- [ ] I have added tests that prove my fix is effective or that my
feature works
- [ ] I have checked that new and existing unit tests pass locally with
my changes

### Release Notes

<!--
Check each box that your changes affect. If none of the boxes relate to
your changes, release notes aren't required.

For each box you select, include information after the relevant heading
that describes the impact of your changes that a user might notice and
any actions they must take to implement updates.
-->

- [ ] Protocol:
- [ ] Nodes (Validators and Full nodes):
- [ ] Indexer:
- [ ] JSON-RPC:
- [ ] GraphQL:
- [ ] CLI:
- [ ] Rust SDK:
- [ ] REST API:
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