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

add mpm deploy subcommand #3495

Merged
merged 6 commits into from
Jul 4, 2022
Merged

add mpm deploy subcommand #3495

merged 6 commits into from
Jul 4, 2022

Conversation

pause125
Copy link
Collaborator

Pull request type

Please check the type of change your PR introduces:

  • Bugfix
  • Feature
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Documentation content changes
  • Other (please describe):

What is the current behavior?

Issue Number: #3462

What is the new behavior?

  • Add mpm deploy subcommand.

Other information

Draft 版本,目前已经将 starcoin dev deploy 命令的功能移植到 mpm deploy。 使用命令如下:

mpm deploy --rpc ws://127.0.0.1:9871 --local-account-dir ~/.starcoin/main/account_vaults/ --password xxxx /path/to/package

考虑到 mpm 不应该过度依赖 starcoin node,因此只支持 rpc 链接; 因为需要签名,所以需要 account 已经 password 来unlock 账号。也许将这些 rpc 参数放到一个配置文件, 将密码参数放到一个 .secret 文件中更方便一点?

是否有其他的需求和建议? @jolestar @WGB5445

@WGB5445
Copy link
Contributor

WGB5445 commented Jun 30, 2022

我觉得可以增加一个私钥部署的选项,这样更方便,不需要使用 node 保存一个账户信息

@pause125
Copy link
Collaborator Author

我觉得可以增加一个私钥部署的选项,这样更方便,不需要使用 node 保存一个账户信息

这样确实要方便一些。我研究一下怎么实现。

@jolestar
Copy link
Member

以前设计 account provider 的时候考虑过环境变量私钥的 account provider 选项 #3259

@pause125
Copy link
Collaborator Author

以前设计 account provider 的时候考虑过环境变量私钥的 account provider 选项 #3259

那这个有没有实现呢?

@jolestar
Copy link
Member

以前设计 account provider 的时候考虑过环境变量私钥的 account provider 选项 #3259

那这个有没有实现呢?

issue 应该是 #3215 这个。

当时没实现,主要是觉得环境变量不太安全,也可以通过直接将私钥导入到 account db 来实现,因为 console 场景下,account 的所有命令都是可以用的。但如果到 mpm 的场景,还有通过 ci 来部署合约的场景,感觉这个是需要的。

@pause125 pause125 marked this pull request as ready for review July 2, 2022 13:09
@pause125
Copy link
Collaborator Author

pause125 commented Jul 3, 2022

我觉得可以增加一个私钥部署的选项,这样更方便,不需要使用 node 保存一个账户信息

Done.

现在同时支持从 local-account-dir 和 secret-file 部署合约。 secret-file 用于保存私钥。 使用命令分别如下:

mpm deploy --rpc ws://127.0.0.1:9871 --local-account-dir ~/.starcoin/main/account_vaults/ --password xxxx /path/to/package
mpm deploy --rpc ws://127.0.0.1:9871 --secret-file ~/.starcoin/main/.secret /path/to/package

@codecov
Copy link

codecov bot commented Jul 3, 2022

Codecov Report

Merging #3495 (b779ea0) into master (3b269ce) will decrease coverage by 1.52%.
The diff coverage is 6.67%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3495      +/-   ##
==========================================
- Coverage   31.12%   29.61%   -1.51%     
==========================================
  Files         500      588      +88     
  Lines       46861    49557    +2696     
  Branches    22139    23332    +1193     
==========================================
+ Hits        14582    14670      +88     
- Misses      17965    20623    +2658     
+ Partials    14314    14264      -50     
Flag Coverage Δ
unittests 29.61% <6.67%> (-1.51%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
account/api/src/provider.rs 75.00% <ø> (ø)
account/provider/src/lib.rs 100.00% <ø> (ø)
account/provider/src/private_key_provider.rs 0.00% <0.00%> (ø)
account/provider/src/provider/mod.rs 5.89% <0.00%> (-4.11%) ⬇️
cmd/starcoin/src/account/import_cmd.rs 0.00% <0.00%> (ø)
vm/move-package-manager/src/lib.rs 7.70% <ø> (ø)
config/src/account_provider_config.rs 28.58% <20.52%> (-13.09%) ⬇️
cmd/starcoin/src/lib.rs 3.45% <0.00%> (-96.55%) ⬇️
cmd/starcoin/src/cli_state.rs 8.55% <0.00%> (-40.23%) ⬇️
vm/natives/src/account.rs 12.50% <0.00%> (-12.50%) ⬇️
... and 157 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3b269ce...b779ea0. Read the comment docs.

account/provider/src/private_key_provider.rs Outdated Show resolved Hide resolved
account/provider/src/private_key_provider.rs Outdated Show resolved Hide resolved
account/provider/src/private_key_provider.rs Outdated Show resolved Hide resolved
vm/move-package-manager/src/deployment.rs Outdated Show resolved Hide resolved
@pause125
Copy link
Collaborator Author

pause125 commented Jul 3, 2022

新增从环境变量读取 private key,设置好私钥的环境变量 STARCOIN_PRIVATE_KEY 后,使用如下命令:

mpm deploy --rpc ws://127.0.0.1:9871 --from-env /path/to/package

@pause125
Copy link
Collaborator Author

pause125 commented Jul 3, 2022

run benchmark 运行到 use criterion 步骤时失败了,从日志上也看不出来是哪里错误了。在 developer-workflow 只提到了 ./scripts/check_commit.sh 检查,然而这似乎并不是全部。有更详细明确的文档描述该如何进行提交前的检查吗?

@pause125
Copy link
Collaborator Author

pause125 commented Jul 3, 2022

run benchmark 运行到 use criterion 步骤时失败了,从日志上也看不出来是哪里错误了。在 developer-workflow 只提到了 ./scripts/check_commit.sh 检查,然而这似乎并不是全部。有更详细明确的文档描述该如何进行提交前的检查吗?

重新 rebase master 后提交,run benchmask 运行成功了。也许是上一次提交时,正好遇到其他 PR 被合并了,从而导致 run benchmark 失败?

@jolestar
Copy link
Member

jolestar commented Jul 4, 2022

run benchmark 运行到 use criterion 步骤时失败了,从日志上也看不出来是哪里错误了。在 developer-workflow 只提到了 ./scripts/check_commit.sh 检查,然而这似乎并不是全部。有更详细明确的文档描述该如何进行提交前的检查吗?

重新 rebase master 后提交,run benchmask 运行成功了。也许是上一次提交时,正好遇到其他 PR 被合并了,从而导致 run benchmark 失败?

benchmark 不太稳定,所以现在主要关注 build and test 这个 task.

Copy link
Member

@jolestar jolestar left a comment

Choose a reason for hiding this comment

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

LGTM

@jolestar jolestar linked an issue Jul 4, 2022 that may be closed by this pull request
@jolestar jolestar merged commit 7471c93 into starcoinorg:master Jul 4, 2022
@pause125
Copy link
Collaborator Author

pause125 commented Jul 4, 2022

啥时候更新相关文档比较合适?merge 了就更新,还是等下一次 release 后再更新?

@jolestar
Copy link
Member

jolestar commented Jul 4, 2022

啥时候更新相关文档比较合适?merge 了就更新,还是等下一次 release 后再更新?

文档可以更新了,不过文档那边 merge 之前等这边 release 一个版本。争取这周 release 一个版本吧。

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.

[Feature Request] mpm supports contract deployment
3 participants