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: open tcx scan_keystores api[R2D2-13481] #141

Closed
wants to merge 6 commits into from

Conversation

xiaoguang1010
Copy link
Contributor

Summary of Changes

open tcx scan_keystores api

Motivation and Context

How Has This Been Tested? (Test Plan)

Other information

Screenshots (if appropriate):

Final checklist

  • Did you test both iOS and Android(if applicable)?
  • Is a security review needed(consenlabs/security)?

Security checklist (only for leader check)

  • No backdoor risk
    • Check for unknown network request urls, and script/shell files with unclear purposes,
    • The backend service cannot expose leaked data interfaces for various reasons (even for testing purposes)
  • No network communication protocol risk
    • Check whether to introduce unsafe network calls such as http/ws
  • No import potentially risk 3rd library
    • Check whether 3rd dependent library is import
    • Don't use an unknown third-party library
    • Check the 3rd library sources are fetched from normal sources, such as npm, gomodule, maven, cocoapod, Do not use unknown sources
    • Check github Dependabot alerts, Whether to add new issues
  • Private data not exposed
    • Check whether there are exclusive ApiKey, privatekey and other private information uploaded to git
    • Check if the packaged keystore has been uploaded to git

@xiaoguang1010 xiaoguang1010 requested a review from XuNeal December 31, 2024 03:54
@@ -444,6 +444,8 @@ pub fn scan_keystores() -> Result<ScanKeystoresResult> {

if version == HdKeystore::VERSION || version == PrivateKeystore::VERSION {
let keystore = Keystore::from_json(&contents)?;
//Verify that the keystore has been migrated
let is_migrated = is_migrated_keystore(&keystore.id());
Copy link

Choose a reason for hiding this comment

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

新创建的 wallet,is_migrated 字段会是什么值?

Copy link

@xwartz xwartz Jan 8, 2025

Choose a reason for hiding this comment

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

应该要区分出来

  • 215 新创建的
  • 215 迁移过的
  • 215 未迁移的

已经迁移过的 keystore,前端可以不管,未迁移的前端恢复 wallet 时,要标记未迁移状态,后续会进行迁移。

所以,前端得区分出上面三种类型的 keystore

Copy link
Contributor Author

@xiaoguang1010 xiaoguang1010 Jan 8, 2025

Choose a reason for hiding this comment

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

新创建的 wallet,is_migrated 字段会是什么值?

新创建的wallet应该会是false,这个只有_migrated.json里面存在的钱包id,才为true。

Copy link
Contributor Author

@xiaoguang1010 xiaoguang1010 Jan 8, 2025

Choose a reason for hiding this comment

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

应该要区分出来

  • 215 新创建的
  • 215 迁移过的
  • 215 未迁移的

已经迁移过的 keystore,前端可以不管,未迁移的前端恢复 wallet 时,要标记未迁移状态,后续会进行迁移。

所以,前端得区分出上面三种类型的 keystore

我的理解:

  1. “scan_keystores”接口返回的is_migrated标识,if ture -->迁移过的,if false-->新建的
  2. 如果要确定哪些是未迁移的,那么就需要调用“scan_legacy_keystores”,如果返回的keystore不包含
    在“scan_keystores”接口返回的列表中,则表示这个keystore是未迁移的。
  3. scan_keystores接口只返回已经迁移过的以及新建的keystore信息,不返回未迁移keystone信息。
    @XuNeal 不知道我理解是否有问题。

Copy link

Choose a reason for hiding this comment

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

那和我理解不太一样了,我以为 scan_keystores 接口返回的是所有的 keystore,包括 215 之前的(迁移和未迁移的)和 215 之后创建的

Copy link
Contributor Author

Choose a reason for hiding this comment

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

旧的keystone可以通过“scan_legacy_keystores”来获取到。

(WALLET_V1_DIR, true) => "migrated".to_string(),
(WALLET_V1_DIR, false) => "unmigrated".to_string(),
(WALLET_V2_DIR, true) => "migrated".to_string(),
(WALLET_V2_DIR, false) => "new".to_string(),
Copy link
Collaborator

Choose a reason for hiding this comment

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

@xiaoguang1010 需要确认这里如果两个老的 keystore 都有相同的助记词,并且都迁移之后,这里是否会正确的返回migrated

@@ -687,6 +691,9 @@ pub struct LegacyKeystoreResult {
pub created_at: ::prost::alloc::string::String,
#[prost(message, repeated, tag = "5")]
pub accounts: ::prost::alloc::vec::Vec<AccountResponse>,
/// This field can have one of three values: "migrated", "unmigrated", or "new".
#[prost(string, tag = "6")]
pub status: ::prost::alloc::string::String,
Copy link
Collaborator

Choose a reason for hiding this comment

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

这里直接叫 status 太过于generic 了。建议改成 migration_status

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants