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

Multisig address list is abnormal #347

Open
Keith-CY opened this issue Dec 30, 2023 · 4 comments
Open

Multisig address list is abnormal #347

Keith-CY opened this issue Dec 30, 2023 · 4 comments
Assignees
Labels
bug Something isn't working

Comments

@Keith-CY
Copy link
Member

以下 100% 重现,配置为:
Linux 最新 release 版本。
我有三个 neuron 账户,按次序记为 A B C
用 B 下面的第一个收款地址创建一个 2-of-3 多签地址
然后:
如果关闭 neuron 时,账户设置为 A,则打开后不论如何切换账户,都看不到之前创建的多签地址
如果关闭 neuron 时,账户设置为 B,则打开后不论如何切换账户,都能正常使用
如果关闭 neuron 时,账户设置为 C,则打开后不论如何切换账户,尽管都能看到多签账户,但无法签名

Reproducible, but wallets A, B, C should all be in Neuron.

Otherwise

如果有一个钱包不在 Neuron 中, 例如刚刚分享的多签地址集合配置, 会有额外的问题
以刚刚分享的三个多签地址的配置为例
一开始是切换到 C 钱包重启 Neuron 也看不到地址列表
然后我导出配置, 删除多签地址列表, 切换到 C 钱包, 导入地址列表, 可以看到 3 个地址, 但是切换到 A 钱包, 查看地址列表, 只有 1 个地址了, 再切换会 C 钱包(不重启), 也只有 1 个地址

Screen.Recording.2023-12-30.at.16.02.51.mov
@Keith-CY Keith-CY added the bug Something isn't working label Dec 30, 2023
@Keith-CY Keith-CY added this to Neuron Dec 30, 2023
@Keith-CY
Copy link
Member Author

Notice the address ends with lf appears in the 1 address list and 3 address list, but they are 2 different entries, because it has an alias in 1 address list but has no alias in 3 address list

@Keith-CY Keith-CY moved this to 🆕 New in Neuron Dec 30, 2023
@Keith-CY
Copy link
Member Author

Keith-CY commented Dec 30, 2023

Possibly caused by https://github.com/nervosnetwork/neuron/blob/develop/packages/neuron-wallet/src/controllers/multisig.ts#L82 and https://github.com/nervosnetwork/neuron/blob/develop/packages/neuron-wallet/src/controllers/multisig.ts#L90

The multisig config is bound to a specific wallet, so launching with different wallets loads different configs


I would suggest an optimization as follows:

  1. share the multisig config globally;
  2. filter addresses that do not belong to the active wallet out when the multisig address list is displayed

BTW, similar logic appears at nervosnetwork/neuron#3000 (comment) so we should highlight the rules in PRD to reduce the barrier of understanding

@yanguoyu
Copy link

yanguoyu commented Jan 2, 2024

I guess it's normal, in the previous the multisig config was designed to bind to the wallet. We can change it to bind to the global, but we should not filter the multisig address if every address of the multisig config does not belong to the current wallet, because someone wants to monitor the multisig address and create a transaction without the sign.

@Danie0918 Danie0918 assigned Danie0918 and unassigned FrederLu and silySuper Jan 2, 2024
@Danie0918 Danie0918 moved this from 🆕 New to 📌Planning in Neuron Jan 2, 2024
@Keith-CY
Copy link
Member Author

Keith-CY commented Jan 8, 2024

PR: nervosnetwork/neuron#3010

@Keith-CY Keith-CY moved this from 📌Planning to 👀 Testing in Neuron Jan 8, 2024
@Danie0918 Danie0918 assigned silySuper and unassigned yanguoyu and Danie0918 Jan 15, 2024
@Danie0918 Danie0918 moved this from 👀 Testing to 🚩Pre Release in Neuron Jan 19, 2024
@Danie0918 Danie0918 moved this from 🚩Pre Release to 📌Planning in Neuron Jan 23, 2024
@Danie0918 Danie0918 moved this from 📌Planning to 🚩Pre Release in Neuron Mar 9, 2024
@Danie0918 Danie0918 moved this from 🚩Pre Release to ✅ Done in Neuron Aug 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: ✅ Done
Development

No branches or pull requests

5 participants