Skip to content

feat: Manage Nervos DAO with multisig address#3298

Merged
Keith-CY merged 11 commits into
developfrom
feat-425
Feb 12, 2025
Merged

feat: Manage Nervos DAO with multisig address#3298
Keith-CY merged 11 commits into
developfrom
feat-425

Conversation

@devchenyan

@devchenyan devchenyan commented Jan 14, 2025

Copy link
Copy Markdown
Contributor

Magickbase/neuron-public-issues#425

Screen-2025-01-16-050516.mp4

@devchenyan
devchenyan marked this pull request as ready for review January 15, 2025 21:05
@Danie0918
Danie0918 requested review from Keith-CY, homura and yanguoyu and removed request for homura January 20, 2025 01:34
@Danie0918

Copy link
Copy Markdown

@homura @yanguoyu Please have a review.

Comment thread packages/neuron-ui/src/components/MultisigAddressNervosDAODialog/hooks.ts Outdated
Comment thread packages/neuron-wallet/src/services/tx/transaction-generator.ts
Comment thread packages/neuron-ui/src/components/MultisigAddressNervosDAODialog/hooks.ts Outdated
@yanguoyu

Copy link
Copy Markdown
Contributor

I guess it will be better to back to the multisig address table after the send success dialog opens and closes
image

Comment thread packages/neuron-wallet/src/services/cells.ts Outdated
Comment thread packages/neuron-wallet/src/services/cells.ts Outdated
Comment thread packages/neuron-wallet/src/services/cells.ts Outdated
Comment thread packages/neuron-wallet/src/services/transaction-sender.ts Outdated
Comment thread packages/neuron-wallet/src/services/tx/transaction-generator.ts Outdated
Comment thread packages/neuron-ui/src/components/DepositDialog/hooks.ts Outdated
@Danie0918
Danie0918 requested review from homura and yanguoyu January 26, 2025 01:22
@Danie0918

Copy link
Copy Markdown

@homura @yanguoyu Please have a review.

@silySuper

silySuper commented Feb 7, 2025

Copy link
Copy Markdown

/package
Packaging for test is done in 13195084522. @silySuper

@silySuper

Copy link
Copy Markdown

[Dao deposit]
1.When deposit successfully,history page does not show records
截屏2025-02-07 17 35 04

history page:no 17:28 and 17:24 records
截屏2025-02-07 17 35 33

2.When deposit successfully balance does not change .original balance is 900ckb,after two deposits(102CKB every record) it is 899 ckb.

截屏2025-02-07 17 50 27 截屏2025-02-07 17 50 37

3.Deposit need add success tip after deposit successfully

[dao withdraw]
4.view transaction detail jump to undefined page,status is before withdraw.

2025-02-07.17.58.37.mov

Comment thread packages/neuron-wallet/src/services/cells.ts Outdated
@silySuper

silySuper commented Feb 8, 2025

Copy link
Copy Markdown

5.[dao deposit]
In deposit window,deposit amount does not change from 0 to max ,change from 0 ->bigger than max ->max

2025-02-08.10.14.35.mov

6.When enter wrong password ,it shows wallet is not correct,maybe shows password is wrong is better.
截屏2025-02-08 10 15 44

7.Data is highlight in concise data,does it have other meaning?
截屏2025-02-08 10 18 56

8.When no more than 102CKB,tip does not show ,and can also proceed.After enter right password it shows wrong wallet.
截屏2025-02-08 11 19 38

2025-02-08.11.21.54.mov

9.[dao withdraw]
In dark mode,font is black.In expect it should be white.
截屏2025-02-08 11 24 29

截屏2025-02-08 11 27 59

10.withdraw UI is different from UI document.
UI document:
image

actual UI:
截屏2025-02-08 11 31 59

@Danie0918
Danie0918 requested a review from yanguoyu February 11, 2025 01:11
@Danie0918

Copy link
Copy Markdown

@yanguoyu @silySuper Please have a review.

@Danie0918
Danie0918 requested a review from silySuper February 11, 2025 01:42
@silySuper

silySuper commented Feb 11, 2025

Copy link
Copy Markdown

/package
Packaging for test is done in 13254311338. @silySuper

@silySuper

Copy link
Copy Markdown

1.same as before
2.same as before
3.same as before
4.same as before
5.same as before
6.same as before
7..same as before
8.same as before
9.verified
10.same as before

please check whether code is submitted correctly. @devchenyan

@devchenyan

Copy link
Copy Markdown
Contributor Author
  1. Transaction history will not display transactions of multisig addresses
  2. Since the outputs are also the multisig address itself, the balance remains the same
  3. Fixed
  4. Fixed
  5. Needs to be modified together with NervosDAO
  6. General logic for verifying multisig address signatures
  7. Data style when data !== '0x'
  8. Needs to be modified together with NervosDAO
  9. Fixed
  10. The UI document matches the actual UI

@Keith-CY

Keith-CY commented Feb 11, 2025

Copy link
Copy Markdown
Contributor

/package
Packaging for test is done in 13257953306. @Keith-CY

@silySuper

Copy link
Copy Markdown
  1. Transaction history will not display transactions of multisig addresses
  2. Since the outputs are also the multisig address itself, the balance remains the same
  3. Fixed
  4. Fixed
  5. Needs to be modified together with NervosDAO
  6. General logic for verifying multisig address signatures
  7. Data style when data !== '0x'
  8. Needs to be modified together with NervosDAO
  9. Fixed
  10. The UI document matches the actual UI

2.This may cause problem when no balance to deposit but show enough balance.
5.Nervos dao is normal,this is video:

2025-02-11.12.25.25.mov

6.Approve shows like this when password is wrong:
截屏2025-02-11 12 23 40

in Nervos Dao shows like this when password is wrong:

截屏2025-02-11 12 24 45

8.Nervos Dao show tip when balance is not enough and proceed button is grey:
截屏2025-02-11 12 34 35

10.This is UI document,before upload wrong.(issue before amend also)
image

@devchenyan

devchenyan commented Feb 11, 2025

Copy link
Copy Markdown
Contributor Author

2、 fixed
5、 fixed
6、 Please check wallet, is not the correct wallet in the screenshot.
image
Wrong password is prompted when using the correct wallet.
image

8、fixed
10、The text in the UI design does not meet the requirements of the scenario, please refer to the requirements prototype design. Magickbase/neuron-public-issues#425
image

@silySuper

silySuper commented Feb 12, 2025

Copy link
Copy Markdown

/package
Packaging for test is done in 13276883182. @silySuper

@silySuper

silySuper commented Feb 12, 2025

Copy link
Copy Markdown

2.verified
3.verified
4.verified
5.verified
6.It is my right wallet,I found that in approve function if I click sign&export it shows not correct wallet,but if I click confirm it shows incorrect password,it is a history bug.I will open another link issue to this bug. link issue :Neuron (view)
8.verified

@Keith-CY
Keith-CY added this pull request to the merge queue Feb 12, 2025
Merged via the queue into develop with commit 95823eb Feb 12, 2025
@Keith-CY
Keith-CY deleted the feat-425 branch February 12, 2025 03:46
@Keith-CY Keith-CY mentioned this pull request Feb 12, 2025
@thetdy

thetdy commented Mar 11, 2025

Copy link
Copy Markdown

when using a ledger with multisig it only seems to work sending ckb but not for the DAO deposits.

tested on linux and windows.

image

@Keith-CY

Copy link
Copy Markdown
Contributor

when using a ledger with multisig it only seems to work sending ckb but not for the DAO deposits.

tested on linux and windows.

image

@Danie0918 please check this

@Danie0918

Danie0918 commented Mar 12, 2025

Copy link
Copy Markdown

when using a ledger with multisig it only seems to work sending ckb but not for the DAO deposits.

tested on linux and windows.

image

Thanks for your feedback! We have confirmed that Ledger does not support DAO deposits and withdrawals for multisig addresses and will address this issue.Magickbase/neuron-public-issues#446

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.

7 participants