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

feat(sui): add init multisig command #349

Merged
merged 17 commits into from
Aug 29, 2024

Conversation

npty
Copy link
Member

@npty npty commented Aug 28, 2024

Description

This PR added the init command to multisig.js, in order to make the multisig flow smoother.

The init command receives the following params from the cli options:

  • base64PublicKeys
  • threshold
  • schemaTypes
  • weights

Then, it constructs the multisig object save into the config file.

@npty npty changed the base branch from main to chore/gh-workflow-for-sui-scripts August 28, 2024 08:23
@npty npty self-assigned this Aug 28, 2024
@npty npty changed the title feat: add init multisig feat(sui): add init multisig command Aug 28, 2024
@npty npty marked this pull request as ready for review August 28, 2024 09:44
@npty npty requested a review from a team as a code owner August 28, 2024 09:44
@npty npty requested review from jcs47 and sdaveas and removed request for a team August 28, 2024 09:44
.github/workflows/test-sui.yaml Outdated Show resolved Hide resolved
.github/workflows/test-sui.yaml Outdated Show resolved Hide resolved
sui/multisig.js Outdated Show resolved Hide resolved
sui/multisig.js Outdated Show resolved Hide resolved
sui/multisig.js Show resolved Hide resolved
@@ -153,13 +213,21 @@ if (require.main === module) {
addBaseOptions(program);

program.addOption(new Option('--txBlockPath <file>', 'path to unsigned tx block'));
program.addOption(new Option('--action <action>', 'action').choices(['sign', 'combine', 'execute']).makeOptionMandatory(true));
program.addOption(new Option('--action <action>', 'action').choices(['sign', 'combine', 'execute', 'init']).makeOptionMandatory(true));
Copy link
Member

Choose a reason for hiding this comment

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

@blockchainguyy Refactor this to use Commands like we do for other Sui scripts

.github/workflows/test-sui.yaml Outdated Show resolved Hide resolved

- name: Submit Signed Tx File
run: |
node sui/multisig.js --txBlockPath ./tx-upgrade.json --signatureFilePath ./combined.json --action combine --signatures signature-1.json signature-2.json
Copy link
Member

Choose a reason for hiding this comment

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

Test if gateway approve still works post upgrade

Copy link
Member

Choose a reason for hiding this comment

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

Is the contract address being updated to the new package id after upgrade?

Copy link
Member Author

Choose a reason for hiding this comment

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

@milapsheth Yes, it'll be upgraded to the new package id. Approval works with either the upgraded contract or the original contract. In this case, should we implement some mechanism to guard access to all prior versions of the package, because they're still exist on-chain?

Copy link
Member

Choose a reason for hiding this comment

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

Foivos is looking into the upgrade mechanism to create a design for our contracts. We want some methods to be accessible on the old contract potentially, and some to not be depending on upgrade versioning

@npty npty merged commit e513875 into chore/gh-workflow-for-sui-scripts Aug 29, 2024
5 checks passed
@npty npty deleted the feat/add-init-multisig branch August 29, 2024 11:01
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