-
Notifications
You must be signed in to change notification settings - Fork 11
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
Fix/response-pub #378
base: onekey
Are you sure you want to change the base?
Fix/response-pub #378
Conversation
Walkthrough此次更改主要集中在多个文件中,涉及对类和类型的修改。主要更新包括在响应对象中添加新的 Changes
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 38
🧹 Outside diff range comments (2)
packages/core/src/api/cardano/CardanoGetPublicKey.ts (1)
Line range hint
8-8
: 更新类文档类的功能有所变化,建议更新文档。
在类定义前添加简短说明:
/** * 处理 Cardano 公钥获取。 * 返回的对象现包含 'pub' 字段,'publicKey' 已弃用。 */ export default class CardanoGetPublicKey extends BaseMethod<CardanoPublicKeyParams[]> {这有助于其他开发者快速了解类的最新功能。
packages/core/src/api/allnetwork/AllNetworkGetAddress.ts (1)
'sui' 网络配置变更影响验证
发现 'suiGetAddress' 在多个文件中仍有调用。移除
dependOnMethodName
和getParams
可能影响这些功能。建议:
- 手动检查这些调用是否依赖于已移除的配置。
- 确保所有相关功能正常运行。
🔗 Analysis chain
Line range hint
1-319
: 'sui' 网络配置变更'sui' 网络配置简化了:
- 移除了
dependOnMethodName
。- 移除了
getParams
函数。这些变更可能影响
generateMethodName
方法对 'sui' 网络的处理。建议:
- 确认这是有意为之,不会破坏现有功能。
- 更新相关文档,反映这一变化。
- 考虑添加测试,确保 'sui' 网络仍能正常工作。
请运行以下脚本,验证 'sui' 网络相关功能:
Also applies to: 336-476
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # 描述:检查 'sui' 网络相关的方法调用 # 测试:搜索 'sui' 网络相关的方法调用 echo "Searching for 'sui' network method calls:" rg --type typescript "suiGetAddress|sui.*network" ./packagesLength of output: 196
Script:
#!/bin/bash # 描述:检查 'sui' 网络相关的方法调用 # 测试:搜索 'sui' 网络相关的方法调用 echo "Searching for 'sui' network method calls:" rg "suiGetAddress|sui.*network" ./packages --glob "*.ts"Length of output: 5096
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
📒 Files selected for processing (29)
- packages/connect-examples/expo-example/webpack.config.js (1 hunks)
- packages/core/src/api/alephium/AlephiumGetAddress.ts (1 hunks)
- packages/core/src/api/allnetwork/AllNetworkGetAddress.ts (1 hunks)
- packages/core/src/api/aptos/AptosGetAddress.ts (2 hunks)
- packages/core/src/api/aptos/AptosGetPublicKey.ts (2 hunks)
- packages/core/src/api/cardano/CardanoGetPublicKey.ts (1 hunks)
- packages/core/src/api/cosmos/CosmosGetPublicKey.ts (2 hunks)
- packages/core/src/api/evm/EVMGetPublicKey.ts (2 hunks)
- packages/core/src/api/lightning/LnurlAuth.ts (1 hunks)
- packages/core/src/api/nostr/NostrGetPublicKey.ts (1 hunks)
- packages/core/src/api/polkadot/PolkadotGetAddress.ts (2 hunks)
- packages/core/src/api/sui/SuiGetAddress.ts (2 hunks)
- packages/core/src/api/sui/SuiGetPublicKey.ts (2 hunks)
- packages/core/src/api/ton/TonGetAddress.ts (1 hunks)
- packages/core/src/api/xrp/XrpGetAddress.ts (2 hunks)
- packages/core/src/types/api/alephiumGetAddress.ts (1 hunks)
- packages/core/src/types/api/allNetworkGetAddress.ts (2 hunks)
- packages/core/src/types/api/aptosGetAddress.ts (1 hunks)
- packages/core/src/types/api/aptosGetPublicKey.ts (1 hunks)
- packages/core/src/types/api/cardanoGetPublicKey.ts (1 hunks)
- packages/core/src/types/api/cosmosGetPublicKey.ts (1 hunks)
- packages/core/src/types/api/evmGetPublicKey.ts (1 hunks)
- packages/core/src/types/api/lnurlAuth.ts (1 hunks)
- packages/core/src/types/api/nostrGetPublicKey.ts (1 hunks)
- packages/core/src/types/api/polkadotGetAddress.ts (1 hunks)
- packages/core/src/types/api/suiGetAddress.ts (1 hunks)
- packages/core/src/types/api/suiGetPublicKey.ts (1 hunks)
- packages/core/src/types/api/tonGetAddress.ts (1 hunks)
- packages/core/src/types/api/xrpGetAddress.ts (1 hunks)
🧰 Additional context used
🔇 Additional comments (24)
packages/core/src/types/api/aptosGetAddress.ts (2)
6-6
: 新属性pub
添加得当。新的可选属性
pub
很好地满足了需求。它保持了向后兼容性,同时为公钥提供了新的存储位置。
Line range hint
1-31
: 请验证这些更改对代码库其他部分的影响。虽然更改保持了向后兼容性,但建议:
- 更新使用
publicKey
的现有代码,改用pub
。- 检查并更新相关文档。
- 考虑添加迁移指南,帮助用户从
publicKey
过渡到pub
。运行以下脚本来查找可能需要更新的地方:
packages/core/src/types/api/alephiumGetAddress.ts (2)
5-8
: 清晰的弃用注释弃用注释简洁明了,指导用户使用新的
pub
属性。这符合PR的目标。
Line range hint
1-33
: 总体评价:代码结构清晰,变更合理这些更改符合PR的目标,代码结构良好。新旧属性的处理方式恰当,有助于平稳过渡。建议为新属性添加简短文档,以进一步提高代码质量。
packages/core/src/types/api/polkadotGetAddress.ts (2)
6-6
: 新属性pub
添加正确。新的
pub
属性替代了旧的publicKey
,符合 PR 的目标。这个改动有助于代码的演进。
6-10
: 总体改动符合预期且实施得当。新增
pub
属性并弃用publicKey
的做法改进了代码结构,同时保持了向后兼容性。这些变更与 PR 的目标一致,有助于代码库的演进。packages/core/src/types/api/tonGetAddress.ts (1)
Line range hint
1-38
: 总体改动简洁有效。这次更新主要针对
TonAddress
类型。新增pub
字段替代publicKey
,同时保留了向后兼容性。其他部分未变,保持了接口稳定性。这种渐进式更新方法很好,让用户能平滑过渡到新 API。
packages/core/src/types/api/cardanoGetPublicKey.ts (2)
7-7
: 新属性pub
添加得当。新增的
pub
属性符合 PR 目标,替代了旧的publicKey
属性。这个改动简洁明了,提高了代码的可读性。
7-11
: 总体改动符合预期,提升了代码质量。这些更改巧妙地引入了新属性
pub
,同时保持了向后兼容性。弃用旧的publicKey
属性的做法恰当,有助于代码的长期维护。这些改动与 PR 的目标一致,增强了 SDK 的可用性。packages/core/src/types/api/evmGetPublicKey.ts (2)
Line range hint
1-32
: 总体来说,这些更改提升了 API 的灵活性和一致性。主要改进包括:
- 将
publicKey
重命名为pub
,提高了命名一致性。- 新增的函数重载支持批处理操作。
这些变更很好地平衡了新功能和向后兼容性。但是,请注意:
- 确保所有使用
publicKey
的地方都更新为pub
。- 新的函数重载可能会影响依赖
node
和xpub
属性的代码。建议运行全面的测试套件,特别关注使用
evmGetPublicKey
的部分,以确保这些更改不会导致意外问题。
Line range hint
29-32
: 新的函数重载很棒,但要注意潜在的兼容性问题。添加
useBatch
参数提供了更灵活的批处理选项,这是个很好的改进。但是,从返回类型中排除node
和xpub
属性可能会影响依赖这些属性的现有代码。请运行以下脚本来检查是否有代码依赖这些被排除的属性:
✅ Verification successful
请运行以下修正后的脚本来检查是否有代码依赖被排除的
node
或xpub
属性:
兼容性验证通过
经过检查,没有发现代码依赖被排除的
node
或xpub
属性。这意味着更改不会影响现有代码。🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # 描述:检查是否有代码依赖 evmGetPublicKey 返回的 node 或 xpub 属性 # 测试:搜索使用 evmGetPublicKey 及其 node 或 xpub 属性的代码 rg --type typescript "evmGetPublicKey.*\.(node|xpub)"Length of output: 90
Script:
#!/bin/bash # 描述:检查是否有代码依赖 evmGetPublicKey 返回的 node 或 xpub 属性 # 修正:使用正确的文件类型 rg --type ts "evmGetPublicKey.*\.(node|xpub)"Length of output: 47
packages/core/src/api/aptos/AptosGetPublicKey.ts (2)
5-5
: 导入更新正确新增的
AptosPublicKey
类型导入与类中的变更一致。这有助于提高代码的类型安全性。
61-61
: 验证更新正确将验证从 'publicKey' 改为 'pub' 与新的响应结构一致。这确保了数据完整性。
packages/core/src/api/sui/SuiGetPublicKey.ts (2)
5-5
: 导入更改获批!新增的
SuiPublicKey
导入很棒。它让代码更清晰,类型更安全。
56-62
: 🧹 Nitpick (assertive)请解释新增
pub
属性的目的新增的
pub
属性似乎与publicKey
重复。这是为了向后兼容吗?如果publicKey
将被弃用,建议用@deprecated
标记它。{ path: serializedPath((this.params as unknown as any[])[index].address_n), publicKey, // @deprecated 使用 `pub` 替代 pub: publicKey, }这样可以清楚地表明意图,帮助其他开发者理解变化。
需要我帮忙实施这个改动吗?
packages/core/src/api/cardano/CardanoGetPublicKey.ts (1)
66-68
: 验证更新正确将验证从 'publicKey' 改为 'pub' 很合适。这确保了新属性存在于响应中。
packages/core/src/api/cosmos/CosmosGetPublicKey.ts (2)
5-5
: 导入语句更新正确。导入
CosmosAddress
类型提高了代码的类型安全性和可读性。
75-75
: 验证更新正确。将验证从
publicKey
更新为pub
与新的响应结构一致。这确保了新属性存在于响应中。packages/core/src/api/evm/EVMGetPublicKey.ts (2)
77-79
: 验证逻辑更新,很好!将验证重点从 'publicKey' 转移到 'pub' 与新增字段的变化保持一致。这确保了所有响应中都包含新字段,提高了代码的健壮性。
Line range hint
1-102
: 总体评价:代码更新合理,前瞻性强这次更新很好地引入了
pub
字段,同时保持了向后兼容性。changes 贯穿整个文件,保持了一致性。建议:
- 用 JSDoc 标记
publicKey
为已废弃。- 考虑小幅重构以提高代码简洁性。
整体而言,这些修改提高了代码的可维护性和未来适应性。做得好!
packages/core/src/api/allnetwork/AllNetworkGetAddress.ts (1)
320-335
: 错误处理逻辑改进新的错误处理更清晰、更具体。主要变化:
- 区分了 Error 实例和其他错误类型。
- 引入了
errorCode
字段,提供更详细的错误信息。- 使用
handleHardwareError
函数处理硬件相关错误。建议:
- 考虑为未知错误添加日志,便于后续调试。
- 可以进一步细化错误类型,提供更精确的错误信息。
packages/core/src/api/sui/SuiGetAddress.ts (3)
23-24
: 简化了shouldConfirm
的判断逻辑
shouldConfirm
的赋值逻辑清晰,直接判断this.payload.showOnOneKey
或bundle
中是否有showOnOneKey
为true
的项。
76-85
: 确保地址生成方式一致当不需要确认时,使用
publicKeyToAddress
方法生成地址;需要确认时,通过设备调用SuiGetAddress
获取地址。请确保这两种方式生成的地址一致,以避免潜在的问题。建议在测试环境中验证两种方法生成的地址是否一致。
95-96
:postPreviousAddressMessage
的异步调用可能引发问题在循环中多次调用
this.postPreviousAddressMessage(result)
,需要确保该方法的异步处理不会导致意外行为。请确认
postPreviousAddressMessage
方法的执行方式,确保不会引入异步处理问题。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
📒 Files selected for processing (2)
- packages/core/src/api/aptos/AptosGetAddress.ts (2 hunks)
- packages/core/src/api/sui/SuiGetAddress.ts (2 hunks)
🧰 Additional context used
🔇 Additional comments (5)
packages/core/src/api/aptos/AptosGetAddress.ts (4)
25-27
: 代码改进很好!代码简洁明了,使用了现代JavaScript特性。类型断言提高了类型安全性。这个变更解决了之前的审查意见。做得好!
108-125
: 兼容性处理很周到为不支持批量公钥检索的设备提供了回退逻辑,很好地确保了兼容性。使用Promise.all进行并行执行提高了性能。代码结构清晰,遵循最佳实践。做得很棒!
132-132
: 返回语句简化得很好使用三元运算符处理bundle和非bundle情况,代码简洁高效。正确处理了单个和多个响应场景。干得漂亮!
95-100
:⚠️ Potential issue建议重新考虑字段冗余
之前的审查意见建议移除已弃用的
publicKey
字段。目前结果对象仍包含publicKey
和pub
两个字段。如果publicKey
确实已弃用,建议仅保留pub
字段,避免冗余。请重新评估这部分代码。packages/core/src/api/sui/SuiGetAddress.ts (1)
23-25
: 逻辑正确,代码实现准确。
const result = { | ||
path: serializedPath(param.address_n), | ||
address: res.message.address?.toLowerCase() ?? '', | ||
}; | ||
if (this.shouldConfirm) { | ||
this.postPreviousAddressMessage(result); | ||
} | ||
return result; | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
非批量处理的结果缺少 publicKey
字段
在非批量处理时,结果对象只包含 path
和 address
。但在批量处理时,结果还包含 publicKey
和 pub
字段。这会导致返回结果不一致,可能引发问题。建议在非批量处理的结果中也加入 publicKey
和 pub
字段。
应用以下修改:
const result = {
path: serializedPath(param.address_n),
address: res.message.address?.toLowerCase() ?? '',
+ publicKey: res.message.publicKey,
+ pub: res.message.publicKey,
};
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
const result = { | |
path: serializedPath(param.address_n), | |
address: res.message.address?.toLowerCase() ?? '', | |
}; | |
if (this.shouldConfirm) { | |
this.postPreviousAddressMessage(result); | |
} | |
return result; | |
}) | |
const result = { | |
path: serializedPath(param.address_n), | |
address: res.message.address?.toLowerCase() ?? '', | |
publicKey: res.message.publicKey, | |
pub: res.message.publicKey, | |
}; | |
if (this.shouldConfirm) { | |
this.postPreviousAddressMessage(result); | |
} | |
return result; | |
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
📒 Files selected for processing (2)
- packages/core/src/api/cardano/CardanoGetPublicKey.ts (1 hunks)
- packages/core/src/types/api/cardanoGetPublicKey.ts (1 hunks)
🧰 Additional context used
@@ -4,7 +4,7 @@ import { PROTO } from '../../constants'; | |||
export type CardanoPublicKey = { | |||
path: number[]; | |||
serializedPath: string; | |||
publicKey: string; | |||
xpub: string; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codebase verification
重命名 publicKey
为 xpub
可能引发兼容性问题
在以下文件中,CardanoPublicKey
被引用,可能需要更新以适应此更改:
packages/hd-transport/src/types/messages.ts
packages/core/src/api/cardano/CardanoGetPublicKey.ts
packages/core/src/api/cardano/CardanoGetAddress.ts
建议:
- 为
publicKey
保留并添加@deprecated
标签。 - 更新相关文件中的引用,确保一致性。
- 检查和更新所有使用
CardanoPublicKey
的代码部分。
🔗 Analysis chain
属性重命名可能导致兼容性问题
publicKey
改为 xpub
是个重大变更。
建议:
- 考虑保留
publicKey
,用@deprecated
标记。 - 更新文档,说明此变更。
- 检查使用
CardanoPublicKey
的代码。
需要验证影响范围吗?
之前的命令无法识别文件类型。请运行以下脚本以查找 CardanoPublicKey
的使用位置。
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 搜索使用 CardanoPublicKey 的地方
rg --type typescript "CardanoPublicKey"
Length of output: 74
Script:
#!/bin/bash
# 搜索 CardanoPublicKey 的使用位置
rg "CardanoPublicKey" --glob "*.ts" --glob "*.tsx"
Length of output: 1554
xpub: message.xpub, | ||
node: message.node, | ||
}); | ||
} | ||
|
||
validateResult(responses, ['publicKey'], { | ||
validateResult(responses, ['pub'], { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
考虑标记 'publicKey' 为弃用
之前的代码中使用了 'publicKey' 字段。为了向后兼容,建议在响应对象中保留 'publicKey' 字段,并标记为弃用。
建议修改如下:
responses.push({
path: batch.address_n,
serializedPath: serializedPath(batch.address_n),
+ pub: message.xpub,
+ publicKey: message.xpub, // @deprecated 请使用 'pub'
xpub: message.xpub,
node: message.node,
});
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
xpub: message.xpub, | |
node: message.node, | |
}); | |
} | |
validateResult(responses, ['publicKey'], { | |
validateResult(responses, ['pub'], { | |
pub: message.xpub, | |
publicKey: message.xpub, // @deprecated 请使用 'pub' | |
xpub: message.xpub, | |
node: message.node, | |
}); | |
} | |
validateResult(responses, ['pub'], { |
修复响应对象中的 'pub' 字段缺失
在第60行,响应对象中没有包含 'pub' 字段。但在第65行,validateResult
函数正在检查 'pub' 字段。这将导致验证失败。
请在响应对象中添加 'pub' 字段,并将 message.xpub
赋值给它。
建议修改如下:
responses.push({
path: batch.address_n,
serializedPath: serializedPath(batch.address_n),
+ pub: message.xpub,
xpub: message.xpub,
node: message.node,
});
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
xpub: message.xpub, | |
node: message.node, | |
}); | |
} | |
validateResult(responses, ['publicKey'], { | |
validateResult(responses, ['pub'], { | |
xpub: message.xpub, | |
node: message.node, | |
pub: message.xpub, | |
}); | |
} | |
validateResult(responses, ['pub'], { |
改动点:
webpack.config.js 中添加环境变量配置
增加 pub 字段,标识原有的字段为deprecated
需要重点关注接口 suiGetAddress, aptosGetAddress, InurlAuth,
原有接口通过Promise.resolve 返回,但是因为已经标识了async 这里应该不需要再包一层
Summary by CodeRabbit
新功能
pub
属性,以替代publicKey
属性,增强了返回数据结构。文档
publicKey
属性为弃用,建议用户使用新的pub
属性。修复